Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Spring framework 6.2 placeholder parser rewrite breaks the secret manager prefix sm:// #3391

Closed
elg-entur opened this issue Nov 25, 2024 · 24 comments
Assignees
Labels
priority: p2 type: enhancement New feature or request

Comments

@elg-entur
Copy link

elg-entur commented Nov 25, 2024

Describe the bug
Please provide details of the problem, including the version of Spring Cloud that you
are using.

spring-cloud-gcp-starter-secretmanager: 5.8.0
Spring framework 6.2.0 (i.e, spring boot 3.4.0)

The rewrite of springs property placeholder parser (6.1 vs 6.2) changes the way default values are parsed. This change breaks hardcoded prefix of the secret manager, sm://.

For example a property defined as secret: ${sm://MY_SECRET} will be be parsed differently in spring framework 6.1 and 6.2. In 6.1 it will be resolved correctly to the actual secret. In 6.2 it will be parsed as //MY_SECRET (unless a property called sm is defined) which is clearly wrong

The workaround is to change the prefix in each application config to sm\://, e.g., secret: ${sm\://MY_SECRET}, but this is not only ugly but not at all intuitive to do.

At the very least I expect the documentation to be updated with the workaround.

Would it be possible to either

  1. Make the prefix configurable
  2. Add an alternative to the current prefix which does not contain :, such as sm//

Workaround (yaml)

unquoted_secret: ${sm\://MY_SECRET}
single_quoted_secret: '${sm\://MY_SECRET}'
double_quoted_secret: "${sm\\://MY_SECRET}"
@keesvandieren
Copy link

For our project, the workaround to use sm\:// does not work.

We've rolled back to spring-boot 3.3.6 to ensure spring-framework 6.1.x is used.

@elg-entur
Copy link
Author

If the secret is in a quoted string it should be double escaped, as so secret: "${sm\\://MY_SECRET}"

@keesvandieren
Copy link

It is in a properties file

@headegg
Copy link

headegg commented Dec 3, 2024

Thanks for raising this and providing a workaround. Mine was in a single quoted string in a helm values.yaml file and a single backslash did the trick.

spring.datasource.password: '${sm\://{{ .Values.env }}-database-password}'

@aptituz
Copy link

aptituz commented Dec 5, 2024

I can also confirm that the workaround with escaping the double-colon works. Any plans from google side to address this issue with a fix, though?

@bendb-instacart
Copy link

Registering my interest in a bugfix - this is quite a problematic breaking change from Spring. I wish that I had a good solution to offer.

@JoeWang1127
Copy link
Contributor

We don't support spring boot 3.4.0 right now and we've been working on the support, see #3320.

Please see the compatibility with spring project versions.

@diegomarquezp
Copy link
Contributor

This will be solved by Framework 6.2.2, it's in their next milestone's roadmap - see spring-projects/spring-framework#34124. We are still working on upgrading to Cloud 2024 as per #3320 (comment).

@snicoll
Copy link
Contributor

snicoll commented Dec 24, 2024

The rewrite of springs property placeholder parser (6.1 vs 6.2) changes the way default values are parsed. This change breaks hardcoded prefix of the secret manager, sm://.

For the record, this was never meant to be supported in the first place. : always was a reserved keyword that separates the key from the fallback. In 6.2 we introduced consistent way to escape it (as you found out).

That being said, we've decided to restore this use case for backward compatible reason with 6.x but we might revisit this in the future. In short, please don't rely on a placeholder with a non escaped reserved keyword in it.

@tompson
Copy link

tompson commented Jan 2, 2025

so just to confirm: currently there is no way to use escape sm:// in properties files correctly to make it work with Spring Boot 3.4 / Spring Framework 6.2 ? /cc @keesvandieren

@keesvandieren
Copy link

For us it didn't work, but for some others it seemed to work?

@joaotab
Copy link

joaotab commented Jan 2, 2025

Worked for us with: api-key: ${sm\://my-api-key-secret} but I read somewhere the escaping required depends if you're using quoted or unquoted strings

@breun
Copy link
Contributor

breun commented Jan 3, 2025

There is currently no version of Spring Cloud GCP yet that is compatible with Spring Cloud 2024.0 / Spring Boot 3.4 / Spring Framework 6.2 (see Compatibility with Spring Project Versions). I would recommend just waiting for Spring Cloud GCP 6 to be released before upgrading.

@mieseprem
Copy link

For those only following this issue (and not keep track on other ones), #3440 looks interesting:

With the Spring Cloud 2024 upgrade, we will introduce a new syntax sm@my_secret which doesn't fall into these issues.

@diegomarquezp
Copy link
Contributor

Thanks @mieseprem for pointing folks to that issue.

Indeed we will soon support an additional syntax sm@my_secret. This will solve the problems given the special treatment of the colon character.
However, in order to not break any code we will still support sm://, although with a warning when used.
This work depends directly on Spring Boot 3.4.2 to be released by January 23rd, so we will soon follow with an upgrade.

@HenryXiloj
Copy link

I tested with Spring Boot 3.4.2, and it works perfectly on my end. I used ${sm://projects//secrets/} in application.yml.

@mieseprem
Copy link

mieseprem commented Feb 3, 2025

update: pls ignore my question. This question will be answered already there: #3320 (comment)

--

Hej @diegomarquezp , one question regarding this Statement:

This work depends directly on Spring Boot 3.4.2 to be released by January 23rd, so we will soon follow with an upgrade.

When is 'soon' planned?

I ask, because 5.x is still the latest/current release.

Compatibility with Spring Project Versions does already list the 6.0 release, but it is nowhere else mentioned.

  • Spring IO website states 5.10 as latest Release
  • Maven doesn't know any 6.0 release

@diegomarquezp
Copy link
Contributor

Hi all, Spring Cloud GCP 6.0.0 has been released and fixes this issue by providing support for a new syntax for secrets in the form sm@my_secret_id. Additionally, we followed up on how did the rewrite pointed out by @elg-entur
break the behavior of secrets in the form sm://my_secret (it would now be interpreted as "compute sm or default to //my_secret). However, the Spring folks fixed this regression as discussed in spring-projects/spring-framework#34124 and was included in spring-framework:6.2.2 (part of spring-boot:3.4.2), which is part of spring-cloud-gcp:6.0.0.

You can find more details on the release description. I'll be closing this for now. Please file a new issue if anything was found with this spring-cloud-gcp 6.0.0 release.

@breun
Copy link
Contributor

breun commented Feb 6, 2025

@diegomarquezp I see the Spring Cloud GCP Secret Manager documentation still mentions the sm:// syntax instead of sm@.

@diegomarquezp
Copy link
Contributor

Thanks @breun. I submitted a job to update the docs under the same link. This should be reflected in a few hours

@breun
Copy link
Contributor

breun commented Feb 11, 2025

@diegomarquezp I don't think that change did anything to replace sm:// with sm@ in the documentation?

@ccudennec-otto
Copy link

@diegomarquezp : I also don't see any mentions of sm@ in the current documentation.

As we now get the following warning in the logs, it would be helpful to update the documentation accordingly:

Detected usage of deprecated prefix sm://. This may be removed in a future version of Spring Cloud GCP. Please use the new prefix sm@ instead.

Thanks!

@breun
Copy link
Contributor

breun commented Feb 18, 2025

@diegomarquezp
Copy link
Contributor

@breun https://googlecloudplatform.github.io/spring-cloud-gcp/6.0.1/reference/html/index.html#secret-manager has the correct documentation. We are looking into correcting the already published 6.0.0 documentation.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
priority: p2 type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests