Fix image name parsing (kyma-project#342)

* Fix image name parsing

* simplify validation

* test

* Adjust test to provide a valid image name

.github/actions/create-k3d-cluster/action.yaml

@@ -1,16 +1,17 @@
-name: 'Create k3d cluster'
-description: 'Action for creating single cluster'
+name: "Create k3d cluster"
+description: "Action for creating single cluster"
 
 runs:
-  using: 'composite'
+  using: "composite"
   steps:
     - name: create k3d cluster
       uses: AbsaOSS/k3d-action@4e8b3239042be1dc0aed6c5eb80c13b18200fc79 #v2.4.0
       with:
         cluster-name: "k3dCluster"
+        k3d-version: "v5.7.4"
         args: >-
           --agents 1
-          --image rancher/k3s:v1.30.0-k3s1
+          --image rancher/k3s:v1.31.2-k3s1
           --port 80:80@loadbalancer
           --port 443:443@loadbalancer
           --wait

Makefile

@@ -2,7 +2,7 @@
 # Image URL to use all building/pushing image targets
 IMG ?= controller:latest
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.27.1
+ENVTEST_K8S_VERSION = 1.30.0
 
 # TEST_COVER_OUT determines path for the output file with coverage 
 TEST_COVER_OUT ?= $(shell pwd)/cover.out
@@ -218,8 +218,8 @@ CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
 ENVTEST ?= $(LOCALBIN)/setup-envtest
 
 ## Tool Versions
-KUSTOMIZE_VERSION ?= v4.5.5
-CONTROLLER_TOOLS_VERSION ?= v0.14.0
+KUSTOMIZE_VERSION ?= v5.5.0
+CONTROLLER_TOOLS_VERSION ?= v0.16.5
 HELM_VERSION ?= v3.13.1
 
 KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"

internal/admission/defaulting_test.go

@@ -48,7 +48,7 @@ func TestTimeout(t *testing.T) {
 	ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testNs,
 		Labels: map[string]string{pkg.NamespaceValidationLabel: pkg.NamespaceValidationEnabled}}}
 	pod := corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "pod", Namespace: testNs},
-		Spec: corev1.PodSpec{Containers: []corev1.Container{{Image: "test:test"}}},
+		Spec: corev1.PodSpec{Containers: []corev1.Container{{Image: "eu-gcr.io/test:test"}}},
 	}
 
 	raw, err := json.Marshal(pod)

internal/validate/image.go

