Fix #1583 #1582. Disallow regex now until implemented.

Signed-off-by: Ville Aikas <[email protected]>

pkg/apis/config/glob.go

@@ -0,0 +1,31 @@
+//
+// Copyright 2022 The Sigstore Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+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
+	}
+	return strings.HasPrefix(image, strings.TrimSuffix(glob, "*"))
+}

pkg/apis/config/glob_test.go

@@ -0,0 +1,40 @@
+// Copyright 2022 The Sigstore Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package config
+
+import (
+	"testing"
+)
+
+func TestGlobMatch(t *testing.T) {
+	tests := []struct {
+		glob  string
+		input string
+		match 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
+	}
+	for _, tc := range tests {
+		got := GlobMatch(tc.input, tc.glob)
+		if got != tc.match {
+			t.Errorf("expected %v for glob: %q input: %q", tc.match, tc.glob, tc.input)
+		}
+	}
+}

pkg/apis/config/image_policies.go

@@ -94,8 +94,7 @@ func (p *ImagePolicyConfig) GetAuthorities(image string) ([]v1alpha1.Authority,
 	// workable so fine for now.
 	for _, v := range p.Policies {
 		for _, pattern := range v.Images {
-			// TODO(vaikas): do the actual glob match.
-			if image == pattern.Glob {
+			if GlobMatch(image, pattern.Glob) {
 				ret = append(ret, pattern.Authorities...)
 			}
 		}

pkg/apis/config/image_policies_test.go

@@ -53,7 +53,8 @@ func TestGetAuthorities(t *testing.T) {
 	if got := c[0].Key.Data; got != want {
 		t.Errorf("Did not get what I wanted %q, got %+v", want, c[0].Key.Data)
 	}
-	c, err = defaults.GetAuthorities("rando*")
+	// Make sure glob matches 'randomstuff*'
+	c, err = defaults.GetAuthorities("randomstuffhere")
 	if err != nil {
 		t.Error("GetMatches Failed =", err)
 	}

pkg/apis/config/store_test.go

@@ -1,18 +1,16 @@
-/*
-Copyright 2020 The Knative Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
+// Copyright 2022 The Sigstore Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
 
 package config
 

pkg/apis/config/testdata/config-image-policies.yaml

@@ -37,7 +37,7 @@ data:
             kms: whatevs
     cluster-image-policy-1: |
       images:
-      - glob: rando*
+      - glob: randomstuff*
         authorities:
         - key:
             data: otherinline here

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go

@@ -16,6 +16,7 @@ package v1alpha1
 
 import (
 	"context"
+	"strings"
 
 	"knative.dev/pkg/apis"
 )
@@ -32,84 +33,115 @@ func (spec *ClusterImagePolicySpec) Validate(ctx context.Context) (errors *apis.
 	return
 }
 
-func (image *ImagePattern) Validate(ctx context.Context) (errors *apis.FieldError) {
+func (image *ImagePattern) Validate(ctx context.Context) *apis.FieldError {
+	var errs *apis.FieldError
 	if image.Regex != "" && image.Glob != "" {
-		errors = errors.Also(apis.ErrMultipleOneOf("regex", "glob")).ViaField("images")
+		errs = errs.Also(apis.ErrMultipleOneOf("regex", "glob"))
 	}
 
 	if image.Regex == "" && image.Glob == "" {
-		errors = errors.Also(apis.ErrMissingOneOf("regex", "glob")).ViaField("images")
+		errs = errs.Also(apis.ErrMissingOneOf("regex", "glob"))
+	}
+
+	if image.Glob != "" {
+		errs = errs.Also(ValidateGlob(image.Glob).ViaField("glob"))
+	}
+
+	if image.Regex != "" {
+		errs = errs.Also(apis.ErrDisallowedFields("regex"))
 	}
 
 	if len(image.Authorities) == 0 {
-		errors = errors.Also(apis.ErrGeneric("At least one authority should be defined")).ViaField("authorities")
+		errs = errs.Also(apis.ErrGeneric("At least one authority should be defined").ViaField("authorities"))
 	}
-	for i, authority := range image.Authorities {
-		errors = errors.Also(authority.Validate(ctx)).ViaFieldIndex("authorities", i)
+	for i := range image.Authorities {
+		errs = errs.Also(image.Authorities[i].Validate(ctx).ViaFieldIndex("authorities", i))
 	}
 
-	return
+	return errs
 }
 
-func (authority *Authority) Validate(ctx context.Context) (errors *apis.FieldError) {
+func (authority *Authority) Validate(ctx context.Context) *apis.FieldError {
+	var errs *apis.FieldError
 	if authority.Key == nil && authority.Keyless == nil {
-		return errors.Also(apis.ErrMissingOneOf("key", "keyless")).ViaField("authority")
+		errs = errs.Also(apis.ErrMissingOneOf("key", "keyless"))
 	}
 	if authority.Key != nil && authority.Keyless != nil {
-		return errors.Also(apis.ErrMultipleOneOf("key", "keyless")).ViaField("authority")
+		errs = errs.Also(apis.ErrMultipleOneOf("key", "keyless"))
 	}
 
 	if authority.Key != nil {
-		errors = errors.Also(authority.Key.Validate(ctx)).ViaField("authority")
+		errs = errs.Also(authority.Key.Validate(ctx).ViaField("key"))
 	}
 	if authority.Keyless != nil {
-		errors = errors.Also(authority.Keyless.Validate(ctx)).ViaField("authority")
+		errs = errs.Also(authority.Keyless.Validate(ctx).ViaField("keyless"))
 	}
 
-	return
+	return errs
 }
 
-func (key *KeyRef) Validate(ctx context.Context) (errors *apis.FieldError) {
+func (key *KeyRef) Validate(ctx context.Context) *apis.FieldError {
+	var errs *apis.FieldError
+
 	if key.Data == "" && key.KMS == "" && key.SecretRef == nil {
-		return errors.Also(apis.ErrMissingOneOf("data", "kms", "secretref")).ViaField("key")
+		errs = errs.Also(apis.ErrMissingOneOf("data", "kms", "secretref"))
 	}
 
 	if key.Data != "" {
 		if key.KMS != "" || key.SecretRef != nil {
-			return errors.Also(apis.ErrMultipleOneOf("data", "kms", "secretref")).ViaField("key")
+			errs = errs.Also(apis.ErrMultipleOneOf("data", "kms", "secretref"))
 		}
 	} else if key.KMS != "" && key.SecretRef != nil {
-		return errors.Also(apis.ErrMultipleOneOf("data", "kms", "secretref")).ViaField("key")
+		errs = errs.Also(apis.ErrMultipleOneOf("data", "kms", "secretref"))
 	}
-	return
+	return errs
 }
 
-func (keyless *KeylessRef) Validate(ctx context.Context) (errors *apis.FieldError) {
+func (keyless *KeylessRef) Validate(ctx context.Context) *apis.FieldError {
+	var errs *apis.FieldError
 	if keyless.URL == nil && keyless.Identities == nil && keyless.CAKey == nil {
-		return errors.Also(apis.ErrMissingOneOf("url", "identities", "ca-key")).ViaField("keyless")
+		errs = errs.Also(apis.ErrMissingOneOf("url", "identities", "ca-key"))
 	}
 
 	if keyless.URL != nil {
 		if keyless.CAKey != nil || keyless.Identities != nil {
-			return errors.Also(apis.ErrMultipleOneOf("url", "identities", "ca-key")).ViaField("keyless")
+			errs = errs.Also(apis.ErrMultipleOneOf("url", "identities", "ca-key"))
 		}
 	} else if keyless.CAKey != nil && keyless.Identities != nil {
-		return errors.Also(apis.ErrMultipleOneOf("url", "identities", "ca-key")).ViaField("keyless")
+		errs = errs.Also(apis.ErrMultipleOneOf("url", "identities", "ca-key"))
 	}
 
 	if keyless.Identities != nil && len(keyless.Identities) == 0 {
-		return errors.Also(apis.ErrGeneric("At least one identity must be provided")).ViaField("keyless")
+		errs = errs.Also(apis.ErrGeneric("At least one identity must be provided"))
 	}
 
 	for i, identity := range keyless.Identities {
-		errors = errors.Also(identity.Validate(ctx)).ViaFieldIndex("identities", i)
+		errs = errs.Also(identity.Validate(ctx).ViaFieldIndex("identities", i))
 	}
-	return
+	return errs
 }
 
-func (identity *Identity) Validate(ctx context.Context) (errors *apis.FieldError) {
+func (identity *Identity) Validate(ctx context.Context) *apis.FieldError {
+	var errs *apis.FieldError
 	if identity.Issuer == "" && identity.Subject == "" {
-		return apis.ErrMissingOneOf("issuer", "subject").ViaField("identity")
+		errs = errs.Also(apis.ErrMissingOneOf("issuer", "subject"))
 	}
-	return
+	return errs
+}
+
+// ValidateGlob makes sure that if there's "*" specified it's the trailing
+// character.
+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")
+	}
+	return nil
 }