Use of kubernetes.io/tls instead of Opaque for Certificates

[rnsv]

Feature Request

Replace Certificate type to use kubernetes.io/tls for web-hooks and api-service implementation

What problem are you trying to solve?

To make tls secret consistent with Kubernetes type(kubernetes.io/tls) that facilitates use of cert generators like cert-manager

How should the problem be solved?

By using tls.key and tls.crt instead of key.pem and crt.pem

File: linkerd2/pkg/k8s/labels.go

    // IdentityIssuerKeyName is the issuer's private key file.
    IdentityIssuerKeyName = "key.pem"

    // IdentityIssuerCrtName is the issuer's certificate file.
    IdentityIssuerCrtName = "crt.pem"

What do you want to happen? Add any considered drawbacks.

None

Any alternatives you've considered?

Alternative is to mount tls.key and tls.crt as subpath that maps to key.pem and crt.pem. Though doable, it is cumbersome.

How would users interact with this feature?

No direct user impact.

[grampelberg]

Hmm, have you looked at what's in edge right now? We basically did exactly this...

[rnsv]

I see key.pem and crt.pem referenced in edge-19.12.2 - https://github.com/linkerd/linkerd2/blob/991542fec647244f3fb19d42d92b093d34583da9/pkg/k8s/labels.go#L223-L228 May be I am looking at the wrong place(If so, please direct to the correct code snippet). I will also download the edge binaries and test it out.

[grampelberg]

@ihcsim @zaharidichev do we have docs written up yet? linkerd install --identity-external-issuer is what you're looking for.

[ihcsim]

@grampelberg I believe this issue is about changing the scheme of the webhooks' secret; not the mTLS stuff.

[grampelberg]

To make tls secret consistent with Kubernetes type(kubernetes.io/tls) that facilitates use of cert generators like cert-manager

That's solved by --identity-external-issuer, isn't it?

[zaharidichev]

@grampelberg the --identity-external-issuer only deals with the linkerd-identity-issuer secret. However, its probably not such a bad idea to transition the rest of the cert secrets to kubernetes.io/tls at some point. Is there a solid reason not to do it?

[zaharidichev]

@grampelberg just to calarify --identity-external-issuer dictates what kind of "scheme" is used for the linkerd-identitiy-issuer secret. If its false, we use linkerd.io/tls with the key.pem and crt.pem keys. If the flag is true we assume the secret was created by cert-manager in the kubernetes.io/tls format and we use the corresponding keys. So in essence we have not really changed much of what was already in there and we still rely on key.pem and crt.pem"

[grampelberg]

I'm not a huge fan of updating the default scheme because of the upgrade requirements right now. It feels like the problem is being solved by allowing the scheme to be plugable.

[zaharidichev]

I think that was the conclusion we reached when we had that conversation last time around.

[rnsv]

I completely agree with @grampelberg that default scheme should be backward compatible. The fix via --identity-external-issuer will work for identity.

That said, my issue started with webhooks and I did not realize until now that there are 2 places where the tls path is being managed. Since the issue is still observable for webhooks in the edge version, I thought I will elaborate a bit on what I am trying to do.

I want to manage all my certs (sp, proxy, tap and identity) via cert manager.

a. -identity-external-issuer would work for identity b. tap has options to specify tls cert and key as part of args c. sp and proxy still are looking for values in default path cred, err := tls.ReadPEMCreds(pkgk8s.MountPathTLSKeyPEM, pkgk8s.MountPathTLSCrtPEM) and is not configurable.

Would it make sense to provide an option (for eg. --use-external-issuer) for webhooks and also make tap arg also consistent with that.

[grampelberg]

That's a good point, I think it makes sense. Helm installs are gonna be painful as you'll need to start passing the caBundle in for the webhooks. As all these certs are point to point (only ever from the api-server to the process) it hasn't been much of a focus for us. You can just update the control plane and make it all work again.

[cypherfox]

@rnsv could you please have a look at #4737. Does that fix all your requirements?