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

feat: add env vars import at application creation #652

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davlgd
Copy link
Contributor

@davlgd davlgd commented Jan 1, 2024

Fix #651

This PR adds a --env/-e option to import env vars from a file during clever create.
If no file name is declared, it uses .env by default.

@davlgd davlgd self-assigned this Jan 1, 2024
@davlgd davlgd requested a review from a team as a code owner January 1, 2024 11:17
@davlgd davlgd changed the title Add .env auto-import (with user confirmation) feat: add .env auto-import (with user confirmation) Jan 1, 2024
@aurrelhebert
Copy link
Member

aurrelhebert commented May 30, 2024

Should we really do that? This means that env vars are stored locally. What would be the win compared to using a config provider for example?

@davlgd
Copy link
Contributor Author

davlgd commented May 30, 2024

Should we really do that? This means that env vars are stored locally. What would be the win compared to using a config provider for example?

When a user creates an application or deploy an existing project needing env vars, he should create the app and then import env vars. As we can now do both in 1 step from API, it could be great to have such a feature directly from clever create (and it simplifies process for 1-click apps/tutorials).

Config provider is a Clever Cloud centric way to share env vars with an app, I'm not sure it covers the same needs.

@davlgd davlgd changed the title feat: add .env auto-import (with user confirmation) feat: add .env import at application creation May 30, 2024
@davlgd davlgd changed the title feat: add .env import at application creation feat: add env vars import at application creation May 30, 2024
@aurrelhebert
Copy link
Member

aurrelhebert commented May 30, 2024

I get the aim, I think that a lot of env vars could be shared to an app with a Config Provider enabling already a 1 click order for most tutorials.

However indeed it could be great to be able to add env variables directly when we order a new application. First I would see something like:

clever create --set-env='key=value'
clever create --set-env='key=value' --set-env='key42=value42' 

And from here I am not sure that we should loaded it from a file, maybe it could something like (we should discuss it I think) :

clever create --set-env-from='.env'
clever create --set-env-from='.env.json'

@davlgd
Copy link
Contributor Author

davlgd commented May 30, 2024

I'm not opposed to a --set-env option, but it's a complementary approach, not an alternative. Why would we ask the user to detail all env vars in a command line if there is already a file containing them ? And a file at the root of the project is a common/standard way to do it.

BTW, --set-env-from seems a bit long to me (but you knew I'll say that 😄 )

Copy link

github-actions bot commented May 30, 2024

🔎 A preview has been automatically published:

  • 🐧 linux 1d3976b5c07841ea7bf23dd01dd75879476284855b73dc545a1d3ca0013c87db
  • 🍏 macos 54edac56e679efdc7891fd963883f1d64427de58ccc81263d5f3a70a5ef8dcae
  • 🪟 win 1986270779dc7609b7f094d4829f86c945bbb1b51000b5c2de484d67e660b9b5

This preview will be deleted once this PR is closed.

@aurrelhebert
Copy link
Member

aurrelhebert commented May 30, 2024

Thanks for your feedback on this, the more I think of if, the less I am convinced. I am afraid that this would enforce a sort of bad practice (having a local env file), that could easily lead to leaking secrets.

I am more in favour to add env vars threw the CLI with --set-env and if there is too many threw a configuration provider. Maybe we could see how we can create a Configuration provider from a local file?

@hsablonniere hsablonniere added this to the After rework/refacto milestone Jun 26, 2024
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.

.env import at app creation
3 participants