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

disambiguation of installMode #1487

Closed
raffaelespazzoli opened this issue Apr 28, 2020 · 8 comments
Closed

disambiguation of installMode #1487

raffaelespazzoli opened this issue Apr 28, 2020 · 8 comments
Labels
triage/unresolved Indicates an issue that can not or will not be resolved.

Comments

@raffaelespazzoli
Copy link

Feature Request

I think the installMode is conflating too many concepts:

  1. where is the operator going to run
  2. which type of resources does it watch: cluster or namespaced
  3. if namespaced resources, which namespace(s) does it watch.

I think #1 should be an installation time choice that has nothing to do with the rest. It can be a filed in the OLM UI, and I think in the new versions of the OLM UI, the user will be allowed to create the namespace on the fly.

#3 makes sense only if the operator watches namespaced resources, which can be deducted from the csv. Also it's not obvious to assume that if the operator watches multiple resources and they are namespaced, then the watched namespaces will be the same for all resources.
If the operator watches a mix of cluster and namespaced resources then the question #3 does not make much sense. The safest thing would be probably to ask the question for each of the namespaced resource.

It seems to be that the MultipleNamespace option is not supported and could probably be removed.

Also the OwnNamespace is a particular case of the SingleNamspace option and it's hard to see why it needs to be called out. However, it should be noted that letting an operator watch the same namespace in which the operator pod is installed is generally to be considered a bad practice if the operator sets a finalizer. In fact under this circumstance, an unavoidable race condition occurs in which when the namespace is deleted, if the operator pod is destroyed before all the finalizers had time to run, some CRs will be stuck and the namespace will not be successfully deleted.
I'd recommend removing the OwnNamespace option, and giving a warning to the customer if they try to set an operator to watch the same namespace in which it is installed using the SingleNamespace option.

@ecordell
Copy link
Member

ecordell commented May 5, 2020

installModes indicate the ways in which an operator can be configured to watch namespaces.

(1), where the operator is going to run, is unrelated to installMode. You can create a CSV or a Subscription in any namespace, and that is where the operator will install and run.

installModes don't indicate which type of resources your operator watches. This is defined by the permissions and clusterPermissions block in the CSV, so I'm not sure I follow the concern you raise in (2).

The one place where installModes can affect permissions is when:

  • An operator indicates it needs rbac access to namespaced resources in the namespaces it watches (via permissions in the CSV)
  • And that operator supports AllNamespace installMode, meaning that if granted permission it can correctly watch all namespaces and provide services in those namespaces.
  • And that operator is installed into a namespace with an OperatorGroup configured to watch AllNamespaces

Then - RBAC that would normally be generated at the namespace-level will be generated at the cluster level (as ClusterRoles), which seems correct given the scenario described above - the operator is configured to watch all namespaces, and has indicated a requirement to have certain permissions in any namespace it watches.

With this explanation, do you still feel there is some ambiguity / conflation of ideas? Or have I missed what you were getting at?

You raised some other good points:

Also it's not obvious to assume that if the operator watches multiple resources and they are namespaced, then the watched namespaces will be the same for all resources.

That is true, but this simplification makes requested permissions easier to reason about, install, and configure. I don't think it will stay this way forever, we just want to have a good UX around it.

It seems to be that the MultipleNamespace option is not supported and could probably be removed.

It's not officially deprecated, but there are enough caveats to using it that we don't surface it or advertise it. (For example, installing two MultiNamespace operators that share an CRD and deploy a ConversionWebhook will cause a problem for one of the two operators).

Also the OwnNamespace is a particular case of the SingleNamspace option and it's hard to see why it needs to be called out.

OwnNamespace is called out precisely because of the issues you mention (plus the additional issue that it will grant anyone with the ability to create workloads in the installation namespace the full permission of the operator).

@raffaelespazzoli
Copy link
Author

@ecordell thanks for your answer. it is clearer now.
One more question: can you describe, or point to a document that describes how permissions are propagated based on the CVS definition and installmode.

That's another area where it's confusing for me: the permissions an operator needs in the namespace it runs, are different than the permissions it needs on a namespace it needs to watch, which is different from the permissions a user needs to create/view the operator's CRs.

Can you explain how all of that works?

@raffaelespazzoli
Copy link
Author

also following the reasoning that in general SingleNamespace is a better than OwnNamespace, I tried to install an operator in SingleNamespace mode, but failed, here is the issue: #1499

I realize this could be just a bug of that operator, but could you explain how all that process should work? @ecordell

@stale
Copy link

stale bot commented Jul 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 4, 2020
@openshift-ci-robot openshift-ci-robot added triage/unresolved Indicates an issue that can not or will not be resolved. and removed wontfix labels Jul 5, 2020
@stale
Copy link

stale bot commented Sep 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Nov 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Jan 3, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@dinhxuanvu
Copy link
Member

This issue is stale. Closed. Please feel free to reopen if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/unresolved Indicates an issue that can not or will not be resolved.
Projects
None yet
Development

No branches or pull requests

4 participants