search

brefphp / bref

Serverless PHP on AWS Lambda
https://bref.sh
MIT License
3.15k stars 365 forks source link

feat: uncouple secrets loader #1652

Closed jaulz closed 7 months ago

jaulz commented 1 year ago

Right now, the logic whether the secrets loader will be used or not is implemented here in the first place. However, I think it should only be used to output a warning. That makes it easier to patch the secrets loader and for example use this proposed change: https://github.com/brefphp/secrets-loader/pull/4

mnapoli commented 1 year ago

I don't understand what this is about honestly.

However, I think it should only be used to output a warning

But the diff still throws an exception, so I'm not sure what you mean.

How are things different with that PR? What can you do that you couldn't do before?

jaulz commented 1 year ago

Actually, this is something I stumbled across when implementing https://github.com/brefphp/secrets-loader/pull/4. In my opinion Bref should only check (but not prevent) loading of secrets via secrets manager. Currently, this package needs to be kept in sync with secrets loader because areThereSecretsToLoad needs to know the syntax of the secrets. At the moment it's easy to keep it in sync but it prevents users from patching the secrets loader with custom logic.

I didn't want to remove the original check to keep the existing behaviour so I kept the exception.

mnapoli commented 7 months ago

Closing to clean up the issue tracker, that's not something I plan on merging right now. Let's reopen the discussion if needed.