-
Notifications
You must be signed in to change notification settings - Fork 292
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
Secret manager does not resolve sm:// to string #2690
Comments
Can you clarify where this logic is located? When I look at a similar use-case, I'm seeing property values being converted from ByteString to String here: https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertySourcesPropertyResolver.java#L60-L62 and here: https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertySourcesPropertyResolver.java#L74-L83 When Since there's already a feature-rich conversion mechanism in place (should the consumer ask for a String type rather than an Object), it seems like the fix might best be located in replacing the use of |
Thank you for your elaborate reply.
So it happens very early. It seems to be tied tto the config initialization phase where spring tries to resolve placeholders. This seems to be a different path that does not make use of the conversion mechanisms. To be clear there is no configuration involved of our own beans. The username/password is required to contact the spring cloud config server which is probably part of this bootstrap phase as it is configured as a configuration import source. The location where this conversion happens is here: We did some tests before without the config server and there string got indeed converted correctly. So you are right about that part. We could work around the problem by using another mechanism to store this bootstrap secret but we hoped to avoid that because it means we need to cope with secrets in properties files again which is exactly what we were trying to get rid of. I will test a bit more to see if 'application' secrets used by our beans are resolved properly (after this bootstrap phase), I assume they will be. I will post back my findings by the end of this week. I'm not sure about the best next step. This seems to be a special bootstrap case where very basic types (string, int, bool, ..) are expected by the underlying system. I also don't see any sort of mechanism to get in between, the system directly talks to the property source. My best solution i could come up with atm is a custom property source that resolves a variant of sm:// (smi://) and converts it to string just for this bootstrap phase. |
I've opened a ticket in Spring Boot. Let's see if they agree this is something that should be resolved within Spring Boot or whether they have different guidance. |
Ok thank you for your help |
spring-projects/spring-boot#39944 fyi this is the ticket on spring boot |
Spring Boot team has targeted a fix to this in Spring Boot 3.3, with potential future backports if it doesn't cause too many customer issues. Since this has been accepted as a bug and will be fixed in Spring Boot, I am closing this issue. Regarding the proposed fix in Spring Cloud GCP: we're concerned existing applications may be depending on the ByteString return type of the current SecretManagerPropertySource, so we don't want to change it from ByteString to String with a better long-term fix on the way. |
Describe the bug
We set up secret manager added a key with a value.
Added the following to our application.properties
The idea is that the password for our spring config server comes from sm.
We debugged the property resolving, it goes trough the code to resolve the sm values.
It also fetches the value but it comes back to the spring resolver code as a protobuf ByteString.
This resolver code will calls String.valueOf(result) which will call the method toString on the ByteString class.
This returns:
Which is obviously not what we expect to get back.
We believe that the class,
SecretManagerPropertySource
should usegetSecretString
on thegetSource()
instead of thegetSecretByteString()
. This will convert the ByteString to a String correctly.The text was updated successfully, but these errors were encountered: