Skip to content

Commit

Permalink
Fix image name parsing (#342) (#343)
Browse files Browse the repository at this point in the history 
* Fix image name parsing

* simplify validation

* test

* Adjust test to provide a valid image name
  • Loading branch information
@halamix2
halamix2 authored Nov 20, 2024
1 parent f53c08a commit 453ae3c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 50 deletions.
2 changes: 1 addition & 1 deletion internal/admission/defaulting_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 14 additions & 34 deletions internal/validate/image.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
35 changes: 20 additions & 15 deletions internal/validate/image_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
})
}
}
Expand Down

0 comments on commit 453ae3c

Please sign in to comment.