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

feature request: make all flags env vars #38

Closed
atlantisbot opened this issue Mar 6, 2018 · 6 comments
Closed

feature request: make all flags env vars #38

atlantisbot opened this issue Mar 6, 2018 · 6 comments
Labels
feature New functionality/enhancement help wanted Good feature for contributors

Comments

@atlantisbot
Copy link

Issue by @robertlabrie
Thursday Oct 19, 2017 at 18:00 GMT
Migrated from hootsuite/atlantis#163
Why was it migrated?


in server.go, it doesn't feel like it'd hurt to do things like ATLANTIS_GH_USER etc. Two use cases:

  1. In k8s everything is an env var, makes parameterizing for different envs easier
  2. In systemd, an envrionmentFile can be turned without a daemon-reload
@atlantisbot
Copy link
Author

Comment by @lkysow
Monday Oct 23, 2017 at 13:51 GMT


Hey @robertlabrie thanks for the ticket. I've got a couple thoughts/questions about this.

  • I don't think just because we can have environment variables for every flag that it necessarily makes sense to do so. It makes the interface to Atlantis more complicated and so I want to be careful with that. If you look at Consul (https://www.consul.io/docs/agent/options.html) you'll see they don't offer many flags as environment variables.
  • The approach we've taken until now is to only have env vars for sensitive data like the github token

So to that end, can you give us more information about your specific use case to help us understand your problem better? It sounds like you're interested in make the variables that change per environment to be set via env vars?

Also, in Kubernetes you can use env vars as args too: https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/#use-environment-variables-to-define-arguments but this is a bit more kludgy.

@atlantisbot
Copy link
Author

Comment by @robertlabrie
Monday Oct 23, 2017 at 14:11 GMT


Hi @lkysow I didn't realize there were any downsides to it, so maybe it's no so trivial. I'm running Atlantis as a systemd service. I had to template both the .service file and an environment file for the sensitive data. Making everything an env var means I just have to template my EnvironmentFile.

Same with kubernetes. You can pass args to a container, but it's this clunky array of args so you end up with ["--gh-user","someuser"]. They seem to treat env vars as a preferred method.

In neither case is it the end of the world, it makes things a little cleaner I think but I'm not so attached to the idea that I'll fork the project and implement it myself. This just came up while I was setting it up for us.

[Unit]
Description=Atlantis

[Service]
Type=simple
User=atlantis
Group=atlantis
EnvironmentFile=/etc/atlantis
ExecStart=/usr/local/bin/atlantis server --gh-hostname sqbu-github.cisco.com --gh-user someuser

@atlantisbot
Copy link
Author

Comment by @robertlabrie
Monday Oct 23, 2017 at 17:53 GMT


So I just had to add something, my chef template for atlantis.service looks like this:

ExecStart=/usr/local/bin/atlantis server --gh-hostname <%= node['atlantis']['gh_hostname'] %> --gh-user <%= node['atlantis']['gh_user'] %> --atlantis-url <%= node['atlantis']['url'] %>

@atlantisbot
Copy link
Author

Comment by @lkysow
Tuesday Oct 24, 2017 at 01:27 GMT


👍 everything makes sense to me. I think we could add env vars for

  • atlantis-url
  • config
  • data-dir
  • gh-hostname
  • gh-user
  • log-level
  • port
  • require-approval

What do you think @anubhavmishra?

@atlantisbot
Copy link
Author

Comment by @anubhavmishra
Wednesday Oct 25, 2017 at 18:06 GMT


Let's add environment variables since it is optional anyway.

@lkysow
Copy link
Member

lkysow commented Mar 20, 2018

@robertlabrie better late than never? Sorry this took so long!

meringu pushed a commit to meringu/atlantis that referenced this issue May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement help wanted Good feature for contributors
Projects
None yet
Development

No branches or pull requests

2 participants