-
Notifications
You must be signed in to change notification settings - Fork 692
Allow to select the source projectId when using secret #2283
Comments
Thanks for your PR.
Yeah I like this idea a lot; would be in favor of letting users specify a custom project id. |
I created a new PR for bootstrap: #2289 Integration test relevance to discuss. TODOs in code where updates are required. |
Regarding property source loading, how would you handle the case where if two projects had 2 secrets with the same secret ID? EDIT: I guess we could consider attaching some kind of additional namespace prefix to the property name that gets loaded. But it feels like it is getting a little bit more complicated. I think we need to consider what the bootstrap properties for multiple project IDs feature will look like to execute this properly. I.e. What property keys will secrets from the non-default projects be loaded under? And then what kind of settings do we need to support - i.e. choosing version and project of secret id, or choosing to include secrets from certain projects, etc. |
Hey I took a look at the PR, my bad; so my comments about name collisions are incorrect; this is like specifying the project-id similar to specifying a version over the default. Could you describe your use-case a bit more? Just want to learn more about it; right now I guess my only concern is that the feature increases the complexity of the settings, and then also there might be unexpected interactions with if a user also sets the version override setting for the specific secret as well. |
In my context, and for cost reason, we have 1 shared project for test and dev environment with 1 Cloud SQL MySQL and 1 Cloud SQL PostGreSQL database. We chose to store the passwords in the secret manager of this shared project. Then, the developer and the other projects can use this database by using the password stored in SecretManager, in addition of other secret stored in their project (like API Keys). But, you have right. If I have the "password" secret name in my default project and in the shared project, how can I specify both ? The best way is to wrap the projectId and the secretId in the same key, something like But I never saw The other solution is to do nothing. The bootstrap configuration work only for secret inside the current project. If you want to load secret from other project, use template (with code example for helping the user to achieve this at the start phase). |
Hmm I see, yeah it makes sense to support this use-case. Let me circle back with our team to discuss more how to best do this; there are some design things I think we need to work out. I see what you're saying with the But we have a couple other settings like choosing the secret version and the secret-name-prefix where I feel users might get confused how these interact with that. Thanks for your PR again; I will put your bootstrap properties PR on hold for now until we figure it out. |
Great. Let me know what is the preferred design. No problem for working on it on my side. |
Hey so I have some ideas for supporting this, I think I will need to rework it quite a bit though. I will implement this in the next few days if my team agrees. In order to support loading secrets from multiple projects, my ideas are to:
|
Hey. I don't know what imply the lazy load (in code) but if you can load secret not at start up but later, it could help cold start duration (and Spring is really not good in it!). But it's a nice to have. You are right for the fully qualified name. But the lazy convenience to omit the current project and the latest version is pleasant. I don't know if it's possible to mix the 2 capabilities. Is something like this is possible?
If so, and if it enforces the Spring best practice, I upvote! And yes, this evolution isn't backward compatible. |
Thanks for your feedback Guillaume. We recently had a discussion with the Secret Manager team that gave us some guidance as well. Based on what you said and their feedback I think we may converge on the following solution: So we are planning for users to be able to specify secrets in their application.properties like this: application.properties
Following the model of Secret Manager on Github Actions: This at least is our current idea; may not be exactly what we ultimately produce but hopefully it gives you a sense. To get this working, we are also semi-blocked on an in issue in Spring-cloud commons here: spring-cloud/spring-cloud-commons#724 |
Great feedback! Is it possible to "reserve" the
Like this, you can specify the Another solution is to reserve the 2 keywords to the
From my usage, I use about 90% of time the project of the current project and more exceptionally the external projects. But, it's only my usage! |
I see, good point. Yeah, since I also foresee most people working within the default project, it makes sense to adopt one of your two suggestions. Will make a note of this. |
Hey @guillaumeblaquiere, we recently submitted #2302 which should offer an easier way of specifying secrets across projects. It introduces the new syntax as we discussed, full docs here: https://github.com/spring-cloud/spring-cloud-gcp/blob/a81143bbd26db0ae7760e8d9aa3147565232f034/docs/src/main/asciidoc/secretmanager.adoc#secret-manager-property-source And should be available in the Going to close but let me know if there's any remaining issues. |
I would like to load easily secrets from more than 1 project. Today, only the default project, or the project set in the properties can be easily used by the SecretManagerTemplate bean
Describe the solution you'd like
The capability to create and read secrets from default project (like today) but also to have the capability to set a projectId for reading the secrets in another project
This implies changes into SecretManagerTemplate and into the bootstrap of secret. On the bootstrap, a solution can be to set the project id in the same way as the versions are defined today.
spring.cloud.gcp.secretmanager.projectId.<secretId>
Open to discuss about the best implementation.
Describe alternatives you've considered
N/A
Additional context
N/A
The text was updated successfully, but these errors were encountered: