Secret manager does not resolve sm:// to string

[pieterjanpintens]

Describe the bug
We set up secret manager added a key with a value.
Added the following to our application.properties

spring.application.name=solution-api-service
spring.config.import=sm://,optional:configserver:http://localhost:8888
spring.cloud.gcp.secretmanager.project-id=${GOOGLE_CLOUD_PROJECT:pj-bu-btxux-dev-eu}
spring.cloud.gcp.secretmanager.allow-default-secret=true
spring.cloud.config.username=${spring.cloud.gcp.secretmanager.project-id}/${spring.application.name}
spring.cloud.config.password=${sm://sm-solution-api-service-config-client-secret}

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:

    public final String toString() {
        return String.format(Locale.ROOT, "<ByteString@%s size=%d contents=\"%s\">", Integer.toHexString(System.identityHashCode(this)), this.size(), this.truncateAndEscapeForDisplay());
    }

Which is obviously not what we expect to get back.

We believe that the class, SecretManagerPropertySource should use getSecretString on the getSource() instead of the getSecretByteString(). This will convert the ByteString to a String correctly.

[pieterjanpintens]

#2691

[burkedavison]

This resolver code will calls String.valueOf(result) which will call the method toString on the ByteString class.

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 convertValueIfNecessary is called, it has the targetType parameter set to String. This results in GenericConversionService being used with sourceType = com.google.protobuf.ByteString$LiteralByteString and targetType = java.lang.String. https://github.com/spring-projects/spring-framework/blob/main/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java#L170-L185. This all results ultimately in the conversion being performed by the converter provided by GcpSecretManagerEnvironmentPostProcessor: https://github.com/GoogleCloudPlatform/spring-cloud-gcp/blob/main/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/GcpSecretManagerEnvironmentPostProcessor.java#L45

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 String.valueOf as an after-the-fact conversion mechanism.

[pieterjanpintens]

Thank you for your elaborate reply.

Can you clarify where this logic is located?

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:
https://github.com/spring-projects/spring-boot/blob/736f712ba9f6f79e5d3d3b019999d6003de50dde/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironmentContributorPlaceholdersResolver.java#L65

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.

[burkedavison]

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.

[pieterjanpintens]

Ok thank you for your help

[pieterjanpintens]

spring-projects/spring-boot#39944 fyi this is the ticket on spring boot

[burkedavison]

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.