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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Vault path-prefixes config #8714

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

davidkuster
Copy link

@davidkuster davidkuster commented Mar 23, 2023

Changes proposed by this PR

Closing #8712 in favor of this approach. Copying the bulk of the description here because the intent is the same.

Support for Vault kv2 was added in #6115. That supports both kv1 and kv2, but as mentioned in the docs, only a single prefix path is supported:

Concourse is currently limited to looking under a single path, meaning enabling only one secrets engine is supported: kv, or kv_v2.

This PR enables multiple prefix paths and then does the lookup of the secrets in each, for each path template configured.

The idea in our org is to enable a smooth transition of teams from kv1 -> kv2. Without this we'd need to do a big bang release of ensuring all teams have migrated their secrets to the new engine, then update the Concourse config to point at kv2. Instead with this change we can support both, and let someone else herd the cats. 馃檪

Note this would potentially double the number of API calls to Vault. I don't see that's a reason to not do it, but something to call out in the docs at a minimum.

  • support multiple Vault prefix paths
  • limited manual testing
  • unit or other tests
  • update docs

Notes to reviewer

Believe the approach in #8712 isn't viable due to var_sources. Instead trying the creation of a separate --path-prefixes config option. I tested locally with the following config:

      CONCOURSE_VAULT_PATH_PREFIX: ""
      CONCOURSE_VAULT_PATH_PREFIXES: "/kv2,/kv1"

Quick example of local testing:

jobs:
- name: hiya
  plan:
  - task: simple-task
    config:
      platform: linux
      image_resource:
        type: registry-image
        source: { repository: busybox }
      run:
        path: echo
        args: ["Hello world!", ((kv2-test.hello)), ((kv1-test.hello))]

Where kv2-test secret only exists in the kv2 path, and kv1-test secret only exists in the kv1 path. This results in Hello world! Hello from kv2! Hello from kv1!

Release Note

  • Add support for multiple Vault path prefixes. This allows for simultaneous support of multiple secret engines, allowing teams to transition between Vault locations at their own pace.

Signed-off-by: David Kuster <david_kuster@comcast.com>
Signed-off-by: David Kuster <david_kuster@comcast.com>
@davidkuster davidkuster marked this pull request as ready for review April 19, 2023 17:56
@davidkuster davidkuster requested a review from a team as a code owner April 19, 2023 17:56
@xtremerui xtremerui self-assigned this May 12, 2023
name := fmt.Sprintf("lookup-template-%d", i)
if _, err := creds.BuildSecretTemplate(name, manager.PathPrefix+tmpl); err != nil {
return err
if manager.PathPrefix != "" && len(manager.PathPrefixes) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why it is not allow for Vault to set both, compared to Credhub?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants