Deployment fails for TLS certificates via cert-manager and ACME

[miclip]

Using a cluster issue for Let's Encrypt over ACME fails. Let's encrypt requires that if the common name is set, the same value must also be listed as a SAN. The yugabyte helm chart is hard-coding the common name. In addition, the TLS secret that is created does not contain the CA, the statefulset is expecting to mount the CA from the TLS secret as a volume.

I believe these are not unique to let's encrypt and corporate PKI solutions will have the same CSR requirements, and not return the CA within the TLS secret cert-manager generates.

[iSignal]

@miclip : Thanks for filing this, this is useful feedback.

1. What do you suggest having in the CN instead? Given that the certificate is common to all pods, I guess we could use the DNS for yb-tserver-0/yb-master-0, which is also one of the SAN entries. Any other suggestions?

2. I do see such documentation on cert-manager docs confirming this.

Additionally, if the Certificate Authority is known, the corresponding CA certificate will be stored in the secret with key ca.crt. For example, with the ACME issuer, the CA is not known and ca.crt will not exist in acme-crt-secret.

But this is confusing. How are we supposed to discover the CA cert in such cases? Is this supposed to a mechanism of distribution outside of k8s and the customer manually specifies the ca.crt in such cases?

@subramanian-neelakantan @bhavin192 @baba230896

[miclip]

1. I need to confirm but I think the CN is no longer required and the SANs (DNS Names) are all you need?
2. They're assuming the CA is public or already trusted. Perhaps supporting an 'additionalTrustedCerts' field in the values file so any custom certs can be trusted within the deployment.

[miclip]

k explain certificate.spec.commonName


KIND:     Certificate
VERSION:  cert-manager.io/v1

FIELD:    commonName <string>

DESCRIPTION:
     CommonName is a common name to be used on the Certificate. The CommonName
     should have a length of 64 characters or fewer to avoid generating invalid
     CSRs. This value is ignored by TLS clients when any subject alt name is
     set. This is x509 behaviour:
     https://tools.ietf.org/html/rfc6125#section-6.4.4

So you should be able to just not set the CN provided you always set the SANs, which should work for self signed and ACME etc. I see in certs that are requested as a result of Ingress annotations, cert-manager never sets the CN.

[iSignal]

@miclip : ok, got it. We are planning to iterate on the cert-manager functionality soon and will take this feedback into account.

[rastaman]

Edit: Oups i forgot to read the full issue before, below patch helps but not for the CA.

I have the same issue, so i used a kustomization to patch the helm chart:

patches:
- patch: |-
    apiVersion: cert-manager.io/v1
    kind: Certificate
    metadata:
      name: yugabyte-tls-client-cert
      namespace: yugabyte
    spec:
      commonName: yugabyte-tls-client-cert.homelab.expansible.io
      dnsNames:
      - yugabyte-tls-client-cert.homelab.expansible.io

(where homelab.expansible.io is my own domain)

However after the patch owrks, it still fails because there is no ca.crt in the certificate provided by Let's Encrypt. I guess i'll open an issue for this too, there is a few lines of helm chart/shell script to rework. Perhaps do you know if it Is already seen in the iteration of the cert-manager functionnality or should i open an issue @iSignal ?