Allow to specify TLS CAs outside of -pd-cluster-secret

[Tema]

trafficstars

Feature Request

Is your feature request related to a problem? Please describe: We use tidb-operator to run TiKV cluster w/o TiDB layer. Our clients are deployed to a separate k8s cluster. It is not safe to share private key of root CA, so our clients have a separate root CA. Now I have to provider PD/TiKV with combined ca.crt from client and server. We rely on cert-manager.io to generate and rotate certificates as recommended on https://docs.pingcap.com/tidb-in-kubernetes/stable/enable-tls-between-components#using-cert-manager, but it does not allow to add an additional ca.crt in the generated secrets.

Describe the feature you'd like: Allow to specify a separate secret for root CA to override ca.crt from -pd-cluster-secret. Or allow security/cacert-path to take precedence of secrete instead of blindly overriding it in https://github.com/pingcap/tidb-operator/blob/62b11ad37ddd3482dfec519770d6c65011e24d6d/pkg/manager/member/pd_member_manager.go#L836-L841 so that it is possible to mount and configure it separately

Describe alternatives you've considered:

• Use security/cacert-path in CR, but it is gets overridden by -pd-cluster-secret mount.
• Use additionalVolumeMount in CR with subPath to mount a different ca.crt under /var/lib/pd-tls, but k8s forbids mounting to the same path
• Add client root CA to /etc/ssl/certs/ca-certificates.crt on the pod (system trust store), but PD and TiKV doesn't seem honoring it.
• Put combined ca.crt into secret for the cert-manager.io Issuer, but cert-mahager.io Certificate does not include it in the generated certificate secrete. The only alternative I can see is not using cert-manager.io, but generate secrete manually, however then I will have to implement in infrastructure to rotate certificates.

Teachability, Documentation, Adoption, Migration Strategy:

[DanielZhangQD]

@Tema Thanks for reporting this issue! The request is reasonable, would you like to contribute to this feature? BTW, with "Or allow security/cacert-path to take precedence of secrete instead of blindly overriding it", how do you want to do this exactly? And could you please describe the update to the CR briefly so that we can reach agreement before coding, thanks!

[Tema]

"Or allow security/cacert-path to take precedence of secrete instead of blindly overriding it", how do you want to do this exactly?

Basically instead of https://github.com/pingcap/tidb-operator/blob/62b11ad37ddd3482dfec519770d6c65011e24d6d/pkg/manager/member/pd_member_manager.go#L836-L841

I thought we can have

if tc.IsTLSClusterEnabled() { 
    if config.Get("security.cacert-path") == nil {
	config.Set("security.cacert-path", path.Join(pdClusterCertPath, tlsSecretRootCAKey)) 
    } 	
} 

The only caveat is if someone already have this defined with some value, which I would say highly unlikely it might break things. Maybe you can put it in the next release note to avoid this chance.

Alternatively, we can always mount -root-ca-secret with optional flag and take path.Join(pdClusterCertPath, tlsSecretRootCAKey) in the snippet above from it if the secret is present.

[mikechengwei]

Is it possible to accomplish your needs by enhancing the additionalVolumeMount configuration, if you configure the same volume name, it will use what you configured. This is similar to the idea of ​​3350. @Tema @DanielZhangQD

[Tema]

@mikechengwei

Is it possible to accomplish your needs by enhancing the additionalVolumeMount configuration, if you configure the same volume name

I've tried that, but k8s forbids mounting to the same name multiple entities and <cluster-name>-pd-cluster-secret is already mounted to the /var/lib/pd-tls. Subpath mounting faces the same issue.

[DanielZhangQD]

The only caveat is if someone already have this defined with some value, which I would say highly unlikely it might break things. Maybe you can put it in the next release note to avoid this chance.

Agree with you.

So we can go with the below solution, right?

1. Configure additionalVolume with the separate ca secret, the additionalVolumeMount, and security.cacert-path
2. Update code as you described above

[Tema]

Sounds good @DanielZhangQD.

[DanielZhangQD]

@Tema Would you like to submit PR and an example yaml in the examples dir for this?

[Tema]

@DanielZhangQD, we have decided to use TiDB layer in our setup. Fortunately, tidb-operator allows to specify separate certificates for client and inter-component already, so it does not block us anymore. However, in future I believe we will still want to have this feature to be able to use Raw TiKV directly. Just not right now, hence I'm deferring work on this PR.

[mikechengwei]

@Tema now，you can try this feature , the same volume name in additionalVolumeMount it will overwrite default volume.