switch to status.lastUnpacked for sole unpacked timestamp, use CEL va…

…lidation for spec.source and status.resolvedSource

Signed-off-by: Ankita Thomas <[email protected]>

README.md

@@ -24,7 +24,7 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A
       name: operatorhubio
     spec:
       source:
-        type: image
+        type: Image
         image:
           ref: quay.io/operatorhubio/catalog:latest
     EOF
@@ -57,7 +57,7 @@ Procedure steps marked with an asterisk (`*`) are likely to change with future A
         Image:
           Poll Interval:  10m0s
           Ref:            quay.io/operatorhubio/catalog:latest
-        Type:             image
+        Type:             Image
     Status:
       Conditions:
         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
           Last Unpacked:      2024-09-12T13:37:52Z
           Ref:                quay.io/operatorhubio/catalog:latest
           Resolved Ref:       quay.io/operatorhubio/catalog@sha256:4453a361198d39d0390fd8c1a7f07b5a5a3ae1e8dac9979ef0c4eba46299df16
-        Type:                 image
+        Type:                 Image
    Events:                   <none>
    ```
 

api/core/v1alpha1/clustercatalog_types.go

@@ -25,7 +25,7 @@ import (
 type SourceType string
 
 const (
-	SourceTypeImage SourceType = "image"
+	SourceTypeImage SourceType = "Image"
 
 	TypeProgressing = "Progressing"
 	TypeServing     = "Serving"
@@ -77,7 +77,7 @@ type ClusterCatalogSpec struct {
 	// Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image:
 	//
 	//  source:
-	//    type: image
+	//    type: Image
 	//    image:
 	//      ref: quay.io/operatorhubio/catalog:latest
 	//
@@ -128,48 +128,56 @@ type ClusterCatalogStatus struct {
 	//    lastUnpacked: "2024-09-10T12:22:13Z"
 	//    ref: quay.io/operatorhubio/catalog:latest
 	//    resolvedRef: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b
-	//  type: image
+	//  type: Image
 	// +optional
 	ResolvedSource *ResolvedCatalogSource `json:"resolvedSource,omitempty"`
 	// contentURL is a cluster-internal URL from which on-cluster components
 	// can read the content of a catalog
 	// +optional
 	ContentURL string `json:"contentURL,omitempty"`
 
-	// LastUnpacked represents the time when the
-	// ClusterCatalog object was last unpacked.
+	// lastUnpacked represents the time when the
+	// ClusterCatalog object was last unpacked successfully.
 	// +optional
 	LastUnpacked metav1.Time `json:"lastUnpacked,omitempty"`
 }
 
 // CatalogSource is a discriminated union of possible sources for a Catalog.
+// CatalogSource contains the sourcing information for a Catalog
+// +union
+// +kubebuilder:validation:XValidation:rule="self.type != 'Image' || has(self.image)",message="source type 'Image' requires image field"
+// +kubebuilder:validation:XValidation:rule="self.type == 'Image' || !has(self.image)",message="image field must only be set for source type 'Image'"
 type CatalogSource struct {
 	// type is a required reference to the type of source the catalog is sourced from.
 	//
-	// Allowed values are ["image"]
+	// Allowed values are ["Image"]
 	//
-	// When this field is set to "image", the ClusterCatalog content will be sourced from an OCI image.
+	// When this field is set to "Image", the ClusterCatalog content will be sourced from an OCI image.
 	// When using an image source, the image field must be set and must be the only field defined for this type.
 	//
 	// +unionDiscriminator
 	// +kubebuilder:validation:Enum:="Image"
 	// +kubebuilder:validation:Required
 	Type SourceType `json:"type"`
-	// 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.
+	// 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.
 	// +optional
 	Image *ImageSource `json:"image,omitempty"`
 }
 
 // ResolvedCatalogSource is a discriminated union of resolution information for a Catalog.
+// ResolvedCatalogSource contains the information about a sourced Catalog
+// +union
+// +kubebuilder:validation:XValidation:rule="self.type != 'Image' || has(self.image)",message="source type 'Image' requires image field"
+// +kubebuilder:validation:XValidation:rule="self.type == 'Image' || !has(self.image)",message="image field must only be set for source type 'Image'"
 type ResolvedCatalogSource struct {
 	// type is a reference to the type of source the catalog is sourced from.
 	//
-	// It will be set to one of the following values: ["image"].
+	// It will be set to one of the following values: ["Image"].
 	//
-	// When this field is set to "image", information about the resolved image source will be set in the 'image' field.
+	// When this field is set to "Image", information about the resolved image source will be set in the 'image' field.
 	//
 	// +unionDiscriminator
-	// +kubebuilder:validation:Enum:="image"
+	// +kubebuilder:validation:Enum:="Image"
 	// +kubebuilder:validation:Required
 	Type SourceType `json:"type"`
 	// image is a field containing resolution information for a catalog sourced from an image.
@@ -182,8 +190,6 @@ type ResolvedImageSource struct {
 	Ref string `json:"ref"`
 	// lastSuccessfulPollAttempt is the time when the resolved source was last successfully polled for new content.
 	LastSuccessfulPollAttempt metav1.Time `json:"lastSuccessfulPollAttempt"`
-	// LastUnpacked is the time when the catalog contents were successfully unpacked.
-	LastUnpacked metav1.Time `json:"lastUnpacked"`
 }
 
 // ImageSource enables users to define the information required for sourcing a Catalog from an OCI image

api/core/v1alpha1/clustercatalog_types_test.go

@@ -2,6 +2,7 @@ package v1alpha1
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"testing"
 	"time"
@@ -68,6 +69,114 @@ func TestPollIntervalCELValidationRules(t *testing.T) {
 	}
 }
 
+func TestSourceCELValidation(t *testing.T) {
+	validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml")
+	pth := "openAPIV3Schema.properties.spec.properties.source"
+	validator, found := validators["v1alpha1"][pth]
+	assert.True(t, found)
+	for name, tc := range map[string]struct {
+		source   CatalogSource
+		wantErrs []string
+	}{
+		"unsupported source": {
+			source: CatalogSource{
+				Type: "unsupportedType",
+			},
+			wantErrs: []string{},
+		},
+		"non-image source with disallowed image field": {
+			source: CatalogSource{
+				Type:  "notImageSourceType",
+				Image: &ImageSource{},
+			},
+			wantErrs: []string{
+				fmt.Sprintf("%s: Invalid value: \"object\": image field must only be set for source type '%s'", pth, SourceTypeImage),
+			},
+		},
+		"image source missing required image field": {
+			source: CatalogSource{
+				Type: SourceTypeImage,
+			},
+			wantErrs: []string{
+				fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage),
+			},
+		},
+		"image source with required image field": {
+			source: CatalogSource{
+				Type:  SourceTypeImage,
+				Image: &ImageSource{},
+			},
+			wantErrs: []string{},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.source) //nolint:gosec
+			require.NoError(t, err)
+			errs := validator(obj, nil)
+			fmt.Println(errs)
+			require.Equal(t, len(tc.wantErrs), len(errs))
+			for i := range tc.wantErrs {
+				got := errs[i].Error()
+				assert.Equal(t, tc.wantErrs[i], got)
+			}
+		})
+	}
+}
+
+func TestResolvedSourceCELValidation(t *testing.T) {
+	validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml")
+	pth := "openAPIV3Schema.properties.status.properties.resolvedSource"
+	validator, found := validators["v1alpha1"][pth]
+
+	assert.True(t, found)
+	for name, tc := range map[string]struct {
+		source   ResolvedCatalogSource
+		wantErrs []string
+	}{
+		"unsupported source": {
+			source: ResolvedCatalogSource{
+				Type: "unsupportedType",
+			},
+			wantErrs: []string{},
+		},
+		"non-image source with disallowed image field": {
+			source: ResolvedCatalogSource{
+				Type:  "notImageSourceType",
+				Image: &ResolvedImageSource{},
+			},
+			wantErrs: []string{
+				fmt.Sprintf("%s: Invalid value: \"object\": image field must only be set for source type '%s'", pth, SourceTypeImage),
+			},
+		},
+		"image source missing required image field": {
+			source: ResolvedCatalogSource{
+				Type: SourceTypeImage,
+			},
+			wantErrs: []string{
+				fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage),
+			},
+		},
+		"image source with required image field": {
+			source: ResolvedCatalogSource{
+				Type:  SourceTypeImage,
+				Image: &ResolvedImageSource{},
+			},
+			wantErrs: []string{},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.source) //nolint:gosec
+			require.NoError(t, err)
+			errs := validator(obj, nil)
+			require.Equal(t, len(tc.wantErrs), len(errs))
+			for i := range tc.wantErrs {
+				got := errs[i].Error()
+				assert.Equal(t, tc.wantErrs[i], got)
+			}
+		})
+	}
+}
+
 // fieldValidatorsFromFile extracts the CEL validators by version and JSONPath from a CRD file and returns
 // a validator func for testing against samples.
 func fieldValidatorsFromFile(t *testing.T, crdFilePath string) map[string]map[string]CELValidateFunc {

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

@@ -68,7 +68,7 @@ spec:
                   Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image:
 
                    source:
-                     type: image
+                     type: Image
                      image:
                        ref: quay.io/operatorhubio/catalog:latest
 
@@ -77,7 +77,7 @@ spec:
                   image:
                     description: 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
+                      is set to "Image" and must be the only field defined for this
                       type.
                     properties:
                       pollInterval:
@@ -107,16 +107,21 @@ spec:
                     description: |-
                       type is a required reference to the type of source the catalog is sourced from.
 
-                      Allowed values are ["image"]
+                      Allowed values are ["Image"]
 
-                      When this field is set to "image", the ClusterCatalog content will be sourced from an OCI image.
+                      When this field is set to "Image", the ClusterCatalog content will be sourced from an OCI image.
                       When using an image source, the image field must be set and must be the only field defined for this type.
                     enum:
                     - Image
                     type: string
                 required:
                 - type
                 type: object
+                x-kubernetes-validations:
+                - message: source type 'Image' requires image field
+                  rule: self.type != 'Image' || has(self.image)
+                - message: image field must only be set for source type 'Image'
+                  rule: self.type == 'Image' || !has(self.image)
             required:
             - source
             type: object
@@ -210,8 +215,8 @@ spec:
                 type: string
               lastUnpacked:
                 description: |-
-                  LastUnpacked represents the time when the
-                  ClusterCatalog object was last unpacked.
+                  lastUnpacked represents the time when the
+                  ClusterCatalog object was last unpacked successfully.
                 format: date-time
                 type: string
               resolvedSource:
@@ -226,7 +231,7 @@ spec:
                      lastUnpacked: "2024-09-10T12:22:13Z"
                      ref: quay.io/operatorhubio/catalog:latest
                      resolvedRef: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b
-                   type: image
+                   type: Image
                 properties:
                   image:
                     description: image is a field containing resolution information
@@ -237,34 +242,33 @@ spec:
                           resolved source was last successfully polled for new content.
                         format: date-time
                         type: string
-                      lastUnpacked:
-                        description: LastUnpacked is the time when the catalog contents
-                          were successfully unpacked.
-                        format: date-time
-                        type: string
                       ref:
                         description: ref contains the resolved sha256 image ref containing
                           Catalog contents.
                         type: string
                     required:
                     - lastSuccessfulPollAttempt
-                    - lastUnpacked
                     - ref
                     type: object
                   type:
                     description: |-
                       type is a reference to the type of source the catalog is sourced from.
 
-                      It will be set to one of the following values: ["image"].
+                      It will be set to one of the following values: ["Image"].
 
-                      When this field is set to "image", information about the resolved image source will be set in the 'image' field.
+                      When this field is set to "Image", information about the resolved image source will be set in the 'image' field.
                     enum:
-                    - image
+                    - Image
                     type: string
                 required:
                 - image
                 - type
                 type: object
+                x-kubernetes-validations:
+                - message: source type 'Image' requires image field
+                  rule: self.type != 'Image' || has(self.image)
+                - message: image field must only be set for source type 'Image'
+                  rule: self.type == 'Image' || !has(self.image)
             type: object
         required:
         - metadata

config/base/crd/kustomization.yaml

@@ -4,11 +4,3 @@
 resources:
 - bases/olm.operatorframework.io_clustercatalogs.yaml
 #+kubebuilder:scaffold:crdkustomizeresource
-
-patches:
-- path: patches/catalog_validation.yaml
-  target:
-    group: apiextensions.k8s.io
-    version: v1
-    kind: CustomResourceDefinition
-    name: catalogs.olm.operatorframework.io

config/base/default/clustercatalogs/default-catalogs.yaml

@@ -5,7 +5,7 @@ metadata:
   namespace: olmv1-system
 spec:
   source:
-    type: image
+    type: Image
     image:
       ref: quay.io/operatorhubio/catalog:latest
       pollInterval: 10m

config/samples/core_v1alpha1_clustercatalog.yaml

@@ -5,7 +5,7 @@ metadata:
 spec:
   priority: 0
   source:
-    type: image
+    type: Image
     image:
       pollInterval: 24h
       ref: quay.io/operatorhubio/catalog:latest