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

Reference a secret instead of directly pasting a base64 encoded pem file to TLSspec.certificateAuthorityData #1570

Open
mhoshi-vm opened this issue Jul 6, 2023 · 3 comments
Labels
enhancement New feature or request state/started Someone is working on it currently

Comments

@mhoshi-vm
Copy link

Is your feature request related to a problem? Please describe.

As written below, we need to copy paste the base64 pem file to the following parameter

https://github.com/vmware-tanzu/pinniped/blob/main/generated/1.20/README.adoc#k8s-api-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-tlsspec

This is pretty hard to automate, for example using tools such as cert-manager that dynamically creates certificates

Describe the solution you'd like

Allow to reference a Kubernetes secret

Describe alternatives you've considered

I thought of using secretgen/secrettemplate to map the secret components, but it can only create kind: secret objects

@cfryanr
Copy link
Member

cfryanr commented Jul 6, 2023

Thanks for creating this issue @mhoshi-vm!

It looks like you are talking about the spec.tls.certificateAuthorityData field of ActiveDirectoryIdentityProvider, LDAPIdentityProvider, and OIDCIdentityProvider.

These are not currently references to a Kubernetes Secret for a few reasons.

  1. There is no confidential data involved.
  2. Is it not especially common for the external identity provider to be on the same cluster, so it is not especially common for the CA certificate to exist in a Secret generated by a tool like cert-manager. Although I do recognize that in those cases it may be convenient to be able to use a secretRef.
  3. There does not seem to be a standard way to reference a CA bundle in a Secret. Some apps like cert-manager use the key ca.crt but that is not part of the definition of the definition of tls secrets, see https://kubernetes.io/docs/concepts/configuration/secret/#tls-secrets. Even cert-manager does not always populate ca.crt in its Secrets, see https://cert-manager.io/docs/projects/trust-manager/#cert-manager-integration-cacrt-vs-tlscrt.

We would want to avoid making a breaking change to remove the existing spec.tls.certificateAuthorityData, so the only way to add such a feature would be to allow multiple ways to specify the CA bundle. However, this could be confusing for users, and something reasonable would need to happen when different CA bundles are specified both ways at the same time.

Could you share more about your use case? What in your use case makes it hard to read the CA bundle from your cert-manager Secret at the time that you are creating the identity provider CR? Is it because you are hoping to install everything in a single step? Have you considered using things like init containers or Jobs to copy the CA bundle to the identity provider CR?

@pinniped-ci-bot pinniped-ci-bot added enhancement New feature or request priority/undecided Not yet prioritized labels Jul 20, 2023
@rooso
Copy link
Contributor

rooso commented Sep 22, 2023

Hi @cfryanr, I'll jump in here and provide my use case :)

I'm talking about the spec.tls.certificateAuthorityData field of JWTAuthenticator.

You're right, there is no confidential data involved in this case. But for customers like us, who are using a Secret Management solution like 1Password Operator or External Secrets Operator, it would be a huge plus to store this kind of Certificates in a Secret.

Why? The secrets can be update in the Secret Operator and will then be automatically replaced in the Kubernetes Secret. Not only for Pinniped, also for all other Deployments that use the CA Certificate from the same Secret. So no more manual update of Certificates in multiple Kubernetes YAML Files.

There is might no standard way for a CA Certificate to be stored in a secret. Might just let use specify the name same way as used to mount a Certificate from Secret to a container as volume:

      volumes:
      - name: custom-ca-cert
        secret:
          secretName: custom-ca-cert
          items:
            - key: ca.crt
              path: ca.crt

@cfryanr
Copy link
Member

cfryanr commented Mar 8, 2024

@mhoshi-vm Were you also talking about the spec.tls.certificateAuthorityData field of JWTAuthenticator? Or were you talking about talking about the spec.tls.certificateAuthorityData field of ActiveDirectoryIdentityProvider, LDAPIdentityProvider, and OIDCIdentityProvider?

@joshuatcasey joshuatcasey added state/started Someone is working on it currently and removed priority/undecided Not yet prioritized labels Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request state/started Someone is working on it currently
Projects
None yet
Development

No branches or pull requests

5 participants