docs(apache): sync template standards#321
Conversation
📝 WalkthroughWalkthroughUpdates Apache chart docs to reference httpd 2.4.68, documents ChangesApache chart updates
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 `@src/pages/playground.astro`:
- Line 24: The Apache playground is wired in via the playground schema, but
`networkPolicy` is being omitted by the schema-driven generation, so
`networkPolicy.egress.extraEgress` never appears. Update the playground
schema/data flow used by `playground.astro` and the Apache schema source so
`networkPolicy` is included in the generated fields, specifically ensuring
`networkPolicy.egress.extraEgress` is exposed for Apache.
🪄 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: 00178711-7dcb-48a3-9df8-22b83e7bbee4
📒 Files selected for processing (2)
src/pages/docs/charts/apache.mdxsrc/pages/playground.astro
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 `@src/data/playground-configs.ts`:
- Around line 261-282: The Apache extraEgress fields in playground-configs are
being serialized independently, which can emit a partial
networkPolicy.egress.extraEgress[0] rule when only CIDR, protocol, or port is
filled. Update the related config handling around the Extra Egress
CIDR/Protocol/Port entries so these fields are treated as one rule and only
serialize extraEgress when the complete to/ports structure is present, using the
existing key paths under networkPolicy.egress.extraEgress.
🪄 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: 6421769d-94b9-4e64-bf14-a32b3400292d
📒 Files selected for processing (1)
src/data/playground-configs.ts
## Summary Align the Apache chart with the corrected template standards checker. Changes: - move the Helm test pod out of `templates/tests/` into `templates/test-connection.yaml` - add canonical `networkPolicy.egress.extraEgress` support while preserving `extraRules` - update Apache NOTES sections to the numbered operational pattern - add helm-unittest coverage for the Helm test pod and `extraEgress` - update chart networking docs for the new NetworkPolicy value Related issue: Closes #633 Site PR: helmforgedev/site#321 ## Validation - `node C:\devops\berlofa\github\helmforge\helmforge-ops\scripts\charts\template-standards-check.js --chart apache --json` - `helm unittest charts/apache` - `make standards-check CHART=apache` - `make validate-chart CHART=apache` - `make site-sync-check CHART=apache` - `make release-check REPO=charts` - `make attribution-check REPO=charts` Result: `apache: FULLY VALIDATED (20 layers)` including k3d behavioral validation for default and all `ci/*.yaml` scenarios. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for extending NetworkPolicy egress rules with an additional `extraEgress` configuration. * Included an example showing how to define extra egress destinations and ports. * **Documentation** * Updated chart notes with clearer, numbered section headings. * Expanded NetworkPolicy guidance to cover the new egress option. * **Tests** * Added coverage for custom egress rules and chart connection checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: MergeCheck <mergecheck@example.invalid>
9c1e55c to
c6494b2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/scripts/playground.ts (1)
768-768: 📐 Maintainability & Code Quality | 🔵 TrivialSlug-specific guard hardcodes 'apache' and its exact key set.
If future charts add similar
extraEgress-style multi-field rule groups, this pattern will need to be copy-pasted per slug. Consider generalizing to aslug -> ruleKeys[]map (or deriving the "coupled key" grouping from the field config itself) so new charts can reuse the same completeness check.♻️ Example generalization
-function pruneIncompleteApacheExtraEgress(changes: { key: string; value: string; defaultValue: string }[]): void { - if (selectedSlug !== 'apache') return; - - const ruleKeys = new Set([ - 'networkPolicy.egress.extraEgress[0].to[0].ipBlock.cidr', - 'networkPolicy.egress.extraEgress[0].ports[0].protocol', - 'networkPolicy.egress.extraEgress[0].ports[0].port', - ]); +const COUPLED_RULE_KEYS: Record<string, string[]> = { + apache: [ + 'networkPolicy.egress.extraEgress[0].to[0].ipBlock.cidr', + 'networkPolicy.egress.extraEgress[0].ports[0].protocol', + 'networkPolicy.egress.extraEgress[0].ports[0].port', + ], +}; + +function pruneIncompleteCoupledRules(changes: { key: string; value: string; defaultValue: string }[]): void { + const keys = COUPLED_RULE_KEYS[selectedSlug]; + if (!keys) return; + const ruleKeys = new Set(keys);Also applies to: 770-774
🤖 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 `@src/scripts/playground.ts` at line 768, The completeness guard in the playground logic is hardcoded to the apache slug and its specific coupled fields, so it won’t scale to new multi-field rule groups. Refactor the check around the selectedSlug handling in the playground flow to use a reusable slug-to-ruleKeys map (or derive grouped keys from the field config) and apply the same completeness validation generically for any chart that needs coupled inputs.
🤖 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.
Nitpick comments:
In `@src/scripts/playground.ts`:
- Line 768: The completeness guard in the playground logic is hardcoded to the
apache slug and its specific coupled fields, so it won’t scale to new
multi-field rule groups. Refactor the check around the selectedSlug handling in
the playground flow to use a reusable slug-to-ruleKeys map (or derive grouped
keys from the field config) and apply the same completeness validation
generically for any chart that needs coupled inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1dcbd0f0-4a9d-4b8e-9124-4c4ab09313d6
📒 Files selected for processing (1)
src/scripts/playground.ts
Summary
Validation
Chart PR: helmforgedev/charts#641 (merged)
Issue: helmforgedev/charts#633
Summary by CodeRabbit
networkPolicy.egress.extraEgressinputs (CIDR, protocol, and port).extraEgress(missing one or more fields) from being applied during playground sync.httpdimage to version2.4.68.extraEgressexample and config reference changes (markingextraRulesas legacy).