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

Allow setting VaultAuth spec kubernetes mount as a default global setting #274

Open
mmckane opened this issue Jun 21, 2023 · 6 comments · May be fixed by #735
Open

Allow setting VaultAuth spec kubernetes mount as a default global setting #274

mmckane opened this issue Jun 21, 2023 · 6 comments · May be fixed by #735
Assignees
Labels
enhancement New feature or request

Comments

@mmckane
Copy link

mmckane commented Jun 21, 2023

Is your feature request related to a problem? Please describe.
Developers currently need to define the vault mount auth endpoint for every namespace they deploy into. This adds overhead to our development teams as we have many clusters and they now need to know the correct vault auth mount point at deployment time for each cluster they deploy to. As we treat clusters and vault role to service account mappings like cattle, this is problematic as the developer now needs to know the mount point for the cluster each time they deploy.

Describe the solution you'd like
Configure a way to globally set the mount so that it can be omitted from VaultAuth spec when using Kubernetes auth. Possibly allow setting this via an ENV that is passed into the chart/deployment/pod that configures the setting for all namespaces, so that developers do not need to provide the specific vault auth mountpoint for the cluster they have been deployed to.

Moving the mount point to the VaultConnection spec might also solve this as that can be globally defaulted, where the developer then only needs to worry about creating a VaultAuth spec that contains the role and service account mapping.

@mmckane mmckane added the enhancement New feature or request label Jun 21, 2023
@kschoche
Copy link
Contributor

Hi @mmckane, thanks for filing this issue and sorry that you're running into this.

It sounds to me like what you're asking for might be the default auth method? You can configure it via the helm chart under defaultAuthMethod and then any CR that does not specify an auth method reference will make use of the default.

I might be missing something though, if for example you're using different mounts per k8s ns/app?

Can you let me know if that would work for you or how we could extend it otherwise?
Thanks!

@mmckane
Copy link
Author

mmckane commented Jun 22, 2023

@kschoche Doesn't defaultAuthMethod also require a service account and vault role to be defined that would apply to each namespace using the default Auth Method? The problem is each namespace in my cluster uses different service account names per k8s best practices and each namespace/SA binds to a different vault role. This doesn't seem like the most secure setup even if I did use the same SA/role everywhere as they would all use the same vault role meaning a tenant in namespace A could create a resource and access namespace B secrets as they would both be bound to the same role.

@xenochor
Copy link

xenochor commented Jul 3, 2023

I agree with @kschoche, please review the patterns afforded by the CSI secrets store driver and Vault provider regarding enabling system-wide defaults for URL, namespace and mount path while allowing mounts within namespaces to only be concerned with secret path, role and, indirectly in the case of CSI secrets store, service account. This is one of the key failings of the other options such as the Vault secrets operator and the external secrets operator.

@mmckane
Copy link
Author

mmckane commented Aug 22, 2023

Ok I am still struggling here. It allows me to create a default vault auth map like this in the operator namespace which is managed and set by the OPS team:

apiVersion: secrets.hashicorp.com/v1beta1
kind: VaultAuth
metadata:
  name: default
  namespace: vault-secrets-operator
  labels:
    control-plane: controller-manager
    component: controller-manager
    app.kubernetes.io/component: controller-manager
spec:
  method: kubernetes
  mount: kubernetes-my-cluster-a

In the developers namespace which is owned by the development team I am hoping to specify and use the following configuration to access and create my secret:

apiVersion: secrets.hashicorp.com/v1beta1
kind: VaultAuth
metadata:
  name: applicationa-vault-auth
  namespace: dev-namespace-a
spec:
  kubernetes:
    role: application-a-role
    serviceAccount: application-a

I would hope that the above would take the missing fields method and mount and combine them to make a valid vault-auth role so my developers can skip specifying the mount. This does not work because the CRD is specifying Mount and Method as required fields with this error when trying to apply:

error validating "test.yaml": error validating data: [ValidationError(VaultAuth.spec): missing required field "method" in com.hashicorp.secrets.v1beta1.VaultAuth.spec, ValidationError(VaultAuth.spec): missing required field "mount" in com.hashicorp.secrets.v1beta1.VaultAuth.spec]; if you choose to ignore these errors, turn validation off with --validate=false

Creating a specification like this works:

apiVersion: secrets.hashicorp.com/v1beta1
kind: VaultAuth
metadata:
  name: default
  namespace: vault-secrets-operator
  labels:
    control-plane: controller-manager
    component: controller-manager
    app.kubernetes.io/component: controller-manager
spec:
  method: kubernetes
  mount: kubernetes-my-cluster-a
  kubernetes:
    role: application-a-role
    serviceAccount: application-a

But now when I use this default vaultAuth specification everyone is forced to utilize a service account with the same name or possibly use the same SA out of the vault-secrets-operator namespace (I am not sure what is true here, but either option is undesirable because the developer looses control to specify the SA that they are using.).

Back to somehow specifying mount outside of the spec and allowing it to be omitted or somehow allowing the combination of the default and developer specified vaultAuth spec would be preferable.

@benashz
Copy link
Collaborator

benashz commented Sep 13, 2023

Fixed with #291

@benashz benashz closed this as completed Sep 13, 2023
@benashz benashz reopened this Sep 13, 2023
@benashz benashz closed this as completed Sep 13, 2023
@benashz benashz reopened this Sep 13, 2023
@benashz benashz closed this as completed Sep 13, 2023
@mmckane
Copy link
Author

mmckane commented Dec 1, 2023

@benashz Can we re-open this. Adding the ability to define the namespace on the resources does not fix this issue. I as a cluster operator now need to manage the service account to role mapping.

The goal here is to allow the developer to define their own service account name to role mapping without needing to know the name of the kubernetes mount which is set and maintained by the cluster operator. More the cluster operator should be able to globally define the kubernetes mount in the vaultAuth spec and the developer should not need to worry about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants