fix bug - #3108
Closed
tharindu1st wants to merge 1 commit into
Closed
Conversation
tharindu1st
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Induwara04,
Krishanx92,
PasanT9,
Piumal1999,
RakhithaRR,
Tharsanan1,
Thushani-Jayasekera,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
lasanthaS,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub and
thivindu
as code owners
August 4, 2026 04:31
Contributor
📝 WalkthroughWalkthroughThe change adds independent enablement for controller and policy-engine ChangesAdmin endpoint configuration and enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GatewayRuntime
participant Envoy
participant HealthCheck
participant AdminClient
GatewayRuntime->>Envoy: Inject admin listener when ROUTER_ADMIN_ENABLED=true
HealthCheck->>Envoy: Request /ready when admin is enabled
HealthCheck->>Envoy: Check router listener over TCP when admin is disabled
AdminClient->>Envoy: Access loopback-bound admin endpoint
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
gateway/gateway-runtime/policy-engine/internal/admin/server_test.go (1)
79-79: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a disabled-path regression test.
The changed test proves only enabled behavior. Add or verify a test that leaves
ConfigDump.Enabledfalse and asserts/config_dumpreturns 404. This protects the policy-engine default-off contract.🤖 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/gateway-runtime/policy-engine/internal/admin/server_test.go` at line 79, Add a regression test alongside the existing config dump test that keeps ConfigDump.Enabled false and asserts the /config_dump request returns HTTP 404, preserving the policy-engine default-off behavior while retaining the enabled-path coverage.gateway/gateway-runtime/policy-engine/internal/admin/server.go (1)
97-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd disabled
/config_dumpcoverage.
TestServer_StartAndStoponly exercisesConfigDump.Enabled = true. Add a server/config whereConfigDump.Enabledis false and assertGET /config_dumpreturns404without calling the dump handler.🤖 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/gateway-runtime/policy-engine/internal/admin/server.go` around lines 97 - 109, Extend TestServer_StartAndStop with a configuration whose ConfigDump.Enabled is false, issue GET /config_dump, and assert a 404 response while verifying the dump handler is not invoked. Keep the existing enabled configuration coverage unchanged.
🤖 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-template.toml`:
- Around line 17-31: Keep controller.admin_server.config_dump.enabled disabled
unless /config_dump is protected by real authentication and authorization.
Update the admin-server configuration around
[controller.admin_server.config_dump] to require the existing policy-engine auth
controls before serving the dump, treating allowed_ips as defense-in-depth only;
do not enable the endpoint based on the IP allowlist alone.
In `@gateway/configs/config.toml`:
- Around line 26-30: Update the config_dump settings in the
policy_engine.admin.config_dump configuration to default to disabled by setting
the relevant enabled flags to false. Preserve explicit local and integration
overrides as the mechanism for enabling both /config_dump routes.
In `@gateway/gateway-runtime/docker-entrypoint.sh`:
- Around line 130-132: Restrict ROUTER_ADMIN_HOST to an explicit loopback
literal before the admin configuration is injected, rejecting or overriding any
non-loopback value while preserving the default. Apply this validation in
gateway/gateway-runtime/docker-entrypoint.sh lines 130-132 and
gateway/gateway-runtime/docker-entrypoint-debug.sh lines 101-103.
In `@gateway/gateway-runtime/router/config/envoy-bootstrap.yaml`:
- Around line 19-24: Validate ROUTER_ADMIN_HOST in both docker-entrypoint.sh and
docker-entrypoint-debug.sh before generating or injecting the Envoy admin
configuration, accepting only loopback addresses and exiting with an error for
any other value. Preserve the existing 127.0.0.1 default and ensure validation
occurs before starting Envoy.
- Around line 19-24: Update the admin-enablement logic in docker-entrypoint.sh
and docker-entrypoint-debug.sh to validate ROUTER_ADMIN_HOST and reject any
value other than the loopback address 127.0.0.1 before injecting the Envoy admin
configuration. Preserve the existing defaulting and startup behavior for the
valid loopback value.
In `@gateway/it/docker-compose.test.yaml`:
- Around line 108-109: Update the readiness mechanism used by the integration
stacks so setup.go’s localhost:9901 poll executes from within gateway-runtime or
uses an explicitly reachable test-only endpoint, while preserving Envoy’s
production loopback binding. Apply the same fix in
gateway/it/docker-compose.test.yaml (108-109),
gateway/it/docker-compose.test.postgres.yaml (196-197),
gateway/it/docker-compose.test.sqlserver.yaml (176-177),
gateway/it/docker-compose.test.vhosts-multi.yaml (78-79), and
gateway/it/docker-compose.test.vhosts-single.yaml (78-79); each site requires
the corresponding readiness-path update.
In
`@kubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yaml`:
- Around line 37-40: Update the Deployment liveness and readiness probes to
avoid kubelet HTTP requests to the loopback-only Envoy admin port 9901: target
the runtime health endpoint through a listener reachable from the kubelet, or
convert both probes to container-local exec checks while preserving
ROUTER_ADMIN_ENABLED=false.
In `@kubernetes/helm/gateway-helm-chart/values.yaml`:
- Around line 884-889: Align Envoy admin port exposure with ROUTER_ADMIN_HOST:
in gateway/docker-compose.yaml (69-72), gateway/distribution/docker-compose.yaml
(66-68), gateway/docker-compose-perf.yaml (69-71), and
gateway/docker-compose.debug.yaml (73-75), remove the published 9901 port unless
the listener is configured to a supported non-loopback address; in
kubernetes/helm/gateway-helm-chart/values.yaml (884-889), keep the Helm Service
exposure toggle consistent with that listener-address requirement rather than
enabling reachability independently.
---
Nitpick comments:
In `@gateway/gateway-runtime/policy-engine/internal/admin/server_test.go`:
- Line 79: Add a regression test alongside the existing config dump test that
keeps ConfigDump.Enabled false and asserts the /config_dump request returns HTTP
404, preserving the policy-engine default-off behavior while retaining the
enabled-path coverage.
In `@gateway/gateway-runtime/policy-engine/internal/admin/server.go`:
- Around line 97-109: Extend TestServer_StartAndStop with a configuration whose
ConfigDump.Enabled is false, issue GET /config_dump, and assert a 404 response
while verifying the dump handler is not invoked. Keep the existing enabled
configuration coverage unchanged.
🪄 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 Plus
Run ID: 49c17dbb-4513-48aa-8fc7-09a899ff6777
📒 Files selected for processing (31)
gateway/configs/config-template.tomlgateway/configs/config.tomlgateway/distribution/docker-compose.yamlgateway/docker-compose-perf.yamlgateway/docker-compose.debug.yamlgateway/docker-compose.yamlgateway/gateway-controller/pkg/adminserver/server.gogateway/gateway-controller/pkg/adminserver/server_test.gogateway/gateway-controller/pkg/config/config.gogateway/gateway-runtime/docker-entrypoint-debug.shgateway/gateway-runtime/docker-entrypoint.shgateway/gateway-runtime/health-check.shgateway/gateway-runtime/policy-engine/configs/envoy-backup.yamlgateway/gateway-runtime/policy-engine/configs/envoy.yamlgateway/gateway-runtime/policy-engine/internal/admin/server.gogateway/gateway-runtime/policy-engine/internal/admin/server_test.gogateway/gateway-runtime/policy-engine/internal/config/config.gogateway/gateway-runtime/router/config/envoy-bootstrap.yamlgateway/it/docker-compose.test.postgres.yamlgateway/it/docker-compose.test.sqlserver.yamlgateway/it/docker-compose.test.vhosts-multi.yamlgateway/it/docker-compose.test.vhosts-single.yamlgateway/it/docker-compose.test.yamlgateway/it/test-config.tomlgateway/it/test-config.vhosts-multi.tomlgateway/it/test-config.vhosts-single.tomlkubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yamlkubernetes/helm/gateway-helm-chart/README.mdkubernetes/helm/gateway-helm-chart/templates/gateway/gateway-config.yamlkubernetes/helm/gateway-helm-chart/templates/gateway/gateway-runtime/deployment.yamlkubernetes/helm/gateway-helm-chart/values.yaml
Comment on lines
+17
to
+31
| # Dedicated admin/debug HTTP server for config dump and xDS sync endpoints. | ||
| # Kept enabled by default because it also serves /health, used by Kubernetes | ||
| # liveness/readiness probes; see admin_server.config_dump below to gate the | ||
| # sensitive /config_dump route specifically. | ||
| enabled = true | ||
| port = 9092 | ||
| allowed_ips = ["*"] | ||
|
|
||
| [controller.admin_server.config_dump] | ||
| # The /config_dump route returns a full snapshot of deployed APIs, policies, | ||
| # and resolved configuration. Off by default — enable only when needed for | ||
| # debugging, and prefer reaching it via `kubectl port-forward` over exposing | ||
| # the admin Service port. | ||
| enabled = false | ||
|
|
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
# Expect: production constructors enforce authentication for enabled config dumps.
rg -n -C 8 \
'NewServer\(|configDumpEnabledMiddleware|ConfigDump|AllowedIPs|BasicAuth|auth' \
gateway/gateway-controller gateway/gateway-runtime || trueRepository: wso2/api-platform
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -u
# Locate the rendered template and focused admin server implementation/search terms without matching every test.
fd -a 'config-template\.toml|server|admin|config_dump|ConfigDump' gateway | sed 's#^\./##' | head -200
echo "---- template relevant sections ----"
if [ -f gateway/configs/config-template.toml ]; then
nl -ba gateway/configs/config-template.toml | sed -n '1,80p;300,360p'
fi
echo "---- focused source terms in go files ----"
rg -n -C 6 \
'ConfigDump|config_dump|config-dump|AllowedIPs|allowed_ips|NewServer|BasicAuth|Allowlisted|AllowlistedIP|ClientIP|IsAllowedIP' \
--glob '*.go' gateway/gateway-controller gateway/gateway-runtime | head -500Repository: wso2/api-platform
Length of output: 46098
🏁 Script executed:
#!/bin/bash
set -u
echo "---- config template relevant sections ----"
sed -n '1,80p;300,360p' gateway/configs/config-template.toml
echo "---- controller admin server implementation outline ----"
ast-grep outline gateway/gateway-controller/pkg/adminserver/server.go || true
echo "---- controller admin server implementation ----"
sed -n '1,260p' gateway/gateway-controller/pkg/adminserver/server.go
echo "---- policy engine admin implementation outline ----"
ast-grep outline gateway/gateway-runtime/policy-engine/internal/admin/server.go || true
echo "---- policy engine admin implementation ----"
sed -n '1,320p' gateway/gateway-runtime/policy-engine/internal/admin/server.go
echo "---- config template admin auth validation references ----"
rg -n -C 5 -C 8 'AdminServer:|basic_auth|allowed_ips|allowed_ips|ConfigDump|config_dump|xds_sync_status' \
gateway/gateway-controller/pkg/config gateway/gateway-runtime/policy-engine --glob '*.go' \
| head -600Repository: wso2/api-platform
Length of output: 50373
Require authentication before enabling the policy-engine config dump.
gateway/configs/config-template.toml exposes allowed_ips = ["*", "127.0.0.1"] and policy_engine.admin.config_dump.enabled = false, but the enabled flag alone still serves the full policy-chain/route dump from any allowed origin. The policy-engine admin server only applies the IP allowlist and does not attach real authentication; enforce authentication and authorization for /config_dump before enabling it, since the IP allowlist is only defense-in-depth.
Applies to: lines 17-31 and 325-338.
🤖 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` around lines 17 - 31, Keep
controller.admin_server.config_dump.enabled disabled unless /config_dump is
protected by real authentication and authorization. Update the admin-server
configuration around [controller.admin_server.config_dump] to require the
existing policy-engine auth controls before serving the dump, treating
allowed_ips as defense-in-depth only; do not enable the endpoint based on the IP
allowlist alone.
Source: Coding guidelines
Comment on lines
+26
to
+30
| [policy_engine.admin.config_dump] | ||
| # Enabled for local development convenience. Off by default in production | ||
| # (see config-template.toml) — /config_dump returns 404 when disabled. | ||
| enabled = true | ||
|
|
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
# Expect: only local or integration paths load gateway/configs/config.toml.
rg -n -C 5 -- \
'config\.toml|config-template\.toml|CONFIG_FILE|CONFIG_PATH|--config' . || trueRepository: wso2/api-platform
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
echo "== candidate files =="
git ls-files | rg '^(gateway|helm|templates|kubernetes)/|configs/config\.toml|config-template\.toml|configLoader|loadEnv|configLoader' || true
echo "== exact config loader / path references in gateway =="
rg -n --glob '*.go' --glob '*.toml' --glob '*.yaml' --glob '*.yml' --glob 'Dockerfile' --glob 'Makefile' \
'gateway/configs/config\.toml|config-template\.toml|policy_engine\.admin\.config_dump|config_dump|CONFIG_FILE|CONFIG_PATH|--config|pol\.config' gateway kubernetes helm templates 2>/dev/null || trueRepository: wso2/api-platform
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -u
echo "== gateway/configs/config.toml relevant sections =="
sed -n '1,80p' gateway/configs/config.toml
echo "== gateway/configs/config-template.toml relevant sections =="
rg -n -C 3 'config_dump|dump|admin' gateway/configs/config-template.toml || true
echo "== git diff for affected files if available =="
git diff -- gateway/configs/config.toml gateway/configs/config-template.toml gateway/gateway-controller/pkg/config/config.go gateway/gateway-runtime/policy-engine/internal/config/config.go | sed -n '1,240p' || true
echo "== focused references to gateway/configs/config.toml and admin config dump defaults =="
rg -n --glob '*.go' --glob '*.toml' --glob '*.yaml' --glob '*.yml' --glob 'Dockerfile' --glob 'Makefile' --glob '*.md' \
'gateway/configs/config\.toml|gateway/configs/config-template\.toml|config_dump\.enabled|policy_engine\.admin\.config_dump|ConfigDump|admin.*config.*dump' .Repository: wso2/api-platform
Length of output: 27998
🏁 Script executed:
#!/bin/bash
set -u
echo "== Makefile dist/config generation =="
sed -n '1,180p' Makefile | rg -n -C 6 'copy|DIST|configs/gateway-controller/config.toml|cp gateway/configs/config.toml' || true
echo "== helm generated config relevant section =="
sed -n '1,60p' kubernetes/helm/gateway-helm-chart/templates/gateway/gateway-config.yaml
sed -n '270,300p' kubernetes/helm/gateway-helm-chart/templates/gateway/gateway-config.yaml
echo "== dist/configs exact files if present in git index/worktree =="
git ls-files | rg '(^|/)dist/|configs/gateway-controller/config\.toml$' || trueRepository: wso2/api-platform
Length of output: 6090
Disable the configured config dumps by default.
gateway/configs/config.toml enables both /config_dump routes, and the packaged package-event-gateway target copies this file into $$DIST/configs/gateway-controller/config.toml. Keep these flags false and enable them only in explicit local or integration overrides.
🤖 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.toml` around lines 26 - 30, Update the config_dump
settings in the policy_engine.admin.config_dump configuration to default to
disabled by setting the relevant enabled flags to false. Preserve explicit local
and integration overrides as the mechanism for enabling both /config_dump
routes.
Comment on lines
+130
to
+132
| export ROUTER_ADMIN_ENABLED="${ROUTER_ADMIN_ENABLED:-false}" | ||
| export ROUTER_ADMIN_HOST="${ROUTER_ADMIN_HOST:-127.0.0.1}" | ||
| export ROUTER_ADMIN_PORT="${ROUTER_ADMIN_PORT:-9901}" |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict the Router admin host in both entrypoints.
Both scripts accept arbitrary ROUTER_ADMIN_HOST values. A non-loopback value can expose Envoy admin endpoints when ROUTER_ADMIN_ENABLED=true.
gateway/gateway-runtime/docker-entrypoint.sh#L130-L132: validate the host as an explicit loopback literal before injecting the admin configuration.gateway/gateway-runtime/docker-entrypoint-debug.sh#L101-L103: apply the same validation before injecting the admin configuration.
📍 Affects 2 files
gateway/gateway-runtime/docker-entrypoint.sh#L130-L132(this comment)gateway/gateway-runtime/docker-entrypoint-debug.sh#L101-L103
🤖 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/gateway-runtime/docker-entrypoint.sh` around lines 130 - 132,
Restrict ROUTER_ADMIN_HOST to an explicit loopback literal before the admin
configuration is injected, rejecting or overriding any non-loopback value while
preserving the default. Apply this validation in
gateway/gateway-runtime/docker-entrypoint.sh lines 130-132 and
gateway/gateway-runtime/docker-entrypoint-debug.sh lines 101-103.
Comment on lines
+19
to
+24
| # The admin interface (config_dump, stats, /runtime_modify, etc.) is intentionally | ||
| # NOT defined here. It is disabled by default and, when enabled via | ||
| # ROUTER_ADMIN_ENABLED, injected at startup by docker-entrypoint.sh through | ||
| # `envoy --config-yaml`, bound to loopback only. See docker-entrypoint.sh for | ||
| # the injected block and health-check.sh for how liveness/readiness are | ||
| # checked without depending on the admin interface. |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
# Expect: ROUTER_ADMIN_HOST accepts only validated loopback addresses.
rg -n -C 8 -- \
'ROUTER_ADMIN_HOST|ROUTER_ADMIN_ENABLED|CONFIG_OVERRIDE' \
gateway/gateway-runtime || trueRepository: wso2/api-platform
Length of output: 28165
🏁 Script executed:
#!/bin/bash
set -u
echo "== scripts file sizes =="
wc -l gateway/gateway-runtime/docker-entrypoint.sh gateway/gateway-runtime/docker-entrypoint-debug.sh
echo "== search for validation helpers/admin host handling in runtime scripts =="
rg -n -i '_validate|validate|localhost|127\.0\.0\.1|0\.0\.0\.0|loopback|admin_host|ROUTER_ADMIN_HOST|set -u|bash' gateway/gateway-runtime/docker-entrypoint.sh gateway/gateway-runtime/docker-entrypoint-debug.sh
echo "== relevant sections =="
sed -n '1,220p' gateway/gateway-runtime/docker-entrypoint.sh
sed -n '1,220p' gateway/gateway-runtime/docker-entrypoint-debug.shRepository: wso2/api-platform
Length of output: 21517
🏁 Script executed:
#!/bin/bash
set -u
echo "== docker-entrypoint remainder =="
sed -n '220,382p' gateway/gateway-runtime/docker-entrypoint.sh
echo "== docker-entrypoint-debug remainder =="
sed -n '220,301p' gateway/gateway-runtime/docker-entrypoint-debug.sh
echo "== all ROUTER_ADMIN references in gateway runtime =="
rg -n 'ROUTER_ADMIN_' gateway/gateway-runtime
echo "== behavioral probe: current shell assignment accepts non-loopback values =="
bash -c '
set -u
export ROUTER_ADMIN_ENABLED=true
export ROUTER_ADMIN_HOST="${ROUTER_ADMIN_HOST:-127.0.0.1}"
printf "enabled=true host=%s port=%s\\n" "$ROUTER_ADMIN_ENABLED" "$ROUTER_ADMIN_HOST"
'
printf '\n'
bash -c '
set -u
export ROUTER_ADMIN_ENABLED=true
export ROUTER_ADMIN_HOST="0.0.0.0"
export ROUTER_ADMIN_PORT="9901"
printf "enabled=%s host=%s address=%s port_value=%s\\n" \
"${ROUTER_ADMIN_ENABLED:-false}" "$ROUTER_ADMIN_HOST" "$ROUTER_ADMIN_HOST" "$ROUTER_ADMIN_PORT"
'Repository: wso2/api-platform
Length of output: 13803
Reject non-loopback ROUTER_ADMIN_HOST values before injecting Envoy config.
docker-entrypoint.sh and docker-entrypoint-debug.sh default ROUTER_ADMIN_HOST to 127.0.0.1, but they accept an environment override such as 0.0.0.0 and write it directly into admin.address.socket_address.address. Add validation and exit on invalid values in both scripts before starting Envoy.
🤖 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/gateway-runtime/router/config/envoy-bootstrap.yaml` around lines 19 -
24, Validate ROUTER_ADMIN_HOST in both docker-entrypoint.sh and
docker-entrypoint-debug.sh before generating or injecting the Envoy admin
configuration, accepting only loopback addresses and exiting with an error for
any other value. Preserve the existing 127.0.0.1 default and ensure validation
occurs before starting Envoy.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
# Expect: every Envoy launch path handles ROUTER_ADMIN_ENABLED consistently.
rg -n -C 6 -- \
'envoy-bootstrap\.yaml|--config-yaml|ROUTER_ADMIN_ENABLED|/ready|drain_router' . || trueRepository: wso2/api-platform
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -u
echo "== locate gateway-runtime scripts/files =="
git ls-files gateway/gateway-runtime | sed -n '1,220p'
echo
echo "== router admin host usages with context =="
rg -n -C 4 'ROUTER_ADMIN_HOST=|ROUTER_ADMIN_ENABLED|validate.*(admin|host|ROUTER)|listen|address:\s*$' gateway/gateway-runtime || true
echo
echo "== all envoy binary/envoy bootstrap command references under gateway/gateway-runtime =="
rg -n -C 4 'envoy[^A-Za-z0-9_/-]|envoy-bootstrap\.yaml|/etc/envoy/envoy\.yaml|--config-yaml|exec.*envoy' gateway/gateway-runtime .github kubernetes gateway -S -g '!vendor' -g '!node_modules' -g '!dist' -g '!build' || trueRepository: wso2/api-platform
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
echo "== exact route 184-343 docker-entrypoint.sh ==" <<'EOF'
cat -n gateway/gateway-runtime/docker-entrypoint.sh | sed -n '184,343p'
EOF
echo
echo "== exact route 145-232 docker-entrypoint-debug.sh ==" <<'EOF'
cat -n gateway/gateway-runtime/docker-entrypoint-debug.sh | sed -n '145,232p'
EOF
echo
echo "== all ROUTER_ADMIN_HOST assignments in gateway/gateway-runtime ==" <<'EOF'
rg -n 'ROUTER_ADMIN_HOST(\+|-|=[^[:space:]]*)?=' gateway/gateway-runtime -SRepository: wso2/api-platform
Length of output: 413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '184,343p' gateway/gateway-runtime/docker-entrypoint.sh | awk '{printf "%5d %s\n", NR+183, $0}'
echo '---'
sed -n '145,232p' gateway/gateway-runtime/docker-entrypoint-debug.sh | awk '{printf "%5d %s\n", NR+144, $0}'
echo '---'
rg -n 'ROUTER_ADMIN_HOST(\+|-|=[^[:space:]]*)?=' gateway/gateway-runtime -SRepository: wso2/api-platform
Length of output: 11845
Enforce loopback-only admin addresses when admin is enabled.
gateway/gateway-runtime/docker-entrypoint.sh and gateway/gateway-runtime/docker-entrypoint-debug.sh default ROUTER_ADMIN_HOST to 127.0.0.1, but they do not reject other values. An overriding ROUTER_ADMIN_HOST would still inject the admin block into envoy --config-yaml, exposing admin endpoints beyond the loopback guarantee documented for envoy-bootstrap.yaml.
🤖 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/gateway-runtime/router/config/envoy-bootstrap.yaml` around lines 19 -
24, Update the admin-enablement logic in docker-entrypoint.sh and
docker-entrypoint-debug.sh to validate ROUTER_ADMIN_HOST and reject any value
other than the loopback address 127.0.0.1 before injecting the Envoy admin
configuration. Preserve the existing defaulting and startup behavior for the
valid loopback value.
Comment on lines
+108
to
+109
| # The IT suite (steps_health.go) exercises Envoy's admin /ready directly. | ||
| - ROUTER_ADMIN_ENABLED=true |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The router-admin enablement does not match the integration test network path.
gateway/it/setup.go:189-197 polls localhost:9901 outside gateway-runtime, but the enabled admin listener is loopback-bound inside that container. The published 9901:9901 port cannot reach it. Move the readiness call into gateway-runtime, or add an explicit test-only reachable endpoint. Do not weaken the production loopback binding.
gateway/it/docker-compose.test.yaml#L108-L109: update the readiness mechanism for the default integration stack.gateway/it/docker-compose.test.postgres.yaml#L196-L197: apply the same readiness-path fix to the PostgreSQL stack.gateway/it/docker-compose.test.sqlserver.yaml#L176-L177: apply the same readiness-path fix to the SQL Server stack.gateway/it/docker-compose.test.vhosts-multi.yaml#L78-L79: apply the same readiness-path fix to the multi-vhost stack.gateway/it/docker-compose.test.vhosts-single.yaml#L78-L79: apply the same readiness-path fix to the single-vhost stack.
📍 Affects 5 files
gateway/it/docker-compose.test.yaml#L108-L109(this comment)gateway/it/docker-compose.test.postgres.yaml#L196-L197gateway/it/docker-compose.test.sqlserver.yaml#L176-L177gateway/it/docker-compose.test.vhosts-multi.yaml#L78-L79gateway/it/docker-compose.test.vhosts-single.yaml#L78-L79
🤖 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/it/docker-compose.test.yaml` around lines 108 - 109, Update the
readiness mechanism used by the integration stacks so setup.go’s localhost:9901
poll executes from within gateway-runtime or uses an explicitly reachable
test-only endpoint, while preserving Envoy’s production loopback binding. Apply
the same fix in gateway/it/docker-compose.test.yaml (108-109),
gateway/it/docker-compose.test.postgres.yaml (196-197),
gateway/it/docker-compose.test.sqlserver.yaml (176-177),
gateway/it/docker-compose.test.vhosts-multi.yaml (78-79), and
gateway/it/docker-compose.test.vhosts-single.yaml (78-79); each site requires
the corresponding readiness-path update.
Comment on lines
+37
to
+40
| # Envoy admin (9901) is intentionally NOT published here — it's disabled by | ||
| # default in the image (ROUTER_ADMIN_ENABLED=false) and, even when enabled, | ||
| # binds loopback-only; publishing it on a Service would defeat that. See | ||
| # kubernetes/helm/gateway-helm-chart's expose.routerAdmin (default false). |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 'ROUTER_ADMIN_ENABLED|9901|/ready' \
gateway/docker-compose.yaml \
kubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yaml \
gateway/gateway-runtimeRepository: wso2/api-platform
Length of output: 29139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Manifest relevant sections =="
sed -n '240,292p' kubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yaml
echo
echo "== health-check.sh =="
sed -n '1,70p' gateway/gateway-runtime/health-check.sh
echo
echo "== docker-entrypoint health/admin sections =="
sed -n '180,205p' gateway/gateway-runtime/docker-entrypoint.sh
echo
echo "== all K8s probe references to router/admin/9901 =="
rg -n -C 4 'ROUTER_ADMIN_ENABLED|port: 9901|/ready|health-check\.sh|livenessProbe|readinessProbe' kubernetes/gateway-operator/internal/controller/resources/api-platform-gateway-k8s-manifests.yaml kubernetes/helm/gateway-helm-chart gateway/gateway-runtime || trueRepository: wso2/api-platform
Length of output: 45175
Move the router probes onto a listener reachable by kubelet.
health-check.sh falls back to an in-pod admin check while ROUTER_ADMIN_ENABLED is true, but the Deployment uses httpGet:9901, which checks Envoy's loopback-only admin from kubelet/pod IP and can block health checks. Point both probes at the runtime health endpoint behind an actual listener, or make the probe run inside the container with exec and keep ROUTER_ADMIN_ENABLED=false.
🤖 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 37 - 40, Update the Deployment liveness and readiness probes to
avoid kubelet HTTP requests to the loopback-only Envoy admin port 9901: target
the runtime health endpoint through a listener reachable from the kubelet, or
convert both probes to container-local exec checks while preserving
ROUTER_ADMIN_ENABLED=false.
Comment on lines
+884
to
+889
| # This is independent of, and in addition to, the process-level defaults: Envoy's | ||
| # admin interface itself is off unless deployment.env.routerAdminEnabled=true (and | ||
| # even then binds loopback-only), and policy-engine's /config_dump route is off | ||
| # unless gateway.config.policy_engine.admin.config_dump.enabled=true. Leave both | ||
| # OFF here regardless of those flags — this toggle controls k8s Service/network | ||
| # reachability, not the admin interface's own enablement. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 'ROUTER_ADMIN_ENABLED|9901:9901|127\.0\.0\.1|admin.*(bind|address)' \
gateway/distribution/docker-compose.yaml \
gateway/docker-compose-perf.yaml \
gateway/docker-compose.debug.yaml \
gateway/docker-compose.yaml \
gateway/gateway-runtime \
kubernetes/helm/gateway-helm-chartRepository: wso2/api-platform
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Helm values relevant lines =="
sed -n '840,900p' kubernetes/helm/gateway-helm-chart/values.yaml
echo
echo "== Helm templates Service definitions mentioning 9901/routerAdmin =="
rg -n -C 8 '9901|routerAdmin|ROUTER_ADMIN_PORT|containerPort: 9901|exposeAdmin|admin' kubernetes/helm/gateway-helm-chart/templates kubernetes/helm/gateway-helm-chart/values.yaml
echo
echo "== Helm README admin references =="
rg -n -C 5 '9901|ROUTER_ADMIN_ENABLED|routerAdmin|admin' kubernetes/helm/gateway-helm-chart/README.md kubernetes/helm/gateway-helm-chart/values.yaml
echo
echo "== Static parsed docker-compose ports/env snippets =="
python3 - <<'PY'
import yaml, pathlib
paths = [
"gateway/docker-compose.yaml",
"gateway/docker-compose.debug.yaml",
"gateway/docker-compose-perf.yaml",
"gateway/distribution/docker-compose.yaml",
]
for p in paths:
data=yaml.safe_load(pathlib.Path(p).read_text())
svc=data.get("services",{})
print(f"\n{p}:")
for name, cfg in svc.items():
print(f" service={name}")
print(f" image={cfg.get('image')}")
print(f" ports={cfg.get('ports')}")
env=cfg.get("environment",{})
if isinstance(env,list):
env={x.split("=",1)[0]:x.split("=",1)[1] if "=" in x else None for x in env}
print(f" ROUTER_ADMIN_ENABLED={env.get('ROUTER_ADMIN_ENABLED')}")
PY
echo
echo "== Static parsing docker-entrypoint admin address injection shape =="
python3 - <<'PY'
from pathlib import Path
p=Path("gateway/gateway-runtime/docker-entrypoint.sh")
text=p.read_text()
start=text.index('if [ "${ROUTER_ADMIN_ENABLED}" = "true" ]; then')
end=text.index('CONFIG_OVERRIDE="${CONFIG_OVERRIDE}', start)
print(text[start:end+80])
PYRepository: wso2/api-platform
Length of output: 50373
Keep the published Envoy admin port aligned with the listener address.
ROUTER_ADMIN_ENABLED=true injects an Envoy admin interface bound to ROUTER_ADMIN_HOST, which defaults to 127.0.0.1, while these Docker Compose files publish container port 9901 to the host as if it were network reachable. Remove the published 9901 port unless ROUTER_ADMIN_HOST is set to a supported non-loopback address, and keep the Helm Service toggle aligned with that listener address.
📍 Affects 5 files
kubernetes/helm/gateway-helm-chart/values.yaml#L884-L889(this comment)gateway/docker-compose.yaml#L69-L72gateway/distribution/docker-compose.yaml#L66-L68gateway/docker-compose-perf.yaml#L69-L71gateway/docker-compose.debug.yaml#L73-L75
🤖 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/gateway-helm-chart/values.yaml` around lines 884 - 889, Align
Envoy admin port exposure with ROUTER_ADMIN_HOST: in gateway/docker-compose.yaml
(69-72), gateway/distribution/docker-compose.yaml (66-68),
gateway/docker-compose-perf.yaml (69-71), and gateway/docker-compose.debug.yaml
(73-75), remove the published 9901 port unless the listener is configured to a
supported non-loopback address; in
kubernetes/helm/gateway-helm-chart/values.yaml (884-889), keep the Helm Service
exposure toggle consistent with that listener-address requirement rather than
enabling reachability independently.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix bug