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

Allow using a custom secret name #2034

Closed
wants to merge 2 commits into from

Conversation

ramondeklein
Copy link
Contributor

The Helm chart to create a new tenant allows to specify .secrets.existingSecret.name (if you already have a secret) or .secrets.name (if you want to create a new secret by the Helm chart). When setting the secret's name, the tenant won't initialize properly and the operator shows the following error message:

main-controller.go:697] error syncing 'minio/tenant': secrets "myminio-env-configuration" not found

After some digging it seems that you also need to set the .tenant.configuration.name value to the secret name to make it work. This isn't obvious from the documentation.

Because these values should always be identical, it doesn't make much sense to use the .tenant.configuration.name value in the Helm chart. This PR sets the tenant configuration to the (existing) secret name, so you only need to set one value. Because the Helm chart won't create a new secret when .secrets.existingSecret.name is set, this value has precedence above .secrets.name.

harshavardhana
harshavardhana previously approved these changes Mar 15, 2024
@harshavardhana harshavardhana requested a review from jiuker March 15, 2024 04:45
shtripat
shtripat previously approved these changes Mar 15, 2024
@ramondeklein ramondeklein dismissed stale reviews from shtripat and harshavardhana via 98c6380 March 15, 2024 12:24
@ramondeklein ramondeklein force-pushed the master branch 2 times, most recently from 98c6380 to ee2b4d0 Compare March 15, 2024 12:32
@ramondeklein
Copy link
Contributor Author

I have added the obsolete.yaml file that checks if tenant.configuration.name is set. If it is, then it fails with a warning that this property isn't used anymore. If you rather not have these compatibility checks, then I can remove it again.

Copy link
Contributor

@jiuker jiuker left a comment

Choose a reason for hiding this comment

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

lgtm

@pjuarezd
Copy link
Member

Closing this in favor of #2032, thank you

@pjuarezd pjuarezd closed this Mar 20, 2024
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.

5 participants