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

Added finalizers for OpenShift in RBAC #65

Closed
wants to merge 1 commit into from

Conversation

bartmeuris
Copy link

Fixes an OpenShift RBAC compatibility issue in helm chart. Found the solution here: spotahome/redis-operator#98

If you don't specifically mention the vaultsecrets/finalizers resource next to the vaultsecrets in the ClusterRole, you get an error like this in the reconciliation loop:

2020-12-28T15:06:04.328Z	INFO	controllers.VaultSecret	Creating a new Secret	{"vaultsecret": "testnamespace/test-vault-secret", "Secret.Namespace": "testnamespace", "Secret.Name": "test-vault-secret"}
2020-12-28T15:06:04.334Z	ERROR	controller	Reconciler error	{"reconcilerGroup": "ricoberger.de", "reconcilerKind": "VaultSecret", "controller": "vaultsecret", "name": "test-vault-secret", "namespace": "testnamespace", "error": "secrets \"test-vault-secret\" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , <nil>"}
github.com/go-logr/zapr.(*zapLogger).Error

I currently wrapped the additional resource in a condition which checks for an OpenShift specific API to be present on the target cluster. Not sure if this is the way to go, and if this condition is even necessary. I have only tested this with Helm 3 on OpenShift 4, and briefly on a local kind instance.

ricoberger added a commit that referenced this pull request Dec 29, 2020
The created RBAC manifests were not in sync with the specified RBAC
markers. We added the RBAC markers for the ConfigMaps and Events and
also for the finalizers. This is needed to fix the bug mentioned in #65.

Now we can generate the RBAC rules via make manifests again. Then we can
use the generated file from confgi/rbac/role.yaml in the Helm chart.
@ricoberger ricoberger mentioned this pull request Dec 29, 2020
@ricoberger
Copy link
Owner

Hi @bartmeuris, thanks for finding the bug and fixing it 🙂.

If you are ok with it, I would merge #67 instead of your PR, which also fixes the markers for the generation of the RBAC rules. They were not added after the migration to the new Operator SDK version and I would like to use make manifest in the future without any hassle.

@bartmeuris
Copy link
Author

Ah that would be a better solution indeed, feel free to close/ignore this PR then :)

@ricoberger
Copy link
Owner

The fix is included in version 1.10.2

@ricoberger ricoberger closed this Dec 30, 2020
@bartmeuris bartmeuris deleted the fix-openshift-rbac branch December 30, 2020 16:46
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.

None yet

2 participants