Skip to content

Commit 5bf00b7

Browse files
committed
Fix #1583 #1582. Disallow regex now until implemented.
Signed-off-by: Ville Aikas <[email protected]>
1 parent c341168 commit 5bf00b7

18 files changed

+220
-73
lines changed

pkg/apis/config/glob.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//
2+
// Copyright 2022 The Sigstore Authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package config
17+
18+
import "strings"
19+
20+
// GlobMatch takes a string and handles only trailing '*' character as a
21+
// wildcard. This is little different from various packages that I was able
22+
// to find, since they handle '*' anywhere in the string as a wildcard. For our
23+
// use we only want to handle it at the end, and hence this effectively turns
24+
// into 'hasPrefix' string matching up to the trailing *.
25+
func GlobMatch(image, glob string) bool {
26+
if !strings.HasSuffix(glob, "*") {
27+
// Doesn't end with *, so do an exact match
28+
return image == glob
29+
}
30+
return strings.HasPrefix(image, strings.TrimSuffix(glob, "*"))
31+
}

pkg/apis/config/glob_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright 2022 The Sigstore Authors.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package config
16+
17+
import (
18+
"testing"
19+
)
20+
21+
func TestGlobMatch(t *testing.T) {
22+
tests := []struct {
23+
glob string
24+
input string
25+
match bool
26+
}{
27+
{glob: "foo", input: "foo", match: true}, // exact match
28+
{glob: "fooo*", input: "foo", match: false}, // prefix too long
29+
{glob: "foo*", input: "foobar", match: true}, // works
30+
{glob: "foo*", input: "foo", match: true}, // works
31+
{glob: "*", input: "foo", match: true}, // matches anything
32+
{glob: "*", input: "bar", match: true}, // matches anything
33+
}
34+
for _, tc := range tests {
35+
got := GlobMatch(tc.input, tc.glob)
36+
if got != tc.match {
37+
t.Errorf("expected %v for glob: %q input: %q", tc.match, tc.glob, tc.input)
38+
}
39+
}
40+
}

