🐛 (helm/v1-alpha): Use namePrefix from config/default/kustomization.yaml as prefix for RBAC rules and project name only if this value cannot be found.

[camilamacedo86]

Closes: #4566

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[camilamacedo86]

HI @mkarlheim

Could you please check this one?

[mkarlheim]

HI @mkarlheim

Could you please check this one?

Awesome, thanks! I'll try to do it shortly.

[mkarlheim]

Hi @camilamacedo86,

using the namePrefix by default and the project name as fallback is an improvement, but the core issue still exists in my opinion:
Executing kubebuilder create api --group testgroup --version v1 --kind Test leads to newly generated ClusterRoles named

• name: testgroup-test-admin-role
• name: testgroup-test-editor-role
• name: testgroup-test-viewer-role

So these generated ClusterRoles are not considered with adding a prefix at all. There must be something like a regex replace like .*-editor-role as the names of the ClusterRoles can differ depending on the apis and apigroups in the kubebuilder project.

[camilamacedo86]

Hi @mkarlheim

Executing kubebuilder create api --group testgroup --version v1 --kind Test leads to newly generated ClusterRoles named

When you change your project you need to re-run the helm plugin
The chart will not be updated by itself.

Also, ensure that you run make manifests and make generate before re-run the helm plugin.
The Chart is only updated when you say for it be done.

[camilamacedo86]

Closing because I think we will need to fix it another way, by changing the scaffolds and adding a prefix in the chart values instead.

[camilamacedo86]

/hold cancel

[camilamacedo86]

Hi @mkarlheim

Can you please verify this one now?
It seems that it addresses your need for now.

However, ultimately, we will need to change the Helm plugin implementation to generate the chart from the kustomize install, using YAML instead. See; #4833

[camilamacedo86]

/test pull-kubebuilder-e2e-k8s-1-33-0

[mkarlheim]

Hi @camilamacedo86,

many thanks for your efforts!

Unfortunately, it is still not the expected behaviour. All roles that are created with the Kustomize/v2 plugin, don't have a prefix in their generated yaml by default.

All the roles are prefixed in the moment when the Kustomize build (make build-installer) is executed as you described in #4833.

The function copyFileWithHelmLogic only considers the roles with fixed names like the metrics, election and leader roles, but not the crd-roles (admin, editor, viewer). The name of these roles is not fixed as it contains resource group and kind (in multgroup projects).

I was also thinking about using the Kustomize build output as the base for the Helm chart generation, as this would adress the issue with the namePrefix.

Another argument for your proposal would be:
In the config/rbac/kustomization.yaml there is a comment saying You can comment the following lines if you do not want those helpers be installed with your Project. Those types of changes wouldn't be considered as well, if someone commented out some roles (e.g.). Using the Kustomize build output as base for the Helm chart generation would also address those types of changes.

Best
Michael

[camilamacedo86]

Hi @mkarlheim

Thank you a lot for your time on this review 👍

Unfortunately, it is still not the expected behaviour. All roles that are created with the Kustomize/v2 plugin, don't have a prefix in their generated yaml by default.

But what we are doing here is ONLY doing what is done with kutomize
We will not change the files name.
The prefix ONLY change the prefix of the resource/RBAC itself ( metadata.name )

I think we need to modify one of the samples scenarios and add a test for it.
So that we can automatically validate and ensure that the result with helm is the same.

The function copyFileWithHelmLogic only considers the roles with fixed names like the metrics, election and leader roles, but not the crd-roles (admin, editor, viewer). The name of these roles is not fixed as it contains resource group and kind (in multgroup projects).

OK, we need to add a code to cover those 👍

In the config/rbac/kustomization.yaml there is a comment saying You can comment the following lines if you do not want those helpers be installed with your Project. Those types of changes wouldn't be considered as well.

OK. Good point for we recheck 👍

I was also thinking about using the Kustomize build output as the base for the Helm chart generation, as this would adress the issue with the namePrefix.

That seems the best way to move forward with the helm plugin.

/hold until check the points above