Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cosigned] remove regex from the image pattern fields #1873

Merged
merged 1 commit into from
May 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions config/300-clusterimagepolicy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions pkg/apis/config/image_policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/config/image_policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/config/testdata/config-image-policies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ data:
-----END PUBLIC KEY-----
cluster-image-policy-4: |
images:
- regex: .*regexstring.*
- glob: regexstring*
authorities:
- name: attestation-0
key:
Expand All @@ -80,7 +80,7 @@ data:
-----END PUBLIC KEY-----
cluster-image-policy-5: |
images:
- regex: .*regexstringtoo.*
- glob: regexstringtoo*
authorities:
- name: attestation-0
key:
Expand All @@ -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:
Expand All @@ -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:
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/cosigned/v1alpha1/clusterimagepolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 2 additions & 10 deletions pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
43 changes: 14 additions & 29 deletions pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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"},
Expand All @@ -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"},
Expand All @@ -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"},
Expand All @@ -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"},
Expand All @@ -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"},
Expand All @@ -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"},
Expand All @@ -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"},
Expand Down
3 changes: 3 additions & 0 deletions pkg/cosign/kubernetes/webhook/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions pkg/cosign/kubernetes/webhook/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down
29 changes: 0 additions & 29 deletions test/testdata/cosigned/invalid/both-regex-and-pattern.yaml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ metadata:
name: image-policy
spec:
images:
- regex: *
- glob: "["
authorities:
- key:
data: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ metadata:
name: image-policy
spec:
images:
- regex: images\..*
- regex: image.*
- glob: images.*
- glob: image*
authorities:
- keyless:
ca-cert:
Expand Down