Refactor config interpolation in gateway#2761
Conversation
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughGateway configuration now uses explicit TOML interpolation instead of automatic ChangesGateway configuration and provisioning
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
gateway/configs/config-template.toml (1)
14-14: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider generating a unique
gateway_iddynamically.The default
gateway_idis still the static"platform-gateway-id"placeholder. Based on learnings, when deploying multiple gateways against a shared database, ensure that deployment scripts or configurations dynamically generate and inject a unique ID (preferably UUIDv7) via theAPIP_GW_CONTROLLER_SERVER_GATEWAY_IDenvironment variable to prevent identification collisions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gateway/configs/config-template.toml` at line 14, Update the gateway_id configuration in the template to require or generate a deployment-specific unique identifier, preferably UUIDv7, through APIP_GW_CONTROLLER_SERVER_GATEWAY_ID instead of falling back to the static "platform-gateway-id" value. Ensure deployment scripts or configuration inputs inject a distinct ID for each gateway instance.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gateway/configs/config.toml`:
- Around line 39-61: Add the missing [controller.storage.postgres] configuration
block to config.toml with explicit env interpolation tokens for its Postgres
settings, and add deployment_sync_enabled under [controller.controlplane] using
the corresponding environment override token and default. Preserve the existing
configuration structure and naming conventions so Docker Compose users can
configure Postgres and deployment sync through api-platform.env.
---
Nitpick comments:
In `@gateway/configs/config-template.toml`:
- Line 14: Update the gateway_id configuration in the template to require or
generate a deployment-specific unique identifier, preferably UUIDv7, through
APIP_GW_CONTROLLER_SERVER_GATEWAY_ID instead of falling back to the static
"platform-gateway-id" value. Ensure deployment scripts or configuration inputs
inject a distinct ID for each gateway instance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f189a96-dd91-4fe6-a94e-a98395682ccf
⛔ Files ignored due to path filters (2)
gateway/gateway-controller/go.sumis excluded by!**/*.sumgateway/gateway-runtime/policy-engine/go.sumis excluded by!**/*.sum
📒 Files selected for processing (24)
.agents/skills/gateway-debug/SKILL.mddocs/gateway/bottom-up-api-deployment-guide.mddocs/gateway/immutable-gateway.mdgateway/.gitignoregateway/DEBUG_GUIDE.mdgateway/README.mdgateway/configs/config-template.tomlgateway/configs/config.tomlgateway/distribution/docker-compose.yamlgateway/docker-compose-perf.yamlgateway/docker-compose.debug.yamlgateway/docker-compose.yamlgateway/gateway-builder/templates/Dockerfile.gateway-controller.tmplgateway/gateway-controller/README.mdgateway/gateway-controller/go.modgateway/gateway-controller/pkg/config/config.gogateway/gateway-controller/pkg/config/config_test.gogateway/gateway-runtime/README.mdgateway/gateway-runtime/policy-engine/go.modgateway/gateway-runtime/policy-engine/internal/config/config.gogateway/gateway-runtime/policy-engine/internal/config/config_test.gogateway/it/test-config.tomlgateway/it/test-config.vhosts-multi.tomlgateway/it/test-config.vhosts-single.toml
💤 Files with no reviewable changes (2)
- gateway/gateway-runtime/policy-engine/go.mod
- gateway/gateway-controller/go.mod
Dependency Validation ResultsDependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: google.golang.org/grpc Dependency name: github.com/gorilla/websocket Dependency name: github.com/testcontainers/testcontainers-go/modules/compose Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: go.opentelemetry.io/otel Dependency name: go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc Dependency name: go.opentelemetry.io/otel/sdk Dependency name: go.opentelemetry.io/otel/trace Dependency name: go.opentelemetry.io/proto/otlp Dependency name: google.golang.org/grpc Dependency name: github.com/testcontainers/testcontainers-go/modules/compose Next Steps
|
1 similar comment
Dependency Validation ResultsDependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: google.golang.org/grpc Dependency name: github.com/gorilla/websocket Dependency name: github.com/testcontainers/testcontainers-go/modules/compose Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: go.opentelemetry.io/otel Dependency name: go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc Dependency name: go.opentelemetry.io/otel/sdk Dependency name: go.opentelemetry.io/otel/trace Dependency name: go.opentelemetry.io/proto/otlp Dependency name: google.golang.org/grpc Dependency name: github.com/testcontainers/testcontainers-go/modules/compose Next Steps
|
e45976c to
66eda2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
kubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yaml (2)
114-119: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy liftMissing encryption key volume mount in the static manifest.
The changes in
keymgmt.gomake the AES-256 encryption key file mandatory; the gateway-controller will unconditionally exit at startup if it is missing. While the Helm chart was updated to enforce and mount this key secret, this static operator manifest lacks the corresponding volume and volume mount for the encryption key.Deploying this manifest will result in a
CrashLoopBackOffas the controller fails to find the key file. Please add a volume (e.g., from a Kubernetes Secret) and mount it to the expected path in this manifest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yaml` around lines 114 - 119, The static gateway-controller manifest is missing the mandatory AES-256 encryption key Secret mount. Update the container volumeMounts near /app/config/config.yaml to mount the key at the path expected by keymgmt.go, and add the corresponding Secret-backed volume in the pod spec using the existing manifest naming conventions.
149-149: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUpdate the mounted controller config to TOML
api-platform-gateway-k8s-manifests.yaml:149still embeds YAML asconfig.yaml, butgateway-controllerloads config with a TOML parser. Convert this manifest payload to TOML and rename the key/path toconfig.tomlso the controller can start with the shipped config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yaml` at line 149, Update the embedded controller configuration in the manifest’s config map from YAML to TOML, and rename the `config.yaml` key and corresponding mounted path to `config.toml`. Preserve all existing configuration values while converting their syntax to valid TOML so gateway-controller can load the shipped config.
🧹 Nitpick comments (2)
portals/ai-workspace/src/pages/appShell/appShellPages/gateways/GatewaySetupSteps.tsx (1)
127-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIcon-only copy button needs an accessible name.
The
IconButtonrenders only a<Copy />glyph with no text, so screen readers announce nothing actionable. You already havecopyLabelin scope — reuse it as the accessible label.♿ Proposed fix
<IconButton size="small" + aria-label={copyLabel} onClick={() => onCopy(clipboardText, copyLabel)} > <Copy /> </IconButton>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/ai-workspace/src/pages/appShell/appShellPages/gateways/GatewaySetupSteps.tsx` around lines 127 - 135, Update the IconButton in GatewaySetupSteps to provide an accessible name using the existing copyLabel value, while preserving its current size, click handler, and Copy icon behavior.kubernetes/helm/resources/apim-gateway-api-operator-demo/README.md (1)
63-68: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueCleanup the raw encryption key file after secret creation.
Both demo README files instruct users to create an AES-256 encryption key on disk but do not include a cleanup step to remove the raw key file afterward. Leaving the key on the filesystem exposes the at-rest encryption secret.
kubernetes/helm/resources/apim-gateway-api-operator-demo/README.md#L63-L68: appendrm default-aesgcm256-v1.binto the bash snippet to clean up the file.kubernetes/helm/resources/gateway-api-operator-demo/README.md#L87-L92: appendrm default-aesgcm256-v1.binto the bash snippet to clean up the file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kubernetes/helm/resources/apim-gateway-api-operator-demo/README.md` around lines 63 - 68, Add a cleanup command after secret creation in both README bash snippets: kubernetes/helm/resources/apim-gateway-api-operator-demo/README.md lines 63-68 and kubernetes/helm/resources/gateway-api-operator-demo/README.md lines 87-92. Remove default-aesgcm256-v1.bin after kubectl create secret completes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/gateway/quick-start-guide.md`:
- Around line 135-139: Remove the https:// scheme from the
APIP_GW_CONTROLLER_CONTROLPLANE_HOST example so it uses plain host:port
formatting in docs/gateway/quick-start-guide.md (lines 135-139) and
gateway/distribution/README.md (lines 87-91); keep the token example unchanged.
In `@kubernetes/conformance/runner/go.mod`:
- Around line 66-69: Update the k8s.io/apiextensions-apiserver dependency in the
Kubernetes module requirements to v0.35.2, aligning it with k8s.io/api,
k8s.io/apimachinery, and k8s.io/client-go while preserving the existing indirect
dependency designation.
In `@kubernetes/helm/gateway-helm-chart/README.md`:
- Around line 37-42: Remove the temporary unencrypted key file immediately after
each Kubernetes Secret creation command: add the cleanup step at
kubernetes/helm/gateway-helm-chart/README.md lines 37-42 and 67-76, and
kubernetes/helm/resources/apim-apigateway-restapi-operator-demo/README.md lines
43-48. Also inspect other demo README instructions, including
gateway-api-operator-demo/README.md, and apply the same cleanup wherever this
key-generation and Secret-upload pattern appears.
- Around line 136-140: Update the secret verification command in the README to
use kubectl describe secret instead of extracting and printing secret data.
Revise the accompanying guidance to confirm default-aesgcm256-v1.bin appears
under Data with 32 bytes, while retaining the controller-log check for
encryption-key errors.
---
Outside diff comments:
In
`@kubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yaml`:
- Around line 114-119: The static gateway-controller manifest is missing the
mandatory AES-256 encryption key Secret mount. Update the container volumeMounts
near /app/config/config.yaml to mount the key at the path expected by
keymgmt.go, and add the corresponding Secret-backed volume in the pod spec using
the existing manifest naming conventions.
- Line 149: Update the embedded controller configuration in the manifest’s
config map from YAML to TOML, and rename the `config.yaml` key and corresponding
mounted path to `config.toml`. Preserve all existing configuration values while
converting their syntax to valid TOML so gateway-controller can load the shipped
config.
---
Nitpick comments:
In `@kubernetes/helm/resources/apim-gateway-api-operator-demo/README.md`:
- Around line 63-68: Add a cleanup command after secret creation in both README
bash snippets:
kubernetes/helm/resources/apim-gateway-api-operator-demo/README.md lines 63-68
and kubernetes/helm/resources/gateway-api-operator-demo/README.md lines 87-92.
Remove default-aesgcm256-v1.bin after kubectl create secret completes.
In
`@portals/ai-workspace/src/pages/appShell/appShellPages/gateways/GatewaySetupSteps.tsx`:
- Around line 127-135: Update the IconButton in GatewaySetupSteps to provide an
accessible name using the existing copyLabel value, while preserving its current
size, click handler, and Copy icon behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 63f562ec-57d8-4505-abcb-b29ca52134be
⛔ Files ignored due to path filters (19)
cli/it/go.sumis excluded by!**/*.sumcli/src/go.sumis excluded by!**/*.sumcommon/go.sumis excluded by!**/*.sumevent-gateway/gateway-builder/go.sumis excluded by!**/*.sumevent-gateway/gateway-controller/go.sumis excluded by!**/*.sumevent-gateway/gateway-runtime/go.sumis excluded by!**/*.sumevent-gateway/it/go.sumis excluded by!**/*.sumgateway/gateway-builder/go.sumis excluded by!**/*.sumgateway/gateway-controller/go.sumis excluded by!**/*.sumgateway/gateway-runtime/policy-engine/go.sumis excluded by!**/*.sumgateway/it/go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumkubernetes/conformance/runner/go.sumis excluded by!**/*.sumkubernetes/gateway-operator/go.sumis excluded by!**/*.sumplatform-api/go.sumis excluded by!**/*.sumsdk/ai/go.sumis excluded by!**/*.sumtests/ai-workspace-cli-e2e/go.sumis excluded by!**/*.sumtests/integration-e2e/go.sumis excluded by!**/*.sumtests/mock-servers/mock-platform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (72)
.agents/skills/gateway-debug/SKILL.mdcli/it/go.modcli/src/go.modcommon/go.moddocs/gateway/bottom-up-api-deployment-guide.mddocs/gateway/immutable-gateway.mddocs/gateway/quick-start-guide.mdevent-gateway/docker-compose.dev.yamlevent-gateway/docker-compose.test.postgres.yamlevent-gateway/docker-compose.test.sqlserver.yamlevent-gateway/docker-compose.yamlevent-gateway/gateway-builder/go.modevent-gateway/gateway-controller/go.modevent-gateway/gateway-runtime/go.modevent-gateway/it/go.modgateway/.gitignoregateway/DEBUG_GUIDE.mdgateway/Makefilegateway/README.mdgateway/configs/config-template.tomlgateway/configs/config.tomlgateway/distribution/README.mdgateway/distribution/docker-compose.yamlgateway/docker-compose-perf.yamlgateway/docker-compose.debug.yamlgateway/docker-compose.yamlgateway/gateway-builder/go.modgateway/gateway-builder/templates/Dockerfile.gateway-controller.tmplgateway/gateway-controller/README.mdgateway/gateway-controller/go.modgateway/gateway-controller/pkg/config/config.gogateway/gateway-controller/pkg/config/config_test.gogateway/gateway-controller/pkg/encryption/aesgcm/keymgmt.gogateway/gateway-controller/pkg/encryption/aesgcm/keymgmt_test.gogateway/gateway-runtime/README.mdgateway/gateway-runtime/policy-engine/go.modgateway/gateway-runtime/policy-engine/internal/config/config.gogateway/gateway-runtime/policy-engine/internal/config/config_test.gogateway/it/go.modgateway/it/test-config.tomlgateway/it/test-config.vhosts-multi.tomlgateway/it/test-config.vhosts-single.tomlgateway/scripts/setup.shkubernetes/conformance/README.mdkubernetes/conformance/install-wso2-gateway.shkubernetes/conformance/runner/go.modkubernetes/gateway-operator/config/gateway_values.yamlkubernetes/gateway-operator/config/samples/operator-config.yamlkubernetes/gateway-operator/docs/CONFIGURATION.mdkubernetes/gateway-operator/go.modkubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yamlkubernetes/helm/gateway-helm-chart/README.mdkubernetes/helm/gateway-helm-chart/templates/gateway/controller/deployment.yamlkubernetes/helm/gateway-helm-chart/templates/gateway/gateway-config.yamlkubernetes/helm/gateway-helm-chart/values-local.yamlkubernetes/helm/gateway-helm-chart/values.yamlkubernetes/helm/operator-helm-chart/values.yamlkubernetes/helm/resources/apigateway-restapi-operator-demo/01-gateway-values-configmap.yamlkubernetes/helm/resources/apigateway-restapi-operator-demo/README.mdkubernetes/helm/resources/apim-apigateway-restapi-operator-demo/01-gateway-values-configmap.yamlkubernetes/helm/resources/apim-apigateway-restapi-operator-demo/README.mdkubernetes/helm/resources/apim-gateway-api-operator-demo/01-gateway-values-configmap.yamlkubernetes/helm/resources/apim-gateway-api-operator-demo/README.mdkubernetes/helm/resources/gateway-api-operator-demo/01-gateway-values-configmap.yamlkubernetes/helm/resources/gateway-api-operator-demo/README.mdplatform-api/go.modportals/ai-workspace/src/pages/appShell/appShellPages/gateways/GatewaySetupSteps.tsxportals/ai-workspace/src/pages/appShell/appShellPages/gateways/ViewGateway.tsxsdk/ai/go.modtests/ai-workspace-cli-e2e/go.modtests/integration-e2e/go.modtests/mock-servers/mock-platform-api/go.mod
💤 Files with no reviewable changes (4)
- event-gateway/docker-compose.dev.yaml
- event-gateway/docker-compose.yaml
- event-gateway/docker-compose.test.sqlserver.yaml
- event-gateway/docker-compose.test.postgres.yaml
🚧 Files skipped from review as they are similar to previous changes (19)
- gateway/.gitignore
- gateway/docker-compose.debug.yaml
- gateway/it/test-config.toml
- gateway/gateway-builder/templates/Dockerfile.gateway-controller.tmpl
- gateway/README.md
- gateway/distribution/docker-compose.yaml
- gateway/gateway-runtime/policy-engine/internal/config/config.go
- docs/gateway/immutable-gateway.md
- docs/gateway/bottom-up-api-deployment-guide.md
- gateway/gateway-runtime/policy-engine/internal/config/config_test.go
- gateway/gateway-runtime/README.md
- gateway/gateway-controller/pkg/config/config_test.go
- gateway/it/test-config.vhosts-single.toml
- gateway/docker-compose-perf.yaml
- gateway/gateway-controller/README.md
- gateway/it/test-config.vhosts-multi.toml
- gateway/configs/config-template.toml
- gateway/gateway-controller/pkg/config/config.go
- gateway/DEBUG_GUIDE.md
| The blocks below extract those env vars and run `dlv` from a single Bash | ||
| invocation. | ||
|
|
||
| > ⚠️ **Config change — the `APIP_GW_` prefix override was removed.** The gateway loaders now read |
There was a problem hiding this comment.
Instead of stating prefix override was removed I guess we have to update the skill to use proper configs.
3 participants
Summary
Completes the gateway's move to the config-interpolation model across every deployment surface (Go, Docker Compose, Kubernetes Helm/operator, conformance, AI Workspace, and docs). The legacy koanf
APIP_GW_*env-override layer is removed.development/demo modeis removed, and at-rest AES-256 encryption is now mandatory and fail-closed - nothing is auto-generated; a provisioning step (setup.sh for Compose, a Secret for Kubernetes) supplies the key.Approach
Gateway (Go) — gateway/gateway-controller, gateway/gateway-runtime/policy-engine
Provisioning & Compose — gateway/
Kubernetes — kubernetes/helm, kubernetes/gateway-operator, kubernetes/conformance
AI Workspace — portals/ai-workspace/.../gateways
Docs
Breaking changes / upgrade notes