fix(envoy-gateway): align template standards#663
Conversation
github-actions
Bot
added
area:schema
area:templates
area:tests
area:values
chart:envoy-gateway
Standards Check (GR-079) — PASSEvery changed chart fully passes standards-check. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds chart validation for externalSecrets/rateLimiting and podLabels, updates certgen Job labels to selector labels, and adds ChangesChart validation and network policy changes
Estimated code review effort: 2 (Simple) | ~15 minutes Related PRs: None identified. Suggested labels: helm-chart, validation, networkpolicy Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 71.6306% |
✅ Security posture acceptable.
![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: 44963f8e12
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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 (2)
charts/envoy-gateway/templates/_helpers.tpl (1)
252-261: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReserved keys hardcoded separately from
selectorLabels.The reserved key list (
app.kubernetes.io/name,/instance,/component) is duplicated here rather than derived fromenvoy-gateway.selectorLabels. If that helper's key set changes later, this validation will silently drift out of sync.♻️ Suggested approach
-{{- $podLabels := .Values.podLabels | default dict -}} -{{- if hasKey $podLabels "app.kubernetes.io/name" -}} -{{- fail "podLabels must not override selector label app.kubernetes.io/name" -}} -{{- end -}} -{{- if hasKey $podLabels "app.kubernetes.io/instance" -}} -{{- fail "podLabels must not override selector label app.kubernetes.io/instance" -}} -{{- end -}} -{{- if hasKey $podLabels "app.kubernetes.io/component" -}} -{{- fail "podLabels must not override selector label app.kubernetes.io/component" -}} -{{- end -}} +{{- $podLabels := .Values.podLabels | default dict -}} +{{- $selectorLabels := include "envoy-gateway.selectorLabels" . | fromYaml -}} +{{- range $key, $_ := $selectorLabels -}} +{{- if hasKey $podLabels $key -}} +{{- fail (printf "podLabels must not override selector label %s" $key) -}} +{{- end -}} +{{- end -}}🤖 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 `@charts/envoy-gateway/templates/_helpers.tpl` around lines 252 - 261, The podLabels validation in the _helpers.tpl helper is hardcoding reserved selector keys instead of reusing the same source as selectorLabels, so it can drift if those keys change. Update the validation logic to derive the reserved-key checks from the envoy-gateway.selectorLabels helper (or its shared key set) and keep the hasKey/fail behavior in sync with that single source of truth.charts/envoy-gateway/values.schema.json (1)
636-652: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfusing naming:
networkPolicies(bool) vsnetworkPolicy(object).Having both
security.networkPolicies(enable flag) andsecurity.networkPolicy(extension object) as near-identical sibling keys is easy to mistype/misread. Consider nesting the flag under the same object (e.g.security.networkPolicy.enabled+security.networkPolicy.extraEgress) or renaming one of them for clarity.🤖 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 `@charts/envoy-gateway/values.schema.json` around lines 636 - 652, The schema currently defines two confusing sibling keys, networkPolicies as a boolean flag and networkPolicy as an object, which are easy to mistype or misread. Update the schema in values.schema.json to use clearer, consistent naming by either nesting the enable flag under networkPolicy as an enabled property with extraEgress, or renaming one of the symbols so the boolean and extension object are not nearly identical siblings. Keep the descriptions and defaults aligned with the new shape so consumers can tell which key controls enablement versus rule extensions.
🤖 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 `@charts/envoy-gateway/templates/validate.yaml`:
- Around line 1-2: The YAML lint failure comes from using a Go template comment
as the first line in the validate template, which makes the parser see template
syntax before valid YAML. Update the top of envoy-gateway.validate’s template to
use a plain YAML comment for the SPDX header, matching the other templates, and
keep the include of envoys-gateway.validate unchanged so yamllint can parse the
file cleanly.
---
Nitpick comments:
In `@charts/envoy-gateway/templates/_helpers.tpl`:
- Around line 252-261: The podLabels validation in the _helpers.tpl helper is
hardcoding reserved selector keys instead of reusing the same source as
selectorLabels, so it can drift if those keys change. Update the validation
logic to derive the reserved-key checks from the envoy-gateway.selectorLabels
helper (or its shared key set) and keep the hasKey/fail behavior in sync with
that single source of truth.
In `@charts/envoy-gateway/values.schema.json`:
- Around line 636-652: The schema currently defines two confusing sibling keys,
networkPolicies as a boolean flag and networkPolicy as an object, which are easy
to mistype or misread. Update the schema in values.schema.json to use clearer,
consistent naming by either nesting the enable flag under networkPolicy as an
enabled property with extraEgress, or renaming one of the symbols so the boolean
and extension object are not nearly identical siblings. Keep the descriptions
and defaults aligned with the new shape so consumers can tell which key controls
enablement versus rule extensions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f81c52f-bb24-43af-9f32-3c2fd20a9bcc
📒 Files selected for processing (9)
charts/envoy-gateway/templates/_helpers.tplcharts/envoy-gateway/templates/certgen-job.yamlcharts/envoy-gateway/templates/networkpolicy.yamlcharts/envoy-gateway/templates/validate.yamlcharts/envoy-gateway/tests/certgen_test.yamlcharts/envoy-gateway/tests/security_test.yamlcharts/envoy-gateway/tests/validation_test.yamlcharts/envoy-gateway/values.schema.jsoncharts/envoy-gateway/values.yaml
mberlofa
force-pushed
the
fix/envoy-gateway-template-standards
branch
from
559049c to
bbf8b0f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@charts/envoy-gateway/templates/validate.yaml`:
- Around line 1-2: The validate.yaml template still needs a yamllint exception
because the bare include directive in envoy-gateway.validate will be flagged by
strict YAML parsing. Update the validate.yaml template to either exclude it from
yamllint or add a file-level yamllint disable marker as the first line, keeping
the existing envoy-gateway.validate include intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ac5626bc-8f50-4bc6-9de2-6d6c315f4014
📒 Files selected for processing (9)
charts/envoy-gateway/templates/_helpers.tplcharts/envoy-gateway/templates/certgen-job.yamlcharts/envoy-gateway/templates/networkpolicy.yamlcharts/envoy-gateway/templates/validate.yamlcharts/envoy-gateway/tests/certgen_test.yamlcharts/envoy-gateway/tests/security_test.yamlcharts/envoy-gateway/tests/validation_test.yamlcharts/envoy-gateway/values.schema.jsoncharts/envoy-gateway/values.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
- charts/envoy-gateway/templates/networkpolicy.yaml
- charts/envoy-gateway/tests/certgen_test.yaml
- charts/envoy-gateway/templates/certgen-job.yaml
- charts/envoy-gateway/tests/validation_test.yaml
- charts/envoy-gateway/tests/security_test.yaml
- charts/envoy-gateway/values.yaml
- charts/envoy-gateway/values.schema.json
- charts/envoy-gateway/templates/_helpers.tpl
mberlofa
force-pushed
the
fix/envoy-gateway-template-standards
branch
from
bbf8b0f to
bed1861
Compare
|
Reviewed all CodeRabbit feedback in review threads and review-summary comments. Changes:
Compatibility note:
Validation:
Note: these two CodeRabbit items were posted in the review summary/body, not as active review threads, so there are no thread IDs to reply to or resolve for them. Existing CodeRabbit inline threads are already resolved. |
1 participant
Summary
security.networkPolicy.extraEgresssupport with schema and unit coverageenvoy-gateway.validateentrypoint for rate limiting, external Redis auth, ExternalSecrets, and reserved pod-label validationsValidation
Site PR: helmforgedev/site#342
Issue: #633
Summary by CodeRabbit
security.networkPolicy.extraEgressto append additional egress rules to the controller NetworkPolicy.podLabelsfrom overriding reserved selector label keys.security.networkPoliciesin the README.