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

Config file envvar overrides #20

Merged
merged 4 commits into from
Nov 8, 2024
Merged

Config file envvar overrides #20

merged 4 commits into from
Nov 8, 2024

Conversation

detjensrobert
Copy link
Contributor

@detjensrobert detjensrobert commented Oct 28, 2024

Switch to building global and challenge configs with Figment instead of directly with Serde. Figment can merge multiple sources together before deserializing, including from environment variables.

This gets us envvar overrides for basically free. Any config in rcds.yaml can be set from envvars -- BEAVERCDS_<path>. Fields that are overridden from envvars can be omitted from the config file, since parsing happens after the merge. This allows users to not need to specify a placeholder value that's immediately replaced.

Previously in #15 we agreed on from_env: subkeys, but that would need us to do all the parsing and substitution ourselves. With this approach this library does these overrides for free, and also gets us very good test capabilities for overrides and parsing and so on.

examples:

registry:
  cluster:
    user:  # ==> BEAVERCDS_REGISTRY_CLUSTER_USER
    pass:  # ==> BEAVERCDS_REGISTRY_CLUSTER_PASS
  # the whole cluster: section can be omitted if both these are set

profiles:
  foo:
    frontend_token:  # ==> BEAVERCDS_PROFILES_FOO_FRONTEND_TOKEN

Fixes #15.

Supersedes #18.

where
E: de::Error,
{
Ok(FromStr::from_str(value).unwrap())
Copy link
Member

Choose a reason for hiding this comment

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

unwrap here probably isn't ideal, I'd prefer returning a de::Error variant (maybe with de::Error::invalid_value?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

value here is always going to be a &str, so AFAICT there's no chance it will ever fail here. I stole this code from Serde's documentation examples so I'm also not really sure why this is doing the from_str dance on an existing &str to begin with.

Copy link

@cacama-valvata cacama-valvata left a comment

Choose a reason for hiding this comment

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

Reviewed synchronously 11/05/24 -- approved

serde-yaml is deprecated and not maintained, serde-yml is the new fork

Signed-off-by: Robert Detjens <[email protected]>
This file is autogenerated and we (users) don't care about the specifics of
what has changed in it, only that it has.

Signed-off-by: Robert Detjens <[email protected]>
@detjensrobert detjensrobert merged commit 3d45a9a into main Nov 8, 2024
3 checks passed
@detjensrobert detjensrobert deleted the dr/config-refactor branch November 8, 2024 02:03
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.

Implement environment variable overrides for config options
3 participants