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

1495 2 app repository cluster role #1498

Merged
merged 7 commits into from
Feb 4, 2020

Conversation

absoludity
Copy link
Contributor

Follows #1497 . Updates the app repository controller to be granted a clusterrole and clusterrolebinding to read AppRepositories.

I've used an easily identifiable and descriptive name for the clusterrole (rather than the templated kubeapps.apprepository.fullname), while ensuring it is unique per Kubeapps installation (appending the namespace).

Ref #1495

@@ -32,6 +32,18 @@ rules:
- jobs
verbs:
- create
---
# Kubeapps can read and watch its own AppRepository resources cluster-wide.
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we enable this behind a feature flag for the moment? that way we avoid "breaking" changes in the chart while the feature is being developed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can put it behind a flag, but didn't see much point given that this doesn't affect existing behaviour, just enables the features which will themselves be behind a feature flag to work properly. Let me update.

Copy link
Contributor

Choose a reason for hiding this comment

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

given that this doesn't affect existing behaviour

well it affects in the sense that we are now installing a cluster wide resource (apart than the CRD)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PTAL :)

@absoludity absoludity changed the base branch from 1495-1-app-repository-cluster-role to master February 4, 2020 09:57
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

LGTM!

@absoludity absoludity merged commit 1dcabc7 into master Feb 4, 2020
@absoludity absoludity deleted the 1495-2-app-repository-cluster-role branch February 4, 2020 22:52
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.

2 participants