Implement support for AWS SecretsManager

[natepage]

Hi there 👋

secrets-loader is great, easy to setup, and works beautifuly with AWS SSM.

However, most of our projects work with both SSM and SecretsManager,
This PR allows people to use natively use secrets from SecretsManager with Bref.

[mnapoli]

It seems the tests are failing because of Undefined array key "AWS_DEFAULT_REGION": you might want to force an AWS_REGION in the test class?

[natepage]

@mnapoli this was a legit error, the SecretsManagerClient shouldn't not be called in the failing test. The issue was triggered by of a contengency between tests because we're using putenv() to set the env variables and it carries between test.
I've explicitly removed the env variable from previous test and it's all green now

[natepage]

@mnapoli I've pushed a last fix for symfony/polyfill-uuid, it's required by async-aws/secrets-manager. However, checking their releases, 1.13.0 was broken and it was fixed 2 days later in 1.13.1

You can see this in their changelog, it fixes the issues with constants: https://github.com/symfony/polyfill/blob/1.x/CHANGELOG.md#1131

[jaulz]

@natepage nice work 😊 Do you already use it? I assume it also needs some adjustments here:
https://github.com/brefphp/bref/blob/master/src/LazySecretsLoader.php#L36 ?

[jaulz]

It would be cool to try to get all the secret values first and afterwards throw the error (including indicating which secrets have failed).

[mnapoli]

In case of a permission issue it's more likely that permissions are not good for all secrets. If not, that's not a common case, I'm fine keeping it simple honestly (easier to maintain).

[mnapoli]

I don't use SecretsManager and I have one hesitation before merging this: isn't it more common to store multiple variables in 1 SecretManager entry?

That's a pattern I've seen multiple times, and I think it's caused by the fact that SecretManager deals well with JSON values (so multiple values in 1 secret) and that secrets are expensive (by the unit). It also means 1 HTTP call (faster and cheaper) to retrieve all values.

For example, that's how secrets are stored for AWS services: https://docs.aws.amazon.com/secretsmanager/latest/userguide/reference_secret_json_structure.html

[jaulz]

I don't use SecretsManager and I have one hesitation before merging this: isn't it more common to store multiple variables in 1 SecretManager entry?

Are you referring to this PR or #7? This PR indeed assumes that all secrets are in JSON format but actually you can also just store simple strings and it would fail then.

This is how this PR works:

Secrets:

DB_CONNECTION: { "host": "example.org", "username": "test", "password": "test" }

serverless.yml:

provider:
  environment:
    DB_CONNECTION: bref-secretsmanager:DB_CONNECTION

Expands to these environment variables:

DB_CONNECTION={ "host": "example.org", "username": "test", "password": "test" }
host=example.org
username=test
password=test

However, I think it should simply expand to the actual value (see #7) and the rest is up to the application to parse it properly:

DB_CONNECTION: { "host": "example.org", "username": "test", "password": "test" }

It's unopinionated and would work exactly the same like the bref-ssm: prefix.

[natepage]

Hey there, sorry I missed the party 😄

@mnapoli agree with you here, all projects I worked on using AWS SecretsManager are storing multiple variables in a single secret.
However, @jaulz got a point, the current implementation wouldn't support secrets that aren't valid JSON values.

What about introducing some kind of processor, similar to what Symfony does with their env variables?
We could support both:

• bref-secretsmanager:MY_SECRET => Simply set env var with whatever value from AWS
• bref-secretsmanager:json:MY_SECRET => JSON decode value from AWS and set individual env vars

I'm happy to push changes to support this if sounds good to you!

[natepage]

The changes to add a json processor were not too hard so I went ahead and did it so you can have a look a decide, I added a test to illustrate how it would work

[natepage]

Hey there, Happy New Year!

A gentle follow up on this one as we would to be able to use this in our projects 😄

[f-lombardo]

Is it OK to put this in the require-dev instead of in the require section?

[natepage]

@f-lombardo my thinking was people don't need this dependency unless they are working with secrets manager, and because ssm is the default implementation of this library it's most likely to be the only use in a majority of cases. But I do not have strong opinions about it 😄

[f-lombardo]

IMHO since async-aws/ssm is in the require section, the same should hold for async-aws/secrets-manager

[f-lombardo]

Hey there, Happy New Year!

A gentle follow up on this one as we would to be able to use this in our projects 😄

Hello @natepage, since I also had to use some new features in a few projects, I created a small fork of this one: https://github.com/f-lombardo/secrets-loader

If you want, you could submit a PR to my repo too. I'll take care of mantaining my project aligned with the original one

[f-lombardo]

Hey there, sorry I missed the party 😄

@mnapoli agree with you here, all projects I worked on using AWS SecretsManager are storing multiple variables in a single secret. However, @jaulz got a point, the current implementation wouldn't support secrets that aren't valid JSON values.

What about introducing some kind of processor, similar to what Symfony does with their env variables? We could support both:

* `bref-secretsmanager:MY_SECRET` => Simply set env var with whatever value from AWS

* `bref-secretsmanager:json:MY_SECRET` => JSON decode value from AWS and set individual env vars

I'm happy to push changes to support this if sounds good to you!

I've implemented something similar here:
https://github.com/f-lombardo/secrets-loader/blob/master/README.md

[natepage]

Hi @mnapoli 👋

Hope you're well, I would like to follow up on this as we're very interested in the functionality 😄