shuttle-hq / shuttle

Build & ship backends without writing any infrastructure files.
https://shuttle.dev
Apache License 2.0
6.14k stars 253 forks source link

[Plugins]: Improve the Shuttle Secrets plugin #1571

Open jonaro00 opened 10 months ago

jonaro00 commented 10 months ago

The current approach to Secrets is Shuttle-native. Evolving the plugin could include support for .env files and allowing the secrets to be set as env variables in more stages of the Shuttle deployment. Ideas are appreciated.


This issue is a part of the Product Feature Poll (all issues). React with :+1: if you want this feature. Comment if you have suggestions related to this feature.

bavalpey commented 10 months ago

I'm thoroughly happy with the simplicity of secrets. It follows similar frameworks such as Cloudflare workers, except is even easier to work with.

Please don't change :(

DevKAVI commented 10 months ago

Yeah I like the current implementation of using Secrets.toml as the main secret store. So there's no need to introduce yet another new file type to the workspace (.env). A simple KV storage for that is more than sufficient, at least in all my use cases.

jonaro00 commented 10 months ago

To clarify the idea: We still want setting up secrets to be as easy as it is now. The idea is mainly about making it easier to customize the approach, such as using a .env file if you want to, and the ability to automatically load all secrets into the environment, like dotenv crates do. Another improvement that is probably coming up is to have it built into the shuttle-runtime crate by default so that you don't even need to add a separate crate to use it. There is also room to reduce the boilerplate required when getting secrets from the SecretStore. How does this sound? @bavalpey @DevKAVI Edit: Perhaps the title of the issue should be "Improve the Secrets plugin"?

bavalpey commented 10 months ago

The idea is mainly about making it easier to customize the approach, such as using a .env file if you want to

I think that having multiple ways to do this makes the feature more complicated, and makes reading other projects slightly more involved. There should really only be 2 files capable of setting secrets, one for local runs and one for deployments. It would also be, well, difficult to differentiate between the deployment secrets and the dev secrets. Having a .env file violates this.

and the ability to automatically load all secrets into the environment, like dotenv crates do.

Now this is great. Definitely would want it to be a deployment flag, though.

Another improvement that is probably coming up is to have it built into the shuttle-runtime crate by default so that you don't even need to add a separate crate to use it.

This makes no difference to me. The only reason I can see to not do this is to avoid pay the compilation cost for an unused module. But I can't imagine the secrets module would have any real impact on compile times.

There is also room to reduce the boilerplate required when getting secrets from the SecretStore. How does this sound?

Now this is something I would be very excited for. It's a bit annoying having to pass around secrets, or throw them into a global static variable. Would love to see this.

jonaro00 commented 10 months ago

It would also be, well, difficult to differentiate between the deployment secrets and the dev secrets. Having a .env file violates this.

Next.js uses .env + .env.local + a few other options. Secrets[.dev].toml is an echo of that pattern.

a deployment flag

How about #[shuttle_secrets::Secrets(set_env = true)] secrets: SecretStore? They would be set at runtime (when loading the secrets) with this approach, in which case it makes sense to have it declared in the macro. A deployment flag would imply it couldn't be applied on a local run.

throw them into a global static variable

env vars are kind of like global variables :smirk:

jonaro00 commented 9 months ago

Another idea to consider: Supporting a way of automatically parsing it into a struct. https://github.com/shuttle-hq/shuttle/pull/1666