Conversation
cwrau
requested review from
marvinWolff,
tasches and
teutonet-bot
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds optional control-plane audit support to the t8s-cluster Helm chart: new helper templates and files for audit policy/webhook, conditional API server args and dynamic file emission, a kubeadmControlPlaneTemplate change to accept contentFrom files, and a new Changes
Sequence Diagram(s)sequenceDiagram
participant Helm as Helm renderer
participant KubeAPIServer as kube-apiserver (control plane)
participant AuditWebhook as Audit Webhook
participant SecretStore as Secret (wazuh-audit-webhook)
Helm->>Helm: Render templates (helpers produce audit files & kubeconfig)
Helm->>KubeAPIServer: Deploy control plane with --audit-policy-file and --audit-webhook-config-file args and mounted files
KubeAPIServer->>AuditWebhook: Send audit events (per policy)
AuditWebhook->>SecretStore: Read token from mounted token file (populated via secret)
AuditWebhook-->>KubeAPIServer: Acknowledge / accept audit payloads
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces audit logging for the control plane, adding several helper templates to manage audit policies and webhook configurations while refactoring the hosted control plane template to use these shared definitions. The review feedback identifies a bug in the mustMerge function usage that will likely cause template failures, points out that using lookup with required will break helm template and dry-run operations, recommends making the hardcoded Wazuh webhook URL configurable, and warns of potential argument conflicts for hosted control planes.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in switch to enable Kubernetes API server audit logging for kubeadm control planes in the t8s-cluster management-cluster ClusterClass, while keeping hosted control planes audited by default.
Changes:
- Introduces
controlPlane.audit(defaultfalse) in chart values and schema. - Refactors hosted control plane audit policy rules into a shared helper template.
- Adds templated generation of audit policy + webhook kubeconfig files and corresponding kube-apiserver flags when auditing is enabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| charts/t8s-cluster/values.yaml | Adds controlPlane.audit value (default off). |
| charts/t8s-cluster/values.schema.json | Adds schema for controlPlane.audit with behavior description. |
| charts/t8s-cluster/templates/management-cluster/clusterClass/hostedControlPlaneTemplate/_hostedControlPlaneTemplateSpec.yaml | Replaces inline audit policy rules with shared helper include. |
| charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl | Adds audit policy/webhook helpers, dynamically injects audit files and kube-apiserver args when enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl`:
- Around line 221-224: The audit args block currently runs for all control
planes when .context.Values.controlPlane.audit is true; wrap the existing
conditional to also check that the control plane is not hosted by gating with
not .context.Values.controlPlane.hosted so the audit-policy-file and
audit-webhook-config-file assignments (the lines setting $args with include
"t8s-cluster.clusterClass.apiServer.auditPolicyPath" and include
"t8s-cluster.clusterClass.apiServer.auditWebhookConfigPath") only execute for
kubeadm control planes; apply the same not-hosted guard to the other audit
branch around the include calls at the second location (the block spanning the
include calls at the later lines) so hosted control planes don’t get duplicate
audit paths or ConfigMap token injection.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8af4685-26bc-42c8-b118-e422bd022aa2
📒 Files selected for processing (4)
charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tplcharts/t8s-cluster/templates/management-cluster/clusterClass/hostedControlPlaneTemplate/_hostedControlPlaneTemplateSpec.yamlcharts/t8s-cluster/values.schema.jsoncharts/t8s-cluster/values.yaml
…(opt-in) This is to be used for management clusters
cwrau
force-pushed
the
feat/t8s-cluster/allow-auditing-kcp-clusters
branch
from
b3cad26 to
0e7c808
Compare
This is to be used for management clusters
Summary by CodeRabbit