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 support for floki to interpret host environmental variables #294

Closed
wants to merge 1 commit into from

Conversation

peter-oneill
Copy link

@peter-oneill peter-oneill commented Jul 4, 2023

Alternative to PR for Tera-based implementation

Why this change?

This allows users to reference environmental variables in floki.yaml, for example to mount a $HOME subdirectory in a container as

docker-switches:
  - -v ${user_env:HOME}/.vim:/home/build/.vim

Any string matching \$\{user_env:([a-zA-Z0-9_]*)\} is captured by floki before deserialization, and the user's environmental variable name following user_env: is substituted in.

Relevant testing

  • New UT added,
  • Tested manually against a local floki.yaml file which successfully mounted a directory declared as ${user_env:HOME}, and printed a host environmental variable in the container in an init command.

Contributor notes

See Teams chat for idea discussion.

Checks

These aren't hard requirements, just guidelines

  • New/modified Rust code formatted with cargo fmt
  • Documentation and README.md updated for this change, if necessary
  • CHANGELOG.md updated for this change

@@ -139,6 +151,17 @@ impl FlokiConfig {

Ok(config)
}

/// Replace all instances of ${user_env:VAR} in the yaml with the value of the local environmental variable "VAR".
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably want to reserve ${<mode>:<value>} completely - so if we see -e ${lol:foo} we might want to throw an error rather than leaving it un-changed (this also allows adding additional things later e.g. -e KERNEL_VERSION=${exec:uname -r} without being a breaking change each time).

This also means that ${user_env:@*&} wants to be an error too - erroring when someone it looks like someone has tried but failed to use a feature is probably better than assuming they weren't aware of the feature and meant to type what they typed (might require an escape hatch for the incredibly rare case where someone wanted the un-expanded value)


Warning:
- Use of host environmental variables may reduce the reproducibility and shareability of your `floki.yaml`.
- If an environmental variable is not found, it is interpreted as a blank string.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting... there's a difference between an envvar being absent and being empty (though by default bash will expand $FOO to "" if FOO is unset - but see set -u and the various ${foo:bar}/%{foo:-bar} syntaxes).

I'm not sure what the right option is here, though I'd probably guess that erroring is more intuitive (the script expects to be using the value, if there is no value the script may well do the wrong thing if it's forced to treat it as "").

@maxdymond
Copy link
Collaborator

Fixed via use of tera

@maxdymond maxdymond closed this Jul 6, 2023
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.

3 participants