Skip to content

fix: Always create the operator ClusterRoleBinding - #637

Open
lfrancke wants to merge 1 commit into
push-xuvzqunkompnfrom
push-yvqzmvlknxvv
Open

fix: Always create the operator ClusterRoleBinding#637
lfrancke wants to merge 1 commit into
push-xuvzqunkompnfrom
push-yvqzmvlknxvv

Conversation

@lfrancke

@lfrancke lfrancke commented Aug 16, 2026

Copy link
Copy Markdown
Member

FYI: Reported by @dervoeti on stackabletech/listener-operator#418

I actually saw this in hive-operator but thought there was a good reason for it I was missing. Seems there is not.
This was added in 2022: #7 but it actually came earlier in stackabletech/zookeeper-operator#247 and Claude tells me that this looks very much like kubebuilder templates. I have not verified that but would trust it. So it seems we bootstrapped parts of that from kubebuilder but back then actually three objects were behind the if: ServiceAccount, ClusterRoleBinding and ClusterRole. @soenkeliebau then split the ClusterRole out and moving only that OUT of the if. I assume none of that was intentional.

@dervoeti suggested renaming all of this to rbac.create but then we should maybe move the ClusterRole back as well. I suggest we merge this as is though and we can always reconsider....

The one problem I have is that this might want a changelog entry which we can't do using templating as far as I'm aware... but at least a release note should be added.

@lfrancke lfrancke self-assigned this Aug 16, 2026
@lfrancke lfrancke moved this to Development: Waiting for Review in Stackable Engineering Aug 16, 2026
@lfrancke
lfrancke marked this pull request as ready for review August 16, 2026 19:07
serviceAccount.create gated both the ServiceAccount and the ClusterRoleBinding,
so with create=false the chart rendered the ClusterRole but nothing bound it. The
operator then ran as the ServiceAccount the user supplied, with none of the
permissions it needs, and nothing said so.

The binding now renders unconditionally, like the ClusterRole in roles.yaml
already does. serviceAccount.create=false means "I supply the ServiceAccount",
and the chart still wires it to the operator ClusterRole.

This is a regression, not the original design. zookeeper-operator#247 (0265e72,
2021-11-11) had the ServiceAccount, the ClusterRole and the ClusterRoleBinding
all inside the same gate, so create=false coherently meant "I provide the RBAC".
operator-templating#7 (2ce5e08, 2021-11-24) split the ClusterRole out into
roles.yaml.j2 without a gate and left the binding behind in one.

Also corrects the comment above the binding, which was kubebuilder scaffold text
about reading secrets in any namespace. The same commit carried other scaffold
leftovers, such as rules granting pods and configmaps under a comment about
accessing Secret objects.

No object changes in the default render. The corrected comment does change it,
because template comments end up in the rendered manifest.

Reported by @dervoeti on listener-operator#418.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Development: Waiting for Review

Development

Successfully merging this pull request may close these issues.

1 participant