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.

Sign up for GitHub

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

Jump to bottom

Add missing RBAC config #44

Merged
merged 2 commits into from
Nov 9, 2023
Merged

Add missing RBAC config #44

merged 2 commits into from
Nov 9, 2023

Conversation

dmendiza
Copy link
Contributor

@dmendiza dmendiza commented Nov 6, 2023 •
edited
Loading

This patch adds the missing RBAC config for BarbicanWorker and BarbicanKeystoneListner that is causing the controller to get stuck in a crash loop.

@dmendiza
Add missing RBAC config
3f29026 
This patch adds the missing RBAC config for BarbicanWorker and
BarbicanKeystoneListnere that is causing the controller to get stuck in
a crash loop.
xek
xek requested changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@xek xek left a comment

Choose a reason for hiding this comment

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

I think these files are generated with make manifests. Are the proper kubebuilder annotations already there? If not, the pull request should update both places, like in this commit: openstack-k8s-operators/keystone-operator@e6927b6

@dmendiza
Add RBAC annotations to Barbican controller
3b0279f 
This patch adds the missing RBAC annotations to the Barbican controller,
and also updates the role.yaml file to use the file generated by make
manifests.
@dmendiza
Copy link
Contributor Author

dmendiza commented Nov 8, 2023

Thanks for the review, @xek . I had to do a bit of digging around in the operator sdk and kubebuilder docs to understand the annotations. IIUC, the annotations are used by make manifests to generate the role.yaml file. I added all the missing annotations to the Barbican controller and then updated the role.yaml file to use the one generated by the makefile. The other files are generated by the openstack-sdk tool when a new api is added, and I think they were left out by mistake in the patch that implemented BarbicanKeystoneListener.

vakwetu
vakwetu approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@vakwetu vakwetu left a comment

Choose a reason for hiding this comment

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

LGTM

@xek xek merged commit 8caeec2 into openstack-k8s-operators:main Nov 9, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vakwetu vakwetu vakwetu approved these changes

@xek xek xek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dmendiza @xek @vakwetu