feat: Vault dynamic secrets Generator

[kristian-lesko]

Problem Statement

The existing Vault provider is well suited for reading static Vault key-value secrets into k8s Secrets; however, reading dynamic secrets is not supported. This Generator is an attempt to extend external-secrets with that functionality.

Related Issue

Based on a discussion in #2036.

Proposed Changes

Add a new VaultDynamicSecret API object with an associated Generator module.

Extend the current pkg/provider/vault code with the possibility to initialize a Vault client from the Generator.

Checklist

• I have read the contribution guidelines
• All commits are signed with git commit --signoff
• My changes have reasonable test coverage
• All tests pass with make test
• I ensured my PR is ready for review with make reviewable

[paul-the-alien]

Greetings!
Thank you for contributing to this project!
If this is your first time contributing, please make
sure to read the Developer and Contributing Process guides.
Please also mind and follow our Code of Conduct.

Useful commands:

• make fmt: Formats the code
• make check-diff: Ensures the branch is clean
• make reviewable: Ensures a PR is ready for review

[kristian-lesko]

Hi @gusfcarvalho @moolen, I've drafted this based on our discussion in #2036.

Would anybody be able to give me a hand with the tests of the new code, please? I've copied over a skeleton for the tests from the provider code, but I cannot seem to get tests including a non-empty mocked Vault response to work. My Golang skills are not very advanced so I'll appreciate any help. Thanks a lot 🙂

[moolen]

Thanks, i'm taking a look at it right now 👀

[moolen]

The response can be deeply nested, this should be a map[string]interface{}.

[kristian-lesko]

I've tried to make this work, but the Generator interface requires the map[string][]byte return type.

The current setup means nested fields are stored in the target Secret as their Go string representation (for example, map[contents:read metadata:read]). Perhaps I could improve that further by JSON.Marshalling them?

That way, the current Generate interface type can be respected, and target users can parse the required values from JSON as needed (while top-level string values would be readily available to use).

[moolen]

Looking at it 👀 this sounds reasonable. It doesn't really make sense to change the interface, as the target Secret can only hold simple key/value pairs, not nested values.

Nested values are pretty rarely used in vault AFAIK, so 👍

[moolen]

I took a look at this again, i think this will help marshaling the map into something useful.

The problem with the current approach is, that you end up with an encoded string, which is a bit unwieldy to work with.

Notice the quotes " and encoded \n characters

apiVersion: v1
data:
  # ["-----BEGIN CERTIFICATE-----\nMIIDW
  ca_chain: WyItLS0tLUJFR0lOIENFUlRJRklDQVRFLS0tLS1cbk1JSURXe...
  # "-----BEGIN CERTIFICATE-----\nMIIDWTCCAkGg
  certificate: Ii0tLS0tQkVHSU4gQ0VSVElGSUNBVEUtLS0tLVxuTUlJRFdUQ0NBa0dn...
  # 1678564972
  expiration: MTY3ODU2NDk3Mg==
  # "-----BEGIN CERTIFICATE-----\nMIIDWzCCAkOgAwIBAgI
  issuing_ca: Ii0tLS0tQkVHSU4gQ0VSVElGSUNBVEUtLS0tLVxuTUlJRFd6Q0NBa09nQXdJQkFnSV...
  # "-----BEGIN RSA PRIVATE KEY-----\nMIIEowIBAAKCA
  private_key: Ii0tLS0tQkVHSU4gUlNBIFBSSVZBVEUgS0VZLS0tLS1cbk1JSUVvd0lCQUFLQ0F...
  # "rsa"
  private_key_type: InJzYSI=
  serial_number: IjUxOjIzOjg3OmNhOmI5OmU5OjBjOjdmO...
immutable: false
kind: Secret
metadata:
  name: pki-example-com
type: Opaque

I don't think the function mentioned above supports a complex type []string like the ca_chain field, thay may need some tweaking and tests 🔨

[kristian-lesko]

Thanks @moolen, I've pushed an update that makes use of (and extends) the function you mentioned (works for me for github-type dynamic secret in live environment).

[moolen]

The approach looks good so far!

I did a bit of research about the vault API surface, it looks like there are a couple of APIs we need to support for this generator. We don't need to support everything right away but we need to design it in a way so we can support it at some point in the future.

