-
Notifications
You must be signed in to change notification settings - Fork 814
Support existing secret for external DB #1783
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Alexandre DEFRASNE <[email protected]>
Signed-off-by: Alexandre DEFRASNE <[email protected]>
Signed-off-by: Alexandre DEFRASNE <[email protected]>
Signed-off-by: Alexandre DEFRASNE <[email protected]>
Signed-off-by: Alexandre DEFRASNE <[email protected]>
Signed-off-by: Alexandre DEFRASNE <[email protected]>
Hi @cest-pas-faux , I don't think these settings expect Best, |
Hi @MinerYang, thanks for this review The
I didn't want to create a breaking change by removing this secret which is shared by different components. |
Hi @MinerYang , any updates on this ? |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
Hi @MinerYang , just a heads up :) |
/remove-lifecycle stale |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
/remove-lifecycle stale |
Hi @MinerYang , did you have any time to look into this PR ? |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
@MinerYang , how should we proceed ? |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
Hi @MinerYang , this feature is still needed. |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
Still needed |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
Still needed |
@MinerYang I believe what @cest-pas-faux is trying to accomplish is support a use-case like cloudnative-pg, which generates a secret with all needed information to connect to the database upon creation, this is a pretty popular way of provisioning databases in kubernetes and should be supported by harbor. @cest-pas-faux it may be easier to get this approved and merged if you remove the breaking change here. people may be using the existingSecret value which currently takes a string, you should be able to check to see if the value is a string or a map and revert to the old logic if it is a string |
Hi, @JefeDavis , thanks for your interest and it could be what's blocking us to move forward (even if if think implementing a detection of string/map would be very dirty instead of just having a small breaking change). However, I'm chasing up since more than a year with no answers, it's hard to guess what's blocking us here, and the only answer I got is unrelated. @MinerYang , let us know if you want this PR closed or this is something you're willing to implement, and if yes, how ? |
interested! I'm waiting to this feature to be merged to deploy harbor with external-secrets! thank you @cest-pas-faux for the good work, let's hope @MinerYang will merge this pr as soon as possible |
thanks for the PR. imo this change is definitely needed to use the harbor helm chart in production. |
Hi,
This PR adds the ability to specify an existing secret for all the DB configurations (host, port, user, password, coreDatabase).
Tests done with this updated chart :
existingSecret.enabled: true
external
and setexistingSecret.enabled: false
Both tests are successful, pull/push images OK.