fix(kibana): align template standards#679
Open
mberlofa wants to merge 3 commits into
Open
Conversation
github-actions
Bot
added
area:docs
area:schema
area:templates
area:tests
area:values
chart:kibana
size:L
labels
Contributor
Standards Check (GR-079) — PASSEvery changed chart fully passes standards-check. |
|
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 (25)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughThe Kibana chart now defaults to bundled Elasticsearch, updates bundled and external Elasticsearch documentation and examples, aligns CI and tests with the new wiring, and adds optional ChangesBundled Elasticsearch default and validation
Network policy extra egress support
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 89.090904% |
✅ 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
Keep Helm test hooks under templates
This hook was moved to the chart-root tests/ directory, but Helm does not render hook manifests from there; I verified with helm install --dry-run=client that a chart with only tests/root-test.yaml reports TEST SUITE: None. In this repository tests/ is used by helm-unittest suites, so packaging this Pod here leaves helm test with no rendered test hook despite the README still instructing users to run it.
ℹ️ 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/kibana/values.yaml (1)
146-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
extraEnvdefault is now a non-empty array — full-replace footgun.Helm list values fully replace rather than merge. Any user who sets their own
extraEnv(e.g., to add proxy vars) will silently dropELASTIC_APM_ACTIVE: "false", undoing the intended default of suppressing APM startup noise, with no warning.Consider either documenting this override caveat inline, or rendering this env var unconditionally from a dedicated boolean (e.g.,
apm.enabled: false) merged in the deployment template so it survivesextraEnvoverrides.💡 Minimal mitigation: warn in the comment
+# -- NOTE: Setting your own `extraEnv` replaces this list entirely (Helm lists +# do not merge). Re-add `ELASTIC_APM_ACTIVE: "false"` if you override this value. extraEnv: - name: ELASTIC_APM_ACTIVE value: "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 `@charts/kibana/values.yaml` around lines 146 - 148, The default extraEnv entry for ELASTIC_APM_ACTIVE is a list value that gets fully replaced by user overrides, so the suppression setting can be lost when consumers set their own extraEnv. Update the Kibana values/template around extraEnv to avoid this footgun: either document inline that extraEnv is a full-replace override and users must re-add ELASTIC_APM_ACTIVE, or move this setting to a dedicated boolean-driven render path in the deployment template so the ELASTIC_APM_ACTIVE env var is always emitted independently of extraEnv.charts/kibana/ci/external-secrets-values.yaml (1)
12-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAll three encryption-key remoteRefs now resolve to the same fake value.
xpack-security-encryption-key,xpack-reporting-encryption-key, andxpack-encrypted-saved-objects-encryption-keyall referencekey: test/password, so any accidental swap betweensecretKeytargets would go undetected by this CI scenario since the resulting values are identical either way.🤖 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/kibana/ci/external-secrets-values.yaml` around lines 12 - 23, The three encryption-key entries in the external-secrets values are all mapped to the same remoteRef value, which makes secretKey swaps impossible to detect in CI. Update the data entries in the helmforge-fake-store configuration so xpack-security-encryption-key, xpack-reporting-encryption-key, and xpack-encrypted-saved-objects-encryption-key each resolve to distinct fake remoteRef keys, keeping the same structure but using unique values to validate the wiring.
🤖 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/kibana/ci/security-values.yaml`:
- Around line 2-3: The Kibana CI values no longer cover the secured
external-Elasticsearch path, since only bundled mode remains in the CI configs.
Update the CI values setup by restoring one scenario that uses the external
Elasticsearch flow with hosts, tls, and auth settings, and keep it separate from
the bundled-elasticsearch case. Use the existing CI values files, especially
external-secrets-values and the bundled-elasticsearch override, to reintroduce
the real secured external cluster coverage.
---
Nitpick comments:
In `@charts/kibana/ci/external-secrets-values.yaml`:
- Around line 12-23: The three encryption-key entries in the external-secrets
values are all mapped to the same remoteRef value, which makes secretKey swaps
impossible to detect in CI. Update the data entries in the helmforge-fake-store
configuration so xpack-security-encryption-key, xpack-reporting-encryption-key,
and xpack-encrypted-saved-objects-encryption-key each resolve to distinct fake
remoteRef keys, keeping the same structure but using unique values to validate
the wiring.
In `@charts/kibana/values.yaml`:
- Around line 146-148: The default extraEnv entry for ELASTIC_APM_ACTIVE is a
list value that gets fully replaced by user overrides, so the suppression
setting can be lost when consumers set their own extraEnv. Update the Kibana
values/template around extraEnv to avoid this footgun: either document inline
that extraEnv is a full-replace override and users must re-add
ELASTIC_APM_ACTIVE, or move this setting to a dedicated boolean-driven render
path in the deployment template so the ELASTIC_APM_ACTIVE env var is always
emitted independently of extraEnv.
🪄 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: 91ea2d57-6818-4c85-8f59-504d4163527d
📒 Files selected for processing (25)
charts/kibana/README.mdcharts/kibana/ci/bundled-elasticsearch-values.yamlcharts/kibana/ci/default-values.yamlcharts/kibana/ci/dual-stack-values.yamlcharts/kibana/ci/external-secrets-values.yamlcharts/kibana/ci/gateway-api-values.yamlcharts/kibana/ci/ingress-values.yamlcharts/kibana/ci/k3d-values.yamlcharts/kibana/ci/network-policy-values.yamlcharts/kibana/ci/persistence-values.yamlcharts/kibana/ci/security-values.yamlcharts/kibana/ci/servicemonitor-values.yamlcharts/kibana/ci/wolfi-values.yamlcharts/kibana/docs/elasticsearch.mdcharts/kibana/examples/bundled-elasticsearch.yamlcharts/kibana/examples/production.yamlcharts/kibana/templates/NOTES.txtcharts/kibana/templates/_helpers.tplcharts/kibana/templates/networkpolicy.yamlcharts/kibana/tests/config_test.yamlcharts/kibana/tests/deployment_test.yamlcharts/kibana/tests/routing_test.yamlcharts/kibana/tests/test-connection.yamlcharts/kibana/values.schema.jsoncharts/kibana/values.yaml
💤 Files with no reviewable changes (1)
- charts/kibana/examples/bundled-elasticsearch.yaml
mberlofa
force-pushed
the
fix/kibana-template-standards
branch
from
1b87dc4 to
c722948
Compare
mberlofa
force-pushed
the
fix/kibana-template-standards
branch
from
c722948 to
3e0d8e8
Compare
Contributor
Author
|
Addressed the still-valid CodeRabbit review-body notes:
Validation completed after the fix:
These two notes were present in the CodeRabbit review summary body, not as separate resolvable review threads, so there was no additional thread ID to resolve for them. |
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.
Summary
templates/tests/sohelm testrenders and executes it, with explicit unittest coverage for the hook pod.networkPolicy.extraEgressand keep NOTES sections numbered.Related
Validation
helm template test charts/kibana | rg -n "helm.sh/hook|test-connection"(hook rendered fromtemplates/tests/test-connection.yaml)helm unittest charts/kibana(28 tests, 3 suites)make template-standards-check CHART=kibananode scripts/charts/validate-chart.js --chart kibana --no-k3dmake validate-chart CHART=kibana TIMEOUT=1200(FULLY VALIDATED, 22 layers)make site-sync-check CHART=kibanapassed with required site updates presentmake release-check REPO=chartspassed with expected GR-077 release warningmake attribution-check REPO=chartspassedSummary by CodeRabbit
New Features
networkPolicy.egress.extraEgressto append additional egress rules.Bug Fixes
Documentation
Tests