chore(charts): controller-gen output file into charts.

[jvanz]

Description

Updates the controller-gen command used to generated the RBAC manifests into the charts/kubewarden-controller directory. This commit also adds the kubebuilder markers to add the missing permissions in the controller roles that was added using manully created roles definitions.

Fix #1483

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.01%. Comparing base (42c079b) to head (0246f81).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1520      +/-   ##
==========================================
- Coverage   80.02%   75.01%   -5.01%     
==========================================
  Files         127      171      +44     
  Lines       16573    21075    +4502     
==========================================
+ Hits        13262    15810    +2548     
- Misses       3311     5051    +1740     
- Partials        0      214     +214     

Flag	Coverage Δ
go-tests	56.99% <100.00%> (?)
rust-tests	79.91% <ø> (-0.11%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[flavio]

Also, I got a bit confused by the names:

• charts/kubewarden-controller/templates/controller-rbac-roles.yaml: this holds all the roles (cluster and not)
• charts/kubewarden-controller/templates/controller-rbac-extras.yaml: this holds all the roles bindings and one ClusterRole
• charts/kubewarden-controller/templates/audit-scanner-rbac.yaml: contains roles and bindings

I would like, if possible, to end up with something like sbomscanner:

• a file with roles
• a file with rolebindings

In our case we would have 4 files:

• audit-scanner-role.yaml
• audit-scanner-rolebinding.yaml
• controller-role.yaml
• controller-rolebinding.yaml

[jvanz]

Also, I got a bit confused by the names:

* `charts/kubewarden-controller/templates/controller-rbac-roles.yaml`: this holds all the roles (cluster and not)

* `charts/kubewarden-controller/templates/controller-rbac-extras.yaml`: this holds all the roles bindings and one `ClusterRole`

* `charts/kubewarden-controller/templates/audit-scanner-rbac.yaml`: contains roles and bindings

I would like, if possible, to end up with something like sbomscanner:

* a file with [roles](https://github.com/kubewarden/sbomscanner/blob/main/charts/sbomscanner/templates/controller/role.yaml)

* a file with [rolebindings](https://github.com/kubewarden/sbomscanner/blob/main/charts/sbomscanner/templates/controller/rolebinding.yaml)

In our case we would have 4 files:

* `audit-scanner-role.yaml`

* `audit-scanner-rolebinding.yaml`

* `controller-role.yaml`

* `controller-rolebinding.yaml`

I'm fine splitting role and rolebindings. I actually did that while I was working on this PR. But I've changed that because I would like to avoid two role files. The controller-rbac-roles.yaml has the roles generated by the controller-gen. It's overwritten every time we run make manifests. Then, instead of having two role files, one for the controller-gen output and another to the "metrics-reader" role, I've decided to rename the file to controller-rbac-extras.yaml. Where it has all the rolebindings and one additional role. Therefore, I avoid more custom code to merge all the roles into a single file.

[flavio]

Good catch from copilot:

• kubewarden-controller/internal/controller/policy_subreconciler_webhook.go

  Line 95 in 42c079b

  err := r.Get(ctx, types.NamespacedName{Name: admissionPolicy.GetUniqueName()}, &webhook)

• kubewarden-controller/internal/controller/policy_subreconciler_webhook.go

  Line 181 in 42c079b

  err := r.Get(ctx, types.NamespacedName{Name: admissionPolicy.GetUniqueName()}, &webhook)

We have to update the annotations.

We also have to understand why all the tests are green despite this change

[flavio]

I've tasked an AI agent to find why the tests are green. This is the answer I got, which seems reasonable to me:

Unit Tests — system:masters Bypasses All RBAC

envtest starts a real kube-apiserver with --authorization-mode=RBAC, so RBAC is technically enabled. However, controller-runtime's testEnv.Start() unconditionally returns a restConfig authenticated as a member of the system:masters group (envtest/server.go):
adminInfo := User{Name: "admin", Groups: []string{"system:masters"}}
system:masters is a hardcoded Kubernetes superuser group that bypasses the RBAC authorizer entirely — not via a ClusterRoleBinding, but at the authorizer level itself. No RBAC policy, no matter how restrictive, is evaluated for this identity. The manager and all reconcilers inherit these credentials, so every r.Get(...) call silently succeeds regardless of what the RBAC manifest says.
Additionally, policy_subreconciler_webhook_test.go doesn't test the reconcile methods at all — it only tests the namespaceSelector() helper in isolation with no client involved, so even in principle there's no API call to be blocked.

E2E Tests — Test Harness Also Runs as system:masters

The e2e tests use a Kind cluster provisioned by sigs.k8s.io/e2e-framework. Kind's generated kubeconfig identifies the user as kubernetes-admin, which is also a member of system:masters. The entire test harness — namespace creation, Helm installs, assertion calls — runs under this identity, bypassing RBAC.
The controller's ServiceAccount does get the restricted Helm chart ClusterRole applied (the one missing get), but the tests only observe effects (e.g., "does a ValidatingWebhookConfiguration with this name now exist?"), not whether the controller reconciled without errors. They don't:

• Inspect the controller's logs for 403 errors
• Check the policy's .status conditions for reconcile failures
• Impersonate the controller's ServiceAccount to validate its permissions directly

I do agree we have to extend the e2e tests, but I do not agree with the remediation paths being proposed. I think we should understand how the GET operation is used, and check for changes that depend on that. I would not grep into the logs

[viccuad]

Here we are hardcoding RBAC, instead of making use of our RBAC files.

Can we instead of defining the structs by hand (why may drift in time), read from file, do a decoder.Decode() into an Unstructured struct, and then apply with the k8sClient.Create(ctx,obj)?

[jvanz]

In my first try I do load the resources from Helm chart files. But that require removing templates calls from files or rendering the files first. If we need to do that I would add e2e test instead of the ginkgo ones. But those are slower as well. Thus, I think hardcode the role here can be a good balance between speed and convenience to detect issue early in the development process.

[viccuad]

Argh, true, I forgot that our output is templated.
I'm ok not doing this, but maybe we should track is as tech debt, I'm scared on forgetting that the tests work as such and slipping regressions in the future.

[flavio]

I'm concerned about these structs going out of sync with the actual RBAC managed by the helm chart.

Can't we leave things as they were and write a test inside of tests done with the e2e-framework? I don't care about them slow, I'm more concerned about having out of sync data.

[viccuad]

I'm convinced now on moving this test to <root>/e2e/. We can call a helm template which uses the default values, to be able to consume the templated YAML.

[jvanz]

@kubewarden/kubewarden-developers I've added again the metric-reader role. While I was updating the telemetry documentation removing the references for Nginx ingress I realized that could be used by the sidecar to give access metrics endpoint via kube-rbac-proxy. Therefore, if we remove it, it may break the integration.

I'll test a little bit more telemetry with this changes.

[jvanz]

@kubewarden/kubewarden-developers I've added again the metric-reader role. While I was updating the telemetry documentation removing the references for Nginx ingress I realized that could be used by the sidecar to give access metrics endpoint via kube-rbac-proxy. Therefore, if we remove it, it may break the integration.

I'll test a little bit more telemetry with this changes.

As far as I can see, the metrics-reader role can be used by operators to give service accounts the credentials to read the metrics from the controller. The binding is missing by design. Just to ensure that we do not break the upgrade for any of our users, if we want to remove the role, I would mark it as deprecated and remove it in a later release.

[viccuad]

Approving! Looking forward to this landing, great work.

I would like to track the tech-debt on the hardcoding of the RBACs on the tests.

[viccuad]

I'm convinced now on moving this test to <root>/e2e/. We can call a helm template which uses the default values, to be able to consume the templated YAML.