Skip to content
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 key with default value is not processed as intended #3440

Open
diegomarquezp opened this issue Jan 6, 2025 · 3 comments
Open
Labels
priority: p2 type: bug Something isn't working

Comments

@diegomarquezp
Copy link
Contributor

diegomarquezp commented Jan 6, 2025

This occurs in the main branch (5.x).
Both cases below use a slight modification of the secretmanager sample. They use the spring.cloud.gcp.secretmanager.allow-default-secret=true setting.

Case 1: normal default value usage

This is not what's suggested but makes sense to try it since it's in the form ${fetch_this:or_default_to_this}.

When using

  @Value("${sm://application-secret:DEFAULT}")
  private String appSecretFromValueWithDefault;

It fails with:

INVALID_ARGUMENT: The provided Secret ID [projects/diegomarquezp-sandbox/secrets/application-secret:DEFAULT/versions/latest]

This means that it tries to resolve the key sm or will default to the string "//application-secret:DEFAULT" if not found.

Case 2: nested placeholder

This is what's suggested in the docs. The docs, however, only explain when the referenced secret is assumed to be non-existent (and this is a test case in the sample, which succeeds)

When using

  @Value("${${sm://application-secret}:DEFAULT}")
  private String appSecretFromValueWithDefaultUsingNestedPlaceholder;

It renders

 <b>Application secret from @Value with default value using nested placeholder:</b> <i>DEFAULT</i><br/>

Analysis

The @Value of the form ${${nested}:default_value} is NOT equivalent to ${nested:default_value}. For example, if the value of nested is foo, the top-level resolution will be ${foo:default_value}, which will in turn try to fetch the property foo or fall back to default_value (2 fetch actions, one for nested, one for foo).

Similarly, for a secret of the form ${${sm://my-secret}:default_value}, where the secret value is bar, it will resolve to a top level placeholder of the form ${bar:default_value}, which in turn will try to resolve the property with key bar (that's not what we want because bar is the actual secret we want to obtain).

The ideal way would be to parse it using the suggested ${key:default} form (i.e. sm://my_secret:default_value), but unfortunately, the : character is considered a default value separator and will be interpreted as _fetch property sm or default to //my_secret:default_value).

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

Possible solutions

  • If this really is a misuse of placeholder resolution, then we can correct the sample code to something of the form ${sm\://my_secret:default_value} (escaping the colon in the "sm protocol" prefix)
  • (open to more suggestions)

Reproducer

See #3439

See also

@diegomarquezp diegomarquezp added the type: bug Something isn't working label Jan 6, 2025
@diegomarquezp
Copy link
Contributor Author

@burkedavison
Copy link
Member

we can correct the sample code to something of the form ${sm://my_secret:default_value} (escaping the colon in the "sm protocol" prefix)

Should we backport sm@ support to 5.x, 4.x, 3.x?
If we did, we could fix the secret manager default-value sample by changing to sm@ on all branches, and provide consistent documentation on how ${sm\://my_secret:default_value} is a deprecated alternative if using the sm:// prefix.

@diegomarquezp
Copy link
Contributor Author

+1 to supporting sm@ on older branches. This can be a follow up of #3411

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants