chore: remove regex from image pattern

Signed-off-by: hectorj2f <hectorf@vmware.com>
sigstore · May 13, 2022 · 872a182 · 872a182
1 parent 20c8c88
commit 872a182
Show file tree

Hide file tree

Showing 12 changed files with 36 additions and 96 deletions.
diff --git a/config/300-clusterimagepolicy.yaml b/config/300-clusterimagepolicy.yaml
@@ -155,8 +155,6 @@ spec:
                     properties:
                       glob:
                         type: string
-                      regex:
-                        type: string
                 policy:
                   description: Policy is an optional policy that can be applied against all the successfully validated Authorities. If no authorities pass, this does not even get evaluated, as the Policy is considered failed.
                   type: object

diff --git a/pkg/apis/config/image_policies.go b/pkg/apis/config/image_policies.go
@@ -19,7 +19,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"regexp"
 
 	webhookcip "github.com/sigstore/cosign/pkg/cosign/kubernetes/webhook/clusterimagepolicy"
 	corev1 "k8s.io/api/core/v1"
@@ -98,12 +97,6 @@ func (p *ImagePolicyConfig) GetMatchingPolicies(image string) (map[string]webhoo
 				} else if matched {
 					ret[k] = v
 				}
-			} else if pattern.Regex != "" {
-				if regex, err := regexp.Compile(pattern.Regex); err != nil {
-					lastError = err
-				} else if regex.MatchString(image) {
-					ret[k] = v
-				}
 			}
 		}
 	}

