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 CLI config overrides and ENV injection #1323

Merged
merged 2 commits into from
Apr 10, 2019

Conversation

patrick-east
Copy link
Contributor

There are two new CLI options added with this change for the opa run
sub command.

--set
--set-file

These allow for overriding config options on the CLI using key=value
options to reference the YAML/JSON config structures. The --set value
expects to take in the value you want to set while the --set-file value is
a path to a file which will be read for the value of the file. This is primarily
useful for secrets mounted as files on a host.

In addition this adds in some simple environment variable injection so that a
deployer can specify environment variables in the config via ${NAME} syntax.
At the time the config is loaded from file it will inject in the environment vars.
This applies to strings set via the --set variable too.

Some things to note:

  • This won't work for configs generated by discovery bundles, it is only for
    configurations loaded by the runtime.
  • This is only done at the time of loading the file. Plugins receive modified
    copies of the config (post injection), and should never be allowed to re-read
    directly from disk.
  • There are security implications of setting secrets in env vars. It is
    recommended to use the file based secrets approach with --set-file
    instead of environment variables.

Fixes: #924

Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just a couple minor comments.

docs/content/docs/configuration.md Outdated Show resolved Hide resolved
docs/content/docs/configuration.md Outdated Show resolved Hide resolved
docs/content/docs/configuration.md Outdated Show resolved Hide resolved
docs/content/docs/configuration.md Show resolved Hide resolved
cmd/run.go Outdated Show resolved Hide resolved
cmd/run.go Outdated Show resolved Hide resolved
docs/content/docs/configuration.md Outdated Show resolved Hide resolved
docs/content/docs/configuration.md Outdated Show resolved Hide resolved
docs/content/docs/configuration.md Outdated Show resolved Hide resolved
@tsandall
Copy link
Member

@patrick-east LGTM. Go ahead and squash your changes and then merge.

We will use it (or a modified version) to allow for setting config easier on the CLI.

Signed-off-by: Patrick East <east.patrick@gmail.com>
There are two new CLI options added with this change for the `opa run`
sub command.

`--set`
`--set-file`

These allow for overriding config options on the CLI using `key=value`
options to reference the YAML/JSON config structures. The `--set` value
expects to take in the value you want to set while the `--set-file` value is
a path to a file which will be read for the value of the file. This is primarily
useful for secrets mounted as files on a host.

In addition this adds in some simple environment variable injection so that a
deployer can specify environment variables in the config via ${NAME} syntax.
At the time the config is loaded from file it will inject in the environment vars.
This applies to strings set via the `--set` variable too.

Some things to note:
* This won't work for configs generated by discovery bundles, it is *only* for
  configurations loaded by the runtime.
* This is only done at the time of loading the file. Plugins receive modified
  copies of the config (post injection), and should never be allowed to re-read
  directly from disk.
* There are security implications of setting secrets in env vars. It is
  recommended to use the file based secrets approach with `--set-file`
  instead of environment variables.

Signed-off-by: Patrick East <east.patrick@gmail.com>
@patrick-east
Copy link
Contributor Author

Squashed and rebased to fix merge conflicts, once the CI passes I'll merge it.

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.

None yet

3 participants