diff --git a/docs/commands/agentbase/gateway/create.md b/docs/commands/agentbase/gateway/create.md index 969ed7c..8ef58f8 100644 --- a/docs/commands/agentbase/gateway/create.md +++ b/docs/commands/agentbase/gateway/create.md @@ -10,6 +10,10 @@ By default the gateway is built from flags (the simple path: no inline targets). For targets, outbound auth, JWT inbound auth, or anything nested, use `--file` with a template produced by `grn agentbase gateway generate`. +Inbound auth defaults to `IAM` (callers authenticate via the platform IAM); +pass `--inbound-mode NONE` to open the gateway or `JWT` to validate caller +JWTs. + The gateway is created asynchronously; this command returns as soon as the service accepts the spec (state `WAITING_CREATING`). Converge with `grn agentbase gateway wait `. @@ -17,7 +21,8 @@ service accepts the spec (state `WAITING_CREATING`). Converge with When `--file` is set it is authoritative and the individual flags below are ignored. `--file` (produced by [generate](generate.md)) is the recommended path for complex nested configuration (targets / outboundAuth / inboundAuth / -privateNetwork). +privateNetwork). A spec that omits `inboundAuth` (or its `mode`) is defaulted +to `{mode: IAM}`; a `jwt` block without `mode: JWT` is rejected. ## Synopsis @@ -95,10 +100,13 @@ Replica count. Sealed at create. **`--inbound-mode`** (string) -Inbound (caller) authentication mode. +Inbound (caller) authentication mode. Callers authenticate via the platform +IAM by default; `NONE` opens the gateway, `JWT` validates caller JWTs (the +`--jwt-*` flags below then build the config). -- Required: Yes (or `--interactive`) -- Possible values: `NONE`, `IAM`, `JWT` +- Required: No +- Default: `IAM` +- Possible values: `NONE`, `IAM`, `JWT` (case-insensitive) **`--client-redirect-uri`** (list<string>) @@ -110,52 +118,56 @@ Allowed client redirect URI for DCR/authorize. *(repeatable)* **`--iam-require-owner`** (boolean) IAM mode only: require the caller to own the resource. Only sent when explicitly -set. +set; rejected unless `--inbound-mode` is `IAM` (the default). - Required: No - Default: `false` **`--jwt-source`** (string) -JWT inbound-auth source. Triggers JWT config assembly when set (or when -`--inbound-mode` is `JWT`). +JWT inbound-auth source. Only applies when `--inbound-mode` is `JWT` (error +otherwise); with `JWT` it triggers the JWT config assembly. - Required: No - Possible values: `DISCOVERY`, `JWKS` **`--jwt-discovery-url`** (string) -JWT OIDC discovery URL (used when `--jwt-source` is `DISCOVERY`). +JWT OIDC discovery URL (used when `--jwt-source` is `DISCOVERY`). Only applies +when `--inbound-mode` is `JWT`. - Required: No **`--jwt-jwks`** (string) -JWT inline JWKS document (used when `--jwt-source` is `JWKS`). +JWT inline JWKS document (used when `--jwt-source` is `JWKS`). Only applies +when `--inbound-mode` is `JWT`. - Required: No **`--allowed-audience`** (list<string>) -Allowed JWT audience. *(repeatable)* +Allowed JWT audience. *(repeatable)* Only applies when `--inbound-mode` is +`JWT`. - Required: No **`--allowed-client`** (list<string>) -Allowed JWT client id. *(repeatable)* +Allowed JWT client id. *(repeatable)* Only applies when `--inbound-mode` is +`JWT`. - Required: No **`--allowed-scope`** (list<string>) -Allowed JWT scope. *(repeatable)* +Allowed JWT scope. *(repeatable)* Only applies when `--inbound-mode` is `JWT`. - Required: No **`--principal-claim`** (string) -JWT principal claim. +JWT principal claim. Only applies when `--inbound-mode` is `JWT`. - Required: No - Default: `sub` @@ -227,15 +239,15 @@ All `grn agentbase` commands accept: ## Examples -Create a simple PUBLIC gateway from flags: +Create a simple PUBLIC gateway from flags (inbound auth defaults to `IAM`; +add `--inbound-mode NONE` to open the gateway): ```bash grn agentbase gateway create \ --name my-gateway \ --network-mode PUBLIC \ --flavor-id fill-flavor-id \ - --replicas 1 \ - --inbound-mode NONE + --replicas 1 grn agentbase gateway wait my-gateway ``` diff --git a/docs/commands/agentbase/gateway/generate.md b/docs/commands/agentbase/gateway/generate.md index 7de6e64..0f0f768 100644 --- a/docs/commands/agentbase/gateway/generate.md +++ b/docs/commands/agentbase/gateway/generate.md @@ -11,7 +11,8 @@ Defaults to YAML (with comments); pass `-o json` for a JSON skeleton. The template keys are the JSON (camelCase) field names, so the file round-trips through `create --file` exactly. Required keys: `name`, `networkMode`, -`flavorId`, `replicas`, `inboundAuth.mode`. Sealed at create: `name`, +`flavorId`, `replicas` (`inboundAuth.mode` is optional — omitted it defaults +to `IAM`; the template ships with `mode: IAM`). Sealed at create: `name`, `networkMode`, `flavorId`, `replicas`, and (PRIVATE) `privateNetwork.vpcId`/`subnetId`. diff --git a/docs/commands/agentbase/runtime/create.md b/docs/commands/agentbase/runtime/create.md index 4922125..8de1885 100644 --- a/docs/commands/agentbase/runtime/create.md +++ b/docs/commands/agentbase/runtime/create.md @@ -8,7 +8,8 @@ Create a new agent runtime. By default the runtime is built from flags: `--name` is prompted for when missing, `--command`/`--args`/`--env` are repeatable, `--network-mode VPC` -places the runtime in a VPC, `--inbound-mode IAM|JWT` configures inbound auth, +places the runtime in a VPC, `--inbound-mode NONE|IAM|JWT` configures inbound +auth (defaults to `IAM` on `PUBLIC` networks; not available in VPC mode), `--allowed-cidrs` restricts inbound CIDRs, and `--protocol MCP` selects the MCP protocol. `imageAuth` is required by the API: with no `--image-auth-*` flags the CLI sends `{enabled: false}` (public image). Prefer `--file` with a @@ -20,7 +21,9 @@ service accepts the spec (state `CREATING`). Converge with `grn agentbase runtime wait `. When `--file` is set it is authoritative — every flag below is ignored and the -file must contain `name`, `imageUrl`, and `flavorId`. +file must contain `name`, `imageUrl`, and `flavorId`. A spec that omits +`inboundAuth` is sent with `inboundAuth: {mode: IAM}` unless the spec puts the +runtime in a VPC (`inboundAuth` is `PUBLIC`-network only). > **Deprecation:** `--min-replicas` is gone. The platform owns the autoscaling > floor (minimum replicas is always 1); only `--max-replicas` is configurable. @@ -178,7 +181,8 @@ Autoscaling: memory scale-up threshold % (10-90). - Required: No - Default: `PUBLIC` -- Constraints: `VPC` requires `--vpc-id` and `--subnet-id`. +- Constraints: `VPC` requires `--vpc-id` and `--subnet-id`; explicitly set but + empty is rejected (an empty `$NETWORK_MODE` expansion used to read as unset). **`--vpc-id`** (string) @@ -213,12 +217,16 @@ traffic on `PUBLIC` networks. **`--inbound-mode`** (string) -`inboundAuth` mode: `NONE`, `IAM`, or `JWT` (unset = `NONE`). +`inboundAuth` mode: `NONE`, `IAM`, or `JWT`. `PUBLIC` networks only — the API +rejects `inboundAuth` on VPC runtimes. Unset = `IAM` on create (callers +authenticate via the platform IAM); pass `NONE` to open the runtime. - Required: No -- Default: (empty = `NONE`) +- Default: `IAM` on create (when the network is `PUBLIC` or unset) - Constraints: `JWT` requires the `--inbound-jwt-*` flags; cannot be combined - with them when set to `IAM`/`NONE`. + with them when set to `IAM`/`NONE`; not available with `--network-mode VPC`; + explicitly set but empty is rejected (an empty `$INBOUND_MODE` expansion + used to read as unset and default to `IAM`). **`--inbound-jwt-source`** (string) diff --git a/docs/commands/agentbase/runtime/generate.md b/docs/commands/agentbase/runtime/generate.md index af0b675..b638d14 100644 --- a/docs/commands/agentbase/runtime/generate.md +++ b/docs/commands/agentbase/runtime/generate.md @@ -10,8 +10,12 @@ and apply with `grn agentbase runtime create --file `. The template ships with `imageAuth: {enabled: false}` — the API requires the field, and `enabled: false` is the public-image default. For a private registry, enable it and fill in `username`/`password` (send-only) or set -`useAgentBaseRegistryCredentials`. The same file (minus the create-only `poc`, -keeping `name`) feeds `runtime update --file`. +`useAgentBaseRegistryCredentials`. The template also includes a live +`inboundAuth: {mode: IAM}` block — the create-time default on `PUBLIC` +networks (the field is not available in VPC mode) — so a template-driven +create or update is explicit about inbound auth instead of relying on the +default/carry-over. Drop the block only for VPC specs. The same file (minus +the create-only `poc`, keeping `name`) feeds `runtime update --file`. Defaults to YAML (with comments); pass `-o json` for a JSON skeleton. diff --git a/docs/commands/agentbase/runtime/update.md b/docs/commands/agentbase/runtime/update.md index 093b2c6..fe22e1d 100644 --- a/docs/commands/agentbase/runtime/update.md +++ b/docs/commands/agentbase/runtime/update.md @@ -14,19 +14,35 @@ wholesale (no element-level merge). Set flags only for what you intend to change — e.g. a bare `--env K=V` replaces the whole environment map, it does not add one key. +An unset `--inbound-mode` carries the current version's inbound auth over, +and the carried value follows the target network: moving the runtime to `VPC` +drops it (inbound auth does not apply on a private network; `--inbound-mode` +is an error there), and opening a `VPC` runtime up to `PUBLIC` defaults it to +`IAM` — never silently to `NONE` (say `--inbound-mode NONE` to open it +deliberately). An explicitly-set-but-empty `--inbound-mode` (e.g. an empty +`$INBOUND_MODE` expansion) is rejected instead of being read as unset. + `imageAuth` is the exception — the API never returns the stored password, so username/password image auth cannot be carried over: re-supply `--image-auth-username`/`--image-auth-password` (or use `--file` with a full spec). Without image auth the update re-sends `{enabled: false}`. When `--file` is set it is authoritative — every flag below is ignored, and -the spec is applied as-is. The file must contain `imageUrl` and `flavorId`; a +the spec is applied as-is except for an omitted `inboundAuth` (see below). The +file must contain `imageUrl` and `flavorId`; a `name` in the file is ignored (the runtime is addressed by the `` positional argument and the name is immutable). The file **must include `imageAuth` explicitly**: a missing `imageAuth` would silently wipe the stored credentials, so it is rejected — `{enabled: false}` drops them deliberately, `username`/`password` or `useAgentBaseRegistryCredentials` keeps them. +A spec that omits `inboundAuth` gets the same treatment as an unset +`--inbound-mode`: the current version's `inboundAuth` is carried over (a +stderr note names the source version), defaulting to `IAM` when there is +nothing to carry (`PUBLIC`-network specs only; drop the `inboundAuth` block +when the spec moves `networkConfig` to `VPC` — `inboundAuth` is +`PUBLIC`-network only, and the CLI rejects a spec that combines the two). + > **Notes:** `--min-replicas` is gone (the platform owns the autoscaling floor — > always 1); only `--max-replicas` is configurable. `--poc` is create-only and > cannot be set or cleared on update. @@ -177,7 +193,8 @@ over; setting any network flag replaces the whole `networkConfig`. - Required: No (carried over from the current version when unset) - Default: (empty = carried over) -- Constraints: `VPC` requires `--vpc-id` and `--subnet-id`. +- Constraints: `VPC` requires `--vpc-id` and `--subnet-id`; explicitly set but + empty is rejected. **`--vpc-id`** (string) @@ -212,12 +229,14 @@ Inbound CIDR allowlist (repeatable, max 20). **`--inbound-mode`** (string) `inboundAuth` mode: `NONE`, `IAM`, or `JWT`. Unset = the current version's -`inboundAuth` is carried over. +`inboundAuth` is carried over; on a `VPC` target (set or carried) unset drops +it, and when there is nothing to carry it defaults to `IAM`. - Required: No (carried over from the current version when unset) -- Default: (empty = carried over) +- Default: (empty = carried over; `IAM` when nothing to carry) - Constraints: `JWT` requires the `--inbound-jwt-*` flags; cannot be combined - with them when set to `IAM`/`NONE`. + with them when set to `IAM`/`NONE`; explicitly set but empty is rejected; + rejected on a `VPC` target (inbound auth is `PUBLIC`-network only). **`--inbound-jwt-source`** (string) @@ -283,11 +302,14 @@ Apply a full-spec file (create template minus `name`); authoritative when set. - Required: No - Default: (empty) -- Constraints: when set, the flags above are ignored; `name` must be present - in the file; `imageAuth` must be present in the file (a missing `imageAuth` +- Constraints: when set, the flags above are ignored; `name` is optional in + the file (ignored — the runtime is addressed by the `` positional + argument); `imageAuth` must be present in the file (a missing `imageAuth` would wipe the stored credentials — set `{enabled: false}` to drop them deliberately, or `username`/`password` / `useAgentBaseRegistryCredentials` - to keep them). + to keep them); an omitted `inboundAuth` is carried over from the current + version (defaulted to `IAM` when there is nothing to carry), and a spec that + combines `inboundAuth` with a `VPC` `networkConfig` is rejected. ## Global options diff --git a/go/cmd/agentbase/gateway.go b/go/cmd/agentbase/gateway.go index 5a696a8..bb6f5e7 100644 --- a/go/cmd/agentbase/gateway.go +++ b/go/cmd/agentbase/gateway.go @@ -86,6 +86,9 @@ By default the gateway is built from flags (the simple path: no inline targets). For targets, outbound auth, JWT inbound auth, or anything nested, use --file with a template produced by 'grn agentbase gateway generate'. +Inbound auth defaults to IAM (callers authenticate via the platform IAM); pass +--inbound-mode NONE to open the gateway or JWT to validate caller JWTs. + The gateway is created asynchronously; this command returns as soon as the service accepts the spec (state WAITING_CREATING). Converge with 'grn agentbase gateway wait '.`, @@ -156,12 +159,13 @@ func buildCreateFromFlags(cmd *cobra.Command) (*gatewaypkg.CreateGatewayRequest, } } + // Inbound auth defaults to IAM (secure by default — the flag default does + // it); the flag only needs to be set to opt out (NONE) or switch to JWT. inboundMode, _ := f.GetString("inbound-mode") - inboundMode, err = cliinput.RequireOrPromptString(inboundMode, "--inbound-mode", "Inbound auth mode (NONE|IAM|JWT)") - if err != nil { - return nil, err + inboundMode = strings.ToUpper(strings.TrimSpace(inboundMode)) + if inboundMode != "NONE" && inboundMode != "IAM" && inboundMode != "JWT" { + return nil, fmt.Errorf("--inbound-mode must be NONE, IAM or JWT (got %q)", inboundMode) } - inboundMode = strings.ToUpper(inboundMode) req := &gatewaypkg.CreateGatewayRequest{ Name: name, @@ -183,11 +187,22 @@ func buildCreateFromFlags(cmd *cobra.Command) (*gatewaypkg.CreateGatewayRequest, req.InboundAuth.ClientRedirectURIs = v } if f.Changed("iam-require-owner") { + if inboundMode != "IAM" { + return nil, fmt.Errorf("--iam-require-owner only applies when --inbound-mode is IAM") + } b, _ := f.GetBool("iam-require-owner") req.InboundAuth.IAMRequireOwner = &b } - if inboundMode == "JWT" || f.Changed("jwt-source") || f.Changed("jwt-discovery-url") || f.Changed("jwt-jwks") { + if inboundMode == "JWT" { req.InboundAuth.JWT = buildJWTFromFlags(cmd) + } else { + // Dangling --jwt-*/--allowed-*/--principal-claim flags would otherwise + // be silently dropped (they only mean anything in JWT mode). + for _, flagName := range []string{"jwt-source", "jwt-discovery-url", "jwt-jwks", "allowed-audience", "allowed-client", "allowed-scope", "principal-claim"} { + if f.Changed(flagName) { + return nil, fmt.Errorf("--%s only applies when --inbound-mode is JWT", flagName) + } + } } // PRIVATE-mode private network (vpcId/subnetId sealed at create). @@ -298,7 +313,7 @@ Defaults to YAML (with comments); pass -o json for a JSON skeleton.`, NetworkMode: "PUBLIC", FlavorID: "fill-flavor-id", Replicas: 1, - InboundAuth: gatewaypkg.InboundAuthRequest{Mode: "NONE"}, + InboundAuth: gatewaypkg.InboundAuthRequest{Mode: "IAM"}, } b, err := json.MarshalIndent(example, "", " ") if err != nil { @@ -318,7 +333,8 @@ Defaults to YAML (with comments); pass -o json for a JSON skeleton.`, const gatewayCreateTemplateYAML = `# Gateway create spec. # Fill in and apply with: grn agentbase gateway create --file # -# Required: name, networkMode, flavorId, replicas, inboundAuth.mode +# Required: name, networkMode, flavorId, replicas (inboundAuth.mode defaults +# to IAM) # Sealed at create (recreate to change): name, networkMode, flavorId, replicas, # and (PRIVATE) privateNetwork.vpcId/subnetId. name: my-gateway # 3-40 chars, [a-z0-9-] @@ -331,7 +347,7 @@ policyGroupId: "" # optional; bind a policy group (FK) # Inbound (caller) authentication. inboundAuth: - mode: NONE # NONE | IAM | JWT (required) + mode: IAM # NONE | IAM | JWT (default IAM) # clientRedirectUris: # DCR/authorize allowlist # - https://app.example.com/callback # iamRequireOwner: true # IAM mode only @@ -761,43 +777,59 @@ func loadCreateSpec(data []byte) (*gatewaypkg.CreateGatewayRequest, error) { return nil, fmt.Errorf("invalid gateway spec: %w", err) } if req.Name == "" || req.NetworkMode == "" || req.FlavorID == "" { - return nil, fmt.Errorf("spec is missing required field(s): name, networkMode, flavorId (and replicas/inboundAuth.mode)") + return nil, fmt.Errorf("spec is missing required field(s): name, networkMode, flavorId (and replicas)") + } + // inboundAuth.mode defaults to IAM (secure by default — the API requires + // the field). A jwt block without a mode is a spec error: the mode must + // say JWT for the jwt config to mean anything. + if req.InboundAuth.Mode == "" { + if req.InboundAuth.JWT != nil { + return nil, fmt.Errorf("spec inboundAuth.jwt requires inboundAuth.mode JWT") + } + req.InboundAuth.Mode = "IAM" } return &req, nil } // --------------------------------------------------------------------------- +// addGatewayCreateFlags registers the gateway create flags (kept as a helper +// so tests can register the identical flag set on a scratch command; the +// --file var is shared with update). +func addGatewayCreateFlags(cmd *cobra.Command) { + f := cmd.Flags() + f.StringVarP(&gatewayCreateName, "name", "n", "", "Gateway name (required without --interactive)") + f.String("display-name", "", "Display name") + f.String("description", "", "Description") + f.String("network-mode", "", "Network mode: PUBLIC | PRIVATE (required, sealed)") + f.String("flavor-id", "", "Flavor id (required, sealed)") + f.Int("replicas", 0, "Replica count 1-10 (required, sealed)") + f.String("inbound-mode", "IAM", "Inbound auth mode: NONE | IAM | JWT (default IAM)") + f.StringArray("client-redirect-uri", nil, "Allowed client redirect URI (repeatable)") + f.Bool("iam-require-owner", false, "IAM mode: require the caller to own the resource") + f.String("jwt-source", "", "JWT source: DISCOVERY | JWKS") + f.String("jwt-discovery-url", "", "JWT OIDC discovery URL (DISCOVERY)") + f.String("jwt-jwks", "", "JWT inline JWKS (JWKS)") + f.StringArray("allowed-audience", nil, "Allowed JWT audience (repeatable)") + f.StringArray("allowed-client", nil, "Allowed JWT client id (repeatable)") + f.StringArray("allowed-scope", nil, "Allowed JWT scope (repeatable)") + f.String("principal-claim", "", "JWT principal claim (default sub)") + f.String("policy-group-id", "", "Policy group id to bind (FK)") + f.String("private-vpc-id", "", "PRIVATE mode: VPC id (sealed)") + f.String("private-subnet-id", "", "PRIVATE mode: subnet id (sealed)") + f.StringArray("private-route", nil, "PRIVATE mode: node route CIDR (repeatable)") + f.Bool("public-endpoint-enabled", false, "PRIVATE mode: expose a public endpoint") + f.StringArray("allowed-cidr", nil, "Inbound client-IP allowlist CIDR (repeatable)") + f.StringArray("host-alias", nil, "/etc/hosts override: ip=host1,host2 (repeatable)") + f.StringVar(&gatewayCreateFile, "file", "", "Apply a spec file (see 'generate'); authoritative when set") +} + func init() { AgentbaseCmd.AddCommand(gatewayCmd) // create - create := gatewayCreateCmd - create.Flags().StringVarP(&gatewayCreateName, "name", "n", "", "Gateway name (required without --interactive)") - create.Flags().String("display-name", "", "Display name") - create.Flags().String("description", "", "Description") - create.Flags().String("network-mode", "", "Network mode: PUBLIC | PRIVATE (required, sealed)") - create.Flags().String("flavor-id", "", "Flavor id (required, sealed)") - create.Flags().Int("replicas", 0, "Replica count 1-10 (required, sealed)") - create.Flags().String("inbound-mode", "", "Inbound auth mode: NONE | IAM | JWT (required)") - create.Flags().StringArray("client-redirect-uri", nil, "Allowed client redirect URI (repeatable)") - create.Flags().Bool("iam-require-owner", false, "IAM mode: require the caller to own the resource") - create.Flags().String("jwt-source", "", "JWT source: DISCOVERY | JWKS") - create.Flags().String("jwt-discovery-url", "", "JWT OIDC discovery URL (DISCOVERY)") - create.Flags().String("jwt-jwks", "", "JWT inline JWKS (JWKS)") - create.Flags().StringArray("allowed-audience", nil, "Allowed JWT audience (repeatable)") - create.Flags().StringArray("allowed-client", nil, "Allowed JWT client id (repeatable)") - create.Flags().StringArray("allowed-scope", nil, "Allowed JWT scope (repeatable)") - create.Flags().String("principal-claim", "", "JWT principal claim (default sub)") - create.Flags().String("policy-group-id", "", "Policy group id to bind (FK)") - create.Flags().String("private-vpc-id", "", "PRIVATE mode: VPC id (sealed)") - create.Flags().String("private-subnet-id", "", "PRIVATE mode: subnet id (sealed)") - create.Flags().StringArray("private-route", nil, "PRIVATE mode: node route CIDR (repeatable)") - create.Flags().Bool("public-endpoint-enabled", false, "PRIVATE mode: expose a public endpoint") - create.Flags().StringArray("allowed-cidr", nil, "Inbound client-IP allowlist CIDR (repeatable)") - create.Flags().StringArray("host-alias", nil, "/etc/hosts override: ip=host1,host2 (repeatable)") - create.Flags().StringVar(&gatewayCreateFile, "file", "", "Apply a spec file (see 'generate'); authoritative when set") - gatewayCmd.AddCommand(create) + addGatewayCreateFlags(gatewayCreateCmd) + gatewayCmd.AddCommand(gatewayCreateCmd) // generate gatewayCmd.AddCommand(gatewayGenerateCmd) diff --git a/go/cmd/agentbase/gateway_test.go b/go/cmd/agentbase/gateway_test.go index 961e866..94f7fe2 100644 --- a/go/cmd/agentbase/gateway_test.go +++ b/go/cmd/agentbase/gateway_test.go @@ -7,6 +7,9 @@ import ( "strings" "testing" + "github.com/spf13/cobra" + + "github.com/greennodehub/greennode-cli/internal/agentbase/cliinput" "github.com/greennodehub/greennode-cli/internal/agentbase/output" ) @@ -79,7 +82,7 @@ func TestGatewayGenerate_RoundTripsThroughLoadCreateSpec(t *testing.T) { if req.Name != "my-gateway" || req.NetworkMode != "PUBLIC" || req.FlavorID != "fill-flavor-id" { t.Errorf("round-trip decoded unexpected spec: %+v", req) } - if req.Replicas != 1 || req.InboundAuth.Mode != "NONE" { + if req.Replicas != 1 || req.InboundAuth.Mode != "IAM" { t.Errorf("round-trip decoded unexpected replicas/inbound: %+v", req) } } @@ -210,6 +213,143 @@ func TestLoadCreateSpec_MissingRequiredFails(t *testing.T) { } } +// TestLoadCreateSpec_InboundDefaultsToIAM verifies the inboundAuth defaulting +// in the --file path: a spec without inboundAuth creates with mode IAM (the +// secure default), while a jwt block without a mode is rejected instead of +// being silently paired with the IAM default. +func TestLoadCreateSpec_InboundDefaultsToIAM(t *testing.T) { + req, err := loadCreateSpec([]byte(` +name: default-inbound-gw +networkMode: PUBLIC +flavorId: f1 +replicas: 1 +`)) + if err != nil { + t.Fatalf("loadCreateSpec: %v", err) + } + if req.InboundAuth.Mode != "IAM" || req.InboundAuth.JWT != nil { + t.Errorf("inboundAuth should default to IAM, got: %+v", req.InboundAuth) + } + + _, err = loadCreateSpec([]byte(` +name: jwt-no-mode-gw +networkMode: PUBLIC +flavorId: f1 +replicas: 1 +inboundAuth: + jwt: + source: DISCOVERY + discoveryUrl: https://idp.example.com/.well-known/openid-configuration +`)) + if err == nil { + t.Fatal("expected error for inboundAuth.jwt without inboundAuth.mode") + } + if !strings.Contains(err.Error(), "inboundAuth.mode JWT") { + t.Errorf("expected inboundAuth.mode error, got: %v", err) + } +} + +// newGatewayCreateCmd builds a fresh command with the create flags, for +// testing buildCreateFromFlags in isolation (flag Changed state must not leak +// between subtests). +func newGatewayCreateCmd(t *testing.T) *cobra.Command { + t.Helper() + cmd := &cobra.Command{} + addGatewayCreateFlags(cmd) + return cmd +} + +// TestBuildCreateFromFlags_InboundAuth covers the inbound-auth flag group on +// the gateway create path: unset --inbound-mode defaults to IAM (no prompt), +// NONE opts out (case-normalized), bad modes error, and the JWT / IAM-only +// flags are rejected outside their modes. +func TestBuildCreateFromFlags_InboundAuth(t *testing.T) { + cliinput.SetInteractive(false) + setBase := func(t *testing.T, cmd *cobra.Command) { + t.Helper() + mustSet(t, cmd, "name", "gw-1") + mustSet(t, cmd, "network-mode", "PUBLIC") + mustSet(t, cmd, "flavor-id", "f1") + mustSet(t, cmd, "replicas", "1") + } + t.Run("unset_defaults_to_iam", func(t *testing.T) { + cmd := newGatewayCreateCmd(t) + setBase(t, cmd) + req, err := buildCreateFromFlags(cmd) + if err != nil { + t.Fatalf("buildCreateFromFlags: %v", err) + } + if req.InboundAuth.Mode != "IAM" || req.InboundAuth.JWT != nil || req.InboundAuth.IAMRequireOwner != nil { + t.Errorf("expected plain IAM default, got: %+v", req.InboundAuth) + } + }) + t.Run("none_opt_out_normalized", func(t *testing.T) { + cmd := newGatewayCreateCmd(t) + setBase(t, cmd) + mustSet(t, cmd, "inbound-mode", "none") + req, err := buildCreateFromFlags(cmd) + if err != nil { + t.Fatalf("buildCreateFromFlags: %v", err) + } + if req.InboundAuth.Mode != "NONE" { + t.Errorf("expected NONE (normalized), got: %+v", req.InboundAuth) + } + }) + t.Run("bad_mode_errors", func(t *testing.T) { + cmd := newGatewayCreateCmd(t) + setBase(t, cmd) + mustSet(t, cmd, "inbound-mode", "TUNNEL") + if _, err := buildCreateFromFlags(cmd); err == nil { + t.Error("expected error for bad --inbound-mode") + } + }) + t.Run("jwt_flags_with_default_iam_error", func(t *testing.T) { + cmd := newGatewayCreateCmd(t) + setBase(t, cmd) + mustSet(t, cmd, "jwt-source", "JWKS") + _, err := buildCreateFromFlags(cmd) + if err == nil || !strings.Contains(err.Error(), "only applies when --inbound-mode is JWT") { + t.Errorf("expected jwt-flag guard error, got: %v", err) + } + }) + t.Run("iam_require_owner_with_none_errors", func(t *testing.T) { + cmd := newGatewayCreateCmd(t) + setBase(t, cmd) + mustSet(t, cmd, "inbound-mode", "NONE") + mustSet(t, cmd, "iam-require-owner", "true") + if _, err := buildCreateFromFlags(cmd); err == nil { + t.Error("expected error for --iam-require-owner with NONE") + } + }) + t.Run("iam_require_owner_ok", func(t *testing.T) { + cmd := newGatewayCreateCmd(t) + setBase(t, cmd) + mustSet(t, cmd, "iam-require-owner", "true") + req, err := buildCreateFromFlags(cmd) + if err != nil { + t.Fatalf("buildCreateFromFlags: %v", err) + } + if req.InboundAuth.IAMRequireOwner == nil || !*req.InboundAuth.IAMRequireOwner { + t.Errorf("expected IAMRequireOwner=true, got: %+v", req.InboundAuth) + } + }) + t.Run("jwt_ok", func(t *testing.T) { + cmd := newGatewayCreateCmd(t) + setBase(t, cmd) + mustSet(t, cmd, "inbound-mode", "JWT") + mustSet(t, cmd, "jwt-source", "DISCOVERY") + mustSet(t, cmd, "jwt-discovery-url", "https://idp/.well-known/openid-configuration") + req, err := buildCreateFromFlags(cmd) + if err != nil { + t.Fatalf("buildCreateFromFlags: %v", err) + } + if req.InboundAuth.Mode != "JWT" || req.InboundAuth.JWT == nil || + req.InboundAuth.JWT.Source != "DISCOVERY" || req.InboundAuth.JWT.DiscoveryURL == "" { + t.Errorf("unexpected jwt config: %+v", req.InboundAuth) + } + }) +} + // TestParseHostAliases covers the ip=host1,host2 flag parser. func TestParseHostAliases(t *testing.T) { out, err := parseHostAliases([]string{"10.0.0.1=foo.local,bar.local", "10.0.0.2=solo.local"}) diff --git a/go/cmd/agentbase/runtime.go b/go/cmd/agentbase/runtime.go index 2ce58e2..3192a47 100644 --- a/go/cmd/agentbase/runtime.go +++ b/go/cmd/agentbase/runtime.go @@ -73,9 +73,12 @@ func newRuntimeClient(ctx context.Context, cmd *cobra.Command) (*runtimepkg.Clie // --------------------------------------------------------------------------- // runtimeSpec holds the mutable spec fields common to create and update. -// networkConfig/inboundAuth/protocol/allowedCidrs are only populated when the -// corresponding flags are set (they are optional server-side; omitting them -// keeps the platform defaults: public network, no inbound auth, HTTP). +// networkConfig/protocol/allowedCidrs are only populated when the corresponding +// flags are set (they are optional server-side; omitting them keeps the +// platform defaults: public network, HTTP). inboundAuth defaults to IAM on +// create (PUBLIC networks only — the API rejects inboundAuth in VPC mode); on +// update an unset --inbound-mode stays nil and the current value is carried +// over by buildUpdateRequest. type runtimeSpec struct { Description string ImageURL string @@ -159,7 +162,7 @@ func readRuntimeSpec(cmd *cobra.Command, forUpdate bool) (*runtimeSpec, error) { return nil, err } allowedCidrs, _ := f.GetStringArray("allowed-cidrs") - inbound, err := readInboundAuth(cmd) + inbound, err := readInboundAuth(cmd, forUpdate, network) if err != nil { return nil, err } @@ -197,6 +200,13 @@ func readNetworkConfig(cmd *cobra.Command) (*runtimepkg.NetworkConfig, error) { f := cmd.Flags() mode, _ := f.GetString("network-mode") if mode == "" { + if f.Changed("network-mode") { + // Set-but-empty is a scripting hazard (an empty $NETWORK_MODE + // expansion): it read as unset, which on create silently meant + // PUBLIC while the script meant to set a network. Refuse it like + // --image-url/--flavor-id. + return nil, fmt.Errorf("--network-mode cannot be empty (PUBLIC or VPC; unset means PUBLIC on create and carries the current network over on update)") + } // A dangling --vpc-id/--subnet-id/--route-cidrs without --network-mode // used to be silently ignored — refuse it instead (it would otherwise // read as "leave the network public" while the user meant VPC). @@ -224,15 +234,35 @@ func readNetworkConfig(cmd *cobra.Command) (*runtimepkg.NetworkConfig, error) { }, nil } -// readInboundAuth builds *InboundAuth only when --inbound-mode is set. NONE is +// readInboundAuth builds *InboundAuth from the --inbound-* flag group. NONE is // open, IAM authenticates callers via the platform IAM, JWT validates caller // JWTs (--inbound-jwt-* then required: DISCOVERY needs --inbound-jwt-discovery-url, // JWKS needs --inbound-jwt-jwks). -func readInboundAuth(cmd *cobra.Command) (*runtimepkg.InboundAuth, error) { +// +// Defaulting: on create an unset --inbound-mode defaults to IAM (secure by +// default), but only on PUBLIC networks — the API rejects inboundAuth in VPC +// mode, so a VPC runtime keeps it unset. On update an unset mode stays nil and +// the current version's inboundAuth is carried over by buildUpdateRequest. +// +// A set-but-empty mode is refused on both paths (35388): an empty $INBOUND_MODE +// expansion is a scripting hazard — on update it read as unset and silently +// dropped inboundAuth (the API's full-spec replacement then reset the mode to +// NONE), on create it silently took the IAM default. +func readInboundAuth(cmd *cobra.Command, forUpdate bool, network *runtimepkg.NetworkConfig) (*runtimepkg.InboundAuth, error) { f := cmd.Flags() mode, _ := f.GetString("inbound-mode") if mode == "" { - return nil, nil + if f.Changed("inbound-mode") { + return nil, fmt.Errorf("--inbound-mode cannot be empty (NONE, IAM or JWT; unset defaults to IAM on create and carries the current mode over on update)") + } + if forUpdate { + return nil, nil + } + if network != nil && network.Mode == "VPC" { + // inboundAuth is PUBLIC-only: a VPC runtime must not carry it. + return nil, nil + } + mode = "IAM" } if mode != "NONE" && mode != "IAM" && mode != "JWT" { return nil, fmt.Errorf("--inbound-mode must be NONE, IAM or JWT (got %q)", mode) @@ -353,9 +383,9 @@ request sends {enabled: false} (public image). Network placement: --network-mode VPC with --vpc-id/--subnet-id and optional repeatable --route-cidrs (unset = PUBLIC). Inbound auth: --inbound-mode -NONE/IAM/JWT (JWT adds --inbound-jwt-source DISCOVERY|JWKS and its flags). -Inbound CIDR allowlist: repeatable --allowed-cidrs. Protocol: --protocol -HTTP|MCP. +NONE/IAM/JWT (unset = IAM on PUBLIC networks; not available in VPC mode; JWT +adds --inbound-jwt-source DISCOVERY|JWKS and its flags). Inbound CIDR +allowlist: repeatable --allowed-cidrs. Protocol: --protocol HTTP|MCP. --file applies a full spec instead ('grn agentbase runtime generate' template) — prefer it to keep the image-auth password (and inline JWKS key sets) out of @@ -381,6 +411,13 @@ service accepts the spec (state CREATING). Converge with if req.ImageAuth == nil { req.ImageAuth = &runtimepkg.ImageAuth{Enabled: false} } + // inboundAuth defaults to IAM on create (secure by default), but + // only on PUBLIC networks — the API rejects inboundAuth in VPC + // mode, so a VPC spec stays without it. + vpc := req.NetworkConfig != nil && req.NetworkConfig.Mode == "VPC" + if req.InboundAuth == nil && !vpc { + req.InboundAuth = &runtimepkg.InboundAuth{Mode: "IAM"} + } return createRuntimeAndPrint(ctx, cmd, req) } name, _ := cmd.Flags().GetString("name") @@ -460,7 +497,7 @@ Defaults to YAML (with comments); pass -o json for a JSON skeleton.`, EnvironmentVariables: map[string]string{}, NetworkConfig: &runtimepkg.NetworkConfig{Mode: "PUBLIC"}, AllowedCidrs: []string{}, - InboundAuth: &runtimepkg.InboundAuth{Mode: "NONE"}, + InboundAuth: &runtimepkg.InboundAuth{Mode: "IAM"}, Protocol: "HTTP", FlavorID: "fill-flavor-id", Autoscaling: runtimepkg.Autoscaling{MaxReplicas: 2, CPUUtilization: 70, MemoryUtilization: 70}, @@ -513,8 +550,8 @@ environmentVariables: # map; may be empty # routeCidrs: [] # egress routes (max 20) # allowedCidrs: # optional inbound CIDR allowlist (max 20) # - 10.0.0.0/8 -# inboundAuth: # optional inbound authentication (omit = NONE) -# mode: JWT # NONE | IAM | JWT +inboundAuth: # inbound authentication (PUBLIC networks only; + mode: IAM # default: IAM. NONE | IAM | JWT) # jwt: # required when mode is JWT # source: DISCOVERY # DISCOVERY | JWKS # discoveryUrl: https://idp.example.com/.well-known/openid-configuration @@ -625,9 +662,19 @@ username/password image auth cannot be carried over: re-supply --image-auth-username/--image-auth-password (or --file with a full spec). Without image auth the update re-sends {enabled: false}. +An unset --inbound-mode carries the current version's inbound auth over, and +the carried value follows the target network: moving the runtime to VPC drops +it (inbound auth does not apply on a private network; --inbound-mode is an +error there), and opening a VPC runtime up to PUBLIC defaults it to IAM (never +silently to NONE) — say --inbound-mode NONE to open it deliberately. + --file applies a full spec instead ('grn agentbase runtime generate' template minus name) — it must include imageAuth explicitly, since a missing imageAuth -would silently wipe stored credentials. +would silently wipe stored credentials. An omitted inboundAuth block is not a +reset either: it is carried over from the current version (defaulted to IAM +when there is nothing to carry). Drop the inboundAuth block only when the spec +moves networkConfig to VPC — inboundAuth is PUBLIC-only and the API rejects it +in VPC mode (the CLI errors on that combination). The update is asynchronous; converge with 'grn agentbase runtime wait '.`, Args: cobra.ExactArgs(1), @@ -655,22 +702,28 @@ The update is asynchronous; converge with 'grn agentbase runtime wait '.`, if createReq.ImageAuth == nil { return fmt.Errorf("spec is missing required field: imageAuth (the API never returns the stored password, so it cannot be carried over; set imageAuth explicitly: {enabled: false} drops it, username/password or useAgentBaseRegistryCredentials keeps it)") } + // inboundAuth carry-over (35379): an omitted block used to be + // forwarded as omitted and the API's full-spec replacement reset + // the mode to NONE. Fetch the current version only when there is + // something to resolve — an explicit block or a VPC spec needs + // no round-trip. + var cur *runtimepkg.AgentRuntimeVersionDto + if createReq.InboundAuth == nil && !specTargetsVPC(createReq.NetworkConfig) { + cur, err = latestVersion(ctx, client, id) + if err != nil { + return err + } + } + req, note, err := updateRequestFromFileSpec(createReq, cur) + if err != nil { + return err + } + if note != "" { + fmt.Fprintf(os.Stderr, "Carrying over: %s\n", note) + } // poc is create-only: a spec file carrying poc is accepted for // convenience but the field is not forwarded on update. - return updateRuntimeAndPrint(ctx, client, cmd, id, &runtimepkg.UpdateAgentRuntimeRequest{ - Description: createReq.Description, - ImageURL: createReq.ImageURL, - ImageAuth: createReq.ImageAuth, - Command: createReq.Command, - Args: createReq.Args, - EnvironmentVariables: createReq.EnvironmentVariables, - NetworkConfig: createReq.NetworkConfig, - AllowedCidrs: createReq.AllowedCidrs, - InboundAuth: createReq.InboundAuth, - Protocol: createReq.Protocol, - FlavorID: createReq.FlavorID, - Autoscaling: createReq.Autoscaling, - }) + return updateRuntimeAndPrint(ctx, client, cmd, id, req) } spec, err := readRuntimeSpec(cmd, true) if err != nil { @@ -704,6 +757,56 @@ func updateRuntimeAndPrint(ctx context.Context, client *runtimepkg.Client, cmd * return output.PrintResource(rt, func() string { return rt.ID }, func() error { return renderRuntimeDetail(rt) }) } +// specTargetsVPC reports whether a spec's networkConfig pins the runtime to +// VPC. A nil networkConfig (or PUBLIC) is a public network — including on +// update, where the API's full-spec replacement resets an omitted +// networkConfig to the PUBLIC default rather than keeping the current one. +func specTargetsVPC(network *runtimepkg.NetworkConfig) bool { + return network != nil && network.Mode == "VPC" +} + +// updateRequestFromFileSpec assembles the update request for the --file path. +// The spec is authoritative for the fields it sets; for an inboundAuth it +// omits, it matches the flag path (35379) instead of forwarding the omission: +// the current version's inboundAuth is carried over (cur may be nil when the +// caller determined no round-trip was needed, or when the current version has +// none — then IAM is the default, never a silent NONE), and a VPC spec keeps +// it unset (inboundAuth is PUBLIC-only; an explicit block on a VPC spec is an +// error). Unit-tested in runtime_test.go. +func updateRequestFromFileSpec(createReq *runtimepkg.CreateAgentRuntimeRequest, cur *runtimepkg.AgentRuntimeVersionDto) (*runtimepkg.UpdateAgentRuntimeRequest, string, error) { + vpc := specTargetsVPC(createReq.NetworkConfig) + if createReq.InboundAuth != nil && vpc { + return nil, "", fmt.Errorf("spec sets inboundAuth on a VPC networkConfig: inboundAuth is PUBLIC-only — drop the inboundAuth block (a VPC runtime carries no inbound auth)") + } + req := &runtimepkg.UpdateAgentRuntimeRequest{ + Description: createReq.Description, + ImageURL: createReq.ImageURL, + ImageAuth: createReq.ImageAuth, + Command: createReq.Command, + Args: createReq.Args, + EnvironmentVariables: createReq.EnvironmentVariables, + NetworkConfig: createReq.NetworkConfig, + AllowedCidrs: createReq.AllowedCidrs, + Protocol: createReq.Protocol, + FlavorID: createReq.FlavorID, + Autoscaling: createReq.Autoscaling, + } + var note string + switch { + case createReq.InboundAuth != nil: + req.InboundAuth = createReq.InboundAuth + case vpc: + // A VPC spec must not carry inboundAuth — leave it unset. + case cur != nil && cur.InboundAuth != nil: + req.InboundAuth = versionInboundAuth(cur) + note = fmt.Sprintf("inboundAuth (from version %d)", cur.Version) + default: + req.InboundAuth = &runtimepkg.InboundAuth{Mode: "IAM"} + note = "inboundAuth (defaulted to IAM; current version has none)" + } + return req, note, nil +} + // imageAuthFlags are the flags that explicitly set the imageAuth spec; any of // them marks it as user-provided on update (otherwise it is derived from the // current version). @@ -735,6 +838,12 @@ func anyChanged(changed func(string) bool, names ...string) bool { // imageAuth is the exception: the API never returns the stored password and an // update with enabled=false wipes the stored credentials, so it cannot be // carried over blindly — see carriedImageAuth. +// +// inboundAuth is PUBLIC-only (the API rejects it in VPC mode) and the carried +// value follows the target network: a VPC target drops it (the PUBLIC→VPC +// transition; an explicit --inbound-mode on a VPC target is an error), and a +// PUBLIC target with nothing to carry (a VPC version opening up) defaults to +// IAM like create does instead of silently opening the runtime (35379). func buildUpdateRequest(spec *runtimeSpec, cur *runtimepkg.AgentRuntimeVersionDto, changed func(string) bool) (*runtimepkg.UpdateAgentRuntimeRequest, []string, error) { req := &runtimepkg.UpdateAgentRuntimeRequest{} var carried []string @@ -782,10 +891,28 @@ func buildUpdateRequest(spec *runtimeSpec, cur *runtimepkg.AgentRuntimeVersionDt carried = append(carried, "allowedCidrs") } if changed("inbound-mode") { + if req.NetworkConfig != nil && req.NetworkConfig.Mode == "VPC" { + return nil, nil, fmt.Errorf("--inbound-mode only applies to PUBLIC networks (the update targets VPC; a VPC runtime carries no inbound auth)") + } req.InboundAuth = spec.Inbound } else { - req.InboundAuth = versionInboundAuth(cur) - carried = append(carried, "inboundAuth") + switch { + case req.NetworkConfig != nil && req.NetworkConfig.Mode == "VPC": + // PUBLIC→VPC: inbound auth does not apply on a private network — + // drop the current version's instead of letting the API 400. + if cur.InboundAuth != nil { + carried = append(carried, "inboundAuth (dropped: target network is VPC)") + } + case cur.InboundAuth != nil: + req.InboundAuth = versionInboundAuth(cur) + carried = append(carried, "inboundAuth") + default: + // VPC→PUBLIC (or a version with no inboundAuth at all): nothing to + // carry — default to IAM like create instead of letting the + // omitted field reset the mode to NONE. + req.InboundAuth = &runtimepkg.InboundAuth{Mode: "IAM"} + carried = append(carried, "inboundAuth (defaulted to IAM; current version has none)") + } } if changed("protocol") { req.Protocol = spec.Protocol @@ -1707,8 +1834,10 @@ func addLogFlags(cmd *cobra.Command) { // addRuntimeSpecFlags registers the mutable spec flags shared by create and // update (everything except --name and --poc, which are create-only). // autoscaling has flag defaults so it is always populated without prompting. -// The network/inbound flags stay unset by default (platform defaults: public -// network, no inbound auth, HTTP). +// The network flags stay unset by default (platform default: public network). +// inbound-mode stays unset by default: create then defaults inboundAuth to +// IAM (PUBLIC networks only — the API rejects inboundAuth in VPC mode), while +// update carries the current version's inboundAuth over. func addRuntimeSpecFlags(cmd *cobra.Command) { f := cmd.Flags() f.String("description", "", "Description") @@ -1725,7 +1854,7 @@ func addRuntimeSpecFlags(cmd *cobra.Command) { f.String("subnet-id", "", "networkConfig subnetId (required with --network-mode VPC)") f.StringArray("route-cidrs", nil, "networkConfig egress route CIDR (repeatable, max 20)") f.StringArray("allowed-cidrs", nil, "Inbound CIDR allowlist (repeatable, max 20)") - f.String("inbound-mode", "", "inboundAuth mode: NONE, IAM or JWT (unset = NONE)") + f.String("inbound-mode", "", "inboundAuth mode: NONE, IAM or JWT (default IAM on create; carried over on update)") f.String("inbound-jwt-source", "", "inboundAuth JWT key source: DISCOVERY or JWKS (with --inbound-mode JWT)") f.String("inbound-jwt-jwks", "", "inboundAuth JWT inline key set JSON (with source JWKS, max 32KiB; prefer --file)") f.String("inbound-jwt-discovery-url", "", "inboundAuth JWT OIDC discovery URL (with source DISCOVERY)") diff --git a/go/cmd/agentbase/runtime_test.go b/go/cmd/agentbase/runtime_test.go index 4d14bda..47a12f1 100644 --- a/go/cmd/agentbase/runtime_test.go +++ b/go/cmd/agentbase/runtime_test.go @@ -45,6 +45,11 @@ func TestRuntimeGenerate_RoundTripsThroughLoadRuntimeSpec(t *testing.T) { if req.Name != "my-runtime" || req.ImageURL != "registry.example.com/my-agent:latest" || req.FlavorID != "fill-flavor-id" { t.Errorf("round-trip decoded unexpected spec: %+v", req) } + // 35379: the template ships an explicit inboundAuth block (mode IAM) — it + // must decode too, so a template-driven update never relies on carry-over. + if req.InboundAuth == nil || req.InboundAuth.Mode != "IAM" { + t.Errorf("template inboundAuth should decode to {mode: IAM}: %+v", req.InboundAuth) + } } // TestRuntimeGenerate_PrintsJSONSkeleton verifies -o json prints a JSON skeleton. @@ -245,6 +250,9 @@ func TestReadRuntimeSpec_OK(t *testing.T) { if s.ImageAuth == nil || s.ImageAuth.Enabled { t.Errorf("imageAuth should default to &ImageAuth{Enabled:false}: %+v", s.ImageAuth) } + if s.Inbound == nil || s.Inbound.Mode != "IAM" { + t.Errorf("inboundAuth should default to &InboundAuth{Mode: IAM} on create: %+v", s.Inbound) + } // Ensure the returned spec round-trips into a request struct. _ = runtimepkg.CreateAgentRuntimeRequest{ ImageURL: s.ImageURL, FlavorID: s.FlavorID, Autoscaling: s.Autoscaling, @@ -261,6 +269,15 @@ func TestReadNetworkConfig(t *testing.T) { t.Errorf("expected (nil, nil), got (%+v, %v)", got, err) } }) + t.Run("set_but_empty_errors", func(t *testing.T) { + // Hardening: an empty $NETWORK_MODE expansion used to read as unset + // (PUBLIC on create) while the script meant to set a network. + cmd := newSpecCmd(t) + mustSet(t, cmd, "network-mode", "") + if _, err := readNetworkConfig(cmd); err == nil { + t.Error("expected error for explicitly-set-but-empty --network-mode") + } + }) t.Run("vpc_without_ids_errors", func(t *testing.T) { cmd := newSpecCmd(t) mustSet(t, cmd, "network-mode", "VPC") @@ -291,21 +308,61 @@ func TestReadNetworkConfig(t *testing.T) { }) } -// TestReadInboundAuth covers the --inbound-* flag group: unset → nil, NONE/IAM -// happy paths, JWT validation matrix (source required; DISCOVERY needs URL; -// JWKS needs valid inline JSON; jwt flags rejected for non-JWT modes). +// TestReadInboundAuth covers the --inbound-* flag group: unset → IAM on create +// (PUBLIC networks only — the API rejects inboundAuth in VPC mode) but nil on +// update (carried over), NONE/IAM happy paths, JWT validation matrix (source +// required; DISCOVERY needs URL; JWKS needs valid inline JSON; jwt flags +// rejected for non-JWT modes, including the IAM default). func TestReadInboundAuth(t *testing.T) { - t.Run("unset_returns_nil", func(t *testing.T) { + t.Run("unset_create_defaults_to_iam", func(t *testing.T) { + cmd := newSpecCmd(t) + got, err := readInboundAuth(cmd, false, nil) + if err != nil || got == nil || got.Mode != "IAM" || got.JWT != nil { + t.Errorf("expected (&InboundAuth{Mode: IAM}, nil), got (%+v, %v)", got, err) + } + }) + t.Run("unset_create_vpc_stays_nil", func(t *testing.T) { + cmd := newSpecCmd(t) + got, err := readInboundAuth(cmd, false, &runtimepkg.NetworkConfig{Mode: "VPC"}) + if err != nil || got != nil { + t.Errorf("expected (nil, nil), got (%+v, %v)", got, err) + } + }) + t.Run("unset_update_stays_nil", func(t *testing.T) { cmd := newSpecCmd(t) - got, err := readInboundAuth(cmd) + got, err := readInboundAuth(cmd, true, nil) if err != nil || got != nil { t.Errorf("expected (nil, nil), got (%+v, %v)", got, err) } }) + t.Run("set_but_empty_update_errors", func(t *testing.T) { + // 35388: an empty $INBOUND_MODE expansion used to read as unset and + // silently dropped inboundAuth on update (the API's full-spec + // replacement then reset the mode to NONE). + cmd := newSpecCmd(t) + mustSet(t, cmd, "inbound-mode", "") + if _, err := readInboundAuth(cmd, true, nil); err == nil { + t.Error("expected error for explicitly-set-but-empty --inbound-mode on update") + } + }) + t.Run("set_but_empty_create_errors", func(t *testing.T) { + cmd := newSpecCmd(t) + mustSet(t, cmd, "inbound-mode", "") + if _, err := readInboundAuth(cmd, false, nil); err == nil { + t.Error("expected error for explicitly-set-but-empty --inbound-mode on create") + } + }) + t.Run("unset_with_jwt_flags_errors", func(t *testing.T) { + cmd := newSpecCmd(t) + mustSet(t, cmd, "inbound-jwt-source", "JWKS") + if _, err := readInboundAuth(cmd, false, nil); err == nil { + t.Error("expected error when jwt flags set with --inbound-mode unset (defaults to IAM)") + } + }) t.Run("none_ok", func(t *testing.T) { cmd := newSpecCmd(t) mustSet(t, cmd, "inbound-mode", "NONE") - got, err := readInboundAuth(cmd) + got, err := readInboundAuth(cmd, false, nil) if err != nil || got == nil || got.Mode != "NONE" || got.JWT != nil { t.Errorf("unexpected: (%+v, %v)", got, err) } @@ -313,7 +370,7 @@ func TestReadInboundAuth(t *testing.T) { t.Run("jwt_without_source_errors", func(t *testing.T) { cmd := newSpecCmd(t) mustSet(t, cmd, "inbound-mode", "JWT") - if _, err := readInboundAuth(cmd); err == nil { + if _, err := readInboundAuth(cmd, false, nil); err == nil { t.Error("expected error when JWT without --inbound-jwt-source") } }) @@ -321,7 +378,7 @@ func TestReadInboundAuth(t *testing.T) { cmd := newSpecCmd(t) mustSet(t, cmd, "inbound-mode", "JWT") mustSet(t, cmd, "inbound-jwt-source", "DISCOVERY") - if _, err := readInboundAuth(cmd); err == nil { + if _, err := readInboundAuth(cmd, false, nil); err == nil { t.Error("expected error when DISCOVERY without --inbound-jwt-discovery-url") } }) @@ -332,7 +389,7 @@ func TestReadInboundAuth(t *testing.T) { mustSet(t, cmd, "inbound-jwt-discovery-url", "https://idp/.well-known/openid-configuration") mustSet(t, cmd, "inbound-jwt-audience", "aud1") mustSet(t, cmd, "inbound-jwt-principal-claim", "sub") - got, err := readInboundAuth(cmd) + got, err := readInboundAuth(cmd, false, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -346,7 +403,7 @@ func TestReadInboundAuth(t *testing.T) { mustSet(t, cmd, "inbound-mode", "JWT") mustSet(t, cmd, "inbound-jwt-source", "JWKS") mustSet(t, cmd, "inbound-jwt-jwks", "{not-json") - if _, err := readInboundAuth(cmd); err == nil { + if _, err := readInboundAuth(cmd, false, nil); err == nil { t.Error("expected error for invalid JWKS JSON") } }) @@ -355,7 +412,7 @@ func TestReadInboundAuth(t *testing.T) { mustSet(t, cmd, "inbound-mode", "JWT") mustSet(t, cmd, "inbound-jwt-source", "JWKS") mustSet(t, cmd, "inbound-jwt-jwks", `{"keys":[]}`) - got, err := readInboundAuth(cmd) + got, err := readInboundAuth(cmd, false, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -367,7 +424,7 @@ func TestReadInboundAuth(t *testing.T) { cmd := newSpecCmd(t) mustSet(t, cmd, "inbound-mode", "IAM") mustSet(t, cmd, "inbound-jwt-source", "JWKS") - if _, err := readInboundAuth(cmd); err == nil { + if _, err := readInboundAuth(cmd, false, nil); err == nil { t.Error("expected error when jwt flags set with --inbound-mode IAM") } }) @@ -513,8 +570,11 @@ func testVersion() *runtimepkg.AgentRuntimeVersionDto { Command: []string{"cur-cmd"}, Args: []string{"cur-arg"}, EnvironmentVariables: map[string]string{"CUR": "1"}, + // PUBLIC + inboundAuth is the only real-world combo that carries one + // (the API rejects inboundAuth in VPC mode, so a VPC version never has + // it). NetworkConfig: &runtimepkg.AgentRuntimeNetworkConfigEntity{ - Mode: "VPC", VpcID: "cur-vpc", SubnetID: "cur-sub", RouteCidrs: []string{"10.0.0.0/8"}, + Mode: "PUBLIC", }, AllowedCidrs: []string{"10.0.0.0/8"}, InboundAuth: &runtimepkg.AgentRuntimeInboundAuthDto{ @@ -530,6 +590,17 @@ func testVersion() *runtimepkg.AgentRuntimeVersionDto { } } +// testVPCVersion is the VPC twin of testVersion: a VPC version never carries +// inboundAuth (PUBLIC-only field), so VPC carry/drop tests use it. +func testVPCVersion() *runtimepkg.AgentRuntimeVersionDto { + cur := testVersion() + cur.NetworkConfig = &runtimepkg.AgentRuntimeNetworkConfigEntity{ + Mode: "VPC", VpcID: "cur-vpc", SubnetID: "cur-sub", RouteCidrs: []string{"10.0.0.0/8"}, + } + cur.InboundAuth = nil + return cur +} + // specFromFlags assembles the update-path spec from a flag command. func specFromFlags(t *testing.T, cmd *cobra.Command) *runtimeSpec { t.Helper() @@ -559,8 +630,7 @@ func TestBuildUpdateRequest(t *testing.T) { req.Autoscaling.MaxReplicas != 3 { t.Errorf("carry-over mismatch: %+v", req) } - if req.NetworkConfig == nil || req.NetworkConfig.Mode != "VPC" || req.NetworkConfig.VpcID != "cur-vpc" || - req.NetworkConfig.SubnetID != "cur-sub" || req.NetworkConfig.RouteCidrs[0] != "10.0.0.0/8" { + if req.NetworkConfig == nil || req.NetworkConfig.Mode != "PUBLIC" || req.NetworkConfig.VpcID != "" { t.Errorf("networkConfig carry-over mismatch: %+v", req.NetworkConfig) } if req.InboundAuth == nil || req.InboundAuth.Mode != "JWT" || req.InboundAuth.JWT == nil || @@ -661,6 +731,91 @@ func TestBuildUpdateRequest(t *testing.T) { t.Errorf("autoscaling should be the flag spec: %+v", req.Autoscaling) } }) + t.Run("carried_none_stays_none", func(t *testing.T) { + // A PUBLIC runtime running open (mode NONE) must not be flipped to IAM + // by a routine update — NONE carries verbatim (35379). + cur := testVersion() + cur.InboundAuth = &runtimepkg.AgentRuntimeInboundAuthDto{Mode: "NONE"} + cmd := newSpecCmd(t) + req, carried, err := buildUpdateRequest(specFromFlags(t, cmd), cur, cmd.Flags().Changed) + if err != nil { + t.Fatalf("buildUpdateRequest: %v", err) + } + if req.InboundAuth == nil || req.InboundAuth.Mode != "NONE" { + t.Errorf("NONE should carry verbatim, not default to IAM: %+v", req.InboundAuth) + } + if !slices.Contains(carried, "inboundAuth") { + t.Errorf("carried list missing inboundAuth: %v", carried) + } + }) + t.Run("nothing_set_on_vpc_carries_network_without_inbound", func(t *testing.T) { + // A VPC version never has inboundAuth: an unchanged update carries the + // VPC networkConfig and keeps inboundAuth unset (no IAM default on a + // VPC target). + cmd := newSpecCmd(t) + req, carried, err := buildUpdateRequest(specFromFlags(t, cmd), testVPCVersion(), cmd.Flags().Changed) + if err != nil { + t.Fatalf("buildUpdateRequest: %v", err) + } + if req.NetworkConfig == nil || req.NetworkConfig.Mode != "VPC" || req.NetworkConfig.VpcID != "cur-vpc" || + req.NetworkConfig.SubnetID != "cur-sub" || req.NetworkConfig.RouteCidrs[0] != "10.0.0.0/8" { + t.Errorf("VPC networkConfig should carry over: %+v", req.NetworkConfig) + } + if req.InboundAuth != nil { + t.Errorf("a VPC target must not carry or default inboundAuth: %+v", req.InboundAuth) + } + if slices.Contains(carried, "inboundAuth") { + t.Errorf("carried list should not mention inboundAuth: %v", carried) + } + }) + t.Run("vpc_to_public_defaults_inbound_to_iam", func(t *testing.T) { + // Opening a VPC runtime up to PUBLIC: nothing to carry — default to + // IAM like create instead of letting the omitted field reset the mode + // to NONE (35379 / hardening). + cmd := newSpecCmd(t) + mustSet(t, cmd, "network-mode", "PUBLIC") + req, carried, err := buildUpdateRequest(specFromFlags(t, cmd), testVPCVersion(), cmd.Flags().Changed) + if err != nil { + t.Fatalf("buildUpdateRequest: %v", err) + } + if req.NetworkConfig == nil || req.NetworkConfig.Mode != "PUBLIC" { + t.Errorf("target network should be PUBLIC: %+v", req.NetworkConfig) + } + if req.InboundAuth == nil || req.InboundAuth.Mode != "IAM" || req.InboundAuth.JWT != nil { + t.Errorf("inboundAuth should default to IAM: %+v", req.InboundAuth) + } + if !slices.Contains(carried, "inboundAuth (defaulted to IAM; current version has none)") { + t.Errorf("carried list should note the IAM default: %v", carried) + } + }) + t.Run("public_to_vpc_drops_carried_inbound", func(t *testing.T) { + // Moving to VPC: inbound auth does not apply on a private network — + // drop the current version's instead of letting the API 400. + cmd := newSpecCmd(t) + mustSet(t, cmd, "network-mode", "VPC") + mustSet(t, cmd, "vpc-id", "v1") + mustSet(t, cmd, "subnet-id", "s1") + req, carried, err := buildUpdateRequest(specFromFlags(t, cmd), testVersion(), cmd.Flags().Changed) + if err != nil { + t.Fatalf("buildUpdateRequest: %v", err) + } + if req.InboundAuth != nil { + t.Errorf("inboundAuth should be dropped on a VPC target: %+v", req.InboundAuth) + } + if !slices.Contains(carried, "inboundAuth (dropped: target network is VPC)") { + t.Errorf("carried list should note the drop: %v", carried) + } + }) + t.Run("explicit_inbound_mode_on_vpc_target_errors", func(t *testing.T) { + cmd := newSpecCmd(t) + mustSet(t, cmd, "network-mode", "VPC") + mustSet(t, cmd, "vpc-id", "v1") + mustSet(t, cmd, "subnet-id", "s1") + mustSet(t, cmd, "inbound-mode", "NONE") + if _, _, err := buildUpdateRequest(specFromFlags(t, cmd), testVersion(), cmd.Flags().Changed); err == nil { + t.Error("expected error for --inbound-mode on a VPC target") + } + }) } // TestCarriedImageAuth_UnsetVersion returns {enabled:false} (imageAuth is @@ -674,3 +829,123 @@ func TestCarriedImageAuth_UnsetVersion(t *testing.T) { t.Errorf("expected &ImageAuth{Enabled:false}, got %+v", ia) } } + +// TestUpdateRequestFromFileSpec covers the --file update path (35379): an +// omitted inboundAuth block is carried over from the current version (or +// defaulted to IAM when there is nothing to carry) instead of being forwarded +// as an omission that the API's full-spec replacement resets to NONE; a VPC +// spec keeps inboundAuth unset and refuses an explicit block. +func TestUpdateRequestFromFileSpec(t *testing.T) { + newSpec := func(mutate func(*runtimepkg.CreateAgentRuntimeRequest)) *runtimepkg.CreateAgentRuntimeRequest { + req := &runtimepkg.CreateAgentRuntimeRequest{ + ImageURL: "img:2", + ImageAuth: &runtimepkg.ImageAuth{Enabled: false}, + FlavorID: "f2", + } + if mutate != nil { + mutate(req) + } + return req + } + t.Run("omitted_inbound_carries_from_current", func(t *testing.T) { + req, note, err := updateRequestFromFileSpec(newSpec(nil), testVersion()) + if err != nil { + t.Fatalf("updateRequestFromFileSpec: %v", err) + } + if req.InboundAuth == nil || req.InboundAuth.Mode != "JWT" || req.InboundAuth.JWT == nil || + req.InboundAuth.JWT.Source != "JWKS" || string(req.InboundAuth.JWT.JWKS) != `{"keys": []}` || + req.InboundAuth.JWT.AllowedAudiences[0] != "aud" || req.InboundAuth.JWT.PrincipalClaim != "sub" { + t.Errorf("inboundAuth should carry the JWT config verbatim: %+v", req.InboundAuth) + } + if !strings.Contains(note, "from version 7") { + t.Errorf("note should name the source version: %q", note) + } + }) + t.Run("omitted_inbound_defaults_to_iam_when_nothing_to_carry", func(t *testing.T) { + cur := testVersion() + cur.InboundAuth = nil + req, note, err := updateRequestFromFileSpec(newSpec(nil), cur) + if err != nil { + t.Fatalf("updateRequestFromFileSpec: %v", err) + } + if req.InboundAuth == nil || req.InboundAuth.Mode != "IAM" || req.InboundAuth.JWT != nil { + t.Errorf("inboundAuth should default to IAM, never NONE: %+v", req.InboundAuth) + } + if !strings.Contains(note, "defaulted to IAM") { + t.Errorf("note should name the IAM default: %q", note) + } + }) + t.Run("explicit_inbound_forwarded_verbatim", func(t *testing.T) { + req, note, err := updateRequestFromFileSpec(newSpec(func(r *runtimepkg.CreateAgentRuntimeRequest) { + r.InboundAuth = &runtimepkg.InboundAuth{Mode: "NONE"} + }), testVersion()) + if err != nil { + t.Fatalf("updateRequestFromFileSpec: %v", err) + } + if req.InboundAuth == nil || req.InboundAuth.Mode != "NONE" { + t.Errorf("explicit inboundAuth should replace the current one: %+v", req.InboundAuth) + } + if note != "" { + t.Errorf("explicit inboundAuth needs no carry note: %q", note) + } + }) + t.Run("vpc_spec_keeps_inbound_unset", func(t *testing.T) { + req, note, err := updateRequestFromFileSpec(newSpec(func(r *runtimepkg.CreateAgentRuntimeRequest) { + r.NetworkConfig = &runtimepkg.NetworkConfig{Mode: "VPC", VpcID: "v1", SubnetID: "s1"} + }), testVersion()) + if err != nil { + t.Fatalf("updateRequestFromFileSpec: %v", err) + } + if req.InboundAuth != nil { + t.Errorf("a VPC spec must not carry or default inboundAuth: %+v", req.InboundAuth) + } + if note != "" { + t.Errorf("a VPC spec needs no carry note: %q", note) + } + }) + t.Run("vpc_spec_on_vpc_current_keeps_inbound_unset_without_current", func(t *testing.T) { + // cur == nil: the caller skips the version round-trip on a VPC spec. + req, note, err := updateRequestFromFileSpec(newSpec(func(r *runtimepkg.CreateAgentRuntimeRequest) { + r.NetworkConfig = &runtimepkg.NetworkConfig{Mode: "VPC", VpcID: "v1", SubnetID: "s1"} + }), nil) + if err != nil { + t.Fatalf("updateRequestFromFileSpec: %v", err) + } + if req.InboundAuth != nil || note != "" { + t.Errorf("VPC spec with no current version: inboundAuth=%+v note=%q", req.InboundAuth, note) + } + }) + t.Run("vpc_spec_with_inbound_errors", func(t *testing.T) { + _, _, err := updateRequestFromFileSpec(newSpec(func(r *runtimepkg.CreateAgentRuntimeRequest) { + r.NetworkConfig = &runtimepkg.NetworkConfig{Mode: "VPC", VpcID: "v1", SubnetID: "s1"} + r.InboundAuth = &runtimepkg.InboundAuth{Mode: "IAM"} + }), nil) + if err == nil { + t.Error("expected error for an inboundAuth block on a VPC spec") + } + }) + t.Run("spec_fields_map_to_request", func(t *testing.T) { + req, _, err := updateRequestFromFileSpec(newSpec(func(r *runtimepkg.CreateAgentRuntimeRequest) { + r.Description = "d" + r.Command = []string{"c"} + r.Args = []string{"a"} + r.EnvironmentVariables = map[string]string{"K": "V"} + r.NetworkConfig = &runtimepkg.NetworkConfig{Mode: "PUBLIC"} + r.AllowedCidrs = []string{"10.0.0.0/8"} + r.Protocol = "MCP" + r.Autoscaling = runtimepkg.Autoscaling{MaxReplicas: 4} + }), nil) + if err != nil { + t.Fatalf("updateRequestFromFileSpec: %v", err) + } + if req.Description != "d" || req.ImageURL != "img:2" || req.Command[0] != "c" || req.Args[0] != "a" || + req.EnvironmentVariables["K"] != "V" || req.AllowedCidrs[0] != "10.0.0.0/8" || + req.Protocol != "MCP" || req.FlavorID != "f2" || req.Autoscaling.MaxReplicas != 4 || + req.ImageAuth == nil || req.ImageAuth.Enabled { + t.Errorf("spec fields not mapped to the update request: %+v", req) + } + if req.InboundAuth == nil || req.InboundAuth.Mode != "IAM" { + t.Errorf("PUBLIC spec with no inbound block should default to IAM: %+v", req.InboundAuth) + } + }) +}