Simplifying namespaces handling

[metacosm]

Current issues

There are still some issues regarding how namespaces are handled. To wit:

• if you generate manifests using the generate-with-watched-namespaces property but have annotations on your controllers that contradict what got generated, and you don't explicitly override the namespaces at runtime, then your controllers might not end up watching the namespaces you intend them to and/or not have the proper permissions to do so
• build-time namespaces values are currently propagated at runtime, which might lead to difficult to ascertain the configuration because, when deploying an operator, the annotation configuration, which might end up being used at runtime if not explicitly overridden, might end up being used.
• propagation of build-time namespaces at runtime makes it tricky to figure out whether default values as provided by quarkus.operator-sdk.namespace need to be applied or not (basically, it is not straightforward to account for all possible cases and detect whether a runtime configuration occurred due to explicit user ask or because of propagation of build-time values).

Proposal to address these issues

• Removal of automatic propagation of build-time values to runtime, with the exception of dev mode where the build-time properties will be made available so that the operator can be developed locally with a setup matching the expected production topology without having to either deploy the operator or manually set runtime values
• Build-time values will be generated as env variables corresponding to the runtime properties in the generated manifests (this is already the case for generated CSV files as of 6.3.3 but this will be extended to the generated Kubernetes descriptors and Helm charts). This will have the advantage of propagating the configuration to runtime in an explicit way, achieving similar effect than automatic propagation without the opaqueness. This will also simplify configuring the controllers at runtime since only the values will need to be changed when deploying as the env variables will already exist

/cc @jcechace @shawkins @csviri

[jcechace]

@metacosm sounds good. However i think a more in depth breakdown is required. Here is my swing at it.

QOSDK Runtime resolution

Runtime Namespace Resolution

• Each controller watched namespaces should follow this priority: QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES -> QUARKUS_OPERATOR_SDK_NAMESPACES -> Default

Note: Ignoring the annotation is possibility, HOWEVER it’s going to be a bit confusing for the developers as it's going to create a difference beween QOSDK and JOSDK (however I am generally fine with it).

Buildtime Namespace Handling

Build time configuration only influences what gets generated into manifest via QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES, QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES and @ControllerConfiguration.namespaces

Every operator deployment should be generated with single QUARKUS_OPERATOR_SDK_NAMESPACES and/or (potentially multiple) QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES


What to Generate and When

1. QUARKUS_OPERATOR_SDK_NAMESPACES is always generated when QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES is explicitly specified
2. QUARKUS_OPERATOR_SDK_NAMESPACES is generated if there is a controller which is neither Annotated nor QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES is specified during build for that controller. In such case the generated value is the JOSDK default (which is "JOSDK_ALL_NAMESPACES")
3. QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES is generated If controller is either annotated with @ControllerConfiguration.namespaces in source code or QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES is set during build and the value is different from that of QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES

ClusterRoleBindings / RoleBindings

