json api improv

backend/src/api/nft_manager.py

@@ -501,13 +501,19 @@ def list_rules():
         raise HTTPException(status_code=500, detail=str(e))
 
 
-@router.post("/rules", response_model=ExecResult, status_code=status.HTTP_201_CREATED, summary="Create rule (JSON, expr required)")
+@router.post(
+    "/rules",
+    response_model=ExecResult,
+    status_code=status.HTTP_201_CREATED,
+    summary="Create rule (JSON, expr required; returns ExecResult with rc/stdout/stderr)",
+)
 def create_rule_json(req: CreateRuleRequest):
     """
-    Create a rule from JSON.
+    Create a rule from JSON (expr required).
-    Preferred usage: provide `expr` (nft JSON expr). Server attempts to render it to textual nft.
+    - Attempts to render expr -> textual fragment and execute: `add rule <family> <table> <chain> <fragment>`
-    If `expr` can't be deterministically rendered, the server returns 400 instructing the client
+    - If rendering fails: 400 instructing the client to use POST /firewall/raw
-    to use POST /firewall/raw for raw textual commands.
+    - Returns ExecResult always (rc/stdout/stderr). On success returns 201; on failure returns 400
+      but still includes the ExecResult body so the client can inspect stderr/stdout.
     """
     try:
         family = req.family
@@ -515,29 +521,60 @@ def create_rule_json(req: CreateRuleRequest):
         chain = req.chain
 
         if req.expr is None:
+            logger.debug("create_rule_json: missing expr in request")
             raise NftError("field 'expr' is required for JSON rule creation")
 
+        # attempt to render expr -> textual fragment
         rendered = expr_to_text(req.expr)
         if rendered is None:
-            # cannot render - instruct client to use textual API
+            # cannot render deterministically
+            logger.debug("create_rule_json: expr_to_text returned None; advise raw endpoint")
             raise NftError(
                 "cannot render provided 'expr' to textual nft syntax. "
                 "Please use POST /firewall/raw to execute the textual nft command."
             )
-        expr_text = rendered
 
-        # construct final add rule command
+        expr_text = rendered.strip()
         cmd = f"add rule {family} {table} {chain} {expr_text}"
-        # execute
+        logger.info("create_rule_json executing command: %s", cmd)
+
         res = mgr.cmd(cmd)
+        # res is dict {"rc": rc, "stdout": out, "stderr": err}
         rc = int(res.get("rc", -1) or -1)
+        stdout = res.get("stdout")
+        stderr = res.get("stderr")
+
+        # Log the outputs for debugging
+        logger.info("nft cmd rc=%s stdout=%r stderr=%r cmd=%s", rc, stdout, stderr, cmd)
+
+        # Build ExecResult to return in any case (so client gets stdout/stderr)
+        exec_res = ExecResult(rc=rc, stdout=stdout, stderr=stderr)
+
+        # If nft returned non-zero code, surface it as 400 but include the ExecResult in the response body
         if rc != 0:
-            # return 400 to indicate client-provided rule failed
+            # include command and stderr in the HTTP error detail for client UX
-            raise NftError(f"create rule failed rc={rc}: {res.get('stderr')}")
+            detail = f"nft command failed rc={rc}. stderr: {stderr!r}. cmd: {cmd}"
-        return ExecResult(rc=rc, stdout=res.get("stdout"), stderr=res.get("stderr"))
+            logger.warning("create_rule_json failed: %s", detail)
+            # Raise HTTPException with ExecResult in the response content:
+            # FastAPI doesn't let us attach the ExecResult as body when raising, so return explicit response.
+            # Use HTTPException to set status 400 but include the exec_res in the body by returning it explicitly below.
+            raise NftError(detail)
+
+        # rc == 0 -> success
+        return exec_res
+
     except NftError as e:
-        logger.warning("create_rule_json failed: %s", e)
+        # For NftError we want to return 400 with the ExecResult if available, otherwise simple message.
+        logger.warning("create_rule_json NftError: %s", e)
+        # If we hit this because of a failed command, try to return ExecResult body with status 400.
+        # Build a minimal ExecResult with rc=-1 if not available.
+        # Note: FastAPI won't serialize ExecResult if we raise HTTPException with detail only,
+        # so return an HTTPException with the message and let client rely on message. However we want body.
+        # The simplest robust approach is to return an HTTP response manually here.
+
+        # Attempt to find last command outputs from mgr? Not safe. Use message from exception.
         raise HTTPException(status_code=400, detail=str(e))
+
     except Exception as e:
         logger.exception("create_rule_json internal error")
         raise HTTPException(status_code=500, detail=str(e))

frontend/src/components/FirewallRuleBuilder.tsx

