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

Customize AwsSecretsManagerEnvironmentRepository. #2357

Conversation

ojecborec
Copy link
Contributor

No description provided.

Copy link
Member

@spencergibb spencergibb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid there can't be breaking changes. Can you describe exactly what you are trying to accomplish with this?

@ojecborec
Copy link
Contributor Author

ojecborec commented Nov 25, 2023

The foundation for this PR is coming from real life experience. I’m reading config properties from 2 sources. One is Git which contains non sensitive data such as server port, context path etc. Every time I release to production I create a new tag, i.e. v1.0. That gives me ability to rollback my config. Then there are secrets, passwords etc. They’re stored in the AWS Secrets Manager. These might change regularly. Rolling back to older version doesn’t work as password might be invalid. So I don’t version secrets. When I make /{application}/{environment}/{version} request it would read secrets from AWS Secrets Manager only labeled as {version}. As these properties don’t exist I get empty response.

I know I can create my own repository but there are several drawbacks.

  1. AwsSecretsManagerEnvironmentRepository is class, not interface.
  2. Extending this class means calling super constructor that has 3 specific arguments which my class might not need. There’s no default constructor.
  3. In order to get instance of awsSmClient I need to create additional builder class/method and build it myself. And repository factory as well just like you do.
  4. I don’t want to copy/paste the whole repository class and amend only one line, that is the AWS request that would ignore version. I would have to keep it in sync with origin in case anything changes.

This looks like too much hassle and risk if anything changes in the upstream for such a small request.

@spencergibb
Copy link
Member

Thanks for the detailed response. Changing from a class to an interface is not possible right now. We can add constructors, if that helps. If you are willing to rework, let me know.

@spencergibb
Copy link
Member

The build failures are checkstyle errors

@ojecborec
Copy link
Contributor Author

Can you have a look at this alternative solution #2358?

@ryanjbaxter
Copy link
Contributor

Should we close this PR then?

@ojecborec
Copy link
Contributor Author

Yes, I’m closing it.

@ojecborec ojecborec closed this Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants