From a0beb0449c385d7cce24c60ecbd070017b239635 Mon Sep 17 00:00:00 2001 From: Halvard Skogsrud Date: Wed, 12 Jan 2022 17:10:30 +1100 Subject: [PATCH] fix(ko): Debug port forwarding for `ko://` images This change ensures that images with the `ko://` prefix have the `dlv` debug port forwarded by default when running `skaffold debug`. The image build process sanitizes the image name by removing the `ko://` prefix and lowercasing the Go import path. This means that the built image name doesn't match the placeholder values in the Kubernetes manifest files. This change sanitizes the image placeholder values from the Kubernetes resource files when matching with the built image names, which in turn means that Skaffold sets up port forwarding. Fixes: #6995 --- pkg/skaffold/deploy/util/util.go | 3 +- pkg/skaffold/deploy/util/util_test.go | 95 +++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/pkg/skaffold/deploy/util/util.go b/pkg/skaffold/deploy/util/util.go index b42ee3263e6..c6b521f437f 100644 --- a/pkg/skaffold/deploy/util/util.go +++ b/pkg/skaffold/deploy/util/util.go @@ -58,7 +58,8 @@ func AddTagsToPodSelector(artifacts []graph.Artifact, deployerArtifacts []graph. m[a.ImageName] = true } for _, artifact := range artifacts { - if _, ok := m[artifact.ImageName]; ok { + imageName := docker.SanitizeImageName(artifact.ImageName) + if _, ok := m[imageName]; ok { podSelector.Add(artifact.Tag) } } diff --git a/pkg/skaffold/deploy/util/util_test.go b/pkg/skaffold/deploy/util/util_test.go index 2daaa6fba07..b9aca0b7ee3 100644 --- a/pkg/skaffold/deploy/util/util_test.go +++ b/pkg/skaffold/deploy/util/util_test.go @@ -19,6 +19,10 @@ package util import ( "testing" + v1 "k8s.io/api/core/v1" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" "github.com/GoogleContainerTools/skaffold/testutil" ) @@ -85,3 +89,94 @@ func TestConsolidateNamespaces(t *testing.T) { }) } } + +func TestAddTagsToPodSelector(t *testing.T) { + tests := []struct { + description string + artifacts []graph.Artifact + deployerArtifacts []graph.Artifact + expectedImages []string + }{ + { + description: "empty image list", + }, + { + description: "non-matching image results in empty list", + artifacts: []graph.Artifact{ + { + ImageName: "my-image", + Tag: "my-image-tag", + }, + }, + deployerArtifacts: []graph.Artifact{ + { + ImageName: "not-my-image", + }, + }, + }, + { + description: "matching images appear in list", + artifacts: []graph.Artifact{ + { + ImageName: "my-image1", + Tag: "registry.example.com/repo/my-image1:tag1", + }, + { + ImageName: "my-image2", + Tag: "registry.example.com/repo/my-image2:tag2", + }, + { + ImageName: "image-not-in-deployer", + Tag: "registry.example.com/repo/my-image3:tag3", + }, + }, + deployerArtifacts: []graph.Artifact{ + { + ImageName: "my-image1", + }, + { + ImageName: "my-image2", + }, + }, + expectedImages: []string{ + "registry.example.com/repo/my-image1:tag1", + "registry.example.com/repo/my-image2:tag2", + }, + }, + { + description: "images from manifest files with ko:// scheme prefix are sanitized before matching", + artifacts: []graph.Artifact{ + { + ImageName: "ko://git.example.com/Foo/bar", + Tag: "registry.example.com/repo/git.example.com/foo/bar:tag", + }, + }, + deployerArtifacts: []graph.Artifact{ + { + ImageName: "git.example.com/foo/bar", + Tag: "ko://git.example.com/Foo/bar", + }, + }, + expectedImages: []string{ + "registry.example.com/repo/git.example.com/foo/bar:tag", + }, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + podSelector := kubernetes.NewImageList() + AddTagsToPodSelector(test.artifacts, test.deployerArtifacts, podSelector) + for _, expectedImage := range test.expectedImages { + if exists := podSelector.Select(&v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + {Image: expectedImage}, + }, + }, + }); !exists { + t.Errorf("expected image list to contain %s", expectedImage) + } + } + }) + } +}