1. If deployment gets generated with either QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_ALL_NAMESPACES" or any QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_ALL_NAMESPACES" then CluseterRoleBindings should be generated generated (Note that such Kubernetes.yaml will be invalid as subject.namespace will be missing -- not much we can do, unless we want to generate kustomize manifests instead of plain k8. I'd personally always generate something like <PLACEHOLDER_OPERATOR_NS> here)

2. If deployment gets generated with either QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_WATCH_CURRENT" or any QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_WATCH_CURRENT" then RoleBindings should be generated generated

3. If deployment gets generated with either QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="namespace1,namespace2" or any QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="namespace1,namespace2 then RoleBindings should be generated generated in namespace1 and namespace2 (note that again, such RoleBindings will require subject.namespace which is again not known at this point.

Note: In order for kubernetes.yaml to require the least amount of manuall modifications metadata.namespace should be generated only on those descriptor where this is required (so the RoleBinding in case 3

[jcechace]

BTW: overall I still think that the ability to specify watch namespaces per controller is introducing a horrible amount of complexity for cases which are not really used in production. ServiceAccount is specified per pod not per controller (which is an abstract concept) so in the end all controllers do have the same permissions in k8 cluster -- the restriction comes from JODK internals not from K8's RBAC.

You are essentially taking an already complex system (K8's RBAC) and making it even more complicated :)

[shawkins]

BTW: overall I still think that the ability to specify watch namespaces per controller is introducing a horrible amount of complexity for cases which are not really used in production.

I second this, but would ask what were the use cases for the original support? Did a user have a case where a set of resources were watched in a single namespace, but others were potentially spead across namespaces? For managed kafka there was some initial thought for the sync component to work this way - have it copy the control plane state into a single namespace, then let the managed kafka operator create namespaces as needed from there. However that was rejected as it either requires two watches / informers, or you have to treat one namespace as special in the reconciliation logic. It could be this is simply allowing users to do something that in general shouldn't be done.

If deployment gets generated with either QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_ALL_NAMESPACES"

If quarkus.kubernetes.namespace is set, it should use that for the subject namespace.

[jcechace]

If quarkus.kubernetes.namespace is set, it should use that for the subject namespace.

Unfortunately it's not that simple.. if that is set, the metadata.namespace of everything will be set to that value (which is not desirable. TLDR... it's bit of a mess :)

[shawkins]

Unfortunately it's not that simple.. if that is set, the metadata.namespace of everything will be set to that value (which is not desirable. TLDR... it's bit of a mess :)

Let me clarify what I mean. Let's ignore the per controller watching for a minute. This really boils down to only a couple of things - the default watching behavior, the deploy namespace, and what you are watching at deploy time. I'd like it to be as simple as:

# default to watch current for non-olm scenarios.  false would be watch all
quarkus.operator.sdk.default-watch-single=true
quarkus.operator.sdk.deploy.namespace=${quarkus.kubernetes.namespace}
quarkus.operator.sdk.deploy.watched-namespaces=default behavior could depend on quarkus.operator.sdk.default-watch-single

Such that if you use quarkus.kubernetes.deploy or quarkus.openshift.deploy it will take care of whatever needs to be manipulated in the manifests to deploy to that namespace and watch those namespaces. Otherwise the manifests would be usable for olm with the addition of target namespaces annotation handling.

[metacosm]

BTW: overall I still think that the ability to specify watch namespaces per controller is introducing a horrible amount of complexity for cases which are not really used in production. ServiceAccount is specified per pod not per controller (which is an abstract concept) so in the end all controllers do have the same permissions in k8 cluster -- the restriction comes from JODK internals not from K8's RBAC.

As far as I'm aware, it is possible to deploy your operator in separate pods and this scenario is supported by the OLM generator that allows you to generate separate bundles for your controllers if needed. That said, it's probably easier and a better practice to separate such controllers into separate projects, maybe.

You are essentially taking an already complex system (K8's RBAC) and making it even more complicated :)

Well, we're not doing anything RBAC related. The complexity comes from provided the proper rights to the proper bits, not extending RBACs.

[metacosm]

BTW: overall I still think that the ability to specify watch namespaces per controller is introducing a horrible amount of complexity for cases which are not really used in production.

I second this, but would ask what were the use cases for the original support? Did a user have a case where a set of resources were watched in a single namespace, but others were potentially spead across namespaces? For managed kafka there was some initial thought for the sync component to work this way - have it copy the control plane state into a single namespace, then let the managed kafka operator create namespaces as needed from there. However that was rejected as it either requires two watches / informers, or you have to treat one namespace as special in the reconciliation logic. It could be this is simply allowing users to do something that in general shouldn't be done.

I don't think there was any particular use case but I don't think that deploying a controller in a namespace while watching a different and also having controllers that watch native kubernetes resources in other specific namespaces is not that far fetched. Of course, we could use only watch all namespaces and make things simpler. The issue, though, is about reducing as much as possible the permissions granted to controllers. If people think that's too much complexity, we can definitely revise things: I just want things to be as simple as possible but no less and I'd certainly be happy to remove complexity from the code. :)

That said, we have users that dynamically change the namespaces they watch at runtime so I suspect removing that complexity is not going to happen :)

If deployment gets generated with either QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_ALL_NAMESPACES"

If quarkus.kubernetes.namespace is set, it should use that for the subject namespace.

That is already was is being done and the extension even lets you know that this is what you should be doing to get a correctly generated manifest.

[metacosm]

If quarkus.kubernetes.namespace is set, it should use that for the subject namespace.

Unfortunately it's not that simple.. if that is set, the metadata.namespace of everything will be set to that value (which is not desirable. TLDR... it's bit of a mess :)

Yes, that's indeed a problem.

[shawkins]

As far as I'm aware, it is possible to deploy your operator in separate pods and this scenario is supported by the OLM generator that allows you to generate separate bundles for your controllers if needed. That said, it's probably easier and a better practice to separate such controllers into separate projects, maybe.

The only practice for that should be separate projects. Only a single jar is being produced and you could via cdi have wirings between your controllers - having them somehow in separate pods greatly confuses things.

I'd like it to be as simple as

@metacosm any thoughts on radical simplification as proposed above?

[metacosm]

I'd like it to be as simple as

@metacosm any thoughts on radical simplification as proposed above?

I think I'd need to see a more detailed explanation. In any case, this kind of change probably wouldn't happen before the next major version.

[metacosm]

@metacosm sounds good. However i think a more in depth breakdown is required. Here is my swing at it.

QOSDK Runtime resolution

Runtime Namespace Resolution

* Each controller watched namespaces should follow this priority:  `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES` -> `QUARKUS_OPERATOR_SDK_NAMESPACES ` -> Default

What does "Default" mean in this context? Also, not clear on the priority here, though I assume that -> means "is of higher priority than", correct?

Note: Ignoring the annotation is possibility, HOWEVER it’s going to be a bit confusing for the developers as it's going to create a difference beween QOSDK and JOSDK (however I am generally fine with it).

Buildtime Namespace Handling

Build time configuration only influences what gets generated into manifest via QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES, QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES and @ControllerConfiguration.namespaces

Every operator deployment should be generated with single QUARKUS_OPERATOR_SDK_NAMESPACES and/or (potentially multiple) QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES


Yes, that is what I'm proposing as well.

What to Generate and When

1. `QUARKUS_OPERATOR_SDK_NAMESPACES` is always generated when `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES` is **explicitly specified**

I think it should always be generated, period. Are there cases where you wouldn't want the env variable to be output in the manifests?

2. `QUARKUS_OPERATOR_SDK_NAMESPACES` is  generated if there is a controller which is neither Annotated nor `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES`  is specified during build for that controller. In such case the generated value is the JOSDK default (which is `"JOSDK_ALL_NAMESPACES"`)

