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

agent: Add implementation for injecting secrets as environment variables #20628

Merged
merged 44 commits into from
May 24, 2023

Conversation

dhuckins
Copy link
Contributor

@dhuckins dhuckins commented May 17, 2023

this PR is the initial implementation of a process runner for injecting secrets via environment variables via vault agent

features

  • restarts on new secret changes
  • handles exit code from child process

It is part of the larger effort to add environment variable support within Vault Agent (VLT-253).

dhuckins and others added 22 commits May 8, 2023 11:16
Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com>
Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
command/agent/exec/exec.go Outdated Show resolved Hide resolved
Comment on lines 47 to 50
// lookupMap is a list of templates indexed by their consul-template ID. This
// is used to ensure all Vault templates have been rendered before returning
// from the runner in the event we're using exit after auth.
lookupMap map[string][]*ctconfig.TemplateConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

"exit after auth" is not applicable in env_template mode, do we still want this map? If so, please update the comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

After speaking with @VioletHynes, it seems we may still want to support the exit_after_auth usecase (doesn't need to be implemented with the map but something to keep in mind).

command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Show resolved Hide resolved
Env: append(os.Environ(), envsToList(newEnvVars)...),
ReloadSignal: nil, // can't reload w/ new env vars
KillSignal: s.config.AgentConfig.Exec.RestartKillSignal,
KillTimeout: 30 * time.Second,
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to make this configurable but maybe post-beta

dhuckins and others added 4 commits May 19, 2023 12:42
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com>
dhuckins added 5 commits May 23, 2023 12:08
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
…nv-var

Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
@averche averche added this to the 1.14 milestone May 23, 2023
dhuckins added 3 commits May 23, 2023 12:26
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
@dhuckins dhuckins changed the title Agent runner env var agent: implementation for injecting secrets via environment variables via vault agent May 23, 2023
@dhuckins dhuckins marked this pull request as ready for review May 23, 2023 17:13
@@ -11,10 +11,11 @@ import (

"github.com/go-test/deep"
ctconfig "github.com/hashicorp/consul-template/config"
"golang.org/x/exp/slices"

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Delete this empty line

ctconfig "github.com/hashicorp/consul-template/config"
"github.com/hashicorp/consul-template/manager"
"github.com/hashicorp/go-hclog"

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: delete this empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's intentional by gofmt, imports are grouped
stdlib
external/third party
internal/first party

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, interesting. I'm not sure if it's different with gofumpt, but I've made an effort to delete these when they pop up in my PRs, and it seems to auto-organize them when I do. I'm happy to keep this as-is!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just ran make fmt and no change? maybe we need to add some import formatting 🤷

command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Show resolved Hide resolved
command/agent/exec/exec.go Show resolved Hide resolved
command/agent/exec/exec.go Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
command/agent/exec/exec.go Outdated Show resolved Hide resolved
@averche averche changed the title agent: implementation for injecting secrets via environment variables via vault agent agent: Add implementation for injecting secrets as environment variables May 23, 2023
dhuckins and others added 2 commits May 23, 2023 14:43
Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Copy link
Contributor

@VioletHynes VioletHynes left a comment

Choose a reason for hiding this comment

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

Looks great!

Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
@dhuckins dhuckins requested review from averche and lursu May 23, 2023 19:33
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
Signed-off-by: Daniel Huckins <dhuckins@users.noreply.github.com>
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

4 participants