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

(feature) [HELM] Allow for creating Role/RoleBinding for loading flags from ConfigMap #2196

Closed
yafanasiev opened this issue Aug 5, 2024 · 2 comments · Fixed by #2198
Closed
Assignees
Labels
enhancement New feature or request p3 Longer term priority

Comments

@yafanasiev
Copy link
Contributor

Requirements

Hi! First of all, thanks for a great project!

We are setting up go-feature-flag to load flag definitions from Kubernetes ConfigMap (relay-proxy itself is deployed in Kubernetes as well) and to make it work we have to add Role and RoleBinding to ServiceAccount created by chart (which is kind of expected as well), however might be not as convenient since you can't have Helm managing all the required resources.

Would you be open to adding those RBAC resources to the chart? Or at least a generic template allowing to pass any YAML to be rendered into resource. I would be happy to do a PR for this.

Just for reference, this is how sample RBAC setup looks like:

kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  name: feature-flags-relay-proxy
rules:
- apiGroups: [""]
  resources:
  - configmaps
  verbs:
  - get
---
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  name: feature-flags-relay-proxy
subjects:
- kind: ServiceAccount
  name: feature-flags-relay-proxy
roleRef:
  kind: Role
  name: feature-flags-relay-proxy
  apiGroup: ""
@yafanasiev yafanasiev added enhancement New feature or request needs-triage A priority should be added to the issue labels Aug 5, 2024
@thomaspoignant thomaspoignant added p3 Longer term priority and removed needs-triage A priority should be added to the issue labels Aug 5, 2024
@thomaspoignant
Copy link
Owner

Hello @yafanasiev,
Yes, it sounds like a good thing to have, but if you work on it just keep in mind the backwards compatibility and it should continue to work as before for people upgrading the charts.

Adding a generic template for including custom YAML seems to be the most appropriate approach to avoid breaking changes. But I am happy to have your point of view on this.

@yafanasiev
Copy link
Contributor Author

I went the generic manifests path, since indeed my use-case may not be as wide-spread and overloading chart with very specific values will just increase the complexity. Please review when you have time #2198

@kodiakhq kodiakhq bot closed this as completed in #2198 Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p3 Longer term priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants