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

When the client is run through a container, it should pick up configuration from environment #563

Closed
atrauzzi opened this issue Jul 24, 2017 · 10 comments

Comments

@atrauzzi
Copy link

atrauzzi commented Jul 24, 2017

If I use a docker-compose.yml file like this:

  hydra:

    image: oryd/hydra

    volumes:
      - "./hydra:/root"

    ports:
      - "4444:4444"

    environment: 
      - DATABASE_URL=postgres://local:local@postgres:5432/hydra?sslmode=disable
      - SYSTEM_SECRET=local
      - ISSUER=https://localhost:9000/
      - CONSENT_URL=http://localhost:9020/consent
      - FORCE_ROOT_CLIENT_CREDENTIALS=local:local

While the container ends up with DATABASE_URL specified, I still have to provide it to run migrations and any other functionality:

docker-compose run --rm --entrypoint "hydra migrate sql postgres://local:local@postgres:5432/hydra?sslmode=disable --skip-tls-verify" hydra

There's probably a way to reference the environment variable that is definitely there (env confirms this for me). It's just that escaping shell variables to be dereferenced inside the container when calling from outside can be a bit wordy.

It would be nice if the hydra client just looked at the current environment for configuration. This is still 12factor frendly as I haven't permanently stored it anywhere as far as hydra is concerned.

@aeneasr
Copy link
Member

aeneasr commented Jul 24, 2017 via email

@atrauzzi
Copy link
Author

atrauzzi commented Jul 24, 2017

That's not really solving the problem, it just moves the headache up a level.

Why wouldn't you just look at the current env for configuration for the CLI? The application server already does it. I already have these values in a compose file as env vars not as env vars in my shell which is cumbersome to manage.

This is about automatically reading those values rather than asking for it to be passed in. By doing this, you simplify the command line experience and benefit from docker conventions.

@aeneasr
Copy link
Member

aeneasr commented Jul 24, 2017 via email

@LandonSchropp
Copy link

@arekkas Our team uses Bash scripts to run docker-compose commands to set up our environment. In our case, the $DATABASE_URL environment variable is stored in the docker-compose but isn't available in the Bash scripts. It would be really handy if this value could be read out of the environment.

@atrauzzi
Copy link
Author

"Typing an url is really not that big of a deal"

In this case, "really not that big of a deal" has been invoked, which has caused me to lose interest in this project.

I sincerely hope in the future they can become more familiar with docker conventions and improve the flexibility of hydra and how well it plays with other ecosystems.

@pnicolcev-tulipretail
Copy link
Contributor

pnicolcev-tulipretail commented May 31, 2018

There was another story around this I think, asking for a flag we could set to use the environment variable.

Currently we work around this by running our migrations container inside the alpine image rather than the scratch image. We bury a shell script in there that fires off hydra migrations with the correct environment variables. It would be preferable not to have to use this workaround though.

edit: I can't find an issue about it but I thought i had created one a while back. I know we talked about it! I really think you should consider it.

@aeneasr
Copy link
Member

aeneasr commented May 31, 2018

In this case, "really not that big of a deal" has been invoked, which has caused me to lose interest in this project.

Yeah, that was badly worded. Sorry for that.

I sincerely hope in the future they can become more familiar with docker conventions and improve the flexibility of hydra and how well it plays with other ecosystems.

Since when is passing arguments to commands not a docker convention? You can literally run docker run oryd/hydra:vX.Y.Z migrate sql database-url, like you do with, for example, kong (docker run kong migrations up [-c /path/to/kong.conf]) or other services.

There was another story around this I think, asking for a flag we could set to use the environment variable.

Yup, we'll add a flag to allow this! Wasn't tracked before, it's now: #896

@pnicolcev-tulipretail
Copy link
Contributor

The reason you can't run docker run oryd/hydra:vX.Y.Z migrate sql database-url in all cases is because the variable expansion required on database-url doesn't actually happen inside the container so whatever system you use to run the command would need to do it (AWS does not). In docker-compose when you give it a command like migrate sql ${DATABASE_URL} it is docker-compose itself that does that variable substitution on ${DATABASE_URL}. I spent a few hours pulling my hair out until I realized this, but it makes sense considering a scratch image doesn't have bash at all. Kubernetes also hides that it does this replacement for you, but AWS? No such luck. The command must be a fixed literal string.

At least, that's my understanding. I can't say I'm a fan of AWS.

@aeneasr
Copy link
Member

aeneasr commented May 31, 2018

Oh, I didn't know that! When running in Docker Swarm / Compose, Kubernetes, or just bare Docker this isn't an issue. Can't say how many times I heard about these little things that frustrate the hell out of you from developers that work with AWS ECS.

If you want, feel free to supply a patch, it should be just ~5 LoCs to get it working.

@aeneasr
Copy link
Member

aeneasr commented Jun 3, 2018

#898

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

No branches or pull requests

4 participants