-
Notifications
You must be signed in to change notification settings - Fork 385
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
Add helper for namespace handling, fix rbac for watchNamespaces #1329
Conversation
This is what gets generated now with
|
76379b0
to
7e63b49
Compare
7e63b49
to
fa05e68
Compare
@rasebo thanks for the PR! Also, I think it'd be useful to update description of |
Fixes: #1323 |
I am considering deploying Functionally, this can go both ways, either having the chart namespace set as Honestly I am not 100% sure that this is a wise approach, but I figured it wouldn't hurt to have the flexibility to customise this. |
@rasebo Do I understand correctly that you want to create an umbrella chart where you would specify ours as a dependency? |
Sorry for taking so long to respond, got down with a nasty virus. Yep, we're using this as a subchart. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, the code looks fine.
@NissesSenap Could you also take a look at it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay with the review.
LGTM
After #1322 I figured there's a better way in which the namespace key can be populated, in the sense that it might be a good idea to include a helper so that a
namespaceOverride
value can also be provided. Quotes were also removed, as they weren't really necessary.I also found a bug in rbac handling, where
Roles
andRoleBinding
objects were being deployed to the namespace wheregrafana-operator
was running instead of the ones defined in thewatchNamespaces
field. This would lead tografana-operator
not having access to said namespaces.Added basic comments in the
values.yaml
file for theOverride
values.Fixes: #1323