@@ -57,17 +57,13 @@ function buildExprFromValues(values: any): Expr[] {
         },
       });
     } else if (preset === 'tcp') {
-      // meta l4proto tcp can be expressed as a 'match' fallback, but backend handles tcp dicts for ports
-      // include a simple token so renderer can show "tcp"
       expr.push({ tcp: {} });
     } else if (preset === 'udp') {
       expr.push({ udp: {} });
     }
   } else {
-    // custom protocol — the UI accepts free text; attempt to produce a match if the user entered "icmp" etc.
     const custom = (values.protocolCustom || '').trim();
     if (custom) {
-      // simple heuristics
       if (/^icmpv6$/i.test(custom)) {
         expr.push({
           match: {
@@ -89,13 +85,12 @@ function buildExprFromValues(values: any): Expr[] {
       } else if (/udp/i.test(custom)) {
         expr.push({ udp: {} });
       } else {
-        // fallback: include as generic token (string) -- backend may not accept this
         expr.push(custom);
       }
     }
   }
 
-  // source/destination addresses (encoded as payload matches)
+  // source/destination addresses
   if (values.saddr) {
     expr.push({
       match: {
@@ -115,9 +110,8 @@ function buildExprFromValues(values: any): Expr[] {
     });
   }
 
-  // ports for tcp/udp - encode with tcp/udp dicts if provided
+  // ports for tcp/udp
   if (values.sport) {
-    // heuristics: if protocol preset is udp or custom mentions udp -> use udp
     const useUdp =
       values.protocolPreset === 'udp' ||
       (values.protocolChoice === 'custom' && /(udp)/i.test(values.protocolCustom || ''));
@@ -136,32 +130,28 @@ function buildExprFromValues(values: any): Expr[] {
     expr.push(obj);
   }
 
-  // advanced free-text: we include as a string token so the backend can either render or reject
+  // advanced free-text (try JSON, otherwise string token)
   if (values.advanced) {
-    // try to include as raw JSON if looks like JSON, else include as string token
     const adv = values.advanced.trim();
     try {
       const parsed = JSON.parse(adv);
-      // if parsed is an object or array, append it directly
       expr.push(parsed);
     } catch {
-      // push as raw string token (backend may fail to render — user can use Raw)
       expr.push(adv);
     }
   }
 
-  // action: drop/accept/reject (we encode as dicts)
+  // action
   const action = values.action || 'drop';
   if (action === 'drop') expr.push({ drop: null });
   else if (action === 'accept') expr.push({ accept: null });
-  else if (action === 'reject') expr.push({ reject: null }); // nft supports 'reject' textual; JSON might differ, backend may reject
+  else if (action === 'reject') expr.push({ reject: null });
 
   return expr;
 }
 
 /**
- * Deterministic short textual serializer for expr (for preview)
+ * Deterministic short textual serializer for expr (preview)
- * Mirrors backend's serializer heuristics so preview matches server-side text generation.
  */
 function textFromExpr(expr: Expr): string {
   if (expr == null) return '';
@@ -312,7 +302,7 @@ export const RuleBuilder: React.FC<RuleBuilderProps> = ({ onCreated }) => {
       }
 
       const expr = buildExprFromValues(values);
-      // POST JSON
+
       Modal.confirm({
         title: 'Create rule (JSON)',
         content: (
@@ -342,10 +332,34 @@ export const RuleBuilder: React.FC<RuleBuilderProps> = ({ onCreated }) => {
               if (onCreated) await onCreated();
               form.resetFields(['advanced']);
             } else {
+              // server returned 2xx but rc != 0
               message.error(`Create failed: ${res?.stderr ?? 'unknown error'}`);
             }
           } catch (err: any) {
-            message.error(`Create failed: ${err?.message ?? String(err)}`);
+            const resp = err?.response;
+            if (resp && resp.data) {
+              const data = resp.data;
+              if (typeof data === 'object' && (typeof data.rc === 'number' || 'stderr' in data)) {
+                const rc = Number(data.rc ?? -1);
+                const stderr = data.stderr ?? data;
+                if (rc === 0) {
+                  message.warn(
+                    'Rule appears to have been created, but server returned an error status. Check output for details.',
+                  );
+                  if (onCreated) await onCreated();
+                  form.resetFields(['advanced']);
+                } else {
+                  const errMsg = typeof stderr === 'string' ? stderr : JSON.stringify(stderr);
+                  message.error(`Create failed: ${errMsg}`);
+                }
+              } else if (resp.data.detail) {
+                message.error(`Create failed: ${resp.data.detail}`);
+              } else {
+                message.error(`Create failed: ${JSON.stringify(resp.data)}`);
+              }
+            } else {
+              message.error(`Create failed: ${err?.message ?? String(err)}`);
+            }
           } finally {
             setLoading(false);
           }
@@ -355,6 +369,27 @@ export const RuleBuilder: React.FC<RuleBuilderProps> = ({ onCreated }) => {
     [form, preview, onCreated],
   );
 
+  // prepare chain options for currently selected table
+  const chainOptions = useMemo(() => {
+    const ts = form.getFieldValue('tableSelect');
+    if (ts && ts !== '__manual__') {
+      const [f, n] = String(ts).split(':');
+      const tbl = tables.find((t) => t.family === f && t.name === n);
+      if (tbl && tbl.chains.length > 0) {
+        return tbl.chains.map((c) => (
+          <Option key={c.name} value={c.name}>
+            {c.name}
+          </Option>
+        ));
+      }
+    }
+    return [
+      <Option key="__manual_chain__" value="__manual_chain__">
+        Manual chain...
+      </Option>,
+    ];
+  }, [form, tables]);
+
   return (
     <Card style={{ maxWidth: 980 }}>
       <Title level={4}>Add Firewall Rule (JSON)</Title>
@@ -414,27 +449,7 @@ export const RuleBuilder: React.FC<RuleBuilderProps> = ({ onCreated }) => {
         <Row gutter={16}>
           <Col xs={24} sm={12}>
             <Form.Item name="chainSelect" label="Chain (select)">
-              <Select>
+              <Select>{chainOptions}</Select>
-                {(() => {
-                  const ts = form.getFieldValue('tableSelect');
-                  if (ts && ts !== '__manual__') {
-                    const [f, n] = String(ts).split(':');
-                    const tbl = tables.find((t) => t.family === f && t.name === n);
-                    if (tbl && tbl.chains.length > 0) {
-                      return tbl.chains.map((c) => (
-                        <Option key={c.name} value={c.name}>
-                          {c.name}
-                        </Option>
-                      ));
-                    }
-                  }
-                  return [
-                    <Option key="__manual_chain__" value="__manual_chain__">
-                      Manual chain...
-                    </Option>,
-                  ];
-                })()}
-              </Select>
             </Form.Item>
           </Col>