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

Reorganized cluster roles, added rules to watch all namespaces #936

Merged

Conversation

jpkrohling
Copy link
Contributor

This PR changes the cluster role to include all the rules from the regular role. Unfortunately, we can't compose roles, as the operator-sdk will either bind a service account for the cluster role, or to a role. It cannot bind a service account to both a role and a cluster role (!!).

Related to the finding above, this PR also fixes a problem that went unnoticed with the CSV, which listed a different service account under clusterPermissions.

Fixes #931.

Signed-off-by: Juraci Paixão Kröhling [email protected]

@jpkrohling
Copy link
Contributor Author

jpkrohling commented Feb 28, 2020

@kevinearls would you be able to test this? It would be good to do the OLM testing as well with this CSV: https://github.com/operator-framework/community-operators/blob/master/docs/testing-operators.md . I believe @jkandasa has done some OLM testing before as well.

@kevinearls
Copy link
Contributor

@jpkrohling Sure I'll test this shortly

Copy link
Contributor

@kevinearls kevinearls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jpkrohling jpkrohling force-pushed the 931-Cluster-wide-operator branch from 36c9638 to 98d3f40 Compare March 2, 2020 10:48
@jpkrohling jpkrohling requested a review from objectiser March 2, 2020 12:21
@jpkrohling
Copy link
Contributor Author

jpkrohling commented Mar 2, 2020

@objectiser would you like to review/give it a try as well?

Copy link
Contributor

@objectiser objectiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jpkrohling jpkrohling merged commit 9d6319d into jaegertracing:master Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Operator cannot create resources in other namespaces even in cluster-wide mode
3 participants