From a8d90e24e7f13fb3da0450abedd50121b17b4cba Mon Sep 17 00:00:00 2001 From: Suleiman Dibirov Date: Thu, 13 Mar 2025 18:26:01 +0200 Subject: [PATCH 1/2] fix(verify): use container name from configuration in verify tests Signed-off-by: Suleiman Dibirov --- pkg/skaffold/verify/docker/verify.go | 16 ++++++--- pkg/skaffold/verify/docker/verify_test.go | 41 +++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/pkg/skaffold/verify/docker/verify.go b/pkg/skaffold/verify/docker/verify.go index 38258f269af..fa325a7bfcd 100644 --- a/pkg/skaffold/verify/docker/verify.go +++ b/pkg/skaffold/verify/docker/verify.go @@ -221,7 +221,10 @@ func (v *Verifier) createAndRunContainer(ctx context.Context, out io.Writer, art if len(tc.Container.Args) != 0 { containerCfg.Cmd = tc.Container.Args } - containerName := v.getContainerName(ctx, artifact.ImageName) + + // Use container name from test case if available, otherwise derive from image + containerName := v.getContainerName(ctx, artifact.ImageName, tc.Container.Name) + opts := dockerutil.ContainerCreateOpts{ Name: containerName, Network: v.network, @@ -301,9 +304,14 @@ func (v *Verifier) containerConfigFromImage(ctx context.Context, taggedImage str return config.Config, err } -func (v *Verifier) getContainerName(ctx context.Context, name string) string { - // this is done to fix the for naming convention of non-skaffold built images which verify supports - name = path.Base(strings.Split(name, ":")[0]) +func (v *Verifier) getContainerName(ctx context.Context, imageName string, containerName string) string { + name := containerName + + // If no container name is provided, derive it from the image name + if name == "" { + name = path.Base(strings.Split(imageName, ":")[0]) + } + currentName := name for { diff --git a/pkg/skaffold/verify/docker/verify_test.go b/pkg/skaffold/verify/docker/verify_test.go index bff65de8c01..e93cc44ac42 100644 --- a/pkg/skaffold/verify/docker/verify_test.go +++ b/pkg/skaffold/verify/docker/verify_test.go @@ -149,3 +149,44 @@ func Test_UseLocalImages(t *testing.T) { t.CheckDeepEqual(expectedPullImgs, fDockerDaemon.PulledImages) }) } + +func TestGetContainerName(t *testing.T) { + ctx := context.TODO() + + tests := []struct { + description string + imageName string + containerName string + expected string + }{ + { + description: "container name specified", + imageName: "gcr.io/cloud-builders/gcloud", + containerName: "custom-container", + expected: "custom-container", + }, + { + description: "container name not specified", + imageName: "gcr.io/cloud-builders/gcloud", + containerName: "", + expected: "gcloud", + }, + } + + fakeDockerDaemon := &fakeDockerDaemon{ + LocalDaemon: docker.NewLocalDaemon(&testutil.FakeAPIClient{}, nil, false, nil), + } + + verifier := &Verifier{ + client: fakeDockerDaemon, + } + + for _, test := range tests { + testutil.Run( + t, test.description, func(t *testutil.T) { + actual := verifier.getContainerName(ctx, test.imageName, test.containerName) + t.CheckDeepEqual(test.expected, actual) + }, + ) + } +} From 8df99597e422480febda00b3a2dd6fba03eac1b0 Mon Sep 17 00:00:00 2001 From: Suleiman Dibirov Date: Mon, 17 Mar 2025 20:47:47 +0200 Subject: [PATCH 2/2] add container name validator for verify Signed-off-by: Suleiman Dibirov --- pkg/skaffold/verify/docker/verify.go | 5 ++++- pkg/skaffold/verify/docker/verify_test.go | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/skaffold/verify/docker/verify.go b/pkg/skaffold/verify/docker/verify.go index fa325a7bfcd..cf7c85711b2 100644 --- a/pkg/skaffold/verify/docker/verify.go +++ b/pkg/skaffold/verify/docker/verify.go @@ -22,6 +22,7 @@ import ( "io" "math" "path" + "regexp" "strings" "sync" "time" @@ -49,6 +50,8 @@ import ( "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util" ) +var validContainerNameRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_.-]*$`) + // Verifier verifies deployments using Docker libs/CLI. type Verifier struct { logger log.Logger @@ -308,7 +311,7 @@ func (v *Verifier) getContainerName(ctx context.Context, imageName string, conta name := containerName // If no container name is provided, derive it from the image name - if name == "" { + if name == "" || !validContainerNameRegex.MatchString(name) { name = path.Base(strings.Split(imageName, ":")[0]) } diff --git a/pkg/skaffold/verify/docker/verify_test.go b/pkg/skaffold/verify/docker/verify_test.go index e93cc44ac42..0021832702c 100644 --- a/pkg/skaffold/verify/docker/verify_test.go +++ b/pkg/skaffold/verify/docker/verify_test.go @@ -165,6 +165,12 @@ func TestGetContainerName(t *testing.T) { containerName: "custom-container", expected: "custom-container", }, + { + description: "invalid container name specified", + imageName: "gcr.io/cloud-builders/gcloud", + containerName: "gcr.io/cloud-builders/gcloud", + expected: "gcloud", + }, { description: "container name not specified", imageName: "gcr.io/cloud-builders/gcloud",