Don't try to build etcd-manager secrets for cilium twice

[olemarkus]

Ciliium etcd mode is broken at the moment as both etcd TLS builder and cilium builder tries to create certificates. As the logic for cilium certs is slightly different, I am keeping the older logic and skipping cilium in the etcd TLS builder.

/kind bug

@johngmyers @justinsb it may be that you want to cherry-pick this one before the 1.21.0 release.

[olemarkus]

/retest

[olemarkus]

/retest

[johngmyers]

/assign

[johngmyers]

I would rather you remove the code for the manager, peers, and clients certs from CiliumBuilder. It would result in less code overall and less risk of divergence between the two cases.

[johngmyers]

Suggested change

	// The certs for cilium etcd is managed by CiliumBuilder
	// The certs for cilium etcd are managed by CiliumBuilder

[olemarkus]

I agree that things can be consolidated here. But I am not convinced putting it in etcd_manager_tls is the best option. There will be some if cilium do this and if not apiserver do that in that logic in there.

I will follow up on refactoring this in a later PR. It is not that important to resolve this regression.

[johngmyers]

What different logic would that be? The only difference I see is a nil check that is absent from the Cilium version.

[olemarkus]

Client ca is installed not only on apiservers, but on all nodes.

I think this should go into 1.21 and therefore would like to make a small change as possible right now.

[johngmyers]

So the difference is that the "cilium" etcd has a different client CA "etcd-clients-ca-cilium", whereas the others share a client CA "etcd-clients-ca".

A simple fix that would reduce code duplication would be

		// Because API server can only have a single client certificate for etcd, we need to share a client CA
  		keys["etcd-clients-ca"] = "etcd-clients-ca"
                // ...except for Cilium
                if k == "cilium" {
  		    keys["etcd-clients-ca"] = "etcd-clients-ca-cilium"
                }

That the etcd-clients-ca-cilium private key is exposed to all the nodes and the nodes issue their own client certs is inexplicable. It squanders the security benefit of using kops-controller for bootstrap, at least with respect to Cilium etcd. But that's a different ticket.

[olemarkus]

I still suggest merging this one and cherry-pick to 1.21 and fixing all the other issues in a follow-up.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johngmyers

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [johngmyers]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment