Simply and clarify the secret fetching process #33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SecretDefinitions, which contain relevant data around fetching a specified set of secrets. Added more tests to support functionality and improve coverage
This was
linked to
issues
Mar 10, 2020
|
@robison I updated the README to reflect these changes. |
robison
reviewed
Mar 19, 2020
robison
approved these changes
Mar 19, 2020
unixist
reviewed
Mar 19, 2020
|
|
||
| // SecretFetcher inspects the environment for variables that | ||
| // define secret definitions. The variables are used to guide | ||
| // the SecretFetcher in acquring and outputting the specified secrets |
| parallelReader.AsyncRequestKeyPath(secretPath) | ||
| secretResult := parallelReader.ReadSecretResult() | ||
| def := &SecretDefinition{ | ||
| envkey: envKey, |
There was a problem hiding this comment.
nit: keep compound word vars camel case? envKey
| continue | ||
| } | ||
|
|
||
| // look for a corresponding secretDestinationPrefix key. |
There was a problem hiding this comment.
If we're going to say that case doesn't matter, I suggest making it not matter at all within the string. For example, this code currently only equates "SUFFIX" and "suffix".
If the case differs in a substring, e.g. "SUFFIX" and "SUFfix", this code wont match. I personally prefer vars to be truly case sensitive. But as it is, it's neither truly case sensitive nor insensitive.
There was a problem hiding this comment.
Looks like lines changed since my comment. I'll let you figure it out and whether you want to change it :)
unixist
reviewed
Mar 19, 2020
|
|
||
| --- | ||
|
|
||
| **Plurar Secrets** |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR simplifies the secret fetching process with the introduction of
SecretDefinitions, which contain relevant data around fetching a specified set of secrets.Externally, a Secret Definition is defined via:
VAULT_SECRET_THING=my/vault/path/to/a/keyeither with a correspondingDAYTONA_SECRET_DESTINATION_THING=/path/to/outputor a-secret-envargument. Please note that the suffix is no longer required to match the final key name in the vault path.Before this change, note that the suffix
THINGinVAULT_SECRET_THINGwas required to match the key name of the vault path:This PR introduces arbitrary suffix identifiers:
Secret Definition Decoder Guide
VAULT_SECRET_: Singular Secret Storage Path PrefixVAULT_SECRETS_: Plural Secret Storage Path PrefixDAYTONA_SECRET_DESTINATION_: Secret Destination PrefixsecretID-SUFFIX: The unique secret identifier that can be used to tie a Secret Storage Path Prefix to a corresponding destination prefixSingular Secrets
VAULT_SECRET_<secretID-SUFFIX>=<SECRET-APEX>DAYTONA_SECRET_DESTINATION_<secretID-SUFFIX>=<FILE-PATH>This PR also now enables support for using Plural Secret Storage Path Prefixes
VAULT_SECRETS_<secretID-SUFFIX>with Secret Destination PrefixesDAYTONA_SECRET_DESTINATION_, which intended to deprecate the-secret-pathcommand line flag.Thank you @robison for authoring the additional tests.