fix(config): validate EnvoyProxySpec in EnvoyGateway - #9961
Conversation
Signed-off-by: zirain <zirain2009@gmail.com>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: zirain <zirain2009@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4828415db6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| cfgCopy := r.snapshotConfig() | ||
| if cfgCopy != nil && cfgCopy.EnvoyGateway != nil { | ||
| r.logger.Info("running hook", "envoyGateway", cfgCopy.EnvoyGateway.EnvoyGatewaySpec) |
There was a problem hiding this comment.
When envoyProxy.bootstrap.value or a raw Kubernetes workload patch contains inline credentials or other sensitive values, this Info-level log reflect-serializes the entire EnvoyGatewaySpec on every startup or reload, exposing those values to anyone with access to controller logs. Log only non-sensitive identifiers here, or move a redacted representation behind debug-level logging.
Useful? React with 👍 / 👎.
| if eg.EnvoyProxy != nil { | ||
| if err := validateEnvoyProxySpec(eg.EnvoyProxy); err != nil { | ||
| return fmt.Errorf("invalid EnvoyProxy template: %w", err) | ||
| } |
There was a problem hiding this comment.
When the file-backed default contains an invalid or prohibited envoyProxy.bootstrap override, this call still succeeds because validateEnvoyProxySpec explicitly skips bootstrap validation. Standalone EnvoyProxy resources additionally pass through bootstrap.Validate in internal/gatewayapi/translator.go, but the merged default spec is never sent through that path; an override that changes dynamic_resources or the xDS cluster can therefore be accepted and leave Envoy unable to connect to the control plane. Add the internal bootstrap validation in the config-loading validation path.
Useful? React with 👍 / 👎.
| if eg.EnvoyProxy != nil { | ||
| if err := validateEnvoyProxySpec(eg.EnvoyProxy); err != nil { | ||
| return fmt.Errorf("invalid EnvoyProxy template: %w", err) | ||
| } |
There was a problem hiding this comment.
When the embedded default sets mergeGateways: true together with mergeBackends: {}, the new validation returns nil because the mutual-exclusion rule exists only as an EnvoyProxy CRD CEL annotation and validateEnvoyProxySpec does not mirror it. EnvoyGateway files do not pass through Kubernetes admission, so the controller accepts the configuration and the translator subsequently marks every affected Gateway NotAccepted for this exact conflict. Mirror the EnvoyProxy CEL invariants in the validation used for embedded file configuration.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9961 +/- ##
==========================================
- Coverage 81.34% 81.33% -0.01%
==========================================
Files 264 264
Lines 40967 40972 +5
==========================================
Hits 33326 33326
- Misses 7640 7645 +5
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
fixes: #9956