Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Re-think Secrets Manager integration #607

Closed
maciejwalkowiak opened this issue Jun 19, 2020 · 4 comments
Closed

Re-think Secrets Manager integration #607

maciejwalkowiak opened this issue Jun 19, 2020 · 4 comments
Labels
component: secrets-manager Secrets Manager integration related issue

Comments

@maciejwalkowiak
Copy link
Contributor

maciejwalkowiak commented Jun 19, 2020

Secrets Manager integration is very opinionated and based on the reported issues I think our opinions don't match users needs.

Spring Cloud AWS Secrets Manager configuration expect secrets to follow a naming pattern:

{prefix}/{defaultContext}
{prefix}/{defaultContext}{profile-separator}{profile-1}
{prefix}/{defaultContext}{profile-separator}{profile-2}
{prefix}/{appName}
{prefix}/{appName}{profile-separator}{profile-1}
{prefix}/{appName}{profile-separator}{profile-2}
  1. Following this naming pattern is not always possible as different conventions may be used in users projects.
  2. It prevents users from loading secrets stored independently (think some-api-key secret) (Allow adding any arbitrary AWS Secrets Manager secrets  #515).
  3. It extends application startup time as secrets for each active profile is loaded
  4. Unless fail-fast is set to false, users are forced to give permissions to read all expected secrets or application will fail to start. If fail-fast is set to false users may not notice that critical secrets have not been loaded (aws.secretsmanager.failFast should only fail application startup if reading all secrets has failed #468).
  5. Even with fail-fast set to true, application still starts without an error if secret is not found.

See other Secret Manager related issues: https://github.com/spring-cloud/spring-cloud-aws/issues?q=is%3Aopen+is%3Aissue+label%3A%22component%3A+secrets-manager%22

This issue is mean to be a place to discuss if/how should we proceed with Secrets Manager integration

@eddumelendez
Copy link
Contributor

eddumelendez commented Oct 19, 2020

yes, seems very opinionated. For me, would be like this:

  • Prefix should not be required neither have a default value
  • DefaultContext should not exist
  • Property name should not exist. So, if we have a new property include-application-name with true we can add it.
  • Profile separator 🤷🏽‍♂️
  • Profile, should be the one that is active. So, we load just that profile instead of everything.

@maciejwalkowiak WDYT? I agree that the new module should take care of this.

@eddumelendez
Copy link
Contributor

this should also applied for new parameter store module, right?

@agentIsaac
Copy link

Requiring the forward slash on the prefix is definitely something I'd like to see go away too.

@maciejwalkowiak
Copy link
Contributor Author

With the ability to load secrets through Spring Boot 2.4 config data loader and Secrets Manager integration introduced in #721, all of the issues have been resolved.

It is possible to load independent secrets like:

spring.config.import: aws-secretsmanager:my-secret;another-secret

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: secrets-manager Secrets Manager integration related issue
Development

No branches or pull requests

3 participants