diff --git a/pkg/apis/config/image_policies_test.go b/pkg/apis/config/image_policies_test.go
@@ -77,8 +77,8 @@ func TestGetAuthorities(t *testing.T) {
 	if got := c[matchedPolicy].Authorities[0].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("randomregexstringstuff")
+	// Make sure regex matches "regexstring*"
+	c, err = defaults.GetMatchingPolicies("regexstringstuff")
 	checkGetMatches(t, c, err)
 	matchedPolicy = "cluster-image-policy-4"
 	want = inlineKeyData

diff --git a/pkg/apis/config/testdata/config-image-policies.yaml b/pkg/apis/config/testdata/config-image-policies.yaml
@@ -69,7 +69,7 @@ data:
             -----END PUBLIC KEY-----
     cluster-image-policy-4: |
       images:
-      - regex: .*regexstring.*
+      - glob: regexstring*
       authorities:
       - name: attestation-0
         key:
@@ -80,7 +80,7 @@ data:
             -----END PUBLIC KEY-----
     cluster-image-policy-5: |
       images:
-      - regex: .*regexstringtoo.*
+      - glob: regexstringtoo*
       authorities:
       - name: attestation-0
         key:
@@ -107,7 +107,7 @@ data:
         data: "cip level cue here"
     cluster-image-policy-source-oci: |
       images:
-      - regex: .*sourceocionly.*
+      - glob: sourceocionly*
       authorities:
       - name: attestation-0
         key:
@@ -116,7 +116,7 @@ data:
         - oci: "example.registry.com/alternative/signature"
     cluster-image-policy-source-oci-signature-pull-secrets: |
       images:
-      - regex: .*sourceocisignaturepullsecrets.*
+      - glob: sourceocisignaturepullsecrets*
       authorities:
       - name: attestation-0
         key:

diff --git a/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_types.go b/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_types.go
@@ -67,8 +67,6 @@ type ClusterImagePolicySpec struct {
 type ImagePattern struct {
 	// +optional
 	Glob string `json:"glob,omitempty"`
-	// +optional
-	Regex string `json:"regex,omitempty"`
 }
 
 // The authorities block defines the rules for discovering and

diff --git a/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go b/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go
@@ -49,22 +49,14 @@ func (spec *ClusterImagePolicySpec) Validate(ctx context.Context) (errors *apis.
 
 func (image *ImagePattern) Validate(ctx context.Context) *apis.FieldError {
 	var errs *apis.FieldError
-	if image.Regex != "" && image.Glob != "" {
-		errs = errs.Also(apis.ErrMultipleOneOf("regex", "glob"))
-	}
-
-	if image.Regex == "" && image.Glob == "" {
-		errs = errs.Also(apis.ErrMissingOneOf("regex", "glob"))
+	if image.Glob == "" {
+		errs = errs.Also(apis.ErrMissingField("glob"))
 	}
 
 	if image.Glob != "" {
 		errs = errs.Also(ValidateGlob(image.Glob).ViaField("glob"))
 	}
 
-	if image.Regex != "" {
-		errs = errs.Also(ValidateRegex(image.Regex).ViaField("regex"))
-	}
-
 	return errs
 }
 

diff --git a/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go b/pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go
@@ -31,24 +31,9 @@ func TestImagePatternValidation(t *testing.T) {
 		policy      ClusterImagePolicy
 	}{
 		{
-			name:        "Should fail when both regex and glob are present",
+			name:        "Should fail when glob is not present",
 			expectErr:   true,
-			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{
-						{
-							Regex: "//",
-							Glob:  "**",
-						},
-					},
-				},
-			},
-		},
-		{
-			name:        "Should fail when neither regex nor glob are present",
-			expectErr:   true,
-			errorString: "expected exactly one, got neither: spec.images[0].glob, spec.images[0].regex\nmissing field(s): spec.authorities",
+			errorString: "missing field(s): spec.authorities, spec.images[0].glob",
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
 					Images: []ImagePattern{
@@ -80,27 +65,27 @@ func TestImagePatternValidation(t *testing.T) {
 			},
 		},
 		{
-			name:        "Should fail when regex is invalid: %v",
+			name:        "Should fail when glob is invalid: %v",
 			expectErr:   true,
-			errorString: "invalid value: *: spec.images[0].regex\nregex is invalid: error parsing regexp: missing argument to repetition operator: `*`\nmissing field(s): spec.authorities",
+			errorString: "missing field(s): spec.authorities",
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
 					Images: []ImagePattern{
 						{
-							Regex: "*",
+							Glob: "]",
 						},
 					},
 				},
 			},
 		},
 		{
-			name:      "Should pass when regex is valid: %v",
+			name:      "Should pass when glob is valid: %v",
 			expectErr: false,
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
 					Images: []ImagePattern{
 						{
-							Regex: ".*",
+							Glob: "gcr.io/*",
 						},
 					},
 					Authorities: []Authority{
@@ -389,7 +374,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 			expectErr: false,
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
-					Images: []ImagePattern{{Regex: ".*"}},
+					Images: []ImagePattern{{Glob: "gcr.io/*"}},
 					Authorities: []Authority{
 						{
 							Key:     &KeyRef{KMS: "kms://key/path"},
@@ -405,7 +390,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 			errorString: "missing field(s): spec.authorities[0].source[0].oci",
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
-					Images: []ImagePattern{{Regex: ".*"}},
+					Images: []ImagePattern{{Glob: "gcr.io/*"}},
 					Authorities: []Authority{
 						{
 							Key:     &KeyRef{KMS: "kms://key/path"},
@@ -420,7 +405,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 			expectErr: false,
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
-					Images: []ImagePattern{{Regex: ".*"}},
+					Images: []ImagePattern{{Glob: "gcr.io/*"}},
 					Authorities: []Authority{
 						{
 							Key: &KeyRef{KMS: "kms://key/path"},
@@ -438,7 +423,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 			expectErr: false,
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
-					Images: []ImagePattern{{Regex: ".*"}},
+					Images: []ImagePattern{{Glob: "gcr.io/*"}},
 					Authorities: []Authority{
 						{
 							Key: &KeyRef{KMS: "kms://key/path"},
@@ -456,7 +441,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 			expectErr: false,
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
-					Images: []ImagePattern{{Regex: ".*"}},
+					Images: []ImagePattern{{Glob: "gcr.io/*"}},
 					Authorities: []Authority{
 						{
 							Key: &KeyRef{KMS: "kms://key/path"},
@@ -479,7 +464,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 			errorString: "missing field(s): spec.authorities[0].source[0].signaturePullSecrets[0].name",
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
-					Images: []ImagePattern{{Regex: ".*"}},
+					Images: []ImagePattern{{Glob: "gcr.io/*"}},
 					Authorities: []Authority{
 						{
 							Key: &KeyRef{KMS: "kms://key/path"},
@@ -501,7 +486,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 			expectErr: false,
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
-					Images: []ImagePattern{{Regex: ".*"}},
+					Images: []ImagePattern{{Glob: "gcr.io/*"}},
 					Authorities: []Authority{
 						{
 							Key: &KeyRef{KMS: "kms://key/path"},

diff --git a/pkg/cosign/kubernetes/webhook/validator.go b/pkg/cosign/kubernetes/webhook/validator.go
@@ -204,12 +204,15 @@ func (v *Validator) validatePodSpec(ctx context.Context, namespace string, ps *c
 						}
 					}
 				}
+				logging.FromContext(ctx).Errorf("policies: for %v", policies)
 			}
 
 			if passedPolicyChecks {
 				logging.FromContext(ctx).Debugf("Found at least one matching policy and it was validated for %s", ref.Name())
 				continue
 			}
+			logging.FromContext(ctx).Errorf("ref: for %v", ref)
+			logging.FromContext(ctx).Errorf("container Keys: for %v", containerKeys)
 
 			if _, err := valid(ctx, ref, nil, containerKeys, ociremote.WithRemoteOptions(remote.WithAuthFromKeychain(kc))); err != nil {
 				errorField := apis.ErrGeneric(err.Error(), "image").ViaFieldIndex(field, i)

diff --git a/pkg/cosign/kubernetes/webhook/validator_test.go b/pkg/cosign/kubernetes/webhook/validator_test.go
@@ -251,7 +251,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 					Policies: map[string]webhookcip.ClusterImagePolicy{
 						"cluster-image-policy": {
 							Images: []v1alpha1.ImagePattern{{
-								Regex: ".*",
+								Glob: "gcr.io/*/*",
 							}},
 							Authorities: []webhookcip.Authority{
 								{
@@ -285,7 +285,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 					Policies: map[string]webhookcip.ClusterImagePolicy{
 						"cluster-image-policy-keyless": {
 							Images: []v1alpha1.ImagePattern{{
-								Regex: ".*",
+								Glob: "gcr.io/*/*",
 							}},
 							Authorities: []webhookcip.Authority{
 								{
@@ -328,7 +328,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 					Policies: map[string]webhookcip.ClusterImagePolicy{
 						"cluster-image-policy-keyless": {
 							Images: []v1alpha1.ImagePattern{{
-								Regex: ".*",
+								Glob: "gcr.io/*/*",
 							}},
 							Authorities: []webhookcip.Authority{
 								{
@@ -371,7 +371,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 					Policies: map[string]webhookcip.ClusterImagePolicy{
 						"cluster-image-policy-keyless": {
 							Images: []v1alpha1.ImagePattern{{
-								Regex: ".*",
+								Glob: "gcr.io/*/*",
 							}},
 							Authorities: []webhookcip.Authority{
 								{
@@ -419,7 +419,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 					Policies: map[string]webhookcip.ClusterImagePolicy{
 						"cluster-image-policy-keyless": {
 							Images: []v1alpha1.ImagePattern{{
-								Regex: ".*",
+								Glob: "gcr.io/*/*",
 							}},
 							Authorities: []webhookcip.Authority{
 								{
@@ -452,7 +452,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 					Policies: map[string]webhookcip.ClusterImagePolicy{
 						"cluster-image-policy-keyless": {
 							Images: []v1alpha1.ImagePattern{{
-								Regex: ".*",
+								Glob: "gcr.io/*/*",
 							}},
 							Authorities: []webhookcip.Authority{
 								{
@@ -498,7 +498,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 					Policies: map[string]webhookcip.ClusterImagePolicy{
 						"cluster-image-policy": {
 							Images: []v1alpha1.ImagePattern{{
-								Regex: ".*",
+								Glob: "gcr.io/*/*",
 							}},
 							Authorities: []webhookcip.Authority{
 								{
@@ -550,7 +550,7 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw==
 					Policies: map[string]webhookcip.ClusterImagePolicy{
 						"cluster-image-policy": {
 							Images: []v1alpha1.ImagePattern{{
-								Regex: ".*",
+								Glob: "gcr.io/*/*",
 							}},
 							Authorities: []webhookcip.Authority{
 								{

diff --git a/test/testdata/cosigned/invalid/both-regex-and-pattern.yaml b/test/testdata/cosigned/invalid/both-regex-and-pattern.yaml
diff --git a/...tdata/cosigned/invalid/invalid-regex.yaml → ...stdata/cosigned/invalid/invalid-glob.yaml b/...tdata/cosigned/invalid/invalid-regex.yaml → ...stdata/cosigned/invalid/invalid-glob.yaml
@@ -18,7 +18,7 @@ metadata:
   name: image-policy
 spec:
   images:
-  - regex: *
+  - glob: "["
   authorities:
   - key:
       data: |

diff --git a/...ta/cosigned/valid/valid-policy-regex.yaml → ...testdata/cosigned/valid/valid-policy.yaml b/...ta/cosigned/valid/valid-policy-regex.yaml → ...testdata/cosigned/valid/valid-policy.yaml
@@ -18,8 +18,8 @@ metadata:
   name: image-policy
 spec:
   images:
-  - regex: images\..*
-  - regex: image.*
+  - glob: images.*
+  - glob: image*
   authorities:
   - keyless:
       ca-cert: