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

Helm: ensure RBAC rules are up to date with the latest autogenerated manifest #175

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

ebr
Copy link
Contributor

@ebr ebr commented Mar 7, 2022

Updates RBAC rules in the Helm template to match those generated by the Operator Framework based on the application code.

Why are these changes needed?

The update was initially needed because after deploying the kuberay-operator nightly build, its service account was unable to access leases in the coordination.k8s.io API group. Further investigation revealed that the Helm manifest was even further outdated, which is fixed by this PR.

Fixes #174

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@Jeffwan
Copy link
Collaborator

Jeffwan commented Mar 8, 2022

@ebr Thanks for the contribution. We did talk about the consistency between kustomize and helm.. ideally, we should have source of truth and the other one can be generated from it. However, currently seems there's no a perfect way to make it.

I will add some notes in development.md later to reminder contributors to make corresponding change if RBAC is changed in the PR.

@chenk008
Copy link
Contributor

chenk008 commented Mar 8, 2022

Add a checker in CI?

@ebr
Copy link
Contributor Author

ebr commented Mar 8, 2022

Add a checker in CI?

That would be helpful. Ideally CI itself would automate the chore of copying the rules: from the Kustomize manifest into the Helm template, but I'm a bit unsure about the whole "allowing CI to commit back to the repo" thing.

Generally, is Helm to be supported going forward? looks like the Kustomize manifests are ahead of the chart in a few other ways. We're likely to switch to Kustomize, but this wasn't obvious to me at all, and I wonder if this is confusing to other users.

@ebr ebr force-pushed the master branch 3 times, most recently from 5e49482 to 3d3d437 Compare March 8, 2022 21:28
@ebr
Copy link
Contributor Author

ebr commented Mar 8, 2022

(sorry about all those force-pushes to our source repo; I made a mistake when opening the PR, but rather than opening a new one, I'm hoping for this one to get merged).

@Jeffwan
Copy link
Collaborator

Jeffwan commented Mar 10, 2022

(sorry about all those force-pushes to our source repo; I made a mistake when opening the PR, but rather than opening a new one, I'm hoping for this one to get merged).

That's not a problem. :D

@Jeffwan
Copy link
Collaborator

Jeffwan commented Mar 10, 2022

Let's work on the CI in a separate PR.

@Jeffwan Jeffwan merged commit c24cff6 into ray-project:master Mar 10, 2022
chenk008 pushed a commit that referenced this pull request Mar 22, 2022
…ole in the Helm chart (#175)

Co-authored-by: Eugene Brodsky <[email protected]>
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
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.

[Bug] RBAC defined in Helm is out of sync with latest RBAC manifests
3 participants