quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.56k stars 2.62k forks source link

kubernetes-config: Ability to switch off RBAC resource generation #32761

Open edeandrea opened 1 year ago

edeandrea commented 1 year ago

Description

When using the kubernetes-config extension it seems to generate a RoleBinding and a ServiceAccount. It would be nice to be able to disable this. In some instances when combining with the kubernetes extension (and other variants) the user may provide their own RoleBinding and/or ServiceAccount objects.

This is essentially the same as from #25756 which was implemented for the kubernetes-client.

@alexgroom feel free to add any additional detail.

Implementation ideas

No response

quarkus-bot[bot] commented 1 year ago

/cc @Sgitario (kubernetes), @geoand (kubernetes), @iocanel (kubernetes)

geoand commented 1 year ago

I think it makes sense. I'll take care of it

alexgroom commented 1 year ago

I found using the OpenShift deployment plugin the build would fail unless you were Project admin (and able to modify RoleBindings and ServiceAccounts), which shouldn't really be assumed. A dev with Project Edit should still be able to use this feature to update their app. So an OFF switch would be very sensible here

geoand commented 1 year ago

I'll use this as an opportunity to refactor the extension to follow the latest Smallrye Config practices

geoand commented 1 year ago

Actually, I see that the code also sets a Role or a ClusterRole in addition to the ServiceAccount and RoleBinding, so I am not sure if that should be disabled as well...

Sgitario commented 1 year ago

Actually, I see that the code also sets a Role or a ClusterRole in addition to the ServiceAccount and RoleBinding, so I am not sure if that should be disabled as well...

At the moment, the Role and ClusterRole generation can be disabled. Though, I think the new property should disable also these two resources as well.

geoand commented 1 year ago

At the moment, the Role and ClusterRole generation can be disabl

But not for quarkus-kubernetes-config, or am I missing something?

Sgitario commented 1 year ago

At the moment, the Role and ClusterRole generation can be disabl

But not for quarkus-kubernetes-config, or am I missing something?

Yes, it can be disabled for the quarkus-kubernetes-config extension using the property quarkus.kubernetes-config.secrets-role-config.generate=false: https://quarkus.io/version/main/guides/kubernetes-config#quarkus-kubernetes-config_quarkus.kubernetes-config.secrets-role-config.generate

geoand commented 1 year ago

Ah okay, yeah, we are talking about the same thing then :).

It seems to me that we need to deprecate the old property and introduce two new ones, one for each Role / ClusterRole and one for ServiceAccount and RoleBinding. WDYT?

Sgitario commented 1 year ago

Ah okay, yeah, we are talking about the same thing then :).

It seems to me that we need to deprecate the old property and introduce two new ones, one for each Role / ClusterRole and one for ServiceAccount and RoleBinding. WDYT?

You can only generate either a Role or a ClusterRole at the same time, so we don't need a property for each and I would not also deprecate the existing property, but adding a new property to also control the generation of the ServiceAccount and RoleBinding.

The only thing is that the new property would also disable the generation of the Role/ClusterRole resource.

geoand commented 1 year ago

You can only generate either a Role or a ClusterRole at the same time

Right hence the use of / :)

geoand commented 1 year ago

I would not also deprecate the existing property, but adding a new property to also control the generation of the ServiceAccount and RoleBinding

I don't agree, because the current property is called generate which means it's generic and therefore it's confusing to only have it control some manifests

Sgitario commented 1 year ago

I would not also deprecate the existing property, but adding a new property to also control the generation of the ServiceAccount and RoleBinding

I don't agree, because the current property is called generate which means it's generic and therefore it's confusing to only have it control some manifests

generate is part of quarkus.kubernetes-config.secrets-role-config which I think it was meant to control only the secret role / secret cluster role to use.

I think keeping it as it is, it makes sense to use cases when you want to generate the Role/ClusterRole, but not the Service Account/RoleBinding.

Anyway, if you all disagree with this and prefer having a property quarkus.kubernetes-config.generate-rbac (similar to the existing one in kubernetes-client), it's also fine with me.

geoand commented 1 year ago

Let's have @iocanel break the tie :)

edeandrea commented 1 year ago

Wouldn't consistency between the kubernetes-client & kubernetes-config be good?

geoand commented 1 year ago

Of course, but does rhe client have the same feature flags?

edeandrea commented 1 year ago

Gentle ping on this issue @geoand / @Sgitario / @iocanel

edeandrea commented 10 months ago

Another gentle ping on this issue @geoand / @Sgitario / @iocanel ...

geoand commented 10 months ago

Given that Jose has moved on to other things and that Ioannis and I are deep into doing other high priority Quarkus related things, I don't see this being picked up any time soon

edeandrea commented 2 months ago

I'm happy to work on this if you could give me a gentle nudge to a starting point...

geoand commented 2 months ago

I would start here