Skip to content

Commit 20acf03

Browse files
everettraveneverettraven
authored
API updates based on external review (#443)
Signed-off-by: everettraven <[email protected]> Signed-off-by: Mikalai Radchuk <[email protected]>
1 parent 9cf2e70 commit 20acf03

File tree

12 files changed

+786
-271
lines changed

12 files changed

+786
-271
lines changed

‎api/core/v1alpha1/clustercatalog_types.go‎

Lines changed: 193 additions & 75 deletions
Large diffs are not rendered by default.

‎api/core/v1alpha1/clustercatalog_types_test.go‎

Lines changed: 258 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,54 +4,141 @@ import (
44
"context"
55
"fmt"
66
"os"
7+
"strings"
78
"testing"
8-
"time"
99

1010
"github.com/stretchr/testify/assert"
1111
"github.com/stretchr/testify/require"
1212
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
1313
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1414
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
1515
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel"
16-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1716
"k8s.io/apimachinery/pkg/runtime"
1817
"k8s.io/apimachinery/pkg/util/validation/field"
1918
celconfig "k8s.io/apiserver/pkg/apis/cel"
19+
"k8s.io/utils/ptr"
2020
"sigs.k8s.io/yaml"
2121
)
2222

23-
func TestPollIntervalCELValidationRules(t *testing.T) {
23+
func TestImageSourceCELValidationRules(t *testing.T) {
2424
validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml")
25-
pth := "openAPIV3Schema.properties.spec"
25+
pth := "openAPIV3Schema.properties.spec.properties.source.properties.image"
2626
validator, found := validators["v1alpha1"][pth]
2727
require.True(t, found)
2828

2929
for name, tc := range map[string]struct {
30-
spec ClusterCatalogSpec
30+
spec ImageSource
3131
wantErrs []string
3232
}{
33-
"digest based image ref, poll interval not allowed, poll interval specified": {
34-
spec: ClusterCatalogSpec{
35-
Source: CatalogSource{
36-
Type: SourceTypeImage,
37-
Image: &ImageSource{
38-
Ref: "docker.io/test-image@sha256:asdf98234sd",
39-
PollInterval: &metav1.Duration{Duration: time.Minute},
40-
},
41-
},
33+
"valid digest based image ref, poll interval not allowed, poll interval specified": {
34+
spec: ImageSource{
35+
Ref: "docker.io/test-image@sha256:abcdef123456789abcdef123456789abc",
36+
PollIntervalMinutes: ptr.To(1),
4237
},
4338
wantErrs: []string{
44-
"openAPIV3Schema.properties.spec: Invalid value: \"object\": cannot specify PollInterval while using digest-based image",
39+
"openAPIV3Schema.properties.spec.properties.source.properties.image: Invalid value: \"object\": cannot specify pollIntervalMinutes while using digest-based image",
4540
},
4641
},
47-
"digest based image ref, poll interval not allowed, poll interval not specified": {
48-
spec: ClusterCatalogSpec{
49-
Source: CatalogSource{
50-
Type: SourceTypeImage,
51-
Image: &ImageSource{
52-
Ref: "docker.io/example/test-catalog@sha256:asdf123",
53-
},
54-
},
42+
"valid digest based image ref, poll interval not allowed, poll interval not specified": {
43+
spec: ImageSource{
44+
Ref: "docker.io/test-image@sha256:abcdef123456789abcdef123456789abc",
45+
},
46+
wantErrs: []string{},
47+
},
48+
"invalid digest based image ref, invalid domain": {
49+
spec: ImageSource{
50+
Ref: "-quay+docker/foo/bar@sha256:abcdef123456789abcdef123456789abc",
51+
},
52+
wantErrs: []string{
53+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": must start with a valid domain. valid domains must be alphanumeric characters (lowercase and uppercase) separated by the \".\" character.",
54+
},
55+
},
56+
"invalid digest based image ref, invalid name": {
57+
spec: ImageSource{
58+
Ref: "docker.io/FOO/BAR@sha256:abcdef123456789abcdef123456789abc",
59+
},
60+
wantErrs: []string{
61+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
62+
},
63+
},
64+
"invalid digest based image ref, invalid digest algorithm": {
65+
spec: ImageSource{
66+
Ref: "docker.io/foo/bar@99-problems:abcdef123456789abcdef123456789abc",
67+
},
68+
wantErrs: []string{
69+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": digest algorithm is not valid. valid algorithms must start with an uppercase or lowercase alpha character followed by alphanumeric characters and may contain the \"-\", \"_\", \"+\", and \".\" characters.",
70+
},
71+
},
72+
"invalid digest based image ref, too short digest encoding": {
73+
spec: ImageSource{
74+
Ref: "docker.io/foo/bar@sha256:abcdef123456789",
75+
},
76+
wantErrs: []string{
77+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": digest is not valid. the encoded string must be at least 32 characters",
78+
},
79+
},
80+
"invalid digest based image ref, invalid characters in digest encoding": {
81+
spec: ImageSource{
82+
Ref: "docker.io/foo/bar@sha256:XYZxy123456789abcdef123456789abc",
83+
},
84+
wantErrs: []string{
85+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": digest is not valid. the encoded string must only contain hex characters (A-F, a-f, 0-9)",
86+
},
87+
},
88+
"invalid image ref, no tag or digest": {
89+
spec: ImageSource{
90+
Ref: "docker.io/foo/bar",
91+
},
92+
wantErrs: []string{
93+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": must end with a digest or a tag",
94+
},
95+
},
96+
"invalid tag based image ref, tag too long": {
97+
spec: ImageSource{
98+
Ref: fmt.Sprintf("docker.io/foo/bar:%s", strings.Repeat("x", 128)),
99+
},
100+
wantErrs: []string{
101+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": tag is invalid. the tag must not be more than 127 characters",
102+
},
103+
},
104+
"invalid tag based image ref, tag contains invalid characters": {
105+
spec: ImageSource{
106+
Ref: "docker.io/foo/bar:-foo_bar-",
107+
},
108+
wantErrs: []string{
109+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": tag is invalid. valid tags must begin with a word character (alphanumeric + \"_\") followed by word characters or \".\", and \"-\" characters",
110+
},
111+
},
112+
"valid tag based image ref": {
113+
spec: ImageSource{
114+
Ref: "docker.io/foo/bar:v1.0.0",
115+
},
116+
wantErrs: []string{},
117+
},
118+
"valid tag based image ref, pollIntervalMinutes specified": {
119+
spec: ImageSource{
120+
Ref: "docker.io/foo/bar:v1.0.0",
121+
PollIntervalMinutes: ptr.To(5),
122+
},
123+
wantErrs: []string{},
124+
},
125+
"invalid image ref, only domain with port": {
126+
spec: ImageSource{
127+
Ref: "docker.io:8080",
128+
},
129+
wantErrs: []string{
130+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
131+
},
132+
},
133+
"valid image ref, domain with port": {
134+
spec: ImageSource{
135+
Ref: "my-subdomain.docker.io:8080/foo/bar:latest",
136+
},
137+
wantErrs: []string{},
138+
},
139+
"valid image ref, tag ends with hyphen": {
140+
spec: ImageSource{
141+
Ref: "my-subdomain.docker.io:8080/foo/bar:latest-",
55142
},
56143
wantErrs: []string{},
57144
},
@@ -60,6 +147,143 @@ func TestPollIntervalCELValidationRules(t *testing.T) {
60147
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.spec) //nolint:gosec
61148
require.NoError(t, err)
62149
errs := validator(obj, nil)
150+
require.Equal(t, len(tc.wantErrs), len(errs), "want", tc.wantErrs, "got", errs)
151+
for i := range tc.wantErrs {
152+
got := errs[i].Error()
153+
assert.Equal(t, tc.wantErrs[i], got)
154+
}
155+
})
156+
}
157+
}
158+
159+
func TestResolvedImageSourceCELValidation(t *testing.T) {
160+
validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml")
161+
pth := "openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref"
162+
validator, found := validators["v1alpha1"][pth]
163+
require.True(t, found)
164+
165+
for name, tc := range map[string]struct {
166+
spec ImageSource
167+
wantErrs []string
168+
}{
169+
"valid digest based image ref": {
170+
spec: ImageSource{
171+
Ref: "docker.io/test-image@sha256:abcdef123456789abcdef123456789abc",
172+
},
173+
wantErrs: []string{},
174+
},
175+
"invalid digest based image ref, invalid domain": {
176+
spec: ImageSource{
177+
Ref: "-quay+docker/foo/bar@sha256:abcdef123456789abcdef123456789abc",
178+
},
179+
wantErrs: []string{
180+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must start with a valid domain. valid domains must be alphanumeric characters (lowercase and uppercase) separated by the \".\" character.",
181+
},
182+
},
183+
"invalid digest based image ref, invalid name": {
184+
spec: ImageSource{
185+
Ref: "docker.io/FOO/BAR@sha256:abcdef123456789abcdef123456789abc",
186+
},
187+
wantErrs: []string{
188+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
189+
},
190+
},
191+
"invalid digest based image ref, invalid digest algorithm": {
192+
spec: ImageSource{
193+
Ref: "docker.io/foo/bar@99-problems:abcdef123456789abcdef123456789abc",
194+
},
195+
wantErrs: []string{
196+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": digest algorithm is not valid. valid algorithms must start with an uppercase or lowercase alpha character followed by alphanumeric characters and may contain the \"-\", \"_\", \"+\", and \".\" characters.",
197+
},
198+
},
199+
"invalid digest based image ref, too short digest encoding": {
200+
spec: ImageSource{
201+
Ref: "docker.io/foo/bar@sha256:abcdef123456789",
202+
},
203+
wantErrs: []string{
204+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": digest is not valid. the encoded string must be at least 32 characters",
205+
},
206+
},
207+
"invalid digest based image ref, invalid characters in digest encoding": {
208+
spec: ImageSource{
209+
Ref: "docker.io/foo/bar@sha256:XYZxy123456789abcdef123456789abc",
210+
},
211+
wantErrs: []string{
212+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": digest is not valid. the encoded string must only contain hex characters (A-F, a-f, 0-9)",
213+
},
214+
},
215+
"invalid image ref, no digest": {
216+
spec: ImageSource{
217+
Ref: "docker.io/foo/bar",
218+
},
219+
wantErrs: []string{
220+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest",
221+
},
222+
},
223+
"invalid image ref, only domain with port": {
224+
spec: ImageSource{
225+
Ref: "docker.io:8080",
226+
},
227+
wantErrs: []string{
228+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
229+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest",
230+
},
231+
},
232+
"invalid image ref, tag-based ref": {
233+
spec: ImageSource{
234+
Ref: "docker.io/foo/bar:latest",
235+
},
236+
wantErrs: []string{
237+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest",
238+
},
239+
},
240+
} {
241+
t.Run(name, func(t *testing.T) {
242+
errs := validator(tc.spec.Ref, nil)
243+
require.Equal(t, len(tc.wantErrs), len(errs), "want", tc.wantErrs, "got", errs)
244+
for i := range tc.wantErrs {
245+
got := errs[i].Error()
246+
assert.Equal(t, tc.wantErrs[i], got)
247+
}
248+
})
249+
}
250+
}
251+
252+
func TestClusterCatalogURLsCELValidation(t *testing.T) {
253+
validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml")
254+
pth := "openAPIV3Schema.properties.status.properties.urls.properties.base"
255+
validator, found := validators["v1alpha1"][pth]
256+
require.True(t, found)
257+
for name, tc := range map[string]struct {
258+
urls ClusterCatalogURLs
259+
wantErrs []string
260+
}{
261+
"base is valid": {
262+
urls: ClusterCatalogURLs{
263+
Base: "https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio",
264+
},
265+
wantErrs: []string{},
266+
},
267+
"base is invalid, scheme is not one of http or https": {
268+
urls: ClusterCatalogURLs{
269+
Base: "file://somefilepath",
270+
},
271+
wantErrs: []string{
272+
fmt.Sprintf("%s: Invalid value: \"string\": scheme must be either http or https", pth),
273+
},
274+
},
275+
"base is invalid": {
276+
urls: ClusterCatalogURLs{
277+
Base: "notevenarealURL",
278+
},
279+
wantErrs: []string{
280+
fmt.Sprintf("%s: Invalid value: \"string\": must be a valid URL", pth),
281+
},
282+
},
283+
} {
284+
t.Run(name, func(t *testing.T) {
285+
errs := validator(tc.urls.Base, nil)
286+
fmt.Println(errs)
63287
require.Equal(t, len(tc.wantErrs), len(errs))
64288
for i := range tc.wantErrs {
65289
got := errs[i].Error()
@@ -83,13 +307,15 @@ func TestSourceCELValidation(t *testing.T) {
83307
Type: SourceTypeImage,
84308
},
85309
wantErrs: []string{
86-
fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage),
310+
fmt.Sprintf("%s: Invalid value: \"object\": image is required when source type is %s, and forbidden otherwise", pth, SourceTypeImage),
87311
},
88312
},
89313
"image source with required image field": {
90314
source: CatalogSource{
91-
Type: SourceTypeImage,
92-
Image: &ImageSource{},
315+
Type: SourceTypeImage,
316+
Image: &ImageSource{
317+
Ref: "docker.io/foo/bar:latest",
318+
},
93319
},
94320
wantErrs: []string{},
95321
},
@@ -123,13 +349,15 @@ func TestResolvedSourceCELValidation(t *testing.T) {
123349
Type: SourceTypeImage,
124350
},
125351
wantErrs: []string{
126-
fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage),
352+
fmt.Sprintf("%s: Invalid value: \"object\": image is required when source type is %s, and forbidden otherwise", pth, SourceTypeImage),
127353
},
128354
},
129355
"image source with required image field": {
130356
source: ResolvedCatalogSource{
131-
Type: SourceTypeImage,
132-
Image: &ResolvedImageSource{},
357+
Type: SourceTypeImage,
358+
Image: &ResolvedImageSource{
359+
Ref: "docker.io/foo/bar@sha256:abcdef123456789abcdef123456789abc",
360+
},
133361
},
134362
wantErrs: []string{},
135363
},
@@ -149,6 +377,7 @@ func TestResolvedSourceCELValidation(t *testing.T) {
149377

150378
// fieldValidatorsFromFile extracts the CEL validators by version and JSONPath from a CRD file and returns
151379
// a validator func for testing against samples.
380+
// nolint:unparam
152381
func fieldValidatorsFromFile(t *testing.T, crdFilePath string) map[string]map[string]CELValidateFunc {
153382
data, err := os.ReadFile(crdFilePath)
154383
require.NoError(t, err)

‎api/core/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 8 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)