Skip to content

Commit 29d21c7

Browse files
grokspawnankitathomas
grokspawn
and
ankitathomas
authored
⚠️ types tightening (#384)
* types tightening * switch to status.lastUnpacked for sole unpacked timestamp, use CEL validation for spec.source and status.resolvedSource Signed-off-by: Ankita Thomas <[email protected]> * rename unpacker result's lastTransitionTime to unpackTime, set observedGeneration in conditions * restructure validation checks to only check for image source type Signed-off-by: Ankita Thomas <[email protected]> * fail on missing validator in types test Signed-off-by: Ankita Thomas <[email protected]> * don't skip bad image refs when testing for needsUnpack allow bad image refs to pass through to unpacker to propagate error to Progressing status Signed-off-by: Ankita Thomas <[email protected]> * compare catalog generation and condition ObservedGeneration for needsUnpack Signed-off-by: Ankita Thomas <[email protected]> --------- Signed-off-by: Ankita Thomas <[email protected]> Co-authored-by: Ankita Thomas <[email protected]>
1 parent db8617f commit 29d21c7

16 files changed

+277
-230
lines changed

‎README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A
2424
name: operatorhubio
2525
spec:
2626
source:
27-
type: image
27+
type: Image
2828
image:
2929
ref: quay.io/operatorhubio/catalog:latest
3030
EOF
@@ -57,7 +57,7 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A
5757
Image:
5858
Poll Interval: 10m0s
5959
Ref: quay.io/operatorhubio/catalog:latest
60-
Type: image
60+
Type: Image
6161
Status:
6262
Conditions:
6363
Last Transition Time: 2024-09-12T13:37:53Z
@@ -79,7 +79,7 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A
7979
Last Unpacked: 2024-09-12T13:37:52Z
8080
Ref: quay.io/operatorhubio/catalog:latest
8181
Resolved Ref: quay.io/operatorhubio/catalog@sha256:4453a361198d39d0390fd8c1a7f07b5a5a3ae1e8dac9979ef0c4eba46299df16
82-
Type: image
82+
Type: Image
8383
Events: <none>
8484
```
8585

‎api/core/v1alpha1/clustercatalog_types.go‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
type SourceType string
2626

2727
const (
28-
SourceTypeImage SourceType = "image"
28+
SourceTypeImage SourceType = "Image"
2929

3030
TypeProgressing = "Progressing"
3131
TypeServing = "Serving"
@@ -77,7 +77,7 @@ type ClusterCatalogSpec struct {
7777
// Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image:
7878
//
7979
// source:
80-
// type: image
80+
// type: Image
8181
// image:
8282
// ref: quay.io/operatorhubio/catalog:latest
8383
//
@@ -91,7 +91,7 @@ type ClusterCatalogSpec struct {
9191
// When omitted, the default priority is 0.
9292
// +kubebuilder:default:=0
9393
// +optional
94-
Priority int32 `json:"priority,omitempty"`
94+
Priority int32 `json:"priority"`
9595
}
9696

9797
// ClusterCatalogStatus defines the observed state of ClusterCatalog
@@ -128,51 +128,54 @@ type ClusterCatalogStatus struct {
128128
// lastUnpacked: "2024-09-10T12:22:13Z"
129129
// ref: quay.io/operatorhubio/catalog:latest
130130
// resolvedRef: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b
131-
// type: image
131+
// type: Image
132132
// +optional
133133
ResolvedSource *ResolvedCatalogSource `json:"resolvedSource,omitempty"`
134134
// contentURL is a cluster-internal URL from which on-cluster components
135135
// can read the content of a catalog
136136
// +optional
137137
ContentURL string `json:"contentURL,omitempty"`
138-
// observedGeneration is the most recent generation observed for this ClusterCatalog. It corresponds to the
139-
// ClusterCatalog's generation, which is updated on mutation by the API Server.
140-
// +optional
141-
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
138+
142139
// lastUnpacked represents the time when the
143-
// ClusterCatalog object was last unpacked.
140+
// ClusterCatalog object was last unpacked successfully.
144141
// +optional
145142
LastUnpacked metav1.Time `json:"lastUnpacked,omitempty"`
146143
}
147144

148145
// CatalogSource is a discriminated union of possible sources for a Catalog.
146+
// CatalogSource contains the sourcing information for a Catalog
147+
// +union
148+
// +kubebuilder:validation:XValidation:rule="self.type == 'Image' && has(self.image)",message="source type 'Image' requires image field"
149149
type CatalogSource struct {
150150
// type is a required reference to the type of source the catalog is sourced from.
151151
//
152-
// Allowed values are ["image"]
152+
// Allowed values are ["Image"]
153153
//
154-
// When this field is set to "image", the ClusterCatalog content will be sourced from an OCI image.
154+
// When this field is set to "Image", the ClusterCatalog content will be sourced from an OCI image.
155155
// When using an image source, the image field must be set and must be the only field defined for this type.
156156
//
157157
// +unionDiscriminator
158-
// +kubebuilder:validation:Enum:="image"
158+
// +kubebuilder:validation:Enum:="Image"
159159
// +kubebuilder:validation:Required
160160
Type SourceType `json:"type"`
161-
// image is used to configure how catalog contents are sourced from an OCI image. This field must be set when type is set to "image" and must be the only field defined for this type.
161+
// image is used to configure how catalog contents are sourced from an OCI image. This field must be set when type is set to "Image" and must be the only field defined for this type.
162162
// +optional
163163
Image *ImageSource `json:"image,omitempty"`
164164
}
165165

166166
// ResolvedCatalogSource is a discriminated union of resolution information for a Catalog.
167+
// ResolvedCatalogSource contains the information about a sourced Catalog
168+
// +union
169+
// +kubebuilder:validation:XValidation:rule="self.type == 'Image' && has(self.image)",message="source type 'Image' requires image field"
167170
type ResolvedCatalogSource struct {
168171
// type is a reference to the type of source the catalog is sourced from.
169172
//
170-
// It will be set to one of the following values: ["image"].
173+
// It will be set to one of the following values: ["Image"].
171174
//
172-
// When this field is set to "image", information about the resolved image source will be set in the 'image' field.
175+
// When this field is set to "Image", information about the resolved image source will be set in the 'image' field.
173176
//
174177
// +unionDiscriminator
175-
// +kubebuilder:validation:Enum:="image"
178+
// +kubebuilder:validation:Enum:="Image"
176179
// +kubebuilder:validation:Required
177180
Type SourceType `json:"type"`
178181
// image is a field containing resolution information for a catalog sourced from an image.
@@ -181,14 +184,10 @@ type ResolvedCatalogSource struct {
181184

182185
// ResolvedImageSource provides information about the resolved source of a Catalog sourced from an image.
183186
type ResolvedImageSource struct {
184-
// ref is the reference to a container image containing Catalog contents.
187+
// ref contains the resolved sha256 image ref containing Catalog contents.
185188
Ref string `json:"ref"`
186-
// resolvedRef is the resolved sha256 image ref containing Catalog contents.
187-
ResolvedRef string `json:"resolvedRef"`
188-
// lastPollAttempt is the time when the source image was last polled for new content.
189-
LastPollAttempt metav1.Time `json:"lastPollAttempt"`
190-
// lastUnpacked is the last time when the Catalog contents were successfully unpacked.
191-
LastUnpacked metav1.Time `json:"lastUnpacked"`
189+
// lastSuccessfulPollAttempt is the time when the resolved source was last successfully polled for new content.
190+
LastSuccessfulPollAttempt metav1.Time `json:"lastSuccessfulPollAttempt"`
192191
}
193192

194193
// ImageSource enables users to define the information required for sourcing a Catalog from an OCI image

‎api/core/v1alpha1/clustercatalog_types_test.go‎

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package v1alpha1
22

33
import (
44
"context"
5+
"fmt"
56
"os"
67
"testing"
78
"time"
@@ -23,7 +24,7 @@ func TestPollIntervalCELValidationRules(t *testing.T) {
2324
validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml")
2425
pth := "openAPIV3Schema.properties.spec"
2526
validator, found := validators["v1alpha1"][pth]
26-
assert.True(t, found)
27+
require.True(t, found)
2728

2829
for name, tc := range map[string]struct {
2930
spec ClusterCatalogSpec
@@ -68,6 +69,84 @@ func TestPollIntervalCELValidationRules(t *testing.T) {
6869
}
6970
}
7071

72+
func TestSourceCELValidation(t *testing.T) {
73+
validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml")
74+
pth := "openAPIV3Schema.properties.spec.properties.source"
75+
validator, found := validators["v1alpha1"][pth]
76+
require.True(t, found)
77+
for name, tc := range map[string]struct {
78+
source CatalogSource
79+
wantErrs []string
80+
}{
81+
"image source missing required image field": {
82+
source: CatalogSource{
83+
Type: SourceTypeImage,
84+
},
85+
wantErrs: []string{
86+
fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage),
87+
},
88+
},
89+
"image source with required image field": {
90+
source: CatalogSource{
91+
Type: SourceTypeImage,
92+
Image: &ImageSource{},
93+
},
94+
wantErrs: []string{},
95+
},
96+
} {
97+
t.Run(name, func(t *testing.T) {
98+
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.source) //nolint:gosec
99+
require.NoError(t, err)
100+
errs := validator(obj, nil)
101+
fmt.Println(errs)
102+
require.Equal(t, len(tc.wantErrs), len(errs))
103+
for i := range tc.wantErrs {
104+
got := errs[i].Error()
105+
assert.Equal(t, tc.wantErrs[i], got)
106+
}
107+
})
108+
}
109+
}
110+
111+
func TestResolvedSourceCELValidation(t *testing.T) {
112+
validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml")
113+
pth := "openAPIV3Schema.properties.status.properties.resolvedSource"
114+
validator, found := validators["v1alpha1"][pth]
115+
116+
require.True(t, found)
117+
for name, tc := range map[string]struct {
118+
source ResolvedCatalogSource
119+
wantErrs []string
120+
}{
121+
"image source missing required image field": {
122+
source: ResolvedCatalogSource{
123+
Type: SourceTypeImage,
124+
},
125+
wantErrs: []string{
126+
fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage),
127+
},
128+
},
129+
"image source with required image field": {
130+
source: ResolvedCatalogSource{
131+
Type: SourceTypeImage,
132+
Image: &ResolvedImageSource{},
133+
},
134+
wantErrs: []string{},
135+
},
136+
} {
137+
t.Run(name, func(t *testing.T) {
138+
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.source) //nolint:gosec
139+
require.NoError(t, err)
140+
errs := validator(obj, nil)
141+
require.Equal(t, len(tc.wantErrs), len(errs))
142+
for i := range tc.wantErrs {
143+
got := errs[i].Error()
144+
assert.Equal(t, tc.wantErrs[i], got)
145+
}
146+
})
147+
}
148+
}
149+
71150
// fieldValidatorsFromFile extracts the CEL validators by version and JSONPath from a CRD file and returns
72151
// a validator func for testing against samples.
73152
func fieldValidatorsFromFile(t *testing.T, crdFilePath string) map[string]map[string]CELValidateFunc {

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

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml‎

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ spec:
6868
Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image:
6969
7070
source:
71-
type: image
71+
type: Image
7272
image:
7373
ref: quay.io/operatorhubio/catalog:latest
7474
@@ -77,7 +77,7 @@ spec:
7777
image:
7878
description: image is used to configure how catalog contents are
7979
sourced from an OCI image. This field must be set when type
80-
is set to "image" and must be the only field defined for this
80+
is set to "Image" and must be the only field defined for this
8181
type.
8282
properties:
8383
pollInterval:
@@ -107,16 +107,19 @@ spec:
107107
description: |-
108108
type is a required reference to the type of source the catalog is sourced from.
109109
110-
Allowed values are ["image"]
110+
Allowed values are ["Image"]
111111
112-
When this field is set to "image", the ClusterCatalog content will be sourced from an OCI image.
112+
When this field is set to "Image", the ClusterCatalog content will be sourced from an OCI image.
113113
When using an image source, the image field must be set and must be the only field defined for this type.
114114
enum:
115-
- image
115+
- Image
116116
type: string
117117
required:
118118
- type
119119
type: object
120+
x-kubernetes-validations:
121+
- message: source type 'Image' requires image field
122+
rule: self.type == 'Image' && has(self.image)
120123
required:
121124
- source
122125
type: object
@@ -211,15 +214,9 @@ spec:
211214
lastUnpacked:
212215
description: |-
213216
lastUnpacked represents the time when the
214-
ClusterCatalog object was last unpacked.
217+
ClusterCatalog object was last unpacked successfully.
215218
format: date-time
216219
type: string
217-
observedGeneration:
218-
description: |-
219-
observedGeneration is the most recent generation observed for this ClusterCatalog. It corresponds to the
220-
ClusterCatalog's generation, which is updated on mutation by the API Server.
221-
format: int64
222-
type: integer
223220
resolvedSource:
224221
description: |-
225222
resolvedSource contains information about the resolved source based on the source type.
@@ -232,50 +229,42 @@ spec:
232229
lastUnpacked: "2024-09-10T12:22:13Z"
233230
ref: quay.io/operatorhubio/catalog:latest
234231
resolvedRef: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b
235-
type: image
232+
type: Image
236233
properties:
237234
image:
238235
description: image is a field containing resolution information
239236
for a catalog sourced from an image.
240237
properties:
241-
lastPollAttempt:
242-
description: lastPollAttempt is the time when the source image
243-
was last polled for new content.
244-
format: date-time
245-
type: string
246-
lastUnpacked:
247-
description: lastUnpacked is the last time when the Catalog
248-
contents were successfully unpacked.
238+
lastSuccessfulPollAttempt:
239+
description: lastSuccessfulPollAttempt is the time when the
240+
resolved source was last successfully polled for new content.
249241
format: date-time
250242
type: string
251243
ref:
252-
description: ref is the reference to a container image containing
244+
description: ref contains the resolved sha256 image ref containing
253245
Catalog contents.
254246
type: string
255-
resolvedRef:
256-
description: resolvedRef is the resolved sha256 image ref
257-
containing Catalog contents.
258-
type: string
259247
required:
260-
- lastPollAttempt
261-
- lastUnpacked
248+
- lastSuccessfulPollAttempt
262249
- ref
263-
- resolvedRef
264250
type: object
265251
type:
266252
description: |-
267253
type is a reference to the type of source the catalog is sourced from.
268254
269-
It will be set to one of the following values: ["image"].
255+
It will be set to one of the following values: ["Image"].
270256
271-
When this field is set to "image", information about the resolved image source will be set in the 'image' field.
257+
When this field is set to "Image", information about the resolved image source will be set in the 'image' field.
272258
enum:
273-
- image
259+
- Image
274260
type: string
275261
required:
276262
- image
277263
- type
278264
type: object
265+
x-kubernetes-validations:
266+
- message: source type 'Image' requires image field
267+
rule: self.type == 'Image' && has(self.image)
279268
type: object
280269
required:
281270
- metadata

‎config/base/crd/kustomization.yaml‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,3 @@
44
resources:
55
- bases/olm.operatorframework.io_clustercatalogs.yaml
66
#+kubebuilder:scaffold:crdkustomizeresource
7-
8-
patches:
9-
- path: patches/catalog_validation.yaml
10-
target:
11-
group: apiextensions.k8s.io
12-
version: v1
13-
kind: CustomResourceDefinition
14-
name: catalogs.olm.operatorframework.io

‎config/base/crd/patches/catalog_validation.yaml‎

Lines changed: 0 additions & 6 deletions
This file was deleted.

0 commit comments

Comments
 (0)