I don't understand what's the point of this, as watching all namespaces will be the default all the time unless a runtime property is used.

3. `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES`  is generated If controller is either annotated with `@ControllerConfiguration.namespaces` in source code or `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES` is set during build  and the value is different from that of `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES`

To make things simpler, I think that all controllers should have their associated env variable generated all the time with the resolved value (either the default one, the one from the annotation, or the one from the generate-with-watched-namespaces property)

ClusterRoleBindings / RoleBindings

1. If deployment gets generated with either  `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_ALL_NAMESPACES"` or any `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_ALL_NAMESPACES"` then CluseterRoleBindings should be generated generated (Note that such Kubernetes.yaml will be invalid as subject.namespace will be missing -- not much we can do, unless we want to generate kustomize manifests instead of plain k8. I'd personally always generate something like `<PLACEHOLDER_OPERATOR_NS>` here)

That's how things are currently done, though we don't use a placeholder. I debated using one and settled on not setting the namespace at all but we definitely could output a placeholder if that's easier.

2. If deployment gets generated with either  `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_WATCH_CURRENT"` or any `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_WATCH_CURRENT"` then RoleBindings should be generated generated

This is the case as well.

3. If deployment gets generated with either  `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="namespace1,namespace2"` or any `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="namespace1,namespace2` then RoleBindings should be generated generated in `namespace1` and `namespace2` (note that again, such RoleBindings will require `subject.namespace` which is again not known at this point.

I'm not sure that setting subject.namespace is needed when using ClusterRole for this. But, yes, a RoleBinding is generated per watched namespace, currently.

Note: In order for kubernetes.yaml to require the least amount of manuall modifications metadata.namespace should be generated only on those descriptor where this is required (so the RoleBinding in case 3.

Not sure I'm following.

[shawkins]

I think I'd need to see a more detailed explanation. In any case, this kind of change probably wouldn't happen before the next major version.

I'm assuming there would still be something like quarkus.operator-sdk.namespaces, but you would not set it directly.

quarkus.operator.sdk.default-watch-single=true

If true the manifests will be generated with RoleBindings - this would be expected most of the time. Only if you were not targeting usage with OLM and wanted your operator default behavior to watch multiple would you set it to false. If true quarkus.operator-sdk.namespaces (for the purposes of generating the manifests) would effectively be JOSDK_WATCH_CURRENT, otherwise JOSDK_ALL_NAMESPACES.

The other properties:

quarkus.operator.sdk.deploy.namespace=${quarkus.kubernetes.namespace} quarkus.operator.sdk.deploy.watched-namespaces=default behavior could depend on quarkus.operator.sdk.default-watch-current

Would be used when quarkus.something.deploy is used. If quarkus.kubernetes.namespace is not specified, quarkus.operator.sdk.deploy.namespace would default to current. This of course would be the namespace where the operator manifest is applied. If ClusterRoleBindings are in use, then the subject namespace would use this as well.

quarkus.operator.sdk.deploy.watched-namespaces hides most of the complexity. It would likely default to watch current or watch all - whatever quarkus.operator.sdk.default-watch-single implies. If you override it, then something would have to at least take responsibility for converting RoleBindings into ClusterRoleBindings should watch all be specified and the default behavior is default-watch-single=true. This will also take responsibility for setting the necessary env for QUARKUS_OPERATOR_SDK_NAMESPACES.

What's not covered by doing it this way:

• for non-olm what if I want to install into a fixed namespace only? If that's something that needs to be supported then there would have to be some option to treat the deploy namespace as a runtime property to affect the manifest generation.
• for non-olm what if I want to watch a fixed single or multiple namespaces? I think this should be covered by the ability to set the namespaces on the controller annotation.

[jcechace]

BTW: overall I still think that the ability to specify watch namespaces per controller is introducing a horrible amount of complexity for cases which are not really used in production. ServiceAccount is specified per pod not per controller (which is an abstract concept) so in the end all controllers do have the same permissions in k8 cluster -- the restriction comes from JODK internals not from K8's RBAC.

As far as I'm aware, it is possible to deploy your operator in separate pods and this scenario is supported by the OLM generator that allows you to generate separate bundles for your controllers if needed. That said, it's probably easier and a better practice to separate such controllers into separate projects, maybe.

Well technically CSV allows for a list of deployments -- in practice I've never seen such operator. That said... if you generate controllers into sepparate bundles you will esentially create sepparate opperators. In such case it indeed makes sense to have them in sepparate projects/modules

You are essentially taking an already complex system (K8's RBAC) and making it even more complicated :)

Well, we're not doing anything RBAC related. The complexity comes from provided the proper rights to the proper bits, not extending RBACs.

True but RBAC is still invovled when doing anthing related to roles

[jcechace]

quarkus.kubernetes.deploy

Lets pretend the ability to have quarkus deploy the Operator into the kluser is not even an option... You are never ever going to use that outside of testing. Ask Fabric8... there is a reason why they are not around anymore.

[jcechace]

@metacosm sounds good. However i think a more in depth breakdown is required. Here is my swing at it.

QOSDK Runtime resolution

Runtime Namespace Resolution

* Each controller watched namespaces should follow this priority:  `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES` -> `QUARKUS_OPERATOR_SDK_NAMESPACES ` -> Default

What does "Default" mean in this context? Also, not clear on the priority here, though I assume that -> means "is of higher priority than", correct?

Indeed.. it was form highes priority to lowest. Default but would JOSDK_ALL_NAMESPACES I guess (provided we cut the annotation out of runtime alltogether).

Note: Ignoring the annotation is possibility, HOWEVER it’s going to be a bit confusing for the developers as it's going to create a difference beween QOSDK and JOSDK (however I am generally fine with it).

Buildtime Namespace Handling

Build time configuration only influences what gets generated into manifest via QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES, QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES and @ControllerConfiguration.namespaces Every operator deployment should be generated with single QUARKUS_OPERATOR_SDK_NAMESPACES and/or (potentially multiple) QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES

Yes, that is what I'm proposing as well.

What to Generate and When

1. `QUARKUS_OPERATOR_SDK_NAMESPACES` is always generated when `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES` is **explicitly specified**

I think it should always be generated, period. Are there cases where you wouldn't want the env variable to be output in the manifests?

This was a breakdown so I really specifically talked about when this is explcitly specified during build. But since you asked for the example...

Assume we have two controlles, con1 which ought to watch ns1 and con2 which ought to watch ns2. So the generated manifest would contains

QUARKUS_OPERATOR_SDK_CONTROLLERS_CON1_NAMESPACES=ns1 QUARKUS_OPERATOR_SDK_CONTROLLERS_CON2_NAMESPACES=ns2

What would be the point of having QUARKUS_OPERATOR_SDK_NAMESPACES present? I mean sure, it can be there, it just wont do anything.

2. `QUARKUS_OPERATOR_SDK_NAMESPACES` is  generated if there is a controller which is neither Annotated nor `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES`  is specified during build for that controller. In such case the generated value is the JOSDK default (which is `"JOSDK_ALL_NAMESPACES"`)

I don't understand what's the point of this, as watching all namespaces will be the default all the time unless a runtime property is used.

3. `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_NAMESPACES`  is generated If controller is either annotated with `@ControllerConfiguration.namespaces` in source code or `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES` is set during build  and the value is different from that of `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES`

To make things simpler, I think that all controllers should have their associated env variable generated all the time with the resolved value (either the default one, the one from the annotation, or the one from the generate-with-watched-namespaces property)

ClusterRoleBindings / RoleBindings

1. If deployment gets generated with either  `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_ALL_NAMESPACES"` or any `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_ALL_NAMESPACES"` then CluseterRoleBindings should be generated generated (Note that such Kubernetes.yaml will be invalid as subject.namespace will be missing -- not much we can do, unless we want to generate kustomize manifests instead of plain k8. I'd personally always generate something like `<PLACEHOLDER_OPERATOR_NS>` here)

That's how things are currently done, though we don't use a placeholder. I debated using one and settled on not setting the namespace at all but we definitely could output a placeholder if that's easier.

replacing a value in yaml is easier than adding a new property. But it's not a big deal.

2. If deployment gets generated with either  `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_WATCH_CURRENT"` or any `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="JOSDK_WATCH_CURRENT"` then RoleBindings should be generated generated

This is the case as well.

3. If deployment gets generated with either  `QUARKUS_OPERATOR_SDK_GENERATE_WITH_WATCHED_NAMESPACES="namespace1,namespace2"` or any `QUARKUS_OPERATOR_SDK_CONTROLLERS_<NAME>_GENERATE_WITH_WATCHED_NAMESPACES="namespace1,namespace2` then RoleBindings should be generated generated in `namespace1` and `namespace2` (note that again, such RoleBindings will require `subject.namespace` which is again not known at this point.

I'm not sure that setting subject.namespace is needed when using ClusterRole for this. But, yes, a RoleBinding is generated per watched namespace, currently.

You are creating a RoleBinding in the watched namespace to a service account (subject) from a different namespace (that where operator is deployed). It has to be... It doesn't really matter if you are binding a Role or a ClusterRole.

Note: In order for kubernetes.yaml to require the least amount of manuall modifications metadata.namespace should be generated only on those descriptor where this is required (so the RoleBinding in case 3.

Not sure I'm following.

Ignore that part... those _GENERATE_WITH_NAMESPACE variables are not doing anyhting incorrect in this regard. I mixed things up with quarkus.kubernetes.namespace.

[shawkins]

Lets pretend the ability to have quarkus deploy the Operator into the kluser is not even an option...

I would want the quarkus folks to weigh in on that as well.

You are never ever going to use that outside of testing. Ask Fabric8... there is a reason why they are not around anymore.

I'm not sure I follow what you are saying here.

In any case let's categorize more into the deployment scenarios:

• olm - we don't need the operator sdk to do much other than to put the mapping from the target namespaces annotation to the josdk namespaces property during the bundle generation.
• quarkus - can be satisfied with the proposed deploy properties. As you say it may just be for testing / maven driven deployment (which is something that you can do today). Let's just leave this here for now since it's not something you want to consider.
• other - there are several cases here:
  • support multiple deployment modes (e.g. single ns and all namespaces) - This is simply not supportable by a single static set of manifests. If we want the quarkus JOSDK to support this option, then it will need options to generate multiple sets of manifests.
  • support a fixed single deployment mode - a fixed set of watched namespaces would be set on the Controller annotations.
  • support a flexiable single deployment mode - something like quarkus.operator.sdk.default-watch-single would control whether the base manifests have RoleBindings or ClusterRoleBindings it would be up to the user from there to modify those manifest to override the appropriate environment variable(s) to watch a given set of namespaces.
  • As mentioned above if the install namespace is fixed, then that as well would require some additional option.

[jcechace]

• olm - we don't need the operator sdk to do much other than to put the mapping from the target namespaces annotation to the josdk namespaces property during the bundle generation.
• quarkus - can be satisfied with the proposed deploy properties. As you say it may just be for testing / maven driven deployment (which is something that you can do today). Let's just leave this here for now since it's not something you want to consider.

• other - there are several cases here:

  • support multiple deployment modes (e.g. single ns and all namespaces) - This is simply not supportable by a single static set of manifests. If we want the quarkus JOSDK to support this option, then it will need options to generate multiple sets of manifests.

Agreed. That's ideally something which would be made available via Helm chart and it is IMHO out of scope for this analysis .

• support a fixed single deployment mode - a fixed set of watched namespaces would be set on the Controller annotations.
• support a flexiable single deployment mode - something like quarkus.operator.sdk.default-watch-single would control whether the base manifests have RoleBindings or ClusterRoleBindings it would be up to the user from there to modify those manifest to override the appropriate environment variable(s) to watch a given set of namespaces.
• As mentioned above if the install namespace is fixed, then that as well would require some additional option.

I'm not sure these are two diffrent cases. The problem at hand is to be able to clearly define what is generated and to have a clearly define resolution at runtime which can be predictably configured.

[shawkins]

I'm not sure these are two diffrent cases. The problem at hand is to be able to clearly define what is generated and to have a clearly define resolution at runtime which can be predictably configured.

Yes, we agree on the goal. I just want make sure we cut out as much as possible, so that there's less to document / consider.

To sum up what I'm advocating:

• Continue to treat Controller.namespaces if specified as forcing that behavior.
• Remove the ability to set / generate the namespaces via properties per controller.
• quarkus.operator-sdk.namespaces is set indirectly
• quarkus.kubernetes.namespace - this already handles the fixed install namespace scenario. If set it will be set as the namespace in the manifests including the clusterrolebinding subjects. In my opinion this would typically not be set for operator development. It could also be used in a templating fashion as mentioned in an earler comment.
• quarkus.operator.sdk.default-watch-single=true - defaults to true so that manifests produced out-of-the-box, regardless of whether quarkus.kubernetes.namespace is set, will work because rolebindings will be generated. If true quarkus.operator-sdk.namespaces should default to JOSDK_WATCH_CURRENT, or if false quarkus.operator-sdk.namespaces should default to JOSDK_ALL_NAMESPACES.

If well defined / separate deployment through quarkus is desirable, we can also have:

• quarkus.operator.sdk.deploy.namespace=${quarkus.kubernetes.namespace}
• quarkus.operator.sdk.deploy.watched-namespaces=default behavior could depend on quarkus.operator.sdk.default-watch-single

Otherwise it would be sufficient to keep the property:

• quarkus.operator-sdk.generate-with-watched-namespaces - such that the user could do something like: mvn ... -Dquarkus.kubernetes.deploy -Dquarkus.kubernetes.namespace=operator_deploy_target -Dquarkus.operator-sdk.generate-with-watched-namespaces=namespaces ... And treat the generated manifests as a throw away.

Beyond that it doesn't seem like we need anything else.

Why introduce a quarkus.operator.sdk.default-watch-single rather than just recommending or defaulting to: -quarkus.operator-sdk.namespaces=JOSDK_WATCH_CURRENT -quarkus.operator-sdk.generate-with-watched-namespaces=JOSDK_WATCH_CURRENT

There are several thoughts here:

1. It seems clearer to leave quarkus.operator-sdk.namespaces effectively as a runtime concern.
2. It doesn't seem like a user would need the ability to default to watch a fixed set of namespaces - for example would you expect someone to use quarkus.operator-sdk.namespaces=foo but also expect the ability to override foo via the deployment?
3. It may also not be clear that they can't simply override to foo,bar - unless they also utilize the generate-with-watched-namespaces property. That should be more explicit with default-watch-single (or whatever naming makes sense).

However if the conditional defaulting behavior is problematic to implement, then using quarkus.operator-sdk.namespaces instead of quarkus.operator.sdk.default-watch-single could be sufficiently explained.

[jcechace]

I'm not sure these are two diffrent cases. The problem at hand is to be able to clearly define what is generated and to have a clearly define resolution at runtime which can be predictably configured.

Yes, we agree on the goal. I just want make sure we cut out as much as possible, so that there's less to document / consider.

To sum up what I'm advocating:

• Continue to treat Controller.namespaces if specified as forcing that behavior. This is not a good idea -> Annotation is something inherently hidden on runtime. It creates confusing scenarios where you configure the runtime properties on deployment and the config is not honored (because some compile time thing took precedence over runtime -- which is illogical). I think @metacosm is on the same boat with this one.

[shawkins]

I think @metacosm is on the same boat with this one.

Keep in mind that I'm advocated for not supporting any properties to individually override the controllers, nor for users to directly set quarkus.operator-sdk.namespaces. That should eliminate the confusion over the role of the annotation property - it would be used only if you want to forcibly set the behavior of the controller. Otherwise it should not be specified.

[jcechace]

I think @metacosm is on the same boat with this one.

Keep in mind that I'm advocated for not supporting any properties to individually override the controllers, nor for users to directly set quarkus.operator-sdk.namespaces. That should eliminate the confusion over the role of the annotation property - it would be used only if you want to forcibly set the behavior of the controller. Otherwise it should not be specified.

If the user sets QUARKUS_OPERATOR_SDK_NAMESPACES they expect the operator to manage those namespaces. Having some internal config change the behaviour of single controller silently is really confusing IMHO.

So while I would agree that settings at controller level are not required (at the same time I don't have particularly strong feelings against them, just that they complicate the implmenetation) I am really against the idea of having build time configuration which sets watched namespaces but can't be overrriden on runtime by any means.

[shawkins]

I am really against the idea of having build time configuration which sets watched namespaces but can't be overrriden on runtime by any means.

I am not suggesting that. I don't want Controller.namespaces to be seen as anything other than as a hard-coded value. If you don't want it to be hard-coded, then you wouldn't use it. The absense of the value should mean let this be managed for you based upon other configuration.

[jcechace]

I am really against the idea of having build time configuration which sets watched namespaces but can't be overrriden on runtime by any means.

I am not suggesting that. I don't want Controller.namespaces to be seen as anything other than as a hard-coded value. If you don't want it to be hard-coded, then you wouldn't use it. The absense of the value should mean let this be managed for you based upon other configuration.

It would be a hardcoded value which seemingly has a runtime override -- except it doesn't. Lets agree to disagree on this one. In the end the decision is on QOSDK guys.

[metacosm]

Lets pretend the ability to have quarkus deploy the Operator into the kluser is not even an option...

I would want the quarkus folks to weigh in on that as well.

We had discussions on that front when we decided to remove the duplication of namespaces at both build and run times. We concluded that people probably would rarely use generated manifests because they're not flexible enough to properly deploy an operator so minimal efforts are done to ensure they are generated in a way that they should work for testing purposes (that's the goal of the generate-with-watched-namespaces property: you basically know how your test environment looks like, in a static way, and you just want to be able to deploy to your cluster using kubectl or maybe quarkus deploy.

For development, the operator is assumed to be running locally but maybe that's a wrong assumption on my part?

You are never ever going to use that outside of testing. Ask Fabric8... there is a reason why they are not around anymore.

I'm not sure I follow what you are saying here.

In any case let's categorize more into the deployment scenarios:

* olm - we don't need the operator sdk to do much other than to put the mapping from the target namespaces annotation to the josdk namespaces property during the bundle generation.

👍🏼

* quarkus - can be satisfied with the proposed deploy properties.  As you say it may just be for testing / maven driven deployment (which is something that you can do today).  Let's just leave this here for now since it's not something you want to consider.

That's the idea, yes, to be only used for testing.

* other - there are several cases here:

  * support multiple deployment modes (e.g. single ns and all namespaces) - This is simply not supportable by a single static set of manifests.  If we want the quarkus JOSDK to support this option, then it will need options to generate multiple sets of manifests.
  * support a fixed single deployment mode - a fixed set of watched namespaces would be set on the Controller annotations.
  * support a flexiable single deployment mode - something like quarkus.operator.sdk.default-watch-single would control whether the base manifests have RoleBindings or ClusterRoleBindings it would be up to the user from there to modify those manifest to override the appropriate environment variable(s) to watch a given set of namespaces.
  * As mentioned above if the install namespace is fixed, then that as well would require some additional option.

We now have Helm chart generation as an option as well.

[jcechace]

• olm - we don't need the operator sdk to do much other than to put the mapping from the target namespaces annotation to the josdk namespaces property during the bundle generation.

👍🏼

This is not entirely accurate. Actually whether the controller is set to watch the current/single or all namespaces matters.

Annotationg a controller with @ControllerConfiguration(namespaces = Constants.WATCH_CURRENT_NAMESPACE) will lead to set of permissions being generated in the CSV.

Annotationg a controller with @ControllerConfiguration(namespaces = Constants.WATCH_ALL_NAMESPACES) will lead to set of clusterPermissions being generated in the CSV.

Although there might be cases when you want the later, the former is usually what is desired -- even when the opperator is supposed to watch all namespaces -- due to how OLM handles roles (esentially if you want to watch all namespaces, OLM will create all role bindings for you based on operator groups).

[shawkins]

This is not entirely accurate. Actually whether the controller is set to watch the current/single or all namespaces matters.

Again, this is why I'm advocating for only using the namespaces in the annotation if you are hard-coding that behavior. In any other case it is simply adding confusion having it set there and then elsewhere via configuration.

[jcechace]

This is not entirely accurate. Actually whether the controller is set to watch the current/single or all namespaces matters.

Again, this is why I'm advocating for only using the namespaces in the annotation if you are hard-coding that behavior. In any other case it is simply adding confusion having it set there and then elsewhere via configuration.

I still don't follow what you want the use of the annotation to be... Previously you talked about how it should be preferred during runtime...

[shawkins]

I still don't follow what you want the use of the annotation to be... Previously you talked about how it should be preferred during runtime...

If the annotation \@ControllerConfiguration.namespaces parameter is set in the code, then that would be treated as a hard-coded value forcing that watching behavior. This would be the moral equivalent in the go sdk of doing something like:

mgr, err := ctrl.NewManager(cfg, manager.Options{Namespace: ""})

That declaratively means you will only ever watch all namespaces. It will never use a different namespace, or multiple namespaces. That gives a very definitive purpose for using that annotation parameter.

If left unset, then it means what you watch will be determined at runtime.

We now have Helm chart generation as an option as well.

So referring to these as .deploy properties probably doesn't make sense. Let me try a different tact, since discussing new properties or behavioral changes isn't working.

Imagine I'm a greenfield developer using QOSDK.

When creating the the \@ControllerConfiguration I don't see anything in the Javadocs saying what is overridable by configuration and what is not, nor the affect that the setting will have on the generated manifests. Let's say I'm lucky and didn't set the namespaces parameterl; ideally the behavior of launching the operator in the IDE, use the target/kubernetes directly (or quarkus deploy), or bundling everything up with olm should work without having to set additional stuff in the application.properties.

However that is not the case. The direct usage of the manifests and olm bundling won't work. Now I have to go hunting for some property to affect the generation of the manifests. Let's say I find quarkus.operator-sdk.generate-with-watched-namespaces and set it to watch current or a target namespace. I can use the manifests, but things still are a little off - if I launch in the IDE it's still going to watch all namespaces, but if I deploy using the manifests it's watching the single namespace. So I either accept that nuance or I have to override quarkus.operator-sdk.namespaces to match quarkus.operator-sdk.generate-with-watched-namespaces.

What I'm looking for is more information upfront or principled defaults that will simply work for the most basic scenario. If out-of-the box is just too messy given legacy concerns, then we'll need more steps and explanation shown in the olm deployment docs to call out exactly what and why you'll set via application.properties.

[jcechace]

I still don't follow what you want the use of the annotation to be... Previously you talked about how it should be preferred during runtime...

If the annotation @ControllerConfiguration.namespaces parameter is set in the code, then that would be treated as a hard-coded value forcing that watching behavior. This would be the moral equivalent in the go sdk of doing something like:

mgr, err := ctrl.NewManager(cfg, manager.Options{Namespace: ""})

That declaratively means you will only ever watch all namespaces. It will never use a different namespace, or multiple namespaces. That gives a very definitive purpose for using that annotation parameter.

If left unset, then it means what you watch will be determined at runtime.

Ok, got it. The thing is that GO then doesn't expose a standard environment variable which is supposed to this behvaiour. However I get what you mean and I wouldn't object too much if this was the case -- provied I can ommit the annotation and still generate the kubernetes.yaml (and thus consecutively the CSV) as if the annotation was present there with "whatch current" as value -- the reason being described in my previous comment abotu permissions and clusterPermissions.

[metacosm]

The way I see things is, and the way they've evolved is that the controller annotation is historically the only way to change which namespaces are watched in JOSDK. QOSDK changes things because then you can override the annotation value with properties or env variables (if the extension lets you, of course).

For a greenfield developer, the annotation is a quick way (I'd argue) to change the settings while running in dev mode: you change the annotation value, QOSDK picks up the change, re-configures things and restart the operator and you're off to the races. Historically, QOSDK has been developed for developer joy while writing the operator. Of course, then reality occurs and has to mess everything up! 😁 If you do a good job at developing your operator, at some point you'll want to deploy it to a cluster. That's where the kube / OLM / Helm manifests come into play. The question thus becomes how to make that transition from an operator running in dev mode locally to a cluster-deployed operator as frictionless as possible.

If you start with the idea that the annotation is the main way to change the namespaces during development (and again, if you use pure JOSDK, it's the only way to do it), then you need to be able to override that value at runtime so that you can still develop things locally as you want but change things dynamically after your operator is deployed. The alternative being that you otherwise need to ensure that the annotation values are either removed before you deploy or that they match exactly the deployment environment (which is not realistic).

One valid point that could be made is that namespaces are not likely to change much after the topology of the operator has been established so maybe the "emphasis" of making them easy to change during development is ill-founded. That said, I think it's still useful to be able to easily experiment with this aspect during the initial stages of the operator development.

We could argue on the value of being able to set different individual namespaces for different controllers. I don't have enough visibility on users' projects and environments to be able to tell either way. Historically, people have had the opportunity to set things this way and unless there's overwhelming evidence that it's not needed, I'd rather not remove such a feature.

So these are the constraints I have to work with and which led to propose the solution outline in this issue's description. Of note, this solution is not completely implementable at this point because removing the propagation of build time properties to runtime prevents Quarkus from being able to properly match environment variable names to the associated property. This will be addressed in a future Quarkus release. Alternatively, I am also considering #733 to address this issue but there is also another issue there with Quarkus that prevents the automated relocation of the quarkus.operator-sdk prefixed properties to the qosdk prefixed one at this point, thus preventing the transition from being done transparently, which is, imo, a must.

[jcechace]

@metacosm thanks for the reasoning. One question if I may.. When you say that the annotation is the only way to set the watched namespaces in vanilla JOSDK, I suppose you mean the only declarative way, right? I'm having issues imagining how would be such operator usable if you could really only hardcode this without the ability to provide runtime configuration.

[metacosm]

@metacosm thanks for the reasoning. One question if I may.. When you say that the annotation is the only way to set the watched namespaces in vanilla JOSDK, I suppose you mean the only declarative way, right? I'm having issues imagining how would be such operator usable if you could really only hardcode this without the ability to provide runtime configuration.

JOSDK doesn't address deployment at all. How people deal with watched namespaces at runtime is left to the user. It's, however, feasible to programmatically change the watched namespaces at runtime but there is no declarative way to do it outside of the annotation, indeed.

[csviri]

other - there are several cases here: support multiple deployment modes (e.g. single ns and all namespaces) - This is simply not supportable by a single static set of manifests. If we want the quarkus JOSDK to support this option, then it will need options to generate multiple sets of manifests.

This I think should not be supported, I remember a discussion, to have only helm that covers all the use cases, and the generated manifests to kubernetes.yaml will always watch whole cluster. I think this should be the case. (Maybe support the current namespace too )

[csviri]

Regarding the @ControllerConfiguration.namespaces , to be honest in v5 of JOSDK would remove this property from the annotation. Would still keep the functionality though. I've seens some operators where different controllers had different scope in terms of namespaces, this is not something very extreme or unusual (I think).

But the confusing part is really the annotation. What effectively does it overrides some default, what we can later again override.

[shawkins]

This I think should not be supported, I remember a discussion, to have only helm that covers all the use cases

It needs to be supported by OLM also.

and the generated manifests to kubernetes.yaml will always watch whole cluster. I think this should be the case. (Maybe support the current namespace too )

That could be fine as well, execpt there's no built-in notion of what the deployment namespace is (unless quarkuse.kubernetes.namespace is set) for the operator deployment so the clusterrolebindings subject namespaces are missing.

The thing is that GO then doesn't expose a standard environment variable which is supposed to this behvaiour.

That is correct, however it's quite clear to the user if they do choose to use an env variable how it will be picked up.

Without the clarity of saying the annotation parameter is a fixed value (or removing it entirely as @csviri is open to doing), it's quite odd in how it is handled. Imagine explaining it like it's a quarkus an injected config property:

\@ConfigProperty(name = "quarkus.operator-sdk.controllers.NAME.namespaces", defaultValue="something")

where there would be a natural property hierarchy:

quarkus.operator.namespaces=JOSDK_ALL_NAMESPACES quarkus.operator-sdk.controllers.NAME.namespaces=${quarkus.operator.namespaces}

However that's not quite correct - the default value takes precedence over quarkus.operator.namespaces and also influences the manifest generation. Both of those would be surprising to a quarkus developer.

[csviri]

IMO the precedence order would be clear if (in priority order):

1. controller namespaces explicitly set by property quarkus.operator-sdk.controllers.NAME.namespaces
2. quarkus.operator.namespaces
3. the default from annotation (if removed in future than just watch all namespaces)

the point is that with 2. we always override 3. (but also with 1. the 2. - maybe just some)

[jcechace]

@ConfigProperty(name = "quarkus.operator-sdk.controllers.NAME.namespaces", defaultValue="something")

where there would be a natural property hierarchy:

quarkus.operator.namespaces=JOSDK_ALL_NAMESPACES quarkus.operator-sdk.controllers.NAME.namespaces=${quarkus.operator.namespaces}

However that's not quite correct - the default value takes precedence over quarkus.operator.namespaces and also influences the manifest generation. Both of those would be surprising to a quarkus developer.

Well.. after my change the default wouldn't be used here. due toquarkus.operator-sdk.controllers.NAME.namespaces=${quarkus.operator.namespaces} being explicitly set

IMO the precedence order would be clear if (in priority order):

1. controller namespaces explicitly set by property quarkus.operator-sdk.controllers.NAME.namespaces
2. quarkus.operator.namespaces
3. the default from annotation (if removed in future than just watch all namespaces)

the point is that with 2. we always override 3. (but also with 1. the 2. - maybe just some)

This is exactly the order I was proposing :)

[metacosm]

other - there are several cases here: support multiple deployment modes (e.g. single ns and all namespaces) - This is simply not supportable by a single static set of manifests. If we want the quarkus JOSDK to support this option, then it will need options to generate multiple sets of manifests.

This I think should not be supported, I remember a discussion, to have only helm that covers all the use cases, and the generated manifests to kubernetes.yaml will always watch whole cluster. I think this should be the case. (Maybe support the current namespace too )

This isn't what was discussed. The decision was that another property was introduced to generate manifests with specific namespaces (generate-with-watched-namespaces). Without this property, kubernetes manifests would be considered as potentially invalid, though we could certainly make sure that they'd watch all namespaces by default, indeed. That's just not the case right now.

[jcechace]

other - there are several cases here: support multiple deployment modes (e.g. single ns and all namespaces) - This is simply not supportable by a single static set of manifests. If we want the quarkus JOSDK to support this option, then it will need options to generate multiple sets of manifests.

This I think should not be supported, I remember a discussion, to have only helm that covers all the use cases, and the generated manifests to kubernetes.yaml will always watch whole cluster. I think this should be the case. (Maybe support the current namespace too )

This isn't what was discussed. The decision was that another property was introduced to generate manifests with specific namespaces (generate-with-watched-namespaces). Without this property, kubernetes manifests would be considered as potentially invalid, though we could certainly make sure that they'd watch all namespaces by default, indeed. That's just not the case right now.

One thing we should keep in mind. It might be a good idea to remove the dependency between what is generated for kubernetes.yaml and for the OLM bundle. If I'm not mistaken, currently the configuration for raw k8 manifest is the base for OLM -- However, as I already mentioned, in basically all cases you don't want to have a base rendered for "watching all namespaces" as it creates the issue with generating clusterPermissions where one would want permissions. OLM takes care of setting up all the roles and bindings (essentially "watch all" in OLM doesn't grant any real cluster-wide permissions, instead it grants the namespace scoped permissions to all created namespaces individually).