From e009c96c73db6cc76b3e21f08c9783b5f83845cc Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 6 Feb 2021 17:14:56 +0100 Subject: [PATCH] Error on images from docker.pkg.github.com Since Kubernetes 1.20 containerd is used instead of Docker as the container runtime. Containerd is due to protocol version differences/support/mismatch unabled to pull images hosted at docker.pkg.github.com. The new check in this commit will error when it finds an image from that registry, and suggests to use ghcr.io, which is also hosted and operated by GitHub, and the successor of docker.pkg.github.com. Refs: * https://github.com/containerd/containerd/issues/3291#issuecomment-683700425 * https://docs.github.com/en/packages/guides/migrating-to-github-container-registry-for-docker-images#domain-changes --- checks.md | 17 +++ checks/all/all.go | 2 + .../docker_pkg_github_com_registry.go | 93 ++++++++++++++++ .../docker_pkg_github_com_registry_test.go | 103 ++++++++++++++++++ checks/containerd/helper_test.go | 103 ++++++++++++++++++ 5 files changed, 318 insertions(+) create mode 100644 checks/containerd/docker_pkg_github_com_registry.go create mode 100644 checks/containerd/docker_pkg_github_com_registry_test.go create mode 100644 checks/containerd/helper_test.go diff --git a/checks.md b/checks.md index ee4be0f..648373e 100644 --- a/checks.md +++ b/checks.md @@ -776,3 +776,20 @@ kubectl taint node - ``` Note the trailing `-` on the key; this causes `kubectl` to delete the label or taint. + +## Images hosted on docker.pkg.github.com + +- Name: `docker-pkg-github-com-registry` +- Groups: `containerd`, `doks` + +`containerd` cannot pull container images from `docker.pkg.github.com` due to a [protocol mismatch](https://github.com/containerd/containerd/issues/3291#issuecomment-683700425). As `docker.pkg.github.com` is GitHub's old package registry, [they recommend migrating to `ghcr.io`](https://docs.github.com/en/packages/guides/migrating-to-github-container-registry-for-docker-images#domain-changes). + +### Example + +```yaml +# Not supported: Using "docker.pkg.github.com" as container registry +spec: + containers: + - name: redis + image: docker.pkg.github.com/redis/redis/redis:6 +``` diff --git a/checks/all/all.go b/checks/all/all.go index 2316b42..ee116a8 100644 --- a/checks/all/all.go +++ b/checks/all/all.go @@ -27,4 +27,6 @@ import ( _ "github.com/digitalocean/clusterlint/checks/noop" // Side-effect import to get all the checks in security package registered. _ "github.com/digitalocean/clusterlint/checks/security" + // Side-effect import to get all the checks in containerd package registered. + _ "github.com/digitalocean/clusterlint/checks/containerd" ) diff --git a/checks/containerd/docker_pkg_github_com_registry.go b/checks/containerd/docker_pkg_github_com_registry.go new file mode 100644 index 0000000..6d48d27 --- /dev/null +++ b/checks/containerd/docker_pkg_github_com_registry.go @@ -0,0 +1,93 @@ +/* +Copyright 2021 DigitalOcean + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerd + +import ( + "fmt" + + "github.com/digitalocean/clusterlint/checks" + "github.com/digitalocean/clusterlint/kube" + "github.com/docker/distribution/reference" + corev1 "k8s.io/api/core/v1" +) + +func init() { + checks.Register(&domainNameCheck{}) +} + +type domainNameCheck struct{} + +// Name returns a unique name for this check. +func (l *domainNameCheck) Name() string { + return "docker-pkg-github-com-registry" +} + +// Groups returns a list of group names this check should be part of. +func (l *domainNameCheck) Groups() []string { + return []string{"containerd", "doks"} +} + +// Description returns a detailed human-readable description of what this check +// does. +func (l *domainNameCheck) Description() string { + return "Checks if there are pods with container images that are hosted at the docker.pkg.github.com registry" +} + +// Run runs this check on a set of Kubernetes objects. It can return errors +// (low-priority problems) and errors (high-priority problems) as well as an +// error value indicating that the check failed to run. +func (l *domainNameCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) { + var diagnostics []checks.Diagnostic + for _, pod := range objects.Pods.Items { + diagnostics = append(diagnostics, l.checkTags(pod.Spec.Containers, pod)...) + diagnostics = append(diagnostics, l.checkTags(pod.Spec.InitContainers, pod)...) + } + + return diagnostics, nil +} + +// checkTags checks if the image registry is `docker.pkg.github.com` +// Adds an error if it finds any image that comes from that registry +func (l *domainNameCheck) checkTags(containers []corev1.Container, pod corev1.Pod) []checks.Diagnostic { + var diagnostics []checks.Diagnostic + for _, container := range containers { + namedRef, err := reference.ParseNormalizedNamed(container.Image) + if err != nil { + d := checks.Diagnostic{ + Severity: checks.Warning, + Message: fmt.Sprintf("Image name for container '%s' could not be parsed", container.Name), + Kind: checks.Pod, + Object: &pod.ObjectMeta, + Owners: pod.ObjectMeta.GetOwnerReferences(), + } + diagnostics = append(diagnostics, d) + continue + } + domainNameOnly := reference.Domain(namedRef) + if domainNameOnly == "docker.pkg.github.com" { + d := checks.Diagnostic{ + Severity: checks.Error, + Message: fmt.Sprintf("containerd can't pull images from docker.pkg.github.com, used by container '%s'", container.Name), + Kind: checks.Pod, + Object: &pod.ObjectMeta, + Owners: pod.ObjectMeta.GetOwnerReferences(), + } + diagnostics = append(diagnostics, d) + } + } + return diagnostics +} diff --git a/checks/containerd/docker_pkg_github_com_registry_test.go b/checks/containerd/docker_pkg_github_com_registry_test.go new file mode 100644 index 0000000..bddd0ce --- /dev/null +++ b/checks/containerd/docker_pkg_github_com_registry_test.go @@ -0,0 +1,103 @@ +/* +Copyright 2021 DigitalOcean + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerd + +import ( + "testing" + + "github.com/digitalocean/clusterlint/checks" + "github.com/digitalocean/clusterlint/kube" + "github.com/stretchr/testify/assert" +) + +func TestDomainNameCheckMeta(t *testing.T) { + domainNameCheck := domainNameCheck{} + assert.Equal(t, "docker-pkg-github-com-registry", domainNameCheck.Name()) + assert.Equal(t, []string{"containerd", "doks"}, domainNameCheck.Groups()) + assert.NotEmpty(t, domainNameCheck.Description()) +} + +func TestDomainNameCheckRegistration(t *testing.T) { + domainNameCheck := &domainNameCheck{} + check, err := checks.Get("docker-pkg-github-com-registry") + assert.NoError(t, err) + assert.Equal(t, check, domainNameCheck) +} + +func TestDockerPkgGithubComRegistry(t *testing.T) { + const message = "containerd can't pull images from docker.pkg.github.com, used by container 'bar'" + const invalidMessage = "Image name for container 'bar' could not be parsed" + const severity = checks.Error + const name = "docker-pkg-github-com-registry" + + tests := []struct { + name string + objs *kube.Objects + expected []checks.Diagnostic + }{ + { + name: "no pods", + objs: initPod(), + expected: nil, + }, + { + name: "pod with container image - docker.pkg.github.com/busybox:latest", + objs: container("docker.pkg.github.com/busybox:latest"), + expected: issues(severity, message, checks.Pod, name), + }, + { + name: "pod with container image - docker.pkg.github.com/busybox", + objs: container("docker.pkg.github.com/busybox"), + expected: issues(severity, message, checks.Pod, name), + }, + { + name: "pod with container image - test:5000/repo", + objs: container("test:5000/repo/image"), + expected: nil, + }, + { + name: "pod with container image - ghcr.io/repo:ignore-tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + objs: container("ghcr.io/repo:ignore-tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), + expected: nil, + }, + { + name: "pod with container image - ghcr.io/busybox:v1.2.3", + objs: container("ghcr.io/busybox:v1.2.3"), + expected: nil, + }, + { + name: "pod with init container with invalid image name", + objs: initContainer(""), + expected: issues(checks.Warning, invalidMessage, checks.Pod, name), + }, + { + name: "pod with container with invalid image name", + objs: container(""), + expected: issues(checks.Warning, invalidMessage, checks.Pod, name), + }, + } + + domainNameCheck := domainNameCheck{} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + d, err := domainNameCheck.Run(test.objs) + assert.NoError(t, err) + assert.ElementsMatch(t, test.expected, d) + }) + } +} diff --git a/checks/containerd/helper_test.go b/checks/containerd/helper_test.go new file mode 100644 index 0000000..9ca2402 --- /dev/null +++ b/checks/containerd/helper_test.go @@ -0,0 +1,103 @@ +/* +Copyright 2019 DigitalOcean + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerd + +import ( + "github.com/digitalocean/clusterlint/checks" + "github.com/digitalocean/clusterlint/kube" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func initPod() *kube.Objects { + objs := &kube.Objects{ + Pods: &corev1.PodList{ + Items: []corev1.Pod{ + { + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "pod_foo", Namespace: "k8s"}, + }, + }, + }, + } + return objs +} + +func initMultiplePods() *kube.Objects { + objs := &kube.Objects{ + Pods: &corev1.PodList{ + Items: []corev1.Pod{ + { + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "pod_1", Namespace: "k8s"}, + }, + { + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "pod_2", Namespace: "k8s"}, + }, + }, + }, + } + return objs +} + +func GetObjectMeta() *metav1.ObjectMeta { + objs := initPod() + return &objs.Pods.Items[0].ObjectMeta +} + +func GetOwners() []metav1.OwnerReference { + objs := initPod() + return objs.Pods.Items[0].ObjectMeta.GetOwnerReferences() +} + +func container(image string) *kube.Objects { + objs := initPod() + objs.Pods.Items[0].Spec = corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "bar", + Image: image, + }}, + } + return objs +} + +func initContainer(image string) *kube.Objects { + objs := initPod() + objs.Pods.Items[0].Spec = corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "bar", + Image: image, + }}, + } + return objs +} + +func issues(severity checks.Severity, message string, kind checks.Kind, check string) []checks.Diagnostic { + d := []checks.Diagnostic{ + { + Severity: severity, + Message: message, + Kind: kind, + Object: GetObjectMeta(), + Owners: GetOwners(), + }, + } + return d +}