pkg/apis/config/image_policies.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ func (p *ImagePolicyConfig) GetAuthorities(image string) ([]v1alpha1.Authority,
9494
// workable so fine for now.
9595
for _, v := range p.Policies {
9696
for _, pattern := range v.Images {
97-
// TODO(vaikas): do the actual glob match.
98-
if image == pattern.Glob {
97+
if GlobMatch(image, pattern.Glob) {
9998
ret = append(ret, pattern.Authorities...)
10099
}
101100
}

pkg/apis/config/image_policies_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ func TestGetAuthorities(t *testing.T) {
5353
if got := c[0].Key.Data; got != want {
5454
t.Errorf("Did not get what I wanted %q, got %+v", want, c[0].Key.Data)
5555
}
56-
c, err = defaults.GetAuthorities("rando*")
56+
// Make sure glob matches 'randomstuff*'
57+
c, err = defaults.GetAuthorities("randomstuffhere")
5758
if err != nil {
5859
t.Error("GetMatches Failed =", err)
5960
}

pkg/apis/config/store_test.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,16 @@
1-
/*
2-
Copyright 2020 The Knative Authors.
3-
4-
Licensed under the Apache License, Version 2.0 (the "License");
5-
you may not use this file except in compliance with the License.
6-
You may obtain a copy of the License at
7-
8-
http://www.apache.org/licenses/LICENSE-2.0
9-
10-
Unless required by applicable law or agreed to in writing, software
11-
distributed under the License is distributed on an "AS IS" BASIS,
12-
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13-
See the License for the specific language governing permissions and
14-
limitations under the License.
15-
*/
1+
// Copyright 2022 The Sigstore Authors.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
1614

1715
package config
1816

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ data:
3737
kms: whatevs
3838
cluster-image-policy-1: |
3939
images:
40-
- glob: rando*
40+
- glob: randomstuff*
4141
authorities:
4242
- key:
4343
data: otherinline here

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go

Lines changed: 60 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package v1alpha1
1616

1717
import (
1818
"context"
19+
"strings"
1920

2021
"knative.dev/pkg/apis"
2122
)
@@ -32,84 +33,115 @@ func (spec *ClusterImagePolicySpec) Validate(ctx context.Context) (errors *apis.
3233
return
3334
}
3435

35-
func (image *ImagePattern) Validate(ctx context.Context) (errors *apis.FieldError) {
36+
func (image *ImagePattern) Validate(ctx context.Context) *apis.FieldError {
37+
var errs *apis.FieldError
3638
if image.Regex != "" && image.Glob != "" {
37-
errors = errors.Also(apis.ErrMultipleOneOf("regex", "glob")).ViaField("images")
39+
errs = errs.Also(apis.ErrMultipleOneOf("regex", "glob"))
3840
}
3941

4042
if image.Regex == "" && image.Glob == "" {
41-
errors = errors.Also(apis.ErrMissingOneOf("regex", "glob")).ViaField("images")
43+
errs = errs.Also(apis.ErrMissingOneOf("regex", "glob"))
44+
}
45+
46+
if image.Glob != "" {
47+
errs = errs.Also(ValidateGlob(image.Glob).ViaField("glob"))
48+
}
49+
50+
if image.Regex != "" {
51+
errs = errs.Also(apis.ErrDisallowedFields("regex"))
4252
}
4353

4454
if len(image.Authorities) == 0 {
45-
errors = errors.Also(apis.ErrGeneric("At least one authority should be defined")).ViaField("authorities")
55+
errs = errs.Also(apis.ErrGeneric("At least one authority should be defined").ViaField("authorities"))
4656
}
47-
for i, authority := range image.Authorities {
48-
errors = errors.Also(authority.Validate(ctx)).ViaFieldIndex("authorities", i)
57+
for i := range image.Authorities {
58+
errs = errs.Also(image.Authorities[i].Validate(ctx).ViaFieldIndex("authorities", i))
4959
}
5060

51-
return
61+
return errs
5262
}
5363

54-
func (authority *Authority) Validate(ctx context.Context) (errors *apis.FieldError) {
64+
func (authority *Authority) Validate(ctx context.Context) *apis.FieldError {
65+
var errs *apis.FieldError
5566
if authority.Key == nil && authority.Keyless == nil {
56-
return errors.Also(apis.ErrMissingOneOf("key", "keyless")).ViaField("authority")
67+
errs = errs.Also(apis.ErrMissingOneOf("key", "keyless"))
5768
}
5869
if authority.Key != nil && authority.Keyless != nil {
59-
return errors.Also(apis.ErrMultipleOneOf("key", "keyless")).ViaField("authority")
70+
errs = errs.Also(apis.ErrMultipleOneOf("key", "keyless"))
6071
}
6172

6273
if authority.Key != nil {
63-
errors = errors.Also(authority.Key.Validate(ctx)).ViaField("authority")
74+
errs = errs.Also(authority.Key.Validate(ctx).ViaField("key"))
6475
}
6576
if authority.Keyless != nil {
66-
errors = errors.Also(authority.Keyless.Validate(ctx)).ViaField("authority")
77+
errs = errs.Also(authority.Keyless.Validate(ctx).ViaField("keyless"))
6778
}
6879

69-
return
80+
return errs
7081
}
7182

72-
func (key *KeyRef) Validate(ctx context.Context) (errors *apis.FieldError) {
83+
func (key *KeyRef) Validate(ctx context.Context) *apis.FieldError {
84+
var errs *apis.FieldError
85+
7386
if key.Data == "" && key.KMS == "" && key.SecretRef == nil {
74-
return errors.Also(apis.ErrMissingOneOf("data", "kms", "secretref")).ViaField("key")
87+
errs = errs.Also(apis.ErrMissingOneOf("data", "kms", "secretref"))
7588
}
7689

7790
if key.Data != "" {
7891
if key.KMS != "" || key.SecretRef != nil {
79-
return errors.Also(apis.ErrMultipleOneOf("data", "kms", "secretref")).ViaField("key")
92+
errs = errs.Also(apis.ErrMultipleOneOf("data", "kms", "secretref"))
8093
}
8194
} else if key.KMS != "" && key.SecretRef != nil {
82-
return errors.Also(apis.ErrMultipleOneOf("data", "kms", "secretref")).ViaField("key")
95+
errs = errs.Also(apis.ErrMultipleOneOf("data", "kms", "secretref"))
8396
}
84-
return
97+
return errs
8598
}
8699

87-
func (keyless *KeylessRef) Validate(ctx context.Context) (errors *apis.FieldError) {
100+
func (keyless *KeylessRef) Validate(ctx context.Context) *apis.FieldError {
101+
var errs *apis.FieldError
88102
if keyless.URL == nil && keyless.Identities == nil && keyless.CAKey == nil {
89-
return errors.Also(apis.ErrMissingOneOf("url", "identities", "ca-key")).ViaField("keyless")
103+
errs = errs.Also(apis.ErrMissingOneOf("url", "identities", "ca-key"))
90104
}
91105

92106
if keyless.URL != nil {
93107
if keyless.CAKey != nil || keyless.Identities != nil {
94-
return errors.Also(apis.ErrMultipleOneOf("url", "identities", "ca-key")).ViaField("keyless")
108+
errs = errs.Also(apis.ErrMultipleOneOf("url", "identities", "ca-key"))
95109
}
96110
} else if keyless.CAKey != nil && keyless.Identities != nil {
97-
return errors.Also(apis.ErrMultipleOneOf("url", "identities", "ca-key")).ViaField("keyless")
111+
errs = errs.Also(apis.ErrMultipleOneOf("url", "identities", "ca-key"))
98112
}
99113

100114
if keyless.Identities != nil && len(keyless.Identities) == 0 {
101-
return errors.Also(apis.ErrGeneric("At least one identity must be provided")).ViaField("keyless")
115+
errs = errs.Also(apis.ErrGeneric("At least one identity must be provided"))
102116
}
103117

104118
for i, identity := range keyless.Identities {
105-
errors = errors.Also(identity.Validate(ctx)).ViaFieldIndex("identities", i)
119+
errs = errs.Also(identity.Validate(ctx).ViaFieldIndex("identities", i))
106120
}
107-
return
121+
return errs
108122
}
109123

110-
func (identity *Identity) Validate(ctx context.Context) (errors *apis.FieldError) {
124+
func (identity *Identity) Validate(ctx context.Context) *apis.FieldError {
125+
var errs *apis.FieldError
111126
if identity.Issuer == "" && identity.Subject == "" {
112-
return apis.ErrMissingOneOf("issuer", "subject").ViaField("identity")
127+
errs = errs.Also(apis.ErrMissingOneOf("issuer", "subject"))
113128
}
114-
return
129+
return errs
130+
}
131+
132+
// ValidateGlob makes sure that if there's "*" specified it's the trailing
133+
// character.
134+
func ValidateGlob(glob string) *apis.FieldError {
135+
c := strings.Count(glob, "*")
136+
switch c {
137+
case 0:
138+
return nil
139+
case 1:
140+
if !strings.HasSuffix(glob, "*") {
141+
return apis.ErrInvalidValue(glob, apis.CurrentField, "glob match supports only * as a trailing character")
142+
}
143+
default:
144+
return apis.ErrInvalidValue(glob, apis.CurrentField, "glob match supports only a single * as a trailing character")
145+
}
146+
return nil
115147
}

0 commit comments

Comments
 (0)