search

Nebo15 / confex

Useful helper to read and use application configuration from environment variables.
Other
304 stars 33 forks source link

File Loader for Config #6

Closed rawkode closed 7 years ago

rawkode commented 7 years ago

Adding support for providing a file_configuration_directory and pulling in values from disk.

This will allow Elixir developers to easily run their applications on Docker Swarm, Mesos and Kubernetes and consume their secrets, which tend to be stored in /run/secrets/key

Signed-off-by: David McKay david@rawkode.com

AndrewDryga commented 7 years ago

Hello, @rawkode! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

rawkode commented 7 years ago

I've not "fixed" the "Pipeline with only one function" as this seems to be "convention" in this codebase and I made it that way to match the rest.

If you want this changed, please let me know.

EDIT: credo and dogma failed the build, so I changed it

AndrewDryga commented 7 years ago

Ebert has finished reviewing this Pull Request and has found:

You can see more details about this review at https://ebertapp.io/github/Nebo15/confex/pulls/6.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 6f63464d0b81b4fe847fbf1bf0f81e7387c05334 on rawkode:feature/read-config-from-file into 806256591dcea15eaabe3c22c872afed231efd5d on Nebo15:master.

AndrewDryga commented 7 years ago

I don't think that reading files in run-time is a good idea, since your calls will become IO-bound (read is performed each time you need your configuration, and it's not expected for most of people to be a bottleneck); you may face file permissions issues; path to secrets can be changed in future releases.

Instead, you can mount secrets as environment variables in Kubernetes. I guess it's also possible in other orchestration engines. In this way they will be loaded by container itself and because they never change you won't need to access IO each time.

rawkode commented 7 years ago

Swarm does not, and will not, support secrets being exposed as environment variables as they believe it is a security risk.

Performance of reads did pop into my head and assuming this gets merged, we could add a GenServer to cache.

Secret paths may change, but I already made this configurable

AndrewDryga commented 7 years ago

I guess reading file may be moved to your application code, but you can store path in ENV variable, eg:

:my_app
|> Confex.get_env(:secrets_path)
|> File.read!

In this way we can make sure that performance hit will be always in mind of developer, without limiting flexibility (maybe you want to parse that secret afterwards). I want this library be very explicit about what happens inside.

Also it's important to notice that :system tuples is getting deprecated in latest Ecto, Phoenix and Elixir versions to be replaced with on_init callbacks.

AndrewDryga commented 7 years ago

Hey, @rawkode. I have a good news.

Confex 3.0 introduced support for custom adapters and version 3.2 supports a :system_file adapter that does exactly what do you need. Altrough file path should be absolute, not relative to some application env.

rawkode commented 7 years ago

Awesome, but I launched this a while back to satisfy my needs

https://github.com/GT8Online/weave