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

Add post-create support for env key in app.json #4498

Open
josegonzalez opened this issue Mar 21, 2021 · 11 comments
Open

Add post-create support for env key in app.json #4498

josegonzalez opened this issue Mar 21, 2021 · 11 comments

Comments

@josegonzalez
Copy link
Member

josegonzalez commented Mar 21, 2021

The app.json manifest has a way to specify environment variables on start. We should support this.

  • Support for the secret generator: https://devcenter.heroku.com/articles/app-json-schema#env
  • Support for static values
  • If a TTY is available, ask the user for a value for the ones with no defaults
    • If no TTY is available, the secret is required, and has no current value, bail the deploy.
  • Skips setting if the pre-deploy hook has already run
  • Fix case where deployed apps might re-run this hook section if they haven't been deployed on the latest Dokku
  • Add a new property that allows setting the value in all deploys, not just after first create

Refs #2269

@josegonzalez
Copy link
Member Author

Since the formation key will update on every deploy, should this also run on every deploy? That will also differ from heroku, but I think that's fine?

@josegonzalez josegonzalez added this to Backlog in Release Board via automation Apr 16, 2021
@josegonzalez
Copy link
Member Author

Thinking about this more, while it would be nice, since the secrets aren't actually secrets, this will expose folks to doing not secure things (like putting credentials in plaintext). Going to close for now, but if folks want this, maybe I can be dissuaded.

Release Board automation moved this from Backlog to Done or Closed Oct 9, 2021
@alexgleason
Copy link
Contributor

Just ran into this, can't deploy from scratch cuz of missing env:

    "OTP_SECRET": {
      "description": "One-time password secret",
      "generator": "secret"
    },

I think it's a good idea since it reduces friction and just makes things easier.

@josegonzalez
Copy link
Member Author

How would this work? Would it only apply for the first deploy? Should we support bare values as well? I'm concerned this will lead to folks exposing unencrypted credentials in repositories because it is easier to setup on first pass, which is just bad practice.

@alexgleason
Copy link
Contributor

Yeah, just the first deploy. This is what I'm pushing: https://github.com/mastodon/mastodon/blob/main/app.json

I'd say yes to bare values cuz of this:

    "HEROKU": {
      "description": "Leave this as true",
      "value": "true",
      "required": true
    },

People check secrets into git repos all the time even though they shouldn't. It's widely known to be bad practice, so that's on them. I don't think adding this this encourages it more than anything else. It's also a problem with .env files, CI scripts, etc.

@josegonzalez
Copy link
Member Author

Just because they can doesn't mean we should allow it as well :D

Is this something you'd be interested in working on or sponsoring?

@alexgleason
Copy link
Contributor

I'm sponsoring as @soapbox-pub

Just upped it to $50 🙂

@josegonzalez
Copy link
Member Author

What should we do when there is a required key but there is no default value?

@josegonzalez
Copy link
Member Author

As an example, your app above has LOCAL_DOMAIN which is required but has no value or generator. Heroku gets around this because they do a cute modal that allows folks to fill in the required env vars. Dokku Pro could do something similar since it has a web ui, but that doesn't help the folks using the OSS product (though now I have a new idea for Dokku Pro).

@alexgleason
Copy link
Contributor

Probably nothing, the app should crash on its own if it's really required. Displaying a warning is a nice-to-have.

I think it would be annoying to block the deployment outright, especially for apps that maybe used to work before.

@josegonzalez josegonzalez reopened this Aug 22, 2022
Release Board automation moved this from Done or Closed to Backlog Aug 22, 2022
@josegonzalez
Copy link
Member Author

Seems reasonable, and something we can probably do for the next minor.

@josegonzalez josegonzalez moved this from Backlog to To Do for current release in Release Board Aug 23, 2022
@josegonzalez josegonzalez added this to the v0.32.0 milestone Sep 10, 2023
@josegonzalez josegonzalez modified the milestones: v0.32.0, v0.33.0 Oct 16, 2023
@josegonzalez josegonzalez modified the milestones: v0.33.0, v0.34.0 Jan 20, 2024
@josegonzalez josegonzalez added the estimate: 5h Estimated time: 5 hours label Jan 30, 2024
@josegonzalez josegonzalez modified the milestones: v0.34.0, v0.35.0 Mar 13, 2024
@josegonzalez josegonzalez modified the milestones: v0.35.0, v0.36.0 Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Release Board
To Do for current release
Development

No branches or pull requests

2 participants