Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up handling of target namespaces #302

Closed
metacosm opened this issue Jan 14, 2021 · 1 comment · Fixed by #303
Closed

Clean up handling of target namespaces #302

metacosm opened this issue Jan 14, 2021 · 1 comment · Fixed by #303
Assignees

Comments

@metacosm
Copy link
Collaborator

We need to settle how we configure which namespace(s) a controller targets and how we configure this. Right now, there are conflicting versions, in particular when it comes to interpreting what no passed namespaces means. ControllerConfiguration assumes that the current namespace is the target if the controller doesn't specify any, using the WATCH_ALL_NAMESPACES_MARKER to decide whether the controller watches all namespaces, while other parts of the code assume that if target namespaces is empty then that means that the controller should watch all namespaces.

That historic behavior is problematic, in my opinion, because the default behavior for a controller should be to watch the namespace it is deployed on as it might not have privileges outside of that particular namespace.

We should also remove all the register methods on Operator apart from this one which uses the controller's configuration to appropriately register the controller accordingly.

@metacosm metacosm self-assigned this Jan 14, 2021
metacosm added a commit that referenced this issue Jan 14, 2021
The intent here is that controllers should be registered according to
their configuration. This will become even more relevant when
externalized configuration is in place (see #237). This also means
using ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER in the
namespaces field of the Controller annotation where appropriate since
that's what ControllerConfiguration uses to determine if a controller
should watch all namespaces. This still needs to be unified across the
code base (see #302).
metacosm added a commit that referenced this issue Jan 14, 2021
The intent here is that controllers should be registered according to
their configuration. This will become even more relevant when
externalized configuration is in place (see #237). This also means
using ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER in the
namespaces field of the Controller annotation where appropriate since
that's what ControllerConfiguration uses to determine if a controller
should watch all namespaces. This still needs to be unified across the
code base (see #302).
metacosm added a commit that referenced this issue Jan 19, 2021
The intent here is that controllers should be registered according to
their configuration. This will become even more relevant when
externalized configuration is in place (see #237). This also means
using ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER in the
namespaces field of the Controller annotation where appropriate since
that's what ControllerConfiguration uses to determine if a controller
should watch all namespaces. This still needs to be unified across the
code base (see #302).
metacosm added a commit that referenced this issue Jan 20, 2021
The intent here is that controllers should be registered according to
their configuration. This will become even more relevant when
externalized configuration is in place (see #237). This also means
using ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER in the
namespaces field of the Controller annotation where appropriate since
that's what ControllerConfiguration uses to determine if a controller
should watch all namespaces. This still needs to be unified across the
code base (see #302).
@metacosm
Copy link
Collaborator Author

The decision was made to keep the behavior where not specifying any namespace would result in all namespaces being watched by default by the controller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant