Repository navigation
feat: report tenant network policy enforcement - #204
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The reconcile error paths ignore failures from status persistence, which can hide RBAC/conflict issues and leave Tenant status stale.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Tenant reconciliation wiring and records passive NetworkPolicy enforcement evidence on the Tenant status. This fits into the operator’s multi-tenancy support by surfacing whether the cluster CNI appears to enforce NetworkPolicies.
Changes:
- Wire up
TenantReconcilerincmd/main.goand update reconciliation to setstatus.networkPolicyActiveplusstatus.networkPolicyEnforcementReason. - Extend the Tenant API and CRD schema with
networkPolicyEnforcementReason, plus additive-compatibility tests. - Update RBAC manifests and Helm RBAC/tests to cover Tenant resources and the new informer/watch needs.
File summaries
| File | Description |
|---|---|
| scripts/check_api_crd_compatibility.sh | Runs Tenant compatibility tests in the CRD/API compatibility script. |
| internal/controller/tenant_networkpolicy_test.go | Adds unit tests for Tenant network policy status behavior and fail-closed semantics. |
| internal/controller/tenant_controller.go | Updates reconciliation to compute and persist passive CNI enforcement evidence on Tenant status. |
| config/rbac/role.yaml | Updates generated manager ClusterRole rules for added watch/daemonset list needs. |
| config/crd/bases/agentic.clawdlinux.org_tenants.yaml | Adds CRD schema for status.networkPolicyEnforcementReason. |
| cmd/main.go | Registers the Tenant controller and provides discovery plus DaemonSet clients for detection. |
| charts/tests/operator_rbac_test.yaml | Extends Helm unit tests for updated controller RBAC. |
| charts/templates/rbac.yaml | Extends Helm ClusterRole to include Tenant resources and related permissions. |
| api/v1alpha1/tenant_types.go | Adds NetworkPolicyEnforcementReason to the Tenant status type. |
| api/v1alpha1/tenant_compatibility_test.go | Verifies the field is additive and present in the CRD OpenAPI schema. |
Review details
Suppressed comments (3)
internal/controller/tenant_controller.go:106
- persistTenantStatus() errors are ignored when provisioning secrets fails. If the status update fails, returning nil error can stall retries and drop the updated phase / enforcement status.
if err := r.provisionSecrets(ctx, &tenant); err != nil {
log.Error(err, "failed to provision secrets")
r.persistTenantStatus(ctx, &tenant, log)
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
}
internal/controller/tenant_controller.go:117
- persistTenantStatus() errors are ignored when RBAC configuration fails. If the status write fails (RBAC, conflict), the controller will keep requeuing without surfacing the real error.
if err := r.configureRBAC(ctx, &tenant); err != nil {
log.Error(err, "failed to configure RBAC")
r.persistTenantStatus(ctx, &tenant, log)
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
}
internal/controller/tenant_controller.go:128
- persistTenantStatus() errors are ignored when quota enforcement fails. If the status update fails, the reconcile hides that failure and keeps retrying with stale status.
if err := r.enforceQuotas(ctx, &tenant); err != nil {
log.Error(err, "failed to enforce quotas")
r.persistTenantStatus(ctx, &tenant, log)
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
}
- Files reviewed: 10/10 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Shreyansh Sancheti <43677304+shreyanshjain7174@users.noreply.github.com>
77c7968 to
1a46a40
Compare
Summary
Validation
go build ./...make test-gomake lint./scripts/check_api_crd_compatibility.shhelm unittest chartshelm lint chartsgo test ./...remains locally blocked only by missing envtestetcdassets.