Skip to content

Commit

Permalink
Loosen glob requirements, add tests, remove warnings for now
Browse files Browse the repository at this point in the history 
Signed-off-by: Jason Hall <jason@chainguard.dev>
  • Loading branch information
@imjasonh
imjasonh committed May 24, 2022
1 parent 07b0d1f commit 3561fa8
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 48 deletions.
34 changes: 23 additions & 11 deletions pkg/apis/config/glob.go
View file
Expand Up @@ -46,34 +46,46 @@ var validGlob = regexp.MustCompile(`^[a-zA-Z0-9-_:\/\*\.]+$`)
//
// 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) (match bool, warnings []string, err error) {
func GlobMatch(glob, image string) (bool, error) {
if glob == "*/*" {
warnings = []string{`The glob match "*/*" should be "index.docker.io/*/*"`}
// TODO: Warn that the glob match "*/*" should be "index.docker.io/*/*".
glob = "index.docker.io/*/*"
}
if glob == "*" {
warnings = []string{`The glob match "*" should be "index.docker.io/library/*"`}
// TODO: Warn that the glob match "*" should be "index.docker.io/library/*".
glob = "index.docker.io/library/*"
}

ref, err := name.ParseReference(image, name.WeakValidation)
if err != nil {
return false, warnings, err
return false, err
}

// Reject that glob doesn't look like a regexp
if !validGlob.MatchString(glob) {
return false, warnings, fmt.Errorf("invalid glob %q", glob)
return false, fmt.Errorf("invalid glob %q", glob)
}

// Translate glob to regexp.
glob = strings.ReplaceAll(glob, ".", `\.`) // . in glob means \. in regexp
glob = strings.ReplaceAll(glob, "**", ".+") // ** in glob means .* in regexp
glob = strings.ReplaceAll(glob, "*", "[^/]+") // * in glob means any non-/ in regexp
glob = fmt.Sprintf("^%s$", glob) // glob must match the whole string
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

// TODO: do we want ":" to count as a separator like "/" is?

match, err = regexp.MatchString(glob, ref.Name())
return match, warnings, err
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
}
36 changes: 21 additions & 15 deletions pkg/apis/config/glob_test.go
View file
Expand Up @@ -15,16 +15,14 @@
package config

import (
"reflect"
"testing"
)

func TestGlobMatch(t *testing.T) {
for _, c := range []struct {
image, glob string
wantMatch bool
wantWarnings []string
wantErr bool
image, glob string
wantMatch bool
wantErr bool
}{
{image: "foo", glob: "index.docker.io/library/foo:latest", wantMatch: true},
{image: "foo", glob: "index.docker.io/library/foo:*", wantMatch: true},
Expand All @@ -35,7 +33,10 @@ func TestGlobMatch(t *testing.T) {
{image: "foo", glob: "index.docker.**", wantMatch: true},
{image: "foo", glob: "inde**", wantMatch: true},
{image: "foo", glob: "**", wantMatch: true},
{image: "foo", glob: "foo", wantMatch: false}, // must have index.docker.io/library prefix.
{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

{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},
Expand All @@ -45,33 +46,38 @@ func TestGlobMatch(t *testing.T) {
{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: false}, // must have index.docker.io prefix.
{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

// 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

// 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.
{image: "foo", glob: "*", wantMatch: true,
wantWarnings: []string{`The glob match "*" should be "index.docker.io/library/*"`}},
{image: "myuser/myapp", glob: "*/*", wantMatch: true,
wantWarnings: []string{`The glob match "*/*" should be "index.docker.io/*/*"`}},

// 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, warnings, err := GlobMatch(c.glob, c.image)
match, err := GlobMatch(c.glob, c.image)
if match != c.wantMatch {
t.Errorf("match: got %t, want %t", match, c.wantMatch)
}
if !reflect.DeepEqual(warnings, c.wantWarnings) {
t.Errorf("warnings: got %v, want %v", warnings, c.wantWarnings)
}
if gotErr := err != nil; gotErr != c.wantErr {
t.Errorf("err: got %v, want %t", err, c.wantErr)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/config/image_policies.go
View file
Expand Up @@ -92,7 +92,7 @@ func (p *ImagePolicyConfig) GetMatchingPolicies(image string) (map[string]webhoo
for k, v := range p.Policies {
for _, pattern := range v.Images {
if pattern.Glob != "" {
if matched, _, err := GlobMatch(pattern.Glob, image); err != nil {
if matched, err := GlobMatch(pattern.Glob, image); err != nil {
lastError = err
} else if matched {
ret[k] = v
Expand Down
55 changes: 34 additions & 21 deletions pkg/apis/config/image_policies_test.go
View file
Expand Up @@ -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 {
Expand All @@ -51,61 +64,61 @@ 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*'
c, err = defaults.GetMatchingPolicies("randomstuffhere")
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*"
c, err = defaults.GetMatchingPolicies("regexstringstuff")
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")
Expand All @@ -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)
}

Expand All @@ -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
Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}
}
Expand Down

0 comments on commit 3561fa8

Please sign in to comment.