Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix image name parsing (#342) #343

Merged
merged 1 commit into from
Nov 20, 2024
Merged

Fix image name parsing (#342) #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/admission/defaulting_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading