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

apimanager: Do not set APIManager controller-based ownerReferences on Secrets #575

Merged
merged 1 commit into from
Feb 4, 2021

Conversation

miguelsorianod
Copy link
Contributor

The reason for this is that secrets are often managed by users and users
sometimes manage the secrets themselves by other controllers which also
add controller-based OwnerReferences to the secrets.
In K8s only a single controller-based OwnerReference is allowed per object, which
was preventing users to work with their own controller-based secrets.

An implication of this change is that the responsibility to cleanup
the APIManager secrets deployed belongs to the user.

This changes include an upgrade procedure to remove the APIManager
controller-based ownerReferences from existing APIManager
secrets.

@miguelsorianod miguelsorianod requested a review from eguzki February 2, 2021 15:49
@miguelsorianod miguelsorianod force-pushed the remove-apimanager-secret-ownerreferences branch from d0841a6 to 74fe779 Compare February 2, 2021 16:09
… Secrets

The reason for this is that secrets are often managed by users and users
sometimes manage the secrets themselves by other controllers which also
add controller-based OwnerReferences to the secrets. In K8s only
a single controller-based OwnerReference is allowed per object, which
was preventing users to work with their own controller-based secrets.
An implication of this change is that the responsability to cleanup
the APIManager secrets deployed belongs to the user.
This changes include an upgrade procedure to remove the APIManager
controller-based ownerReferences from existing APIManager
secrets.
@miguelsorianod miguelsorianod force-pushed the remove-apimanager-secret-ownerreferences branch from 74fe779 to 715e8ca Compare February 2, 2021 16:10
@codeclimate
Copy link

codeclimate bot commented Feb 2, 2021

Code Climate has analyzed commit 715e8ca and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@eguzki
Copy link
Member

eguzki commented Feb 3, 2021

Looks good.

question: what about the config maps? shouldn't be free of controller owner reference for the very same reason?

@eguzki eguzki assigned miguelsorianod and unassigned eguzki Feb 3, 2021
@miguelsorianod
Copy link
Contributor Author

Looks good.

question: what about the config maps? shouldn't be free of controller owner reference for the very same reason?

At the moment we do not offer direct configuration through ConfigMaps, the ones we have listed as configurable are Secrets. We have some ConfigMaps but they are managed by the operator. If there's a configurable field in a ConfigMap it's been placed at CR level. For example, the wildcard domain.

In case in the future we add some configurable ConfigMaps we could explore this as it might make sense too. Maybe it depends on each case. Some ConfigMaps might be meant to be exclusive to the APIManager and some others "shared" with the user.

@miguelsorianod miguelsorianod merged commit 407318e into master Feb 4, 2021
@miguelsorianod miguelsorianod deleted the remove-apimanager-secret-ownerreferences branch February 4, 2021 10:52
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.

2 participants