With this PR we support all GET-based APIs:

GET       /aws/creds/:name
GET       /azure/creds/:name
GET       /consul/creds/:name
GET       /rabbitmq/creds/:name
GET       /database/creds/:name
GET       /kubernetes/creds/:role
GET       /terraform/creds/:name

However, we don't support POST-based APIs like:

POST      /aws/sts/:name
GET/POST  /gcp/impersonated-account/:impersonated-account/token
GET/POST  /gcp/static-account/:static-account/token
GET/POST  /gcp/roleset/:roleset/token
POST      /pki/issue/:name
POST      /pki/issuer/:issuer_ref/issue/:name
POST      /ssh/creds/:name
POST      /ssh/issue/:name
POST      /transit/datakey/:type/:name
POST      /transit/random(/:source)(/:bytes)
POST      /transit/hmac/:name(/:algorithm)
POST      /kmip/scope/:scope/role/:role/credential/generate

With that being said i think we need a way to pass two things: a method and parameters:

apiVersion: generators.external-secrets.io/v1alpha1
kind: VaultDynamicSecret
spec:
  path: "/pki/issue/baz"

  # which method to use when making the API call
  # one of: GET, POST, PATCH, LIST, DELETE. defaults to GET
  # 
  method: "POST" 
  
  # a map[string]interface{}
  # that is passed as-is to the vault API
  parameters:
    common_name: "foobar"
    uri_sans: "..."
    ip_sans: "..."

  # renamed this to "provider" 
  provider:
    server: ""
    caBundle: ""
    auth:
      kubernetes:
        mountPath: ""
        role: ""
        serviceAccountRef: 
          name: "foo"

Do you consider to implement that in scope of this PR? It would be great to have that covered.

[kristian-lesko]

Hi @moolen, can you please re-check? I've adjusted the code according to your comments (and added JSON marshalling for the nested output - please see my comment above). Thanks a lot 🙂

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.1% Duplication

[moolen]

I did a quick manual test with vault locally, let me try to document the steps taken:

setup (roughly)

helm install vault hashicorp/vault \
    --set "server.dev.enabled=true" --set injector.enabled=false

# create role
cat > vault-policy-es.hcl <<EOF
path "secret/+/*" {
  capabilities = ["create", "read", "update", "delete", "list"]
}

path "pki/+/*" {
  capabilities = ["create", "read", "update", "delete", "list"]
}
EOF
vault policy write \
    external-secrets-operator \
    vault-policy-es.hcl

# enable kubernetes auth
vault auth enable kubernetes
vault write auth/kubernetes/config \
    kubernetes_host=https://kubernetes.default.svc.cluster.local \
    kubernetes_ca_cert=@kind.ca.pem \
    issuer=https://kubernetes.default.svc.cluster.local
vault write auth/kubernetes/role/external-secrets-operator \
    bound_service_account_names="*" \
    bound_service_account_namespaces="*" \
    policies=external-secrets-operator \
    ttl=1h

# enable pki
vault secrets enable pki
vault write pki/root/generate/internal \
    common_name=external-secrets-dev.io \
    ttl=8760h
vault write pki/roles/example-dot-com \
    allowed_domains=external-secrets-dev.io \
    allow_subdomains=true \
    max_ttl=72h

Dynamic Secret + ExternalSecret

apiVersion: generators.external-secrets.io/v1alpha1
kind: VaultDynamicSecret
metadata:
  name: "pki-example"
spec:
  path: "/pki/issue/example-dot-com"
  method: "POST"
  parameters:
    common_name: "localhost"
    ip_sans: "127.0.0.1,127.0.0.11"
  provider:
    server: "http://vault.default.svc.cluster.local:8200" # localhost:8200 when running port-forward
    auth:
      kubernetes:
        mountPath: "kubernetes"
        role: "external-secrets-operator"
        serviceAccountRef:
          name: "default"
---
apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  name: "pki-example-com"
spec:
  refreshInterval: "768h"
  target:
    name: pki-example-com
  dataFrom:
  - sourceRef:
      generatorRef:
        apiVersion: generators.external-secrets.io/v1alpha1
        kind: VaultDynamicSecret
        name: "pki-example"

resulting secret:

apiVersion: v1
data:
  ca_chain: WyItLS0tLUJFR0lOIENFUlRJRklDQVRFLS0tLS1cbk1JSURXe...
  certificate: Ii0tLS0tQkVHSU4gQ0VSVElGSUNBVEUtLS0tLVxuTUlJRFdUQ0NBa0dn...
  expiration: MTY3ODU2NDk3Mg==
  issuing_ca: Ii0tLS0tQkVHSU4gQ0VSVElGSUNBVEUtLS0tLVxuTUlJRFd6Q0NBa09nQXdJQkFnSV...
  private_key: Ii0tLS0tQkVHSU4gUlNBIFBSSVZBVEUgS0VZLS0tLS1cbk1JSUVvd0lCQUFLQ0F...
  private_key_type: InJzYSI=
  serial_number: IjUxOjIzOjg3OmNhOmI5OmU5OjBjOjdmO...
immutable: false
kind: Secret
metadata:
  name: pki-example-com
type: Opaque

[moolen]

Just some minor comments, can you address those? Otherwise it's just some linter issues that should be straight-forward to fix.
I really like that there are minimal changes in pkg/provider/vault 👍
It's really well structured and easy to read 🏅

pkg/provider/vault needs some serious refactoring 👩‍🔧, but thats a different topic 😉

[kristian-lesko]

Hi @moolen, thanks for the latest comments! I've integrated them into the PR.

A note: moving the ClientWithLoginMock into vault/fake was a slightly larger change than expected (due to dependencies on other types/structs), so I've kept in a separate fixup commit for now. I'll squash it if you agree this is the approach to go.

Thanks a lot 🙂

[moolen]

Did another test run, LGTM! 🎖️
Thank you so much for this contribution ❤️
Do you fancy to write some 📖 docs for this? If not i can do it before we release this, no problem.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.1% Duplication

[moolen]

@gusfcarvalho @knelasevero anyone wants to take look at this PR? If there are no objections i'd be happy to merge this in 🤞

[gusfcarvalho]

Been looking at this PR from a distance 👀 - didn't have the time to properly play with it, but it already feels very good 🥳

I believe we would only be missing vault tokens themselves (as in a short lived vault credential for other apps to consume) in order to be compliant with every possible use case vault could have - which IMO it makes sense to just be another Generator 😄

[kristian-lesko]

Hi @moolen, thanks for the approval & further fixes!

I can definitely try to prepare some docs; just two questions please:

• Should I push them as part of this PR, or separately?
• Where should they go (under Vault provider, Generator guide or somewhere else)?

Thanks a lot 🙂

[moolen]

Let's make an extra PR for that.

I'd add a new entry to the api/components section [1] and add a note to the Vault provider that states that the intended use is for vault kv only, for other backends point to the new generator section.

[1]

[moolen]

@gusfcarvalho

I believe we would only be missing vault tokens themselves

This should be covered with POST /auth/token/create/(:role) HTTP API, it's all pluggable 👍

[kristian-lesko]

Thanks for the merge @moolen :) I'll try to prepare the docs shortly 🚀

[kristian-lesko]

For reference, docs prepared in #2123.

@moolen when can this be expected to be released, please? Thank you very much 🙂

[moolen]

thank you! We'll do a release this week 🚀

[norman-zon]

@gusfcarvalho

I believe we would only be missing vault tokens themselves

This should be covered with POST /auth/token/create/(:role) HTTP API, it's all pluggable 👍

Anyone got this working successfully and can provide a full example? I am having issues getting it to work.

In fact, applying the following manifest:

apiVersion: generators.external-secrets.io/v1alpha1
kind: VaultDynamicSecret
metadata:
  name: "vault-token"
spec:
  path: "/v1/auth/token/create/"
  method: "POST"
  provider:
    server: "https://vault.mycompany.de"
    version: "v1"
    auth:
      kubernetes:
        mountPath: "main-cluster"
        role: "baseproject-project-x"
        serviceAccountRef:
          name: "baseproject"

crashes the external-secrets Pod:

{"level":"info","ts":1684169687.7693796,"msg":"Observed a panic in reconciler: runtime error: invalid memory address or nil pointer dereference","controller":"externalsecret","controllerGroup":"external-secrets.io","controllerKind":"ExternalSecret","ExternalSecret":{"name":"vault-token","namespace":"atlantis"},"namespace":"atlantis","name":"vault-token","reconcileID":"5bc4e939-dcb7-49b4-b4c5-5957a78648a1"}
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1d25267]

