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

Import Secrets #665

Merged
merged 7 commits into from
Dec 6, 2024
Merged

Import Secrets #665

merged 7 commits into from
Dec 6, 2024

Conversation

kraftp
Copy link
Member

@kraftp kraftp commented Dec 5, 2024

Support for importing secrets from a dotenv file. All environment variables in the file will be imported as DBOS Cloud secrets and made available as environment variables to the deployed application. Aim is to simplify environment management. Import a file with:

dbos-cloud app secrets import -d .env

Supported dotenv file syntax: https://dotenvx.com/docs/env-file

@maxdml
Copy link
Contributor

maxdml commented Dec 5, 2024

.env files are not just made for secrets. We should just have some logic in the cloud to source the .env file (presumably, before we source env vars from dbos-config.yaml). And we shouldn't mix this with the secrets logic.

edit: or even easier, merge .env and our config file env section at deploy time in the cloud CLI

@kraftp
Copy link
Member Author

kraftp commented Dec 6, 2024

.env files are not just made for secrets. We should just have some logic in the cloud to source the .env file (presumably, before we source env vars from dbos-config.yaml). And we shouldn't mix this with the secrets logic.

I want to eventually deprecate the dbos-config.yaml env vars, we've repeatedly received feedback (which I agree with) that it's confusing and unintuitive.

I like using the new secrets system for all environment management. After all, secrets are just key-value pairs. Secrets are intuitive and have a nice web UI. This command makes them easy to import from a .env file if that's what you're used to using. I'd rather do it this way than have yet another system based around sourcing the .env file.

@maxdml
Copy link
Contributor

maxdml commented Dec 6, 2024

I want to eventually deprecate the dbos-config.yaml env vars, we've repeatedly received feedback (which I agree with) that it's confusing and unintuitive.

I like that idea. And we can do it at deploy time. But I don't think we should mix this up with our current secrets management, because these take a different management path, namely, they go through secrets management rather than application management.

For example, today if I want to update my env vars, I can just update a file (dbos-config or .env), like the rest of my code, and do dbos-cloud app deploy.

But if we move everything toward secrets, then the deployment logic becomes more difficult on the client side because I now have to do both secrets management then application deployment.

@kraftp
Copy link
Member Author

kraftp commented Dec 6, 2024

I want to eventually deprecate the dbos-config.yaml env vars, we've repeatedly received feedback (which I agree with) that it's confusing and unintuitive.

I like that idea. And we can do it at deploy time. But I don't think we should mix this up with our current secrets management, because these take a different management path, namely, they go through secrets management rather than application management.

For example, today if I want to update my env vars, I can just update a file (dbos-config or .env), like the rest of my code, and do dbos-cloud app deploy.

But if we move everything toward secrets, then the deployment logic becomes more difficult on the client side because I now have to do both secrets management then application deployment.

I'm not convinced, we'll see what feedback we get. The environment and the code are different things and often aren't in the same place (.env often isn't checked into version control for example) so I think it makes sense to separate their management. And the new deployment logic isn't really more difficult--at most, it's two commands instead of one, but now you don't have to worry about futzing around with the dbos-config.yaml.

Also you can always source your .env directly if you really want to, set your start command to source .env && python3 main.py or whatever. I don't know if we should be doing that automatically.

@maxdml
Copy link
Contributor

maxdml commented Dec 6, 2024

I'm not convinced, we'll see what feedback we get. The environment and the code are fundamentally different things and usually aren't in the same place (.env often isn't checked into version control for example) so I think it makes sense to separate their management.

I'd agree that it's a bit all over the place -- I've seen both. In most CI/CD systems the environment is set to some sort of UI, and the actual applications being CI/CD-ed use interpolation syntax (and the later is in version control.)

If we want to split env variables management from code deploy, then we should also replace/alias this secrets API to be an "environment variables" API: it'll otherwise be pretty confusing for users.

@kraftp
Copy link
Member Author

kraftp commented Dec 6, 2024

I'm not convinced, we'll see what feedback we get. The environment and the code are fundamentally different things and usually aren't in the same place (.env often isn't checked into version control for example) so I think it makes sense to separate their management.

I'd agree that it's a bit all over the place -- I've seen both. In most CI/CD systems the environment is set to some sort of UI, and the actual applications being CI/CD-ed use interpolation syntax (and the later is in version control.)

If we want to split env variables management from code deploy, then we should also replace/alias this secrets API to be an "environment variables" API: it'll otherwise be pretty confusing for users.

Yeah, I think it's simpler to have one way to do it. Might adapt later, we'll see. I'll alias it to "environment" and change the docs a bit.

Copy link
Contributor

@maxdml maxdml left a comment

Choose a reason for hiding this comment

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

Let's add a test in the cloud

@kraftp kraftp merged commit 0e304ef into main Dec 6, 2024
5 checks passed
@kraftp kraftp deleted the kraftp/import-secrets branch December 6, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants