diff --git a/pkg/apis/config/glob.go b/pkg/apis/config/glob.go index 640f2275b28..97ed8ef5dfa 100644 --- a/pkg/apis/config/glob.go +++ b/pkg/apis/config/glob.go @@ -15,17 +15,53 @@ package config -import "strings" - -// GlobMatch takes a string and handles only trailing '*' character as a -// wildcard. This is little different from various packages that I was able -// to find, since they handle '*' anywhere in the string as a wildcard. For our -// use we only want to handle it at the end, and hence this effectively turns -// into 'hasPrefix' string matching up to the trailing *. -func GlobMatch(image, glob string) bool { - if !strings.HasSuffix(glob, "*") { - // Doesn't end with *, so do an exact match - return image == glob +import ( + "net/url" + "path/filepath" + "strings" +) + +const ( + ResolvedDockerhubHost = "index.docker.io/" + // Images such as "busybox" reside in the dockerhub "library" repository + // The full resolved image reference would be index.docker.io/library/busybox + DockerhubPublicRepository = "library/" +) + +// GlobMatch will attempt to: +// 1. match the glob first +// 2. When the pattern is /*, therefore missing a host, +// it should match for the resolved image digest in the form of index.docker.io//* +// 3. When the pattern is , it should match for the resolved image digest +// against the official Dockerhub repository in the form of index.docker.io/library/* +func GlobMatch(glob, image string) (bool, error) { + matched, err := filepath.Match(glob, image) + if err != nil { + return false, err + } + + // If matched, return early + if matched { + return matched, nil + } + + // If not matched, check if missing host and default to index.docker.io + u, err := url.Parse(glob) + if err != nil { + return false, err } - return strings.HasPrefix(image, strings.TrimSuffix(glob, "*")) + + if u.Host == "" { + dockerhubGlobPattern := ResolvedDockerhubHost + + // If the image is expected to be part of the Dockerhub official "library" repository + if len(strings.Split(u.Path, "/")) < 2 { + dockerhubGlobPattern += DockerhubPublicRepository + } + + dockerhubGlobPattern += glob + return filepath.Match(dockerhubGlobPattern, image) + } + + return matched, nil } diff --git a/pkg/apis/config/glob_test.go b/pkg/apis/config/glob_test.go index 584b1c49ef0..e86d1b32361 100644 --- a/pkg/apis/config/glob_test.go +++ b/pkg/apis/config/glob_test.go @@ -20,9 +20,10 @@ import ( func TestGlobMatch(t *testing.T) { tests := []struct { - glob string - input string - match bool + glob string + input string + match bool + errString string }{ {glob: "foo", input: "foo", match: true}, // exact match {glob: "fooo*", input: "foo", match: false}, // prefix too long @@ -30,9 +31,36 @@ func TestGlobMatch(t *testing.T) { {glob: "foo*", input: "foo", match: true}, // works {glob: "*", input: "foo", match: true}, // matches anything {glob: "*", input: "bar", match: true}, // matches anything + {glob: "*foo*", input: "1foo2", match: true}, // matches wildcard around + {glob: "*repository/*", input: "repository/image", match: true}, + {glob: "*.repository/*", input: "other.repository/image", match: true}, + {glob: "repository/*", input: "repository/image", match: true}, + {glob: "repository/*", input: "other.repository/image", match: false}, + {glob: "repository/*", input: "index.docker.io/repository/image", match: true}, // Testing resolved digest + {glob: "image", input: "index.docker.io/library/image", match: true}, // Testing resolved digest and official dockerhub public repository + {glob: "[", input: "[", match: false, errString: "syntax error in pattern"}, // Invalid glob pattern + {glob: "gcr.io/projectsigstore/*", input: "gcr.io/projectsigstore/cosign", match: true}, + {glob: "gcr.io/projectsigstore/*", input: "us.gcr.io/projectsigstore/cosign", match: false}, + {glob: "*gcr.io/projectsigstore/*", input: "gcr.io/projectsigstore/cosign", match: true}, + {glob: "*gcr.io/projectsigstore/*", input: "gcr.io/projectsigstore2/cosign", match: false}, + {glob: "*gcr.io/*", input: "us.gcr.io/projectsigstore/cosign", match: false}, // Does not match since '*' only handles until next non-separator character '/' + {glob: "*gcr.io/*/*", input: "us.gcr.io/projectsigstore/cosign", match: true}, // Does match with multiple '*' + {glob: "us.gcr.io/*/*", input: "us.gcr.io/projectsigstore/cosign", match: true}, + {glob: "us.gcr.io/*/*", input: "gcr.io/projectsigstore/cosign", match: false}, + {glob: "*.gcr.io/*/*", input: "asia.gcr.io/projectsigstore/cosign", match: true}, + {glob: "*.gcr.io/*/*", input: "gcr.io/projectsigstore/cosign", match: false}, } for _, tc := range tests { - got := GlobMatch(tc.input, tc.glob) + got, err := GlobMatch(tc.glob, tc.input) + + if tc.errString != "" { + if tc.errString != err.Error() { + t.Errorf("expected %s for error: %s", tc.errString, err.Error()) + } + } else if err != nil { + t.Errorf("unexpected error: %v for glob: %q input: %q", err, tc.glob, tc.input) + } + if got != tc.match { t.Errorf("expected %v for glob: %q input: %q", tc.match, tc.glob, tc.input) } diff --git a/pkg/apis/config/image_policies.go b/pkg/apis/config/image_policies.go index 40a6fa5164c..955befecaa1 100644 --- a/pkg/apis/config/image_policies.go +++ b/pkg/apis/config/image_policies.go @@ -93,7 +93,9 @@ func (p *ImagePolicyConfig) GetMatchingPolicies(image string) (map[string]webhoo for k, v := range p.Policies { for _, pattern := range v.Images { if pattern.Glob != "" { - if GlobMatch(image, pattern.Glob) { + if matched, err := GlobMatch(pattern.Glob, image); err != nil { + lastError = err + } else if matched { ret[k] = v } } else if pattern.Regex != "" { diff --git a/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go b/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go index 0f5aff7bea0..c357b6e5351 100644 --- a/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go +++ b/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go @@ -17,8 +17,8 @@ package v1alpha1 import ( "context" "fmt" + "path/filepath" "regexp" - "strings" "github.com/sigstore/cosign/pkg/apis/utils" "knative.dev/pkg/apis" @@ -202,20 +202,13 @@ func (identity *Identity) Validate(ctx context.Context) *apis.FieldError { return errs } -// ValidateGlob makes sure that if there's "*" specified it's the trailing -// character. +// ValidateGlob glob compilation by testing against empty string func ValidateGlob(glob string) *apis.FieldError { - c := strings.Count(glob, "*") - switch c { - case 0: - return nil - case 1: - if !strings.HasSuffix(glob, "*") { - return apis.ErrInvalidValue(glob, apis.CurrentField, "glob match supports only * as a trailing character") - } - default: - return apis.ErrInvalidValue(glob, apis.CurrentField, "glob match supports only a single * as a trailing character") + _, err := filepath.Match(glob, "") + if err != nil { + return apis.ErrInvalidValue(glob, apis.CurrentField, fmt.Sprintf("glob is invalid: %v", err)) } + return nil } diff --git a/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go b/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go index 9610ca2d1ff..083f2695060 100644 --- a/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go +++ b/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go @@ -33,7 +33,7 @@ func TestImagePatternValidation(t *testing.T) { { name: "Should fail when both regex and glob are present", expectErr: true, - errorString: "expected exactly one, got both: spec.images[0].glob, spec.images[0].regex\ninvalid value: **: spec.images[0].glob\nglob match supports only a single * as a trailing character\nmissing field(s): spec.authorities", + errorString: "expected exactly one, got both: spec.images[0].glob, spec.images[0].regex\nmissing field(s): spec.authorities", policy: ClusterImagePolicy{ Spec: ClusterImagePolicySpec{ Images: []ImagePattern{ @@ -58,28 +58,14 @@ func TestImagePatternValidation(t *testing.T) { }, }, { - name: "Glob should fail with multiple *", + name: "Glob should fail with invalid glob", expectErr: true, - errorString: "invalid value: **: spec.images[0].glob\nglob match supports only a single * as a trailing character\nmissing field(s): spec.authorities", + errorString: "invalid value: [: spec.images[0].glob\nglob is invalid: syntax error in pattern\nmissing field(s): spec.authorities", policy: ClusterImagePolicy{ Spec: ClusterImagePolicySpec{ Images: []ImagePattern{ { - Glob: "**", - }, - }, - }, - }, - }, - { - name: "Glob should fail with non-trailing *", - expectErr: true, - errorString: "invalid value: foo*bar: spec.images[0].glob\nglob match supports only * as a trailing character\nmissing field(s): spec.authorities", - policy: ClusterImagePolicy{ - Spec: ClusterImagePolicySpec{ - Images: []ImagePattern{ - { - Glob: "foo*bar", + Glob: "[", }, }, },