goroutine 475 [running]:
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
    /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:119 +0x1fa
panic({0x2f57dc0, 0x62773a0})
    /opt/hostedtoolcache/go/1.19.7/x64/src/runtime/panic.go:884 +0x212
github.com/external-secrets/external-secrets/pkg/generator/vault.(*Generator).generate(0xc00184c480?, {0x3d7d1f8, 0xc000fc86c0}, 0x3d3eb60?, 0x630da08?, {0x3d8af90, 0xc000eefb60}, {0x3d8f3f8, 0xc00147aee0}, {0xc001290ef8, ...})
    /home/runner/work/external-secrets/external-secrets/pkg/generator/vault/vault.go:86 +0x307
github.com/external-secrets/external-secrets/pkg/generator/vault.(*Generator).Generate(0xc0014490c8?, {0x3d7d1f8, 0xc000fc86c0}, 0xc001290ef8?, {0x3d8af90, 0xc000eefb60}, {0xc001290ef8, 0x8})
    /home/runner/work/external-secrets/external-secrets/pkg/generator/vault/vault.go:58 +0xec
github.com/external-secrets/external-secrets/pkg/controllers/externalsecret.(*Reconciler).handleGenerateSecrets(0xc000a41420, {0x3d7d1f8, 0xc000fc86c0}, {0xc001290ef8, 0x8}, {0x0, 0x0, {0x0, 0x0, 0x0}, ...}, ...)
    /home/runner/work/external-secrets/external-secrets/pkg/controllers/externalsecret/externalsecret_controller_secret.go:116 +0xc6
github.com/external-secrets/external-secrets/pkg/controllers/externalsecret.(*Reconciler).getProviderSecretData(0xc000a41420, {0x3d7d1f8, 0xc000fc86c0}, 0xc0012c0b40)
    /home/runner/work/external-secrets/external-secrets/pkg/controllers/externalsecret/externalsecret_controller_secret.go:59 +0x57a
github.com/external-secrets/external-secrets/pkg/controllers/externalsecret.(*Reconciler).Reconcile(0xc000a41420, {0x3d7d1f8?, 0xc000fc86c0}, {{{0xc001290ef8, 0x8}, {0xc001290f00, 0xb}}})
    /home/runner/work/external-secrets/external-secrets/pkg/controllers/externalsecret/externalsecret_controller.go:187 +0xf85
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x3d7d1f8?, {0x3d7d1f8?, 0xc000fc86c0?}, {{{0xc001290ef8?, 0x2d58680?}, {0xc001290f00?, 0x0?}}})
    /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:122 +0xc8
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc000033900, {0x3d7d150, 0xc0010efd80}, {0x30d6a20?, 0xc000563d40?})
    /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:323 +0x38f
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc000033900, {0x3d7d150, 0xc0010efd80})
    /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:274 +0x1d9
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
    /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:235 +0x85
created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2
    /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:231 +0x333
Stream closed EOF for external-secrets/external-secrets-5849c4b4d7-nwvq9 (external-secrets)

[moolen]

You found a bug!

Can you try this: add parameters: {}, that should do it 🤞

apiVersion: generators.external-secrets.io/v1alpha1
kind: VaultDynamicSecret
metadata:
  name: "vault-token"
spec:
  path: "/v1/auth/token/create/"
  method: "POST"
  parameters: {} # add this here
  provider:
    server: "https://vault.mycompany.de"
    version: "v1"
    auth:
      kubernetes:
        mountPath: "main-cluster"
        role: "baseproject-project-x"
        serviceAccountRef:
          name: "baseproject"

[moolen]

This will be fixed with the upcoming release, see 👆

[norman-zon]

You found a bug!

Can you try this: add parameters: {}, that should do it 🤞

This prevents the crash. But still does not yield a working result.
Now both the VaultDynamicSecret and the externalSecret show Events: <none> and there is no log for them in the external-secrets controller, even on debug-level.

[moolen]

Hey @norman-zon i'll need some time to reproduce this, would you mind creating an issue so we can track that? 🙏

[norman-zon]

I reported the problem in #2324