CoreDNS default image bump to 1.6.6 to resolve CVE

[gjtempleton]

Also updates the default Corefile config to make use of the new lameduck functionality for healthcheck: https://coredns.io/plugins/health/ as per kubeADM: kubernetes/kubernetes#86260

Builds on top of the work done by @michalschott in #7883 however updates the image for all clusters.

Resolves #8332
Resolves #8309

[mikesplain]

This mostly looks good. You probably also want to bump the versions here:

kops/upup/pkg/fi/cloudup/bootstrapchannelbuilder.go

Line 288 in de4a86f

version := "1.3.1-kops.5"

and here:

kops/upup/pkg/fi/cloudup/bootstrapchannelbuilder.go

Line 307 in de4a86f

version := "1.3.1-kops.5"

Those aren't really required anymore but I think it's good for cleanliness of existing deployments and code.

[gjtempleton]

Good point, have updated as suggested.

[joshbranham]

Do we care about the version of CoreDNS as it pertains to K8s version? I know we had a PR a while back we chose not to merge (I think) because the version was ahead of what k/k was using?

[hakman]

Do we care about the version of CoreDNS as it pertains to K8s version? I know we had a PR a while back we chose not to merge (I think) because the version was ahead of what k/k was using?

Good point @joshbranham. The bump to 1.6.x should happen for 1.16+. As discussed in Slack, Kubeadm uses 1.3.x up to Kubernetes 1.15 and 1.6.x for Kubernetes 1.16+

We should also cherrypick this for 1.15+.

[mikesplain]

As discussed during office hours, lets check with @rajansandeep who has previously be in the know with CoreDNS.

Should we consider backporting 1.6.* to k8s 1.15 regardless of what k/k is doing @rajansandeep?

[michalschott]

My three cents, I'd keep CoreDNS to whatever version is suggested to certain version (aka keep 1.3.1 for <= 1.15 and bump to 1.6.X for > 1.16).

[thomaspeitz]

Thanks for your PR @gjtempleton

We are applying your change manually currently. During git diff @kforsthoevel and I found this small issue:

kops/upup/models/cloudup/resources/addons/coredns.addons.k8s.io/k8s-1.12.yaml.template

Line 217 in b4bfdcb

# Workaround for 1.3.1 bug, can be removed after bumping to 1.4+. See: https://github.com/coredns/coredns/pull/2529

- There is still the volume definition in there. You only removed the mounting. From my perspective we should remove the definition as well.

Beside that i really would enjoy a merge of it. Having a CVE in the cluster sounds awful.

Backstory:
We are upgrading to 1.6.6 mainly because we have dns issues since upgrading from 1.14 to 1.15 and want to verify if this solves it. It happens during node exchange.

[gjtempleton]

Hey @thomaspeitz !

Good catch, thanks. I've just pushed up a commit to remove that as well, hadn't noticed as we're running the CA ignoring local storage.

[justinsb]

AFAICT k/k is going to move to 1.6.6 (I haven't seen any objections), and we probably shouldn't have a CVE in the default versions that kops installs, which trumps our "don't change behaviour of old versions" rule. Ideally CoreDNS (and all addons) would backport CVE fixes so we could just bump the patch for older k8s versions, but nobody is doing that (yet!) to my knowledge.

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gjtempleton, justinsb

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 [justinsb]

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

[justinsb]

(I suspect e2e will actually fail because of the skew issue that may be fixed in kubernetes/test-infra#16029 )

[johngmyers]

/test pull-kops-e2e-kubernetes-aws

[johngmyers]

/test pull-kops-e2e-kubernetes-aws