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

CA injection via cert-manager in ClusterSecretStore do not work #9

Closed
volschin opened this issue Jul 11, 2024 · 15 comments · Fixed by external-secrets/external-secrets#3699
Assignees

Comments

@volschin
Copy link

volschin commented Jul 11, 2024

Describe the bug
I tried to setup ClusterSecretStore after deployment of the self signed certificate chain and the helm installation. I tried to inject the ca instead of using the caBundle field statically.

apiVersion: external-secrets.io/v1beta1
kind: ClusterSecretStore
metadata:
  name: bitwarden-secrets-manager
  annotations:
    cert-manager.io/inject-ca-from: external-secrets/bitwarden-tls-certs

But it tells me caBundle is a required field.
Full message from reconciliation:

ClusterSecretStore/external-secrets/bitwarden-secrets-manager dry-run failed (Invalid): ClusterSecretStore.external-secrets.io "bitwarden-secrets-manager" is invalid: spec.provider.bitwardensecretsmanager.caBundle: Required value

To Reproduce
Steps to reproduce the behavior:
My deployment is made with flux. My yaml code can be seen here:
https://github.com/volschin/home-ops/blob/main/kubernetes/apps/external-secrets/external-secrets/stores/bitwarden-secrets/clustersecretstore.yaml

Expected behavior
It should be possible to inject the ca from the generated certificate in the ClusterSecretStore.

** Additional **
In my connected research I found this linkerd issue an pr from some years ago that goes in a same direction an perhaps helps.
linkerd/linkerd2#7353

@Skarlso
Copy link
Collaborator

Skarlso commented Jul 11, 2024

Oh interesting. I don't know of that approach. :) I'll take a look on how to implement that.

@volschin
Copy link
Author

I‘m surprised, because your hack script explicitly waited for the ca-injector to be ready. ;)

@volschin
Copy link
Author

To be honest, at the moment I‘m in a brainstorming phase to use mTLS instead of normal TLS. It would be helpful to have the kind of communication between the services open to such concepts.

@Skarlso
Copy link
Collaborator

Skarlso commented Jul 12, 2024

Hmmm. Not entirely sure how that would work since external secrets isn't necessarily aware of the service. It's handling it as a generic something that has an api.

If it would start checking any certificates it would impact the setup of the whole project too.

It's not impossible I just think it's not necessarily worth the effort. 🤣 but maybe I'm wrong. 😀

@Skarlso
Copy link
Collaborator

Skarlso commented Jul 12, 2024

Hmmm. Then again we are talking about secrets here that potentially could destroy a system. So scratch what I said. Setting up mTLS would be nice.

@Skarlso
Copy link
Collaborator

Skarlso commented Jul 12, 2024

Btw I don't think this ca injector annotation would work here since the caBundle field isn't in the spec. It's under spec provider bitwardenprovider auth. The thing we configure is neither a webhook nor an api service. So I'm not sure how this would work.

@Skarlso
Copy link
Collaborator

Skarlso commented Jul 12, 2024

That said... eso does have it's own injector. I wonder if I could implement something there. I will have to check that out.

@Skarlso
Copy link
Collaborator

Skarlso commented Jul 14, 2024

Hm, this is most likely not going to happen as it requires a bunch of rewrites to our injector and the recommended approach is to either point to a secret or use a bundled certificate. I will add the secret approach as well.

@Skarlso
Copy link
Collaborator

Skarlso commented Jul 14, 2024

Meaning it will get something like this:

// Used to provide custom certificate authority (CA) certificates
// for a secret store. The CAProvider points to a Secret or ConfigMap resource
// that contains a PEM-encoded certificate.
type CAProvider struct {
	// The type of provider to use such as "Secret", or "ConfigMap".
	// +kubebuilder:validation:Enum="Secret";"ConfigMap"
	Type CAProviderType `json:"type"`

	// The name of the object located at the provider type.
	Name string `json:"name"`

	// The key where the CA certificate can be found in the Secret or ConfigMap.
	// +kubebuilder:validation:Optional
	Key string `json:"key,omitempty"`

	// The namespace the Provider type is in.
	// Can only be defined when used in a ClusterSecretStore.
	// +optional
	Namespace *string `json:"namespace,omitempty"`
}

So the source can be a secret or a configmap too.

@Skarlso
Copy link
Collaborator

Skarlso commented Jul 16, 2024

@volschin
Copy link
Author

Looks like a really fine & clean solution. 👍

@brianramseyau
Copy link

brianramseyau commented Jul 27, 2024

I've just hit into this one as well as the caProvider option (which is in the docs: https://github.com/external-secrets/bitwarden-sdk-server?tab=readme-ov-file#external-secrets) is not in the CRD https://github.com/external-secrets/external-secrets/blob/4f62fb396397bc4973f54dce1744e8de39900a9c/apis/externalsecrets/v1beta1/secretsstore_bitwarden_types.go#L20 also lacking the ref in the upstream api docs as well https://external-secrets.io/latest/api/spec/#external-secrets.io/v1beta1.CAProvider

I have resorted to (for now, for testing) copying the CA bundle from the CertManager output secret and adding in into the caBundle string as a bypass, though this is not ideal as any changes upstream (CertManager) could result in a new CA being required and then its invalidated.

I can see you have an MR thats open which looks at a glace to be addressing this, however :)

@volschin
Copy link
Author

@brianramseyau Yes, the upcoming solution would be able to handle a ca certificate rotation also.

@Skarlso
Copy link
Collaborator

Skarlso commented Aug 16, 2024

This will have to be released first in ESO to take effect. So keep an eye out for 0.10.1 for ESO.

@Skarlso
Copy link
Collaborator

Skarlso commented Aug 28, 2024

Finally, ESO 0.10.1 was released :)

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

Successfully merging a pull request may close this issue.

3 participants