fix: invalid telemetry backendref degrades gateway#9406
Conversation
An invalid tracing, access log, or metrics backendRef in the referenced EnvoyProxy previously set the Gateway to Accepted=False (InvalidParameters). Telemetry misconfigurations are now added as a warning on the EnvoyProxy status (Accepted=True, reason Accepted) and only the affected telemetry feature is skipped, so the Gateway and its infrastructure keep running. Fixes envoyproxy#9229 Signed-off-by: vishwas-bm <b_m.vishwas@nokia.com>
…dref-degrades-gateway
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
vishwas-bm
changed the title
![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: 7507206974
ℹ️ 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".
| xdsIR.AccessLog, err = t.processAccessLog(gwCtx, envoyProxy, resources) | ||
| if err != nil { | ||
| status.UpdateGatewayStatusNotAccepted(gwCtx.Gateway, gwapiv1.GatewayReasonInvalidParameters, | ||
| fmt.Sprintf("Invalid access log backendRefs in the referenced EnvoyProxy: %v", err)) | ||
| return | ||
| warnings = append(warnings, fmt.Errorf("invalid access log backendRefs in the referenced EnvoyProxy: %w", err)) |
There was a problem hiding this comment.
When an access log setting contains any invalid matches CEL expression, processAccessLog returns that error before building the IR; this new blanket handling treats it as an invalid backendRef warning and leaves xdsIR.AccessLog nil, so a config with valid file/OTel sinks plus one bad match now keeps the Gateway accepted but removes all access logging. The access-log API documents invalid CEL expressions as ignored, so this path should drop/report only the bad match and reserve feature skipping for actual backendRef resolution failures.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9406 +/- ##
=======================================
Coverage 75.19% 75.20%
=======================================
Files 252 252
Lines 41093 41112 +19
=======================================
+ Hits 30900 30917 +17
- Misses 8090 8091 +1
- Partials 2103 2104 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… logs Invalid CEL match expressions in an EnvoyProxy access log setting no longer cause the whole access log feature to be skipped. Per the AccessLog API, invalid CEL expressions are ignored: only the invalid expressions are dropped while the valid expressions and the rest of the access log (file/OTel sinks) are preserved. Feature skipping remains reserved for backendRef resolution failures. Addresses Codex review feedback on envoyproxy#9406. Signed-off-by: vishwas-bm <b_m.vishwas@nokia.com>
… logs Invalid CEL match expressions in an EnvoyProxy access log setting no longer cause the whole access log feature to be skipped. Per the AccessLog API, invalid CEL expressions are ignored: only the invalid expressions are dropped while the valid expressions and the rest of the access log (file/OTel sinks) are preserved. Feature skipping remains reserved for backendRef resolution failures. Addresses Codex review feedback on envoyproxy#9406. Signed-off-by: vishwas-bm <b_m.vishwas@nokia.com>
vishwas-bm
force-pushed
the
fix/9229-invalid-telemetry-backendref-degrades-gateway
branch
from
9455161 to
79e1321
Compare
1 participant
What type of PR is this?
Bug fix.
What this PR does / why we need it
An invalid
backendRefin anEnvoyProxy's telemetry configuration (tracing, access log, or metrics) previously caused the translator to mark the Gateway asAccepted=False.This PR changes the behavior in
processProxyObservability:EnvoyProxystatusWhich issue(s) this PR fixes
Fixes #9229
PR Checklist
git commit -s). See DCO: Sign your work./api), the API was discussed and agreed before the implementation. The API change can be in a separate PR, or in the same PR, but the API must be agreed before implementation. N/A if this PR does not contain API changes.make generate gen-check,make lint, and the unit-test/coverage build pass. (Flaky e2e failures are not considered breakages, butgen-check,lint, and coverage MUST pass.)release-notes/current/<section>/<pr-number>-<slug>.md(seerelease-notes/current/README.mdfor sections and naming). N/A if this PR does not contain non-trivial changes.make gen-checkand committed the result if API/helm charts/modules changed.release-notes/current/breaking_changes/.