@@ -32,10 +32,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-const (
-	tagDelim = ":"
-)
-
 //go:generate mockery --name=ImageValidatorService
 type ImageValidatorService interface {
 	Validate(ctx context.Context, image string, imagePullCredentials map[string]cliType.AuthConfig) error
@@ -70,21 +66,18 @@ func (s *notaryService) Validate(ctx context.Context, image string, imagePullCre
 		return nil
 	}
 
-	split := strings.Split(image, tagDelim)
-
-	if len(split) != 2 {
-		return pkg.NewValidationFailedErr(errors.New("image name is not formatted correctly"))
+	// strict validation requires image name to contain domain and a tag, and/or sha256
+	ref, err := name.ParseReference(image, name.StrictValidation)
+	if err != nil {
+		return pkg.NewValidationFailedErr(errors.Wrap(err, "image name could not be parsed"))
 	}
 
-	imgRepo := split[0]
-	imgTag := split[1]
-
-	expectedShaBytes, err := s.loggedGetNotaryImageDigestHash(ctx, imgRepo, imgTag)
+	expectedShaBytes, err := s.loggedGetNotaryImageDigestHash(ctx, ref)
 	if err != nil {
 		return err
 	}
 
-	shaImageBytes, shaManifestBytes, err := s.loggedGetRepositoryDigestHash(ctx, image, imagePullCredentials)
+	shaImageBytes, shaManifestBytes, err := s.loggedGetRepositoryDigestHash(ctx, ref, imagePullCredentials)
 	if err != nil {
 		return err
 	}
@@ -111,23 +104,14 @@ func (s *notaryService) isImageAllowed(imgRepo string) bool {
 	return false
 }
 
-func (s *notaryService) loggedGetRepositoryDigestHash(ctx context.Context, image string, imagePullCredentials map[string]cliType.AuthConfig) ([]byte, []byte, error) {
+func (s *notaryService) loggedGetRepositoryDigestHash(ctx context.Context, ref name.Reference, imagePullCredentials map[string]cliType.AuthConfig) ([]byte, []byte, error) {
 	const message = "request to image registry"
 	closeLog := helpers.LogStartTime(ctx, message)
 	defer closeLog()
-	return s.getRepositoryDigestHash(image, imagePullCredentials)
+	return s.getRepositoryDigestHash(ref, imagePullCredentials)
 }
 
-func (s *notaryService) getRepositoryDigestHash(image string, imagePullCredentials map[string]cliType.AuthConfig) ([]byte, []byte, error) {
-	if len(image) == 0 {
-		return nil, nil, pkg.NewValidationFailedErr(errors.New("empty image provided"))
-	}
-
-	ref, err := name.ParseReference(image)
-	if err != nil {
-		return nil, nil, pkg.NewValidationFailedErr(errors.Wrap(err, "ref parse"))
-	}
-
+func (s *notaryService) getRepositoryDigestHash(ref name.Reference, imagePullCredentials map[string]cliType.AuthConfig) ([]byte, []byte, error) {
 	remoteOptions := make([]remote.Option, 0)
 
 	// check if we have credentials for the registry, and use them
@@ -233,30 +217,26 @@ func getImageDigestHash(ref name.Reference, remoteOptions ...remote.Option) ([]b
 	return digestBytes, manifestBytes, nil
 }
 
-func (s *notaryService) loggedGetNotaryImageDigestHash(ctx context.Context, imgRepo, imgTag string) ([]byte, error) {
+func (s *notaryService) loggedGetNotaryImageDigestHash(ctx context.Context, ref name.Reference) ([]byte, error) {
 	const message = "request to notary"
 	closeLog := helpers.LogStartTime(ctx, message)
 	defer closeLog()
-	result, err := s.getNotaryImageDigestHash(ctx, imgRepo, imgTag)
+	result, err := s.getNotaryImageDigestHash(ctx, ref)
 	return result, err
 }
 
-func (s *notaryService) getNotaryImageDigestHash(ctx context.Context, imgRepo, imgTag string) ([]byte, error) {
-	if len(imgRepo) == 0 || len(imgTag) == 0 {
-		return nil, pkg.NewValidationFailedErr(errors.New("empty arguments provided"))
-	}
-
+func (s *notaryService) getNotaryImageDigestHash(ctx context.Context, ref name.Reference) ([]byte, error) {
 	const messageNewRepoClient = "request to notary (NewRepoClient)"
 	closeLog := helpers.LogStartTime(ctx, messageNewRepoClient)
-	c, err := s.RepoFactory.NewRepoClient(imgRepo, s.NotaryConfig)
+	c, err := s.RepoFactory.NewRepoClient(ref.Context().Name(), s.NotaryConfig)
 	closeLog()
 	if err != nil {
 		return nil, pkg.NewUnknownResultErr(err)
 	}
 
 	const messageGetTargetByName = "request to notary (GetTargetByName)"
 	closeLog = helpers.LogStartTime(ctx, messageGetTargetByName)
-	target, err := c.GetTargetByName(imgTag)
+	target, err := c.GetTargetByName(ref.Identifier())
 	closeLog()
 	if err != nil {
 		return nil, parseNotaryErr(err)

internal/validate/image_test.go

@@ -54,12 +54,12 @@ var (
 		hash: []byte{33, 98, 102, 200, 111, 196, 220, 239, 86, 25, 147, 11, 211, 148, 36, 88, 36, 194, 175, 82, 253, 33, 186, 124, 111, 160, 230, 24, 101, 125, 76, 59},
 	}
 	differentHashImage = image{
-		name: "nginx",
+		name: "docker.io/library/nginx",
 		tag:  "latest",
 		hash: []byte{1, 2, 3, 4},
 	}
 	untrustedImage = image{
-		name: "nginx",
+		name: "docker.io/library/nginx",
 		tag:  "untrusted",
 	}
 	unknownImage = image{
@@ -98,26 +98,31 @@ func Test_Validate_ProperIndex_ShouldPass(t *testing.T) {
 func Test_Validate_InvalidImageName_ShouldReturnError(t *testing.T) {
 	cfg := validate.ServiceConfig{NotaryConfig: validate.NotaryConfig{}}
 	f := setupMockFactory()
+	expectedErrMsg := "image name could not be parsed"
 
 	tests := []struct {
-		name           string
-		imageName      string
-		expectedErrMsg string
+		name      string
+		imageName string
 	}{
 		{
-			name:           "image name without semicolon",
-			imageName:      "makapaka",
-			expectedErrMsg: "image name is not formatted correctly",
+			name:      "image name without tag and domain",
+			imageName: "makapaka",
 		},
 		{
-			name:           "",
-			imageName:      ":",
-			expectedErrMsg: "empty arguments provided",
+			name:      "image name without domain",
+			imageName: "makapaka:latest",
 		},
 		{
-			name:           "image name with more than one semicolon", //TODO: IMO it's proper image name, but now is not allowed
-			imageName:      "repo:port/image-name:tag",
-			expectedErrMsg: "image name is not formatted correctly",
+			name:      "image name without tag",
+			imageName: "domain.com/makapaka",
+		},
+		{
+			name:      "",
+			imageName: ":",
+		},
+		{
+			name:      "image name with more than two semicolon in hash",
+			imageName: "repo.com:123/image-name:tag:hash",
 		},
 	}
 	for _, tt := range tests {
@@ -126,7 +131,7 @@ func Test_Validate_InvalidImageName_ShouldReturnError(t *testing.T) {
 
 			err := s.Validate(context.TODO(), tt.imageName, emptyAuthData)
 
-			require.ErrorContains(t, err, tt.expectedErrMsg)
+			require.ErrorContains(t, err, expectedErrMsg)
 		})
 	}
 }