feat: add vault auth namespace option

[blairdrummond]

Edit: I am not familiar with this code-base and am expecting that changes will be required, but can confirm that this change works as intended in my clusters. Look forward to feedback!

Problem Statement

Add Vault.Auth.Namespace field so that you can authenticate in one Vault namespace and use the vault token in a different namespace. This is a common-ish setup

Related Issue

No issue created yet but I can make one.

Proposed Changes

This code adds a new Namespace field in the Vault.Auth config, allowing ESO to authenticate against one Vault Namespace, and use that token in another namespace. This is not a breaking change, but it does introduce a new optional field into the spec. I think this is a fairly well documented Vault setup

I considered alternatives, but the choices would be either:

• refactor all my auth backends to live (duplicated) in every tenant namespace
• create the vault token through an out-of-band process and have ESO read a token instead of using the JWT auth method.

Example manifest

apiVersion: external-secrets.io/v1beta1
kind: ClusterSecretStore
metadata:
  creationTimestamp: "2024-02-16T22:33:23Z"
  generation: 1
  name: tenant-team-a-kv-global
  resourceVersion: "15762515"
  uid: ecc747e0-cb7e-4956-aede-76e0c4266c87
spec:
  conditions:
  - namespaceSelector:
      matchLabels:
        owner: team-a
  provider:
    vault:
      auth:
        # Use the jwt mount in the `admin` namespace
        jwt:
          kubernetesServiceAccountToken:
            serviceAccountRef:
              audiences:
              - vault
              name: vault-sync
              namespace: external-secrets
          path: jwt_mgmt_k8s_platform_v3_eks
          role: k8s
        namespace: admin                       # <-- this is new!
      namespace: admin/tenant-team-a           # <-- example tenant. Use a token from one namespace against another namespace
      path: kv_global
      server: https://vault.example.com
      tls: {}
      version: v2
status:
  capabilities: ReadWrite
  conditions:
  - lastTransitionTime: "2024-02-16T22:33:23Z"
    message: store validated
    reason: Valid
    status: "True"
    type: Ready

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

[gusfcarvalho]

hi @blairdrummond ! Is there a way to add tests for this change? 🤔

[gusfcarvalho]

CI tests are also failing. You can double check which tests are failing with make lint

[blairdrummond]

Pushed the linting fix. I will try reading through the tests and see what could be added... Should I constrain my attention to the pkg/provider/vault tests? Or should I look elsewhere, too?

One thing I would like feedback on is the specific change in the setAuth function. It is very basic and I am not sure if there are concurrency-safety issues here. Putting this logic entirely in the auth function is also a little messy, but on first-stab I wanted to introduce the most transparent approach.

[blairdrummond]

I think I should move the code out of setAuth, and I'll write some tests around permutations of

Vault.Namespace	Vault.Auth.Namepsace
nil	nil
a	nil
nil	a
a	a
a	b

[blairdrummond]

@gusfcarvalho Did a cleanup and added some tests for setAuthNamespace. Finished up the make test and make reviewable steps as well

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[blairdrummond]

Just checking if there's anything else needed on my end?

[Skarlso]

You could shift this value into the if statement since that's where it's used first.

[Skarlso]

Nice! Thanks. :) 🙇