diff --git a/pkg/apis/config/glob.go b/pkg/apis/config/glob.go index 97ed8ef5dfa..ab61b90a4c9 100644 --- a/pkg/apis/config/glob.go +++ b/pkg/apis/config/glob.go @@ -16,9 +16,11 @@ package config import ( - "net/url" - "path/filepath" + "fmt" + "regexp" "strings" + + "github.com/google/go-containerregistry/pkg/name" ) const ( @@ -28,40 +30,62 @@ const ( 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/* +var validGlob = regexp.MustCompile(`^[a-zA-Z0-9-_:\/\*\.]+$`) + +// GlobMatch will return true if the image reference matches the requested glob pattern. +// +// If the image reference is invalid, an error will be returned. +// +// In the glob pattern, the "*" character matches any non-"/" character, and "**" matches any character, including "/". +// +// If the image is a DockerHub official image like "ubuntu" or "debian", the glob that matches it must be something like index.docker.io/library/ubuntu. +// If the image is a DockerHub used-owned image like "myuser/myapp", then the glob that matches it must be something like index.docker.io/myuser/myapp. +// This means that the glob patterns "*" will not match the image name "ubuntu", and "*/*" will not match "myuser/myapp"; the "index.docker.io" prefix is required. +// +// If the image does not specify a tag (e.g., :latest or :v1.2.3), the tag ":latest" will be assumed. +// +// Note that the tag delimiter (":") does not act as a breaking separator for the purposes of a "*" glob. +// To match any tag, the glob should end with ":**". func GlobMatch(glob, image string) (bool, error) { - matched, err := filepath.Match(glob, image) - if err != nil { - return false, err + if glob == "*/*" { + // TODO: Warn that the glob match "*/*" should be "index.docker.io/*/*". + glob = "index.docker.io/*/*" } - - // If matched, return early - if matched { - return matched, nil + if glob == "*" { + // TODO: Warn that the glob match "*" should be "index.docker.io/library/*". + glob = "index.docker.io/library/*" } - // If not matched, check if missing host and default to index.docker.io - u, err := url.Parse(glob) + ref, err := name.ParseReference(image, name.WeakValidation) if err != nil { return false, err } - if u.Host == "" { - dockerhubGlobPattern := ResolvedDockerhubHost + // Reject that glob doesn't look like a regexp + if !validGlob.MatchString(glob) { + return false, fmt.Errorf("invalid glob %q", glob) + } - // If the image is expected to be part of the Dockerhub official "library" repository - if len(strings.Split(u.Path, "/")) < 2 { - dockerhubGlobPattern += DockerhubPublicRepository - } + // Translate glob to regexp. + glob = strings.ReplaceAll(glob, ".", `\.`) // . in glob means \. in regexp + glob = strings.ReplaceAll(glob, "**", "#") // ** in glob means 0+ of any character in regexp + // We replace ** with a placeholder here, rather than `.*` directly, because the next line + // would replace that `*` again, breaking the regexp. So we stash the change with a placeholder, + // then replace the placeholder later to preserve the original intent. + glob = strings.ReplaceAll(glob, "*", "[^/]*") // * in glob means 0+ of any non-`/` character in regexp + glob = strings.ReplaceAll(glob, "#", ".*") + glob = fmt.Sprintf("^%s$", glob) // glob must match the whole string - dockerhubGlobPattern += glob - return filepath.Match(dockerhubGlobPattern, image) - } + // TODO: do we want ":" to count as a separator like "/" is? - return matched, nil + match, err := regexp.MatchString(glob, ref.Name()) + if err != nil { + return false, err + } + if !match && ref.Name() != image { + // If the image was not fully qualified, try matching the glob against the original non-fully-qualified. + // This should be a warning and this behavior should eventually be removed. + match, err = regexp.MatchString(glob, image) + } + return match, err } diff --git a/pkg/apis/config/glob_test.go b/pkg/apis/config/glob_test.go index 8857f59912b..fa1ea3d25a5 100644 --- a/pkg/apis/config/glob_test.go +++ b/pkg/apis/config/glob_test.go @@ -15,67 +15,72 @@ package config import ( - "runtime" "testing" ) func TestGlobMatch(t *testing.T) { - tests := []struct { - glob string - input string - match bool - errString string - windowsMatch *struct { - match bool - } + for _, c := range []struct { + image, glob string + wantMatch bool + wantErr bool }{ - {glob: "foo", input: "foo", match: true}, // exact match - {glob: "fooo*", input: "foo", match: false}, // prefix too long - {glob: "foo*", input: "foobar", match: true}, // works - {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: 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}, - // Does not match since '*' only handles until next non-separator character '/' - // On Windows, '/' is not the separator and therefore it passes - {glob: "*gcr.io/*", input: "us.gcr.io/projectsigstore/cosign", match: false, windowsMatch: &struct{ match bool }{match: true}}, - } - for _, tc := range tests { - got, err := GlobMatch(tc.glob, tc.input) + {image: "foo", glob: "index.docker.io/library/foo:latest", wantMatch: true}, + {image: "foo", glob: "index.docker.io/library/foo:*", wantMatch: true}, + {image: "foo", glob: "index.docker.io/library/*", wantMatch: true}, + {image: "foo", glob: "index.docker.io/library/*:latest", wantMatch: true}, + {image: "foo", glob: "index.docker.io/*/*", wantMatch: true}, + {image: "foo", glob: "index.docker.io/**", wantMatch: true}, + {image: "foo", glob: "index.docker.**", wantMatch: true}, + {image: "foo", glob: "inde**", wantMatch: true}, + {image: "foo", glob: "**", wantMatch: true}, + {image: "foo", glob: "foo", wantMatch: true}, // matches because of deprecated fallback logic. + {image: "foo", glob: "foo*", wantMatch: true}, // * matches 0+ characters + {image: "foo", glob: "foo**", wantMatch: true}, // ** matches 0+ characters - 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) - } + {image: "myuser/myapp", glob: "index.docker.io/myuser/myapp:latest", wantMatch: true}, + {image: "myuser/myapp", glob: "index.docker.io/myuser/myapp:*", wantMatch: true}, + {image: "myuser/myapp", glob: "index.docker.io/myuser/*", wantMatch: true}, + {image: "myuser/myapp", glob: "index.docker.io/myuser/*:latest", wantMatch: true}, + {image: "myuser/myapp", glob: "index.docker.io/*/*", wantMatch: true}, + {image: "myuser/myapp", glob: "index.docker.io/**", wantMatch: true}, + {image: "myuser/myapp", glob: "index.docker.**", wantMatch: true}, + {image: "myuser/myapp", glob: "inde**", wantMatch: true}, + {image: "myuser/myapp", glob: "**", wantMatch: true}, + {image: "myuser/myapp", glob: "myuser/myapp", wantMatch: true}, // matches because of deprecated fallback logic. + {image: "myuser/myapp", glob: "myuser/myapp*", wantMatch: true}, // * matches 0+ characters + {image: "myuser/myapp", glob: "myuser/myapp**", wantMatch: true}, // ** matches 0+ characters - want := tc.match + // Fully qualified refs and globs. + {image: "ghcr.io/foo/bar", glob: "ghcr.io/*/*", wantMatch: true}, + {image: "ghcr.io/foo/bar", glob: "ghcr.io/**", wantMatch: true}, + {image: "ghcr.io/foo", glob: "ghcr.io/*/*", wantMatch: false}, // doesn't match second * + {image: "ghcr.io/foo", glob: "ghcr.io/**", wantMatch: true}, + {image: "ghcr.io/foo", glob: "ghc**", wantMatch: true}, + {image: "ghcr.io/foo", glob: "**", wantMatch: true}, + {image: "ghcr.io/foo", glob: "*/**", wantMatch: true}, + {image: "ghcr.io/foo", glob: "ghcr.io/foo*", wantMatch: true}, // * matches 0+ characters + {image: "ghcr.io/foo", glob: "ghcr.io/foo**", wantMatch: true}, // ** matches 0+ characters - // If OS is Windows, check if there is a different expected match value - if runtime.GOOS == "windows" && tc.windowsMatch != nil { - want = tc.windowsMatch.match - } + // Various error cases. + {image: "prefix-ghcr.io/foo", glob: "ghcr.io/foo", wantMatch: false}, // glob starts at beginning. + {image: "ghcr.io/foo-suffix", glob: "ghcr.io/foo", wantMatch: false}, // glob ends at the end. + {image: "ghcrxio/foo", glob: "ghcr.io/**", wantMatch: false}, // dots in glob are replaced with \., not treated as regexp . + {image: "invalid&name", glob: "**", wantMatch: false, wantErr: true}, // invalid refs are not matched. + {image: "invalid-glob", glob: ".+", wantMatch: false, wantErr: true}, // invalid globs are rejected. + {image: "invalid-glob", glob: "[a-z]*", wantMatch: false, wantErr: true}, // invalid globs are rejected. - if got != want { - t.Errorf("expected %v for glob: %q input: %q", want, tc.glob, tc.input) - } + // Upgrading unqualified globs to assume index.docker.io prefix. + {image: "foo", glob: "*", wantMatch: true}, + {image: "myuser/myapp", glob: "*/*", wantMatch: true}, + } { + t.Run(c.image+"|"+c.glob, func(t *testing.T) { + match, err := GlobMatch(c.glob, c.image) + if match != c.wantMatch { + t.Errorf("match: got %t, want %t", match, c.wantMatch) + } + if gotErr := err != nil; gotErr != c.wantErr { + t.Errorf("err: got %v, want %t", err, c.wantErr) + } + }) } } diff --git a/pkg/apis/config/image_policies_test.go b/pkg/apis/config/image_policies_test.go index 1c9c91a95da..6e14b59a64a 100644 --- a/pkg/apis/config/image_policies_test.go +++ b/pkg/apis/config/image_policies_test.go @@ -42,6 +42,19 @@ func TestDefaultsConfigurationFromFile(t *testing.T) { } func TestGetAuthorities(t *testing.T) { + // TODO: Clean up this test to be table-driven with sub-tests, instead of one big test. + getAuthority := func(t *testing.T, m map[string]webhookcip.ClusterImagePolicy, mp string) webhookcip.Authority { + t.Helper() + cip, found := m[mp] + if !found { + t.Fatalf("failed to find matching policy %q", mp) + } + if len(cip.Authorities) == 0 { + t.Fatalf("no authorities for matching policy %q", mp) + } + return cip.Authorities[0] + } + _, example := ConfigMapsFromTestFile(t, ImagePoliciesConfigName) defaults, err := NewImagePoliciesConfigFromConfigMap(example) if err != nil { @@ -51,7 +64,7 @@ func TestGetAuthorities(t *testing.T) { checkGetMatches(t, c, err) matchedPolicy := "cluster-image-policy-0" want := "inlinedata here" - if got := c[matchedPolicy].Authorities[0].Key.Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } // Make sure glob matches 'randomstuff*' @@ -59,22 +72,22 @@ func TestGetAuthorities(t *testing.T) { checkGetMatches(t, c, err) matchedPolicy = "cluster-image-policy-1" want = "otherinline here" - if got := c[matchedPolicy].Authorities[0].Key.Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } c, err = defaults.GetMatchingPolicies("rando3") checkGetMatches(t, c, err) matchedPolicy = "cluster-image-policy-2" want = "cacert chilling here" - if got := c[matchedPolicy].Authorities[0].Keyless.CACert.Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Keyless.CACert.Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } want = "issuer" - if got := c[matchedPolicy].Authorities[0].Keyless.Identities[0].Issuer; got != want { + if got := getAuthority(t, c, matchedPolicy).Keyless.Identities[0].Issuer; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } want = "subject" - if got := c[matchedPolicy].Authorities[0].Keyless.Identities[0].Subject; got != want { + if got := getAuthority(t, c, matchedPolicy).Keyless.Identities[0].Subject; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } // Make sure regex matches "regexstring*" @@ -82,30 +95,30 @@ func TestGetAuthorities(t *testing.T) { checkGetMatches(t, c, err) matchedPolicy = "cluster-image-policy-4" want = inlineKeyData - if got := c[matchedPolicy].Authorities[0].Key.Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } - checkPublicKey(t, c[matchedPolicy].Authorities[0].Key.PublicKeys[0]) + checkPublicKey(t, getAuthority(t, c, matchedPolicy).Key.PublicKeys[0]) // Test multiline yaml cert c, err = defaults.GetMatchingPolicies("inlinecert") checkGetMatches(t, c, err) matchedPolicy = "cluster-image-policy-3" want = inlineKeyData - if got := c[matchedPolicy].Authorities[0].Key.Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } - checkPublicKey(t, c[matchedPolicy].Authorities[0].Key.PublicKeys[0]) + checkPublicKey(t, getAuthority(t, c, matchedPolicy).Key.PublicKeys[0]) // Test multiline cert but json encoded - c, err = defaults.GetMatchingPolicies("ghcr.io/example/*") + c, err = defaults.GetMatchingPolicies("ghcr.io/example/foo") checkGetMatches(t, c, err) matchedPolicy = "cluster-image-policy-json" want = inlineKeyData - if got := c[matchedPolicy].Authorities[0].Key.Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } - checkPublicKey(t, c[matchedPolicy].Authorities[0].Key.PublicKeys[0]) + checkPublicKey(t, getAuthority(t, c, matchedPolicy).Key.PublicKeys[0]) // Test multiple matches c, err = defaults.GetMatchingPolicies("regexstringtoo") @@ -115,14 +128,14 @@ func TestGetAuthorities(t *testing.T) { } matchedPolicy = "cluster-image-policy-4" want = inlineKeyData - if got := c[matchedPolicy].Authorities[0].Key.Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } - checkPublicKey(t, c[matchedPolicy].Authorities[0].Key.PublicKeys[0]) + checkPublicKey(t, getAuthority(t, c, matchedPolicy).Key.PublicKeys[0]) matchedPolicy = "cluster-image-policy-5" want = "inlinedata here" - if got := c[matchedPolicy].Authorities[0].Key.Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Key.Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } @@ -134,7 +147,7 @@ func TestGetAuthorities(t *testing.T) { } matchedPolicy = "cluster-image-policy-with-policy-attestations" want = "attestation-0" - if got := c[matchedPolicy].Authorities[0].Name; got != want { + if got := getAuthority(t, c, matchedPolicy).Name; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } // Both top & authority policy is using cue @@ -147,11 +160,11 @@ func TestGetAuthorities(t *testing.T) { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } want = "cue" - if got := c[matchedPolicy].Authorities[0].Attestations[0].Type; got != want { + if got := getAuthority(t, c, matchedPolicy).Attestations[0].Type; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } want = "test-cue-here" - if got := c[matchedPolicy].Authorities[0].Attestations[0].Data; got != want { + if got := getAuthority(t, c, matchedPolicy).Attestations[0].Data; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } @@ -165,7 +178,7 @@ func TestGetAuthorities(t *testing.T) { checkSourceOCI(t, c[matchedPolicy].Authorities) want = "example.registry.com/alternative/signature" - if got := c[matchedPolicy].Authorities[0].Sources[0].OCI; got != want { + if got := getAuthority(t, c, matchedPolicy).Sources[0].OCI; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } @@ -178,11 +191,11 @@ func TestGetAuthorities(t *testing.T) { } checkSourceOCI(t, c[matchedPolicy].Authorities) - if got := len(c[matchedPolicy].Authorities[0].Sources[0].SignaturePullSecrets); got != 1 { + if got := len(getAuthority(t, c, matchedPolicy).Sources[0].SignaturePullSecrets); got != 1 { t.Errorf("Did not get what I wanted %d, got %d", 1, got) } want = "examplePullSecret" - if got := c[matchedPolicy].Authorities[0].Sources[0].SignaturePullSecrets[0].Name; got != want { + if got := getAuthority(t, c, matchedPolicy).Sources[0].SignaturePullSecrets[0].Name; got != want { t.Errorf("Did not get what I wanted %q, got %+v", want, got) } }