feat(config): introduce external secret management #11985
Merged
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
github-actions
bot
added
the
domain: external docs
Soak Test ResultsBaseline: a63f402 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±7.0% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±7.0%: Fine details of change detection per experiment.
Fine details of each soak run.
|
prognant
requested review from
leebenson and
lukesteensen
and removed request for
leebenson
jszwedko
requested review from
jszwedko,
spencergilbert and
StephenWakely
and removed request for
lukesteensen
|
Just linking the RFC for reviewers: https://github.com/vectordotdev/vector/blob/master/rfcs/2022-02-24-11552-dd-agent-style-secret-management.md |
jszwedko
reviewed
Apr 8, 2022
There was a problem hiding this comment.
Awesome, nice work @prognant . I left a few comments below, but this mostly looks good.
One thing I think would be nice to do is add a behavior test to serve as an integration test. You could add that here: https://github.com/vectordotdev/vector/tree/master/tests/behavior
Thanks for adding the docs too. I think we'll want a guide for this too, but we can follow up on that.
…secret-management
Co-authored-by: Spencer Gilbert <spencer.gilbert@datadoghq.com>
…ment' into prognant/introduce-secret-management
spencergilbert
approved these changes
Apr 11, 2022
There was a problem hiding this comment.
LGTM, though @jszwedko's points seem reasonable to me as well
|
While working on signal handling I found one major issue, the secret interpolation hook leads to secret management/retrieval/replacement being done on a per file basis, that being said if the secret management is configured in its own file the secret retrieval will just never happen and that's bad. |
jszwedko
reviewed
May 4, 2022
src/config/loading/secret.rs
Outdated
Comment on lines
252
to
256
| let stderr = String::from_utf8_lossy(&output.stderr); | ||
|
|
||
| if !stderr.trim().is_empty() { | ||
| warn!("A secret exec backend wrote a message on stderr: {}.", stderr); | ||
| } |
There was a problem hiding this comment.
I think this won't read very well for multi-line stderr.
I was also thinking we'd proxy it through as it was written which could aid with troubleshooting if the task is hanging, for example. I'd say we could just do command.stderr(Stderr::inherit()), but maybe it would be better to feed it through Vector's log system so it is in a consistent format. That could mean something like:
- Rather than calling
wait_with_outputcallspawnoncommandso that you can read the stderr output as it is written - Call
warn!with each line of stderr output so that it ends up in Vector's logs with the appropriate format - Collect the output asynchronously until the command finishes (I think serde can actually take a async reader too so maybe you could just feed it into that)
I'm happy to just defer this for now though, if you prefer, and open a follow up ticket. That would mean dropping the stderr handling you have here (maybe just doing command.stderr(std::process::Stderr::piped()).
There was a problem hiding this comment.
I'm also realizing the above:
command.stderr(std::process::Stdio::piped());
command.stdin(std::process::Stdio::piped());
command.stdout(std::process::Stdio::piped());
Isn't right, they should be piped to Stderr Stdout Stdin respectively.
There was a problem hiding this comment.
I'm also realizing the above:
command.stderr(std::process::Stdio::piped()); command.stdin(std::process::Stdio::piped()); command.stdout(std::process::Stdio::piped());Isn't right, they should be piped to
StderrStdoutStdinrespectively.
This is confusing but the Stdio::piped() just instructs what to do with stderr/stdout/stdin from the child process and in that case it establishes a pipe between Vector and the child process to read/write to/from stdin/stdio/stderr.
There was a problem hiding this comment.
Doh, yeah, I misread Stdio as Stdout 👍
Soak Test ResultsBaseline: 9658477 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
Soak Test ResultsBaseline: 18d5e27 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
|
@jszwedko the line-by-line reading for stderr is definitely a better solution, I put everything in the same select loop. As far as I checked serde doesn't do async. Anyway based on your insights I reworked the main loop and it is much more readable now. |
Soak Test ResultsBaseline: 18d5e27 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
…secret-management
Soak Test ResultsBaseline: 17adcc2 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
jszwedko
reviewed
May 17, 2022
There was a problem hiding this comment.
Awesome, thanks for these changes @prognant !
I realized we should probably add a new section to the configuration docs, aside from the secrets backend configuration, that describes this feature. I think it would fit here: https://vector.dev/docs/reference/configuration/#how-it-works . Which is generated from
vector/website/cue/reference/configuration.cue
Lines 322 to 371 in aa540a2
Otherwise this looks good!
Soak Test ResultsBaseline: aa540a2 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
Soak Test ResultsBaseline: aa540a2 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
jszwedko
approved these changes
May 18, 2022
Co-authored-by: Jesse Szwedko <jesse@szwedko.me>
Soak Test ResultsBaseline: d03a526 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. Changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%:
Fine details of change detection per experiment.
|
Closed
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.
Allow this config
This allow the configuration, just after env var interpolation, to have strings like
SECRET[backend_1.foo]andSECRET[backend_2.bar]replaced by values retrieved from command specified in secret.backend_1 and secret.backend_2, respectively.