@GrahamcOfBorg build kubernetes kubectl

cc @johanot @offline @NixOS/podman

@saschagrunert That was fast 🚀 Thanks! 🎉 Haven't had the slightest chance to test this though.

Note: Golang v1.15 common name deprecation (https://golang.org/doc/go1.15#commonname , see also: kubernetes/kubernetes#93264) breaks easyCerts and hence the Kubernetes tests are now broken as well. We need to fix easyCerts and we need at least a 20.09 release note about this, since it might break clusters with custom PKI-setups.

I know that this is more a Go 1.15 thing than a Kubernetes thing, but I think it would be good to have the k8s tests passing, before merging this. I won't have time over the weekend, but will look into it first thing next week if no one beats me to it.

Note: Golang v1.15 common name deprecation (https://golang.org/doc/go1.15#commonname , see also: kubernetes/kubernetes#93264) breaks easyCerts and hence the Kubernetes tests are now broken as well. We need to fix easyCerts and we need at least a 20.09 release note about this, since it might break clusters with custom PKI-setups.

I know that this is more a Go 1.15 thing than a Kubernetes thing, but I think it would be good to have the k8s tests passing, before merging this. I won't have time over the weekend, but will look into it first thing next week if no one beats me to it.

I agree 👍

@saschagrunert The tests pass for me when I apply this patch:

diff --git a/nixos/modules/services/cluster/kubernetes/pki.nix b/nixos/modules/services/cluster/kubernetes/pki.nix
index 4275563f1a3..933ae481e96 100644
--- a/nixos/modules/services/cluster/kubernetes/pki.nix
+++ b/nixos/modules/services/cluster/kubernetes/pki.nix
@@ -20,7 +20,7 @@ let
         size = 2048;
     };
     CN = top.masterAddress;
-    hosts = cfg.cfsslAPIExtraSANs;
+    hosts = [top.masterAddress] ++ cfg.cfsslAPIExtraSANs;
   });
 
   cfsslAPITokenBaseName = "apitoken.secret";
@@ -228,7 +228,8 @@ in
             };
             private_key = cert.privateKeyOptions;
             request = {
-              inherit (cert) CN hosts;
+              hosts = [cert.CN] ++ cert.hosts;
+              inherit (cert) CN;
               key = {
                 algo = "rsa";
                 size = 2048;

Same for you?

... basically, it adds the CN to the list of sans for all issued certificates. Using CN still is ok, as long as any hostname mentioned in the CN-field is also present in the SAN-list.

If you want, you can add this patch or something similar to the PR. :) Will you do the honours of condensing some of this info into a release note?

Same for you?

Nice, thank you for the fix! 🙏 Tested it via nixos-shell that everything works as expected ✔️

If you want, you can add this patch or something similar to the PR. :) Will you do the honours of condensing some of this info into a release note?

Sure, I added the patch to this PR. :) I'm not sure any more if we need a release note for that change. Usually the user should not notice any change, right? Or is there any user action required now?

I'm not sure any more if we need a release note for that change.

I guess it's a matter of taste. I don't know what the official policy is in the community.

I do know that some use Kubernetes without easyCerts enabled ( my company included ) and we will be affected by the Golang 1.15 upgrade. Hmm, I don't know. It's more like a Go 1.15 release note I guess.

@srhb any opinions?

Other than that. This PR looks good to merge for me. :)

Sorry, I've been away on vacation.

If there's any doubt whether users of kubernetes should be aware of this upgrade, I see no reason not to include a release note. It doesn't have to be particularly in-depth, but if we know there's a pitfall here regarding go 1.15 and CN/SANS, let's at least mention it. Other than that, LGTM. Can merge and backport as soon as it has a release note :)

Added a release note and bumped Kubernetes to v1.19.1, too.

@GrahamcOfBorg build kubernetes

Hmm, that doesn't sound quite right to me. As I understand it, we will break certificates that have been relying on CN instead of SANs for the host name (because of the go 1.15 bump, not this PR specifically.) Whether or not that causes a lot of cluster breakage (it probably does?) depends on the specifics of the cluster setup, but I think that's what the heads-up should be.

@johanot Can you clarify this - did I misunderstand the issue? If we can't get more specific -- and I haven't thought about it yet -- maybe something generic like:

kubernetes has been upgraded to 1.19.x and the version of go used to build it has been bumped to 1.15. This may have consequences for your existing clusters and their certificates. Please consider the release notes for kubernetes 1.19 carefully before upgrading.

And a link to https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.19.md#changes-by-kind-1 I guess.

The phrasing from the release notes that worries me is this:

Kubernetes is now built with golang 1.15.0-rc.1.
The deprecated, legacy behavior of treating the CommonName field on X.509 serving certificates as a host name when no Subject Alternative Names are present is now disabled by default. It can be temporarily re-enabled by adding the value x509ignoreCN=0 to the GODEBUG environment variable. (#93264, @justaugustus) [SIG API Machinery, Auth, CLI, Cloud Provider, Cluster Lifecycle, Instrumentation, Network, Node, Release, Scalability, Storage and Testing]

@srhb Your comprehension of the issue is correct. The important thing is that users understand that this might cause cluster breakage, depending on how cluster certificates are issued.

Users that have easyCerts = true we got covered (1), but other custom setups are out of our hands, hence the warning.

re 1) Actually this is not entirely true, because easyCerts users still need to rotate/re-issue their certificates. I don't know if we need to write a specific guide for that?
That said; I agree that something like your generic wording followed by a link to the k8s release notes should be sufficient.

Sounds good, I updated the release note and linked it in the text.