From 6d095489c3968166c6784e3244df82cd62e2d869 Mon Sep 17 00:00:00 2001 From: everettraven Date: Tue, 22 Oct 2024 10:43:11 -0400 Subject: [PATCH] API updates based on external review Signed-off-by: everettraven Signed-off-by: Mikalai Radchuk --- api/core/v1alpha1/clustercatalog_types.go | 268 +++++++++++---- .../v1alpha1/clustercatalog_types_test.go | 287 ++++++++++++++-- api/core/v1alpha1/zz_generated.deepcopy.go | 14 +- ....operatorframework.io_clustercatalogs.yaml | 322 +++++++++++++----- .../clustercatalogs/default-catalogs.yaml | 2 +- .../samples/core_v1alpha1_clustercatalog.yaml | 2 +- go.mod | 2 +- .../core/clustercatalog_controller.go | 49 ++- .../core/clustercatalog_controller_test.go | 93 ++--- internal/source/containers_image.go | 12 +- internal/source/unpacker.go | 4 + test/e2e/unpack_test.go | 2 +- 12 files changed, 786 insertions(+), 271 deletions(-) diff --git a/api/core/v1alpha1/clustercatalog_types.go b/api/core/v1alpha1/clustercatalog_types.go index 895f4068..f91cdce2 100644 --- a/api/core/v1alpha1/clustercatalog_types.go +++ b/api/core/v1alpha1/clustercatalog_types.go @@ -24,6 +24,9 @@ import ( // +enum type SourceType string +// AvailabilityMode defines the availability of the catalog +type AvailabilityMode string + const ( SourceTypeImage SourceType = "Image" @@ -31,9 +34,9 @@ const ( TypeServing = "Serving" // Serving reasons - ReasonAvailable = "Available" - ReasonUnavailable = "Unavailable" - ReasonDisabled = "Disabled" + ReasonAvailable = "Available" + ReasonUnavailable = "Unavailable" + ReasonUserSpecifiedUnavailable = "UserSpecifiedUnavailable" // Progressing reasons ReasonSucceeded = "Succeeded" @@ -42,8 +45,8 @@ const ( MetadataNameLabel = "olm.operatorframework.io/metadata.name" - AvailabilityEnabled = "Enabled" - AvailabilityDisabled = "Disabled" + AvailabilityModeAvailable AvailabilityMode = "Available" + AvailabilityModeUnavailable AvailabilityMode = "Unavailable" ) //+kubebuilder:object:root=true @@ -56,10 +59,24 @@ const ( // ClusterCatalog enables users to make File-Based Catalog (FBC) catalog data available to the cluster. // For more information on FBC, see https://olm.operatorframework.io/docs/reference/file-based-catalogs/#docs type ClusterCatalog struct { - metav1.TypeMeta `json:",inline"` + metav1.TypeMeta `json:",inline"` + + // metadata is the standard object's metadata. + // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata metav1.ObjectMeta `json:"metadata"` - Spec ClusterCatalogSpec `json:"spec"` + // spec is the desired state of the ClusterCatalog. + // spec is required. + // The controller will work to ensure that the desired + // catalog is unpacked and served over the catalog content HTTP server. + // +kubebuilder:validation:Required + Spec ClusterCatalogSpec `json:"spec"` + + // status contains information about the state of the ClusterCatalog such as: + // - Whether or not the catalog contents are being served via the catalog content HTTP server + // - Whether or not the ClusterCatalog is progressing to a new state + // - A reference to the source from which the catalog contents were retrieved + // +optional Status ClusterCatalogStatus `json:"status,omitempty"` } @@ -68,15 +85,28 @@ type ClusterCatalog struct { // ClusterCatalogList contains a list of ClusterCatalog type ClusterCatalogList struct { metav1.TypeMeta `json:",inline"` + + // metadata is the standard object's metadata. + // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata metav1.ListMeta `json:"metadata"` + // items is a list of ClusterCatalogs. + // items is required. + // +kubebuilder:validation:Required Items []ClusterCatalog `json:"items"` } // ClusterCatalogSpec defines the desired state of ClusterCatalog -// +kubebuilder:validation:XValidation:rule="!has(self.source.image.pollInterval) || (self.source.image.ref.find('@sha256:') == \"\")",message="cannot specify PollInterval while using digest-based image" type ClusterCatalogSpec struct { - // source is a required field that allows the user to define the source of a Catalog that contains catalog metadata in the File-Based Catalog (FBC) format. + // source allows a user to define the source of a catalog. + // A "catalog" contains information on content that can be installed on a cluster. + // Providing a catalog source makes the contents of the catalog discoverable and usable by + // other on-cluster components. + // These on-cluster components may do a variety of things with this information, such as + // presenting the content in a GUI dashboard or installing content from the catalog on the cluster. + // The catalog source must contain catalog metadata in the File-Based Catalog (FBC) format. + // For more information on FBC, see https://olm.operatorframework.io/docs/reference/file-based-catalogs/#docs. + // source is a required field. // // Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image: // @@ -85,103 +115,136 @@ type ClusterCatalogSpec struct { // image: // ref: quay.io/operatorhubio/catalog:latest // - // For more information on FBC, see https://olm.operatorframework.io/docs/reference/file-based-catalogs/#docs + // +kubebuilder:validation:Required Source CatalogSource `json:"source"` - // priority is an optional field that allows the user to define a priority for a ClusterCatalog. + // priority allows the user to define a priority for a ClusterCatalog. + // priority is optional. + // // A ClusterCatalog's priority is used by clients as a tie-breaker between ClusterCatalogs that meet the client's requirements. - // For example, in the case where multiple ClusterCatalogs provide the same bundle. - // A higher number means higher priority. Negative numbers are also accepted. - // When omitted, the default priority is 0. + // A higher number means higher priority. + // + // It is up to clients to decide how to handle scenarios where multiple ClusterCatalogs with the same priority meet their requirements. + // When deciding how to break the tie in this scenario, it is recommended that clients prompt their users for additional input. + // + // When omitted, the default priority is 0 because that is the zero value of integers. + // + // Negative numbers can be used to specify a priority lower than the default. + // Positive numbers can be used to specify a priority higher than the default. + // + // The lowest possible value is -2147483648. + // The highest possible value is 2147483647. + // // +kubebuilder:default:=0 + // +kubebuilder:validation:minimum:=-2147483648 + // +kubebuilder:validation:maximum:=2147483647 // +optional Priority int32 `json:"priority"` - // Availability is an optional field that allows users to define whether the ClusterCatalog is utilized by the operator-controller. + // availabilityMode allows users to define how the ClusterCatalog is made available to clients on the cluster. + // availabilityMode is optional. + // + // Allowed values are "Available" and "Unavailable" and omitted. // - // Allowed values are : ["Enabled", "Disabled"]. - // If set to "Enabled", the catalog will be used for updates, serving contents, and package installations. + // When omitted, the default value is "Available". // - // If set to "Disabled", catalogd will stop serving the catalog and the cached data will be removed. + // When set to "Available", the catalog contents will be unpacked and served over the catalog content HTTP server. + // Setting the availabilityMode to "Available" tells clients that they should consider this ClusterCatalog + // and its contents as usable. // - // If unspecified, the default value is "Enabled" + // When set to "Unavailable", the catalog contents will no longer be served over the catalog content HTTP server. + // When set to this availabilityMode it should be interpreted the same as the ClusterCatalog not existing. + // Setting the availabilityMode to "Unavailable" can be useful in scenarios where a user may not want + // to delete the ClusterCatalog all together, but would still like it to be treated as if it doesn't exist. // - // +kubebuilder:validation:Enum="Disabled";"Enabled" - // +kubebuilder:default="Enabled" + // +kubebuilder:validation:Enum:="Unavailable";"Available" + // +kubebuilder:default:="Available" // +optional - Availability string `json:"availability,omitempty"` + AvailabilityMode AvailabilityMode `json:"availabilityMode,omitempty"` } // ClusterCatalogStatus defines the observed state of ClusterCatalog type ClusterCatalogStatus struct { // conditions is a representation of the current state for this ClusterCatalog. - // The status is represented by a set of "conditions". // - // Each condition is generally structured in the following format: - // - Type: a string representation of the condition type. More or less the condition "name". - // - Status: a string representation of the state of the condition. Can be one of ["True", "False", "Unknown"]. - // - Reason: a string representation of the reason for the current state of the condition. Typically useful for building automation around particular Type+Reason combinations. - // - Message: a human-readable message that further elaborates on the state of the condition. + // The current condition types are Serving and Progressing. // - // The current set of condition types are: - // - "Serving", which represents whether or not the contents of the catalog are being served via the HTTP(S) web server. - // - "Progressing", which represents whether or not the ClusterCatalog is progressing towards a new state. + // The Serving condition is used to represent whether or not the contents of the catalog is being served via the HTTP(S) web server. + // When it has a status of True and a reason of Available, the contents of the catalog are being served. + // When it has a status of False and a reason of Unavailable, the contents of the catalog are not being served because the contents are not yet available. + // When it has a status of False and a reason of UserSpecifiedUnavailable, the contents of the catalog are not being served because the catalog has been intentionally marked as unavailable. // - // The current set of reasons are: - // - "Succeeded", this reason is set on the "Progressing" condition when progressing to a new state is successful. - // - "Blocked", this reason is set on the "Progressing" condition when the ClusterCatalog controller has encountered an error that requires manual intervention for recovery. - // - "Retrying", this reason is set on the "Progressing" condition when the ClusterCatalog controller has encountered an error that might be resolvable on subsequent reconciliation attempts. - // - "Available", this reason is set on the "Serving" condition when the contents of the ClusterCatalog are being served via an endpoint on the HTTP(S) web server. - // - "Unavailable", this reason is set on the "Serving" condition when there is not an endpoint on the HTTP(S) web server that is serving the contents of the ClusterCatalog. + // The Progressing condition is used to represent whether or not the ClusterCatalog is progressing or is ready to progress towards a new state. + // When it has a status of True and a reason of Retrying, there was an error in the progression of the ClusterCatalog that may be resolved on subsequent reconciliation attempts. + // When it has a status of True and a reason of Succeeded, the ClusterCatalog has successfully progressed to a new state and is ready to continue progressing. + // When it has a status of False and a reason of Blocked, there was an error in the progression of the ClusterCatalog that requires manual intervention for recovery. // + // In the case that the Serving condition is True with reason Available and Progressing is True with reason Retrying, the previously fetched + // catalog contents are still being served via the HTTP(S) web server while we are progressing towards serving a new version of the catalog + // contents. This could occur when we've initially fetched the latest contents from the source for this catalog and when polling for changes + // to the contents we identify that there are updates to the contents. + // + // +listType=map + // +listMapKey=type // +optional Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` // resolvedSource contains information about the resolved source based on the source type. - // - // Below is an example of a resolved source for an image source: - // resolvedSource: - // - // image: - // lastSuccessfulPollAttempt: "2024-09-10T12:22:13Z" - // ref: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b - // type: Image // +optional ResolvedSource *ResolvedCatalogSource `json:"resolvedSource,omitempty"` // urls contains the URLs that can be used to access the catalog. // +optional URLs *ClusterCatalogURLs `json:"urls,omitempty"` - // lastUnpacked represents the time when the - // ClusterCatalog object was last unpacked successfully. + // lastUnpacked represents the last time the contents of the + // catalog were extracted from their source format. As an example, + // when using an Image source, the OCI image will be pulled and the + // image layers written to a file-system backed cache. We refer to the + // act of this extraction from the source format as "unpacking". // +optional - LastUnpacked metav1.Time `json:"lastUnpacked,omitempty"` + LastUnpacked *metav1.Time `json:"lastUnpacked,omitempty"` } // ClusterCatalogURLs contains the URLs that can be used to access the catalog. type ClusterCatalogURLs struct { - // base is a required cluster-internal URL which provides API access for this ClusterCatalog. - // A suffix API access path can be added to retrieve catalog data for the ClusterCatalog. - // Currently, a 'v1' API access provides complete FBC retrival via the path "/api/v1/all", with the general form `{base}/api/v1/all`. + // base is a cluster-internal URL that provides endpoints for + // accessing the content of the catalog. + // + // It is expected that clients append the path for the endpoint they wish + // to access. + // + // Currently, only a single endpoint is served and is accessible at the path + // /api/v1. + // + // The endpoints served for the v1 API are: + // - /all - this endpoint returns the entirety of the catalog contents in the FBC format + // + // As the needs of users and clients of the evolve, new endpoints may be added. + // // +kubebuilder:validation:Required + // +kubebuilder:validation:MaxLength:=525 + // +kubebuilder:validation:XValidation:rule="isURL(self)",message="must be a valid URL" + // +kubebuilder:validation:XValidation:rule="isURL(self) ? (url(self).getScheme() == \"http\" || url(self).getScheme() == \"https\") : true",message="scheme must be either http or https" Base string `json:"base"` } // 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="has(self.type) && self.type == 'Image' ? has(self.image) : !has(self.image)",message="image is required when source type is Image, and forbidden otherwise" type CatalogSource struct { - // type is a required reference to the type of source the catalog is sourced from. + // type is a reference to the type of source the catalog is sourced from. + // type is required. // - // Allowed values are ["Image"] + // The only allowed value is "Image". // - // When this field is set to "Image", the ClusterCatalog content will be sourced from an OCI image. + // When 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 is required when type is Image, and forbidden otherwise. // +optional Image *ImageSource `json:"image,omitempty"` } @@ -189,49 +252,104 @@ type CatalogSource struct { // 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="has(self.type) && self.type == 'Image' ? has(self.image) : !has(self.image)",message="image is required when source type is Image, and forbidden otherwise" type ResolvedCatalogSource struct { // type is a reference to the type of source the catalog is sourced from. + // type is required. // - // It will be set to one of the following values: ["Image"]. + // The only allowed value is "Image". // - // When this field is set to "Image", information about the resolved image source will be set in the 'image' field. + // When set to "Image", information about the resolved image source will be set in the 'image' field. // // +unionDiscriminator // +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. + // This field must be set when type is Image, and forbidden otherwise. Image *ResolvedImageSource `json:"image"` } // ResolvedImageSource provides information about the resolved source of a Catalog sourced from an image. type ResolvedImageSource struct { - // ref contains the resolved sha256 image ref containing Catalog contents. + // ref contains the resolved image digest-based reference. + // The digest format is used so users can use other tooling to fetch the exact + // OCI manifests that were used to extract the catalog contents. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MaxLength:=1000 + // +kubebuilder:validation:XValidation:rule="self.matches('^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])((\\\\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(:[0-9]+)?\\\\b')",message="must start with a valid domain. valid domains must be alphanumeric characters (lowercase and uppercase) separated by the \".\" character." + // +kubebuilder:validation:XValidation:rule="self.find('(\\\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)') != \"\"",message="a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters." + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') != \"\"",message="must end with a digest" + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') != \"\" ? self.find('(@.*:)').matches('(@[A-Za-z][A-Za-z0-9]*([-_+.][A-Za-z][A-Za-z0-9]*)*[:])') : true",message="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." + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') != \"\" ? self.find(':.*$').substring(1).size() >= 32 : true",message="digest is not valid. the encoded string must be at least 32 characters" + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') != \"\" ? self.find(':.*$').matches(':[0-9A-Fa-f]*$') : true",message="digest is not valid. the encoded string must only contain hex characters (A-F, a-f, 0-9)" Ref string `json:"ref"` - // lastSuccessfulPollAttempt is the time when the resolved source was last successfully polled for new content. - LastSuccessfulPollAttempt metav1.Time `json:"lastSuccessfulPollAttempt"` } // ImageSource enables users to define the information required for sourcing a Catalog from an OCI image +// +// If we see that there is a possibly valid digest-based image reference AND pollIntervalMinutes is specified, +// reject the resource since there is no use in polling a digest-based image reference. +// +kubebuilder:validation:XValidation:rule="self.ref.find('(@.*:)') != \"\" ? !has(self.pollIntervalMinutes) : true",message="cannot specify pollIntervalMinutes while using digest-based image" type ImageSource struct { - // ref is a required field that allows the user to define the reference to a container image containing Catalog contents. - // Examples: - // ref: quay.io/operatorhubio/catalog:latest # image reference - // ref: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b # image reference with sha256 digest + // ref allows users to define the reference to a container image containing Catalog contents. + // ref is required. + // ref can not be more than 1000 characters. + // + // A reference can be broken down into 3 parts - the domain, name, and identifier. + // + // The domain is typically the registry where an image is located. + // It must be alphanumeric characters (lowercase and uppercase) separated by the "." character. + // Hyphenation is allowed, but the domain must start and end with alphanumeric characters. + // Specifying a port to use is also allowed by adding the ":" character followed by numeric values. + // The port must be the last value in the domain. + // Some examples of valid domain values are "registry.mydomain.io", "quay.io", "my-registry.io:8080". + // + // The name is typically the repository in the registry where an image is located. + // It must contain lowercase alphanumeric characters separated only by the ".", "_", "__", "-" characters. + // Multiple names can be concatenated with the "/" character. + // The domain and name are combined using the "/" character. + // Some examples of valid name values are "operatorhubio/catalog", "catalog", "my-catalog.prod". + // An example of the domain and name parts of a reference being combined is "quay.io/operatorhubio/catalog". + // + // The identifier is typically the tag or digest for an image reference and is present at the end of the reference. + // It starts with a separator character used to distinguish the end of the name and beginning of the identifier. + // For a digest-based reference, the "@" character is the separator. + // For a tag-based reference, the ":" character is the separator. + // An identifier is required in the reference. + // + // Digest-based references must contain an algorithm reference immediately after the "@" separator. + // The algorithm reference must be followed by the ":" character and an encoded string. + // The algorithm must start with an uppercase or lowercase alpha character followed by alphanumeric characters and may contain the "-", "_", "+", and "." characters. + // Some examples of valid algorithm values are "sha256", "sha256+b64u", "multihash+base58". + // The encoded string following the algorithm must be hex digits (a-f, A-F, 0-9) and must be a minimum of 32 characters. + // + // Tag-based references must begin with a word character (alphanumeric + "_") followed by word characters or ".", and "-" characters. + // The tag must not be longer than 127 characters. + // + // An example of a valid digest-based image reference is "quay.io/operatorhubio/catalog@sha256:200d4ddb2a73594b91358fe6397424e975205bfbe44614f5846033cad64b3f05" + // An example of a valid tag-based image reference is "quay.io/operatorhubio/catalog:latest" + // + // +kubebuilder:validation:Required + // +kubebuilder:validation:MaxLength:=1000 + // +kubebuilder:validation:XValidation:rule="self.matches('^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])((\\\\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(:[0-9]+)?\\\\b')",message="must start with a valid domain. valid domains must be alphanumeric characters (lowercase and uppercase) separated by the \".\" character." + // +kubebuilder:validation:XValidation:rule="self.find('(\\\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)') != \"\"",message="a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters." + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') != \"\" || self.find(':.*$') != \"\"",message="must end with a digest or a tag" + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') == \"\" ? (self.find(':.*$') != \"\" ? self.find(':.*$').substring(1).size() <= 127 : true) : true",message="tag is invalid. the tag must not be more than 127 characters" + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') == \"\" ? (self.find(':.*$') != \"\" ? self.find(':.*$').matches(':[\\\\w][\\\\w.-]*$') : true) : true",message="tag is invalid. valid tags must begin with a word character (alphanumeric + \"_\") followed by word characters or \".\", and \"-\" characters" + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') != \"\" ? self.find('(@.*:)').matches('(@[A-Za-z][A-Za-z0-9]*([-_+.][A-Za-z][A-Za-z0-9]*)*[:])') : true",message="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." + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') != \"\" ? self.find(':.*$').substring(1).size() >= 32 : true",message="digest is not valid. the encoded string must be at least 32 characters" + // +kubebuilder:validation:XValidation:rule="self.find('(@.*:)') != \"\" ? self.find(':.*$').matches(':[0-9A-Fa-f]*$') : true",message="digest is not valid. the encoded string must only contain hex characters (A-F, a-f, 0-9)" Ref string `json:"ref"` - // pollInterval is an optional field that allows the user to set the interval at which the image source should be polled for new content. - // It must be specified as a duration. - // It must not be specified for a catalog image referenced by a sha256 digest. - // Examples: - // pollInterval: 1h # poll the image source every hour - // pollInterval: 30m # poll the image source every 30 minutes - // pollInterval: 1h30m # poll the image source every 1 hour and 30 minutes + + // pollIntervalMinutes allows the user to set the interval, in minutes, at which the image source should be polled for new content. + // pollIntervalMinutes is optional. + // pollIntervalMinutes can not be specified when ref is a digest-based reference. // // When omitted, the image will not be polled for new content. - // +kubebuilder:validation:Format:=duration + // +kubebuilder:validation:Minimum:=1 // +optional - PollInterval *metav1.Duration `json:"pollInterval,omitempty"` + PollIntervalMinutes *int `json:"pollIntervalMinutes,omitempty"` } func init() { diff --git a/api/core/v1alpha1/clustercatalog_types_test.go b/api/core/v1alpha1/clustercatalog_types_test.go index ee106bdb..b0e96cd6 100644 --- a/api/core/v1alpha1/clustercatalog_types_test.go +++ b/api/core/v1alpha1/clustercatalog_types_test.go @@ -4,8 +4,8 @@ import ( "context" "fmt" "os" + "strings" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -13,45 +13,132 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" celconfig "k8s.io/apiserver/pkg/apis/cel" + "k8s.io/utils/ptr" "sigs.k8s.io/yaml" ) -func TestPollIntervalCELValidationRules(t *testing.T) { +func TestImageSourceCELValidationRules(t *testing.T) { validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml") - pth := "openAPIV3Schema.properties.spec" + pth := "openAPIV3Schema.properties.spec.properties.source.properties.image" validator, found := validators["v1alpha1"][pth] require.True(t, found) for name, tc := range map[string]struct { - spec ClusterCatalogSpec + spec ImageSource wantErrs []string }{ - "digest based image ref, poll interval not allowed, poll interval specified": { - spec: ClusterCatalogSpec{ - Source: CatalogSource{ - Type: SourceTypeImage, - Image: &ImageSource{ - Ref: "docker.io/test-image@sha256:asdf98234sd", - PollInterval: &metav1.Duration{Duration: time.Minute}, - }, - }, + "valid digest based image ref, poll interval not allowed, poll interval specified": { + spec: ImageSource{ + Ref: "docker.io/test-image@sha256:abcdef123456789abcdef123456789abc", + PollIntervalMinutes: ptr.To(1), }, wantErrs: []string{ - "openAPIV3Schema.properties.spec: Invalid value: \"object\": cannot specify PollInterval while using digest-based image", + "openAPIV3Schema.properties.spec.properties.source.properties.image: Invalid value: \"object\": cannot specify pollIntervalMinutes while using digest-based image", }, }, - "digest based image ref, poll interval not allowed, poll interval not specified": { - spec: ClusterCatalogSpec{ - Source: CatalogSource{ - Type: SourceTypeImage, - Image: &ImageSource{ - Ref: "docker.io/example/test-catalog@sha256:asdf123", - }, - }, + "valid digest based image ref, poll interval not allowed, poll interval not specified": { + spec: ImageSource{ + Ref: "docker.io/test-image@sha256:abcdef123456789abcdef123456789abc", + }, + wantErrs: []string{}, + }, + "invalid digest based image ref, invalid domain": { + spec: ImageSource{ + Ref: "-quay+docker/foo/bar@sha256:abcdef123456789abcdef123456789abc", + }, + wantErrs: []string{ + "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.", + }, + }, + "invalid digest based image ref, invalid name": { + spec: ImageSource{ + Ref: "docker.io/FOO/BAR@sha256:abcdef123456789abcdef123456789abc", + }, + wantErrs: []string{ + "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.", + }, + }, + "invalid digest based image ref, invalid digest algorithm": { + spec: ImageSource{ + Ref: "docker.io/foo/bar@99-problems:abcdef123456789abcdef123456789abc", + }, + wantErrs: []string{ + "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.", + }, + }, + "invalid digest based image ref, too short digest encoding": { + spec: ImageSource{ + Ref: "docker.io/foo/bar@sha256:abcdef123456789", + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": digest is not valid. the encoded string must be at least 32 characters", + }, + }, + "invalid digest based image ref, invalid characters in digest encoding": { + spec: ImageSource{ + Ref: "docker.io/foo/bar@sha256:XYZxy123456789abcdef123456789abc", + }, + wantErrs: []string{ + "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)", + }, + }, + "invalid image ref, no tag or digest": { + spec: ImageSource{ + Ref: "docker.io/foo/bar", + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": must end with a digest or a tag", + }, + }, + "invalid tag based image ref, tag too long": { + spec: ImageSource{ + Ref: fmt.Sprintf("docker.io/foo/bar:%s", strings.Repeat("x", 128)), + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": tag is invalid. the tag must not be more than 127 characters", + }, + }, + "invalid tag based image ref, tag contains invalid characters": { + spec: ImageSource{ + Ref: "docker.io/foo/bar:-foo_bar-", + }, + wantErrs: []string{ + "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", + }, + }, + "valid tag based image ref": { + spec: ImageSource{ + Ref: "docker.io/foo/bar:v1.0.0", + }, + wantErrs: []string{}, + }, + "valid tag based image ref, pollIntervalMinutes specified": { + spec: ImageSource{ + Ref: "docker.io/foo/bar:v1.0.0", + PollIntervalMinutes: ptr.To(5), + }, + wantErrs: []string{}, + }, + "invalid image ref, only domain with port": { + spec: ImageSource{ + Ref: "docker.io:8080", + }, + wantErrs: []string{ + "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.", + }, + }, + "valid image ref, domain with port": { + spec: ImageSource{ + Ref: "my-subdomain.docker.io:8080/foo/bar:latest", + }, + wantErrs: []string{}, + }, + "valid image ref, tag ends with hyphen": { + spec: ImageSource{ + Ref: "my-subdomain.docker.io:8080/foo/bar:latest-", }, wantErrs: []string{}, }, @@ -60,6 +147,143 @@ func TestPollIntervalCELValidationRules(t *testing.T) { obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.spec) //nolint:gosec require.NoError(t, err) errs := validator(obj, nil) + require.Equal(t, len(tc.wantErrs), len(errs), "want", tc.wantErrs, "got", errs) + for i := range tc.wantErrs { + got := errs[i].Error() + assert.Equal(t, tc.wantErrs[i], got) + } + }) + } +} + +func TestResolvedImageSourceCELValidation(t *testing.T) { + validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml") + pth := "openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref" + validator, found := validators["v1alpha1"][pth] + require.True(t, found) + + for name, tc := range map[string]struct { + spec ImageSource + wantErrs []string + }{ + "valid digest based image ref": { + spec: ImageSource{ + Ref: "docker.io/test-image@sha256:abcdef123456789abcdef123456789abc", + }, + wantErrs: []string{}, + }, + "invalid digest based image ref, invalid domain": { + spec: ImageSource{ + Ref: "-quay+docker/foo/bar@sha256:abcdef123456789abcdef123456789abc", + }, + wantErrs: []string{ + "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.", + }, + }, + "invalid digest based image ref, invalid name": { + spec: ImageSource{ + Ref: "docker.io/FOO/BAR@sha256:abcdef123456789abcdef123456789abc", + }, + wantErrs: []string{ + "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.", + }, + }, + "invalid digest based image ref, invalid digest algorithm": { + spec: ImageSource{ + Ref: "docker.io/foo/bar@99-problems:abcdef123456789abcdef123456789abc", + }, + wantErrs: []string{ + "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.", + }, + }, + "invalid digest based image ref, too short digest encoding": { + spec: ImageSource{ + Ref: "docker.io/foo/bar@sha256:abcdef123456789", + }, + wantErrs: []string{ + "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", + }, + }, + "invalid digest based image ref, invalid characters in digest encoding": { + spec: ImageSource{ + Ref: "docker.io/foo/bar@sha256:XYZxy123456789abcdef123456789abc", + }, + wantErrs: []string{ + "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)", + }, + }, + "invalid image ref, no digest": { + spec: ImageSource{ + Ref: "docker.io/foo/bar", + }, + wantErrs: []string{ + "openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest", + }, + }, + "invalid image ref, only domain with port": { + spec: ImageSource{ + Ref: "docker.io:8080", + }, + wantErrs: []string{ + "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.", + "openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest", + }, + }, + "invalid image ref, tag-based ref": { + spec: ImageSource{ + Ref: "docker.io/foo/bar:latest", + }, + wantErrs: []string{ + "openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest", + }, + }, + } { + t.Run(name, func(t *testing.T) { + errs := validator(tc.spec.Ref, nil) + require.Equal(t, len(tc.wantErrs), len(errs), "want", tc.wantErrs, "got", errs) + for i := range tc.wantErrs { + got := errs[i].Error() + assert.Equal(t, tc.wantErrs[i], got) + } + }) + } +} + +func TestClusterCatalogURLsCELValidation(t *testing.T) { + validators := fieldValidatorsFromFile(t, "../../../config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml") + pth := "openAPIV3Schema.properties.status.properties.urls.properties.base" + validator, found := validators["v1alpha1"][pth] + require.True(t, found) + for name, tc := range map[string]struct { + urls ClusterCatalogURLs + wantErrs []string + }{ + "base is valid": { + urls: ClusterCatalogURLs{ + Base: "https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio", + }, + wantErrs: []string{}, + }, + "base is invalid, scheme is not one of http or https": { + urls: ClusterCatalogURLs{ + Base: "file://somefilepath", + }, + wantErrs: []string{ + fmt.Sprintf("%s: Invalid value: \"string\": scheme must be either http or https", pth), + }, + }, + "base is invalid": { + urls: ClusterCatalogURLs{ + Base: "notevenarealURL", + }, + wantErrs: []string{ + fmt.Sprintf("%s: Invalid value: \"string\": must be a valid URL", pth), + }, + }, + } { + t.Run(name, func(t *testing.T) { + errs := validator(tc.urls.Base, nil) + fmt.Println(errs) require.Equal(t, len(tc.wantErrs), len(errs)) for i := range tc.wantErrs { got := errs[i].Error() @@ -83,13 +307,15 @@ func TestSourceCELValidation(t *testing.T) { Type: SourceTypeImage, }, wantErrs: []string{ - fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage), + fmt.Sprintf("%s: Invalid value: \"object\": image is required when source type is %s, and forbidden otherwise", pth, SourceTypeImage), }, }, "image source with required image field": { source: CatalogSource{ - Type: SourceTypeImage, - Image: &ImageSource{}, + Type: SourceTypeImage, + Image: &ImageSource{ + Ref: "docker.io/foo/bar:latest", + }, }, wantErrs: []string{}, }, @@ -123,13 +349,15 @@ func TestResolvedSourceCELValidation(t *testing.T) { Type: SourceTypeImage, }, wantErrs: []string{ - fmt.Sprintf("%s: Invalid value: \"object\": source type '%s' requires image field", pth, SourceTypeImage), + fmt.Sprintf("%s: Invalid value: \"object\": image is required when source type is %s, and forbidden otherwise", pth, SourceTypeImage), }, }, "image source with required image field": { source: ResolvedCatalogSource{ - Type: SourceTypeImage, - Image: &ResolvedImageSource{}, + Type: SourceTypeImage, + Image: &ResolvedImageSource{ + Ref: "docker.io/foo/bar@sha256:abcdef123456789abcdef123456789abc", + }, }, wantErrs: []string{}, }, @@ -149,6 +377,7 @@ func TestResolvedSourceCELValidation(t *testing.T) { // fieldValidatorsFromFile extracts the CEL validators by version and JSONPath from a CRD file and returns // a validator func for testing against samples. +// nolint:unparam func fieldValidatorsFromFile(t *testing.T, crdFilePath string) map[string]map[string]CELValidateFunc { data, err := os.ReadFile(crdFilePath) require.NoError(t, err) diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go index 24f7bbfc..202d330d 100644 --- a/api/core/v1alpha1/zz_generated.deepcopy.go +++ b/api/core/v1alpha1/zz_generated.deepcopy.go @@ -140,7 +140,10 @@ func (in *ClusterCatalogStatus) DeepCopyInto(out *ClusterCatalogStatus) { *out = new(ClusterCatalogURLs) **out = **in } - in.LastUnpacked.DeepCopyInto(&out.LastUnpacked) + if in.LastUnpacked != nil { + in, out := &in.LastUnpacked, &out.LastUnpacked + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterCatalogStatus. @@ -171,9 +174,9 @@ func (in *ClusterCatalogURLs) DeepCopy() *ClusterCatalogURLs { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ImageSource) DeepCopyInto(out *ImageSource) { *out = *in - if in.PollInterval != nil { - in, out := &in.PollInterval, &out.PollInterval - *out = new(v1.Duration) + if in.PollIntervalMinutes != nil { + in, out := &in.PollIntervalMinutes, &out.PollIntervalMinutes + *out = new(int) **out = **in } } @@ -194,7 +197,7 @@ func (in *ResolvedCatalogSource) DeepCopyInto(out *ResolvedCatalogSource) { if in.Image != nil { in, out := &in.Image, &out.Image *out = new(ResolvedImageSource) - (*in).DeepCopyInto(*out) + **out = **in } } @@ -211,7 +214,6 @@ func (in *ResolvedCatalogSource) DeepCopy() *ResolvedCatalogSource { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResolvedImageSource) DeepCopyInto(out *ResolvedImageSource) { *out = *in - in.LastSuccessfulPollAttempt.DeepCopyInto(&out.LastSuccessfulPollAttempt) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ResolvedImageSource. diff --git a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml index a3a10dc9..ecdf5bdb 100644 --- a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml +++ b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml @@ -49,36 +49,66 @@ spec: metadata: type: object spec: - description: ClusterCatalogSpec defines the desired state of ClusterCatalog + description: |- + spec is the desired state of the ClusterCatalog. + spec is required. + The controller will work to ensure that the desired + catalog is unpacked and served over the catalog content HTTP server. properties: - availability: - default: Enabled + availabilityMode: + default: Available description: |- - Availability is an optional field that allows users to define whether the ClusterCatalog is utilized by the operator-controller. + availabilityMode allows users to define how the ClusterCatalog is made available to clients on the cluster. + availabilityMode is optional. + + Allowed values are "Available" and "Unavailable" and omitted. - Allowed values are : ["Enabled", "Disabled"]. - If set to "Enabled", the catalog will be used for updates, serving contents, and package installations. + When omitted, the default value is "Available". - If set to "Disabled", catalogd will stop serving the catalog and the cached data will be removed. + When set to "Available", the catalog contents will be unpacked and served over the catalog content HTTP server. + Setting the availabilityMode to "Available" tells clients that they should consider this ClusterCatalog + and its contents as usable. - If unspecified, the default value is "Enabled" + When set to "Unavailable", the catalog contents will no longer be served over the catalog content HTTP server. + When set to this availabilityMode it should be interpreted the same as the ClusterCatalog not existing. + Setting the availabilityMode to "Unavailable" can be useful in scenarios where a user may not want + to delete the ClusterCatalog all together, but would still like it to be treated as if it doesn't exist. enum: - - Disabled - - Enabled + - Unavailable + - Available type: string priority: default: 0 description: |- - priority is an optional field that allows the user to define a priority for a ClusterCatalog. + priority allows the user to define a priority for a ClusterCatalog. + priority is optional. + A ClusterCatalog's priority is used by clients as a tie-breaker between ClusterCatalogs that meet the client's requirements. - For example, in the case where multiple ClusterCatalogs provide the same bundle. - A higher number means higher priority. Negative numbers are also accepted. - When omitted, the default priority is 0. + A higher number means higher priority. + + It is up to clients to decide how to handle scenarios where multiple ClusterCatalogs with the same priority meet their requirements. + When deciding how to break the tie in this scenario, it is recommended that clients prompt their users for additional input. + + When omitted, the default priority is 0 because that is the zero value of integers. + + Negative numbers can be used to specify a priority lower than the default. + Positive numbers can be used to specify a priority higher than the default. + + The lowest possible value is -2147483648. + The highest possible value is 2147483647. format: int32 type: integer source: description: |- - source is a required field that allows the user to define the source of a Catalog that contains catalog metadata in the File-Based Catalog (FBC) format. + source allows a user to define the source of a catalog. + A "catalog" contains information on content that can be installed on a cluster. + Providing a catalog source makes the contents of the catalog discoverable and usable by + other on-cluster components. + These on-cluster components may do a variety of things with this information, such as + presenting the content in a GUI dashboard or installing content from the catalog on the cluster. + The catalog source must contain catalog metadata in the File-Based Catalog (FBC) format. + For more information on FBC, see https://olm.operatorframework.io/docs/reference/file-based-catalogs/#docs. + source is a required field. Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image: @@ -86,45 +116,116 @@ spec: type: Image image: ref: quay.io/operatorhubio/catalog:latest - - For more information on FBC, see https://olm.operatorframework.io/docs/reference/file-based-catalogs/#docs properties: 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 - type. + description: |- + image is used to configure how catalog contents are sourced from an OCI image. + This field is required when type is Image, and forbidden otherwise. properties: - pollInterval: + pollIntervalMinutes: description: |- - pollInterval is an optional field that allows the user to set the interval at which the image source should be polled for new content. - It must be specified as a duration. - It must not be specified for a catalog image referenced by a sha256 digest. - Examples: - pollInterval: 1h # poll the image source every hour - pollInterval: 30m # poll the image source every 30 minutes - pollInterval: 1h30m # poll the image source every 1 hour and 30 minutes + pollIntervalMinutes allows the user to set the interval, in minutes, at which the image source should be polled for new content. + pollIntervalMinutes is optional. + pollIntervalMinutes can not be specified when ref is a digest-based reference. When omitted, the image will not be polled for new content. - format: duration - type: string + minimum: 1 + type: integer ref: description: |- - ref is a required field that allows the user to define the reference to a container image containing Catalog contents. - Examples: - ref: quay.io/operatorhubio/catalog:latest # image reference - ref: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b # image reference with sha256 digest + ref allows users to define the reference to a container image containing Catalog contents. + ref is required. + ref can not be more than 1000 characters. + + A reference can be broken down into 3 parts - the domain, name, and identifier. + + The domain is typically the registry where an image is located. + It must be alphanumeric characters (lowercase and uppercase) separated by the "." character. + Hyphenation is allowed, but the domain must start and end with alphanumeric characters. + Specifying a port to use is also allowed by adding the ":" character followed by numeric values. + The port must be the last value in the domain. + Some examples of valid domain values are "registry.mydomain.io", "quay.io", "my-registry.io:8080". + + The name is typically the repository in the registry where an image is located. + It must contain lowercase alphanumeric characters separated only by the ".", "_", "__", "-" characters. + Multiple names can be concatenated with the "/" character. + The domain and name are combined using the "/" character. + Some examples of valid name values are "operatorhubio/catalog", "catalog", "my-catalog.prod". + An example of the domain and name parts of a reference being combined is "quay.io/operatorhubio/catalog". + + The identifier is typically the tag or digest for an image reference and is present at the end of the reference. + It starts with a separator character used to distinguish the end of the name and beginning of the identifier. + For a digest-based reference, the "@" character is the separator. + For a tag-based reference, the ":" character is the separator. + An identifier is required in the reference. + + Digest-based references must contain an algorithm reference immediately after the "@" separator. + The algorithm reference must be followed by the ":" character and an encoded string. + The algorithm must start with an uppercase or lowercase alpha character followed by alphanumeric characters and may contain the "-", "_", "+", and "." characters. + Some examples of valid algorithm values are "sha256", "sha256+b64u", "multihash+base58". + The encoded string following the algorithm must be hex digits (a-f, A-F, 0-9) and must be a minimum of 32 characters. + + Tag-based references must begin with a word character (alphanumeric + "_") followed by word characters or ".", and "-" characters. + The tag must not be longer than 127 characters. + + An example of a valid digest-based image reference is "quay.io/operatorhubio/catalog@sha256:200d4ddb2a73594b91358fe6397424e975205bfbe44614f5846033cad64b3f05" + An example of a valid tag-based image reference is "quay.io/operatorhubio/catalog:latest" + maxLength: 1000 type: string + x-kubernetes-validations: + - message: must start with a valid domain. valid domains must + be alphanumeric characters (lowercase and uppercase) separated + by the "." character. + rule: self.matches('^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])((\\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(:[0-9]+)?\\b') + - message: a valid name is required. valid names must contain + lowercase alphanumeric characters separated only by the + ".", "_", "__", "-" characters. + rule: self.find('(\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)') + != "" + - message: must end with a digest or a tag + rule: self.find('(@.*:)') != "" || self.find(':.*$') != + "" + - message: tag is invalid. the tag must not be more than 127 + characters + rule: 'self.find(''(@.*:)'') == "" ? (self.find('':.*$'') + != "" ? self.find('':.*$'').substring(1).size() <= 127 + : true) : true' + - message: tag is invalid. valid tags must begin with a word + character (alphanumeric + "_") followed by word characters + or ".", and "-" characters + rule: 'self.find(''(@.*:)'') == "" ? (self.find('':.*$'') + != "" ? self.find('':.*$'').matches('':[\\w][\\w.-]*$'') + : true) : true' + - message: 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. + rule: 'self.find(''(@.*:)'') != "" ? self.find(''(@.*:)'').matches(''(@[A-Za-z][A-Za-z0-9]*([-_+.][A-Za-z][A-Za-z0-9]*)*[:])'') + : true' + - message: digest is not valid. the encoded string must be + at least 32 characters + rule: 'self.find(''(@.*:)'') != "" ? self.find('':.*$'').substring(1).size() + >= 32 : true' + - message: digest is not valid. the encoded string must only + contain hex characters (A-F, a-f, 0-9) + rule: 'self.find(''(@.*:)'') != "" ? self.find('':.*$'').matches('':[0-9A-Fa-f]*$'') + : true' required: - ref type: object + x-kubernetes-validations: + - message: cannot specify pollIntervalMinutes while using digest-based + image + rule: 'self.ref.find(''(@.*:)'') != "" ? !has(self.pollIntervalMinutes) + : true' type: description: |- - type is a required reference to the type of source the catalog is sourced from. + type is a reference to the type of source the catalog is sourced from. + type is required. - Allowed values are ["Image"] + The only allowed value is "Image". - When this field is set to "Image", the ClusterCatalog content will be sourced from an OCI image. + When 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 @@ -133,39 +234,40 @@ spec: - type type: object x-kubernetes-validations: - - message: source type 'Image' requires image field - rule: self.type == 'Image' && has(self.image) + - message: image is required when source type is Image, and forbidden + otherwise + rule: 'has(self.type) && self.type == ''Image'' ? has(self.image) + : !has(self.image)' required: - source type: object - x-kubernetes-validations: - - message: cannot specify PollInterval while using digest-based image - rule: '!has(self.source.image.pollInterval) || (self.source.image.ref.find(''@sha256:'') - == "")' status: - description: ClusterCatalogStatus defines the observed state of ClusterCatalog + description: |- + status contains information about the state of the ClusterCatalog such as: + - Whether or not the catalog contents are being served via the catalog content HTTP server + - Whether or not the ClusterCatalog is progressing to a new state + - A reference to the source from which the catalog contents were retrieved properties: conditions: description: |- conditions is a representation of the current state for this ClusterCatalog. - The status is represented by a set of "conditions". - Each condition is generally structured in the following format: - - Type: a string representation of the condition type. More or less the condition "name". - - Status: a string representation of the state of the condition. Can be one of ["True", "False", "Unknown"]. - - Reason: a string representation of the reason for the current state of the condition. Typically useful for building automation around particular Type+Reason combinations. - - Message: a human-readable message that further elaborates on the state of the condition. + The current condition types are Serving and Progressing. + + The Serving condition is used to represent whether or not the contents of the catalog is being served via the HTTP(S) web server. + When it has a status of True and a reason of Available, the contents of the catalog are being served. + When it has a status of False and a reason of Unavailable, the contents of the catalog are not being served because the contents are not yet available. + When it has a status of False and a reason of UserSpecifiedUnavailable, the contents of the catalog are not being served because the catalog has been intentionally marked as unavailable. - The current set of condition types are: - - "Serving", which represents whether or not the contents of the catalog are being served via the HTTP(S) web server. - - "Progressing", which represents whether or not the ClusterCatalog is progressing towards a new state. + The Progressing condition is used to represent whether or not the ClusterCatalog is progressing or is ready to progress towards a new state. + When it has a status of True and a reason of Retrying, there was an error in the progression of the ClusterCatalog that may be resolved on subsequent reconciliation attempts. + When it has a status of True and a reason of Succeeded, the ClusterCatalog has successfully progressed to a new state and is ready to continue progressing. + When it has a status of False and a reason of Blocked, there was an error in the progression of the ClusterCatalog that requires manual intervention for recovery. - The current set of reasons are: - - "Succeeded", this reason is set on the "Progressing" condition when progressing to a new state is successful. - - "Blocked", this reason is set on the "Progressing" condition when the ClusterCatalog controller has encountered an error that requires manual intervention for recovery. - - "Retrying", this reason is set on the "Progressing" condition when the ClusterCatalog controller has encountered an error that might be resolvable on subsequent reconciliation attempts. - - "Available", this reason is set on the "Serving" condition when the contents of the ClusterCatalog are being served via an endpoint on the HTTP(S) web server. - - "Unavailable", this reason is set on the "Serving" condition when there is not an endpoint on the HTTP(S) web server that is serving the contents of the ClusterCatalog. + In the case that the Serving condition is True with reason Available and Progressing is True with reason Retrying, the previously fetched + catalog contents are still being served via the HTTP(S) web server while we are progressing towards serving a new version of the catalog + contents. This could occur when we've initially fetched the latest contents from the source for this catalog and when polling for changes + to the contents we identify that there are updates to the contents. items: description: Condition contains details for one aspect of the current state of this API Resource. @@ -221,48 +323,71 @@ spec: - type type: object type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map lastUnpacked: description: |- - lastUnpacked represents the time when the - ClusterCatalog object was last unpacked successfully. + lastUnpacked represents the last time the contents of the + catalog were extracted from their source format. As an example, + when using an Image source, the OCI image will be pulled and the + image layers written to a file-system backed cache. We refer to the + act of this extraction from the source format as "unpacking". format: date-time type: string resolvedSource: - description: |- - resolvedSource contains information about the resolved source based on the source type. - - Below is an example of a resolved source for an image source: - resolvedSource: - - image: - lastSuccessfulPollAttempt: "2024-09-10T12:22:13Z" - ref: quay.io/operatorhubio/catalog@sha256:c7392b4be033da629f9d665fec30f6901de51ce3adebeff0af579f311ee5cf1b - type: Image + description: resolvedSource contains information about the resolved + source based on the source type. properties: image: - description: image is a field containing resolution information - for a catalog sourced from an image. + description: |- + image is a field containing resolution information for a catalog sourced from an image. + This field must be set when type is Image, and forbidden otherwise. properties: - lastSuccessfulPollAttempt: - description: lastSuccessfulPollAttempt is the time when the - resolved source was last successfully polled for new content. - format: date-time - type: string ref: - description: ref contains the resolved sha256 image ref containing - Catalog contents. + description: |- + ref contains the resolved image digest-based reference. + The digest format is used so users can use other tooling to fetch the exact + OCI manifests that were used to extract the catalog contents. + maxLength: 1000 type: string + x-kubernetes-validations: + - message: must start with a valid domain. valid domains must + be alphanumeric characters (lowercase and uppercase) separated + by the "." character. + rule: self.matches('^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])((\\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(:[0-9]+)?\\b') + - message: a valid name is required. valid names must contain + lowercase alphanumeric characters separated only by the + ".", "_", "__", "-" characters. + rule: self.find('(\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)') + != "" + - message: must end with a digest + rule: self.find('(@.*:)') != "" + - message: 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. + rule: 'self.find(''(@.*:)'') != "" ? self.find(''(@.*:)'').matches(''(@[A-Za-z][A-Za-z0-9]*([-_+.][A-Za-z][A-Za-z0-9]*)*[:])'') + : true' + - message: digest is not valid. the encoded string must be + at least 32 characters + rule: 'self.find(''(@.*:)'') != "" ? self.find('':.*$'').substring(1).size() + >= 32 : true' + - message: digest is not valid. the encoded string must only + contain hex characters (A-F, a-f, 0-9) + rule: 'self.find(''(@.*:)'') != "" ? self.find('':.*$'').matches('':[0-9A-Fa-f]*$'') + : true' required: - - lastSuccessfulPollAttempt - ref type: object type: description: |- type is a reference to the type of source the catalog is sourced from. + type is required. - It will be set to one of the following values: ["Image"]. + The only allowed value is "Image". - When this field is set to "Image", information about the resolved image source will be set in the 'image' field. + When set to "Image", information about the resolved image source will be set in the 'image' field. enum: - Image type: string @@ -271,18 +396,37 @@ spec: - type type: object x-kubernetes-validations: - - message: source type 'Image' requires image field - rule: self.type == 'Image' && has(self.image) + - message: image is required when source type is Image, and forbidden + otherwise + rule: 'has(self.type) && self.type == ''Image'' ? has(self.image) + : !has(self.image)' urls: description: urls contains the URLs that can be used to access the catalog. properties: base: description: |- - base is a required cluster-internal URL which provides API access for this ClusterCatalog. - A suffix API access path can be added to retrieve catalog data for the ClusterCatalog. - Currently, a 'v1' API access provides complete FBC retrival via the path "/api/v1/all", with the general form `{base}/api/v1/all`. + base is a cluster-internal URL that provides endpoints for + accessing the content of the catalog. + + It is expected that clients append the path for the endpoint they wish + to access. + + Currently, only a single endpoint is served and is accessible at the path + /api/v1. + + The endpoints served for the v1 API are: + - /all - this endpoint returns the entirety of the catalog contents in the FBC format + + As the needs of users and clients of the evolve, new endpoints may be added. + maxLength: 525 type: string + x-kubernetes-validations: + - message: must be a valid URL + rule: isURL(self) + - message: scheme must be either http or https + rule: 'isURL(self) ? (url(self).getScheme() == "http" || url(self).getScheme() + == "https") : true' required: - base type: object diff --git a/config/base/default/clustercatalogs/default-catalogs.yaml b/config/base/default/clustercatalogs/default-catalogs.yaml index 23bc9d28..7df6ec96 100644 --- a/config/base/default/clustercatalogs/default-catalogs.yaml +++ b/config/base/default/clustercatalogs/default-catalogs.yaml @@ -8,4 +8,4 @@ spec: type: Image image: ref: quay.io/operatorhubio/catalog:latest - pollInterval: 10m + pollIntervalMinutes: 10 diff --git a/config/samples/core_v1alpha1_clustercatalog.yaml b/config/samples/core_v1alpha1_clustercatalog.yaml index a8f33097..7027c39c 100644 --- a/config/samples/core_v1alpha1_clustercatalog.yaml +++ b/config/samples/core_v1alpha1_clustercatalog.yaml @@ -7,5 +7,5 @@ spec: source: type: Image image: - pollInterval: 24h + pollIntervalMinutes: 1440 ref: quay.io/operatorhubio/catalog:latest diff --git a/go.mod b/go.mod index 0e31b753..aa33336f 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( k8s.io/client-go v0.31.2 k8s.io/component-base v0.31.2 k8s.io/klog/v2 v2.130.1 + k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 sigs.k8s.io/controller-runtime v0.19.0 sigs.k8s.io/yaml v1.4.0 ) @@ -156,7 +157,6 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect - k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect ) diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index f0c880d4..1e7fe453 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -160,7 +161,7 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp l := log.FromContext(ctx) // Check if the catalog availability is set to disabled, if true then // unset base URL, delete it from the cache and set appropriate status - if catalog.Spec.Availability == v1alpha1.AvailabilityDisabled { + if catalog.Spec.AvailabilityMode == v1alpha1.AvailabilityModeUnavailable { // Delete the catalog from local cache err := r.deleteCatalogCache(ctx, catalog) if err != nil { @@ -169,7 +170,7 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp // Set status.conditions[type=Progressing] to False as we are done with // all that needs to be done with the catalog - updateStatusCatalogDisabled(&catalog.Status, catalog.GetGeneration()) + updateStatusProgressingUserSpecifiedUnavailable(&catalog.Status, catalog.GetGeneration()) // Remove the fbcDeletionFinalizer as we do not want a finalizer attached to the catalog // when it is disabled. Because the finalizer serves no purpose now. @@ -215,7 +216,7 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp case catalog.Generation != storedCatalog.observedGeneration: l.Info("unpack required: catalog generation differs from observed generation") needsUnpack = true - case r.needsPoll(storedCatalog.unpackResult.ResolvedSource.Image.LastSuccessfulPollAttempt.Time, catalog): + case r.needsPoll(storedCatalog.unpackResult.LastSuccessfulPollAttempt.Time, catalog): l.Info("unpack required: poll duration has elapsed") needsUnpack = true } @@ -223,7 +224,7 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp if !needsUnpack { // No need to update the status because we've already checked // that it is set correctly. Otherwise, we'd be unpacking again. - return nextPollResult(storedCatalog.unpackResult.ResolvedSource.Image.LastSuccessfulPollAttempt.Time, catalog), nil + return nextPollResult(storedCatalog.unpackResult.LastSuccessfulPollAttempt.Time, catalog), nil } unpackResult, err := r.Unpacker.Unpack(ctx, catalog) @@ -258,7 +259,7 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp observedGeneration: catalog.GetGeneration(), } r.storedCatalogsMu.Unlock() - return nextPollResult(unpackResult.ResolvedSource.Image.LastSuccessfulPollAttempt.Time, catalog), nil + return nextPollResult(unpackResult.LastSuccessfulPollAttempt.Time, catalog), nil } func (r *ClusterCatalogReconciler) getCurrentState(catalog *v1alpha1.ClusterCatalog) (*v1alpha1.ClusterCatalogStatus, storedCatalogData, bool) { @@ -282,8 +283,9 @@ func nextPollResult(lastSuccessfulPoll time.Time, catalog *v1alpha1.ClusterCatal var requeueAfter time.Duration switch catalog.Spec.Source.Type { case v1alpha1.SourceTypeImage: - if catalog.Spec.Source.Image != nil && catalog.Spec.Source.Image.PollInterval != nil { - jitteredDuration := wait.Jitter(catalog.Spec.Source.Image.PollInterval.Duration, requeueJitterMaxFactor) + if catalog.Spec.Source.Image != nil && catalog.Spec.Source.Image.PollIntervalMinutes != nil { + pollDuration := time.Duration(*catalog.Spec.Source.Image.PollIntervalMinutes) * time.Minute + jitteredDuration := wait.Jitter(pollDuration, requeueJitterMaxFactor) requeueAfter = time.Until(lastSuccessfulPoll.Add(jitteredDuration)) } } @@ -303,7 +305,7 @@ func clearUnknownConditions(status *v1alpha1.ClusterCatalogStatus) { func updateStatusProgressing(status *v1alpha1.ClusterCatalogStatus, generation int64, err error) { progressingCond := metav1.Condition{ Type: v1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: v1alpha1.ReasonSucceeded, Message: "Successfully unpacked and stored content from resolved source", ObservedGeneration: generation, @@ -329,7 +331,7 @@ func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result source.Re status.URLs = &v1alpha1.ClusterCatalogURLs{} } status.URLs.Base = baseURL - status.LastUnpacked = metav1.NewTime(result.UnpackTime) + status.LastUnpacked = ptr.To(metav1.NewTime(result.UnpackTime)) meta.SetStatusCondition(&status.Conditions, metav1.Condition{ Type: v1alpha1.TypeServing, Status: metav1.ConditionTrue, @@ -339,21 +341,38 @@ func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result source.Re }) } -func updateStatusCatalogDisabled(status *v1alpha1.ClusterCatalogStatus, generation int64) { +func updateStatusProgressingUserSpecifiedUnavailable(status *v1alpha1.ClusterCatalogStatus, generation int64) { + // Set Progressing condition to True with reason Succeeded + // since we have successfully progressed to the unavailable + // availability mode and are ready to progress to any future + // desired state. progressingCond := metav1.Condition{ Type: v1alpha1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: v1alpha1.ReasonSucceeded, + Message: "Catalog availability mode is set to Unavailable", + ObservedGeneration: generation, + } + + // Set Serving condition to False with reason UserSpecifiedUnavailable + // so that users of this condition are aware that this catalog is + // intentionally not being served + servingCond := metav1.Condition{ + Type: v1alpha1.TypeServing, Status: metav1.ConditionFalse, - Reason: v1alpha1.ReasonDisabled, - Message: "Catalog availability is set to Disabled", + Reason: v1alpha1.ReasonUserSpecifiedUnavailable, + Message: "Catalog availability mode is set to Unavailable", ObservedGeneration: generation, } + meta.SetStatusCondition(&status.Conditions, progressingCond) + meta.SetStatusCondition(&status.Conditions, servingCond) } func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus, generation int64) { status.ResolvedSource = nil status.URLs = nil - status.LastUnpacked = metav1.Time{} + status.LastUnpacked = nil meta.SetStatusCondition(&status.Conditions, metav1.Condition{ Type: v1alpha1.TypeServing, Status: metav1.ConditionFalse, @@ -364,12 +383,12 @@ func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus, generation in func (r *ClusterCatalogReconciler) needsPoll(lastSuccessfulPoll time.Time, catalog *v1alpha1.ClusterCatalog) bool { // If polling is disabled, we don't need to poll. - if catalog.Spec.Source.Image.PollInterval == nil { + if catalog.Spec.Source.Image.PollIntervalMinutes == nil { return false } // Only poll if the next poll time is in the past. - nextPoll := lastSuccessfulPoll.Add(catalog.Spec.Source.Image.PollInterval.Duration) + nextPoll := lastSuccessfulPoll.Add(time.Duration(*catalog.Spec.Source.Image.PollIntervalMinutes) * time.Minute) return nextPoll.Before(time.Now()) } diff --git a/internal/controllers/core/clustercatalog_controller_test.go b/internal/controllers/core/clustercatalog_controller_test.go index 62cc66ad..93a2cbee 100644 --- a/internal/controllers/core/clustercatalog_controller_test.go +++ b/internal/controllers/core/clustercatalog_controller_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -220,7 +221,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, { - name: "valid source type, unpack state == Unpacked, should reflect in status that it's not progressing anymore, and is serving", + name: "valid source type, unpack state == Unpacked, should reflect in status that it's progressing, and is serving", source: &MockSource{ result: &source.Result{ State: source.StateUnpacked, @@ -270,7 +271,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, { Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: catalogdv1alpha1.ReasonSucceeded, }, }, @@ -279,6 +280,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { Ref: "my.org/someimage@someSHA256Digest", }, }, + LastUnpacked: &metav1.Time{}, }, }, }, @@ -393,7 +395,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - LastUnpacked: metav1.Time{}, + LastUnpacked: &metav1.Time{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ @@ -606,11 +608,11 @@ func TestCatalogdControllerReconcile(t *testing.T) { Ref: "my.org/someimage:latest", }, }, - Availability: "Disabled", + AvailabilityMode: catalogdv1alpha1.AvailabilityModeUnavailable, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ URLs: &catalogdv1alpha1.ClusterCatalogURLs{Base: "URL"}, - LastUnpacked: metav1.Time{}, + LastUnpacked: &metav1.Time{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ @@ -642,19 +644,19 @@ func TestCatalogdControllerReconcile(t *testing.T) { Ref: "my.org/someimage:latest", }, }, - Availability: "Disabled", + AvailabilityMode: catalogdv1alpha1.AvailabilityModeUnavailable, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeServing, Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonUnavailable, + Reason: catalogdv1alpha1.ReasonUserSpecifiedUnavailable, }, { Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonDisabled, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonSucceeded, }, }, }, @@ -681,11 +683,11 @@ func TestCatalogdControllerReconcile(t *testing.T) { Ref: "my.org/someimage:latest", }, }, - Availability: "Disabled", + AvailabilityMode: catalogdv1alpha1.AvailabilityModeUnavailable, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ URLs: &catalogdv1alpha1.ClusterCatalogURLs{Base: "URL"}, - LastUnpacked: metav1.Time{}, + LastUnpacked: &metav1.Time{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ @@ -700,7 +702,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, { Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: catalogdv1alpha1.ReasonSucceeded, }, }, @@ -718,19 +720,19 @@ func TestCatalogdControllerReconcile(t *testing.T) { Ref: "my.org/someimage:latest", }, }, - Availability: "Disabled", + AvailabilityMode: catalogdv1alpha1.AvailabilityModeUnavailable, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeServing, Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonUnavailable, + Reason: catalogdv1alpha1.ReasonUserSpecifiedUnavailable, }, { Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, - Reason: catalogdv1alpha1.ReasonDisabled, + Status: metav1.ConditionTrue, + Reason: catalogdv1alpha1.ReasonSucceeded, }, }, }, @@ -770,11 +772,10 @@ func TestCatalogdControllerReconcile(t *testing.T) { } func TestPollingRequeue(t *testing.T) { - now := time.Now() - for name, tc := range map[string]struct { catalog *catalogdv1alpha1.ClusterCatalog expectedRequeueAfter time.Duration + lastPollTime metav1.Time }{ "ClusterCatalog with tag based image ref without any poll interval specified, requeueAfter set to 0, ie polling disabled": { catalog: &catalogdv1alpha1.ClusterCatalog{ @@ -792,6 +793,7 @@ func TestPollingRequeue(t *testing.T) { }, }, expectedRequeueAfter: time.Second * 0, + lastPollTime: metav1.Now(), }, "ClusterCatalog with tag based image ref with poll interval specified, requeueAfter set to wait.jitter(pollInterval)": { catalog: &catalogdv1alpha1.ClusterCatalog{ @@ -803,13 +805,14 @@ func TestPollingRequeue(t *testing.T) { Source: catalogdv1alpha1.CatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "my.org/someimage:latest", - PollInterval: &metav1.Duration{Duration: time.Minute * 5}, + Ref: "my.org/someimage:latest", + PollIntervalMinutes: ptr.To(5), }, }, }, }, expectedRequeueAfter: time.Minute * 5, + lastPollTime: metav1.Now(), }, } { t.Run(name, func(t *testing.T) { @@ -820,10 +823,10 @@ func TestPollingRequeue(t *testing.T) { FS: &fstest.MapFS{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "my.org/someImage@someSHA256Digest", - LastSuccessfulPollAttempt: metav1.NewTime(now), + Ref: "my.org/someImage@someSHA256Digest", }, }, + LastSuccessfulPollAttempt: tc.lastPollTime, }}, Storage: &MockStore{}, storedCatalogs: map[string]storedCatalogData{}, @@ -838,7 +841,6 @@ func TestPollingRequeue(t *testing.T) { func TestPollingReconcilerUnpack(t *testing.T) { oldDigest := "a5d4f4467250074216eb1ba1c36e06a3ab797d81c431427fc2aca97ecaf4e9d8" newDigest := "f42337e7b85a46d83c94694638e2312e10ca16a03542399a65ba783c94a32b63" - now := time.Now() successfulObservedGeneration := int64(2) successfulUnpackStatus := func(mods ...func(status *catalogdv1alpha1.ClusterCatalogStatus)) catalogdv1alpha1.ClusterCatalogStatus { @@ -847,7 +849,7 @@ func TestPollingReconcilerUnpack(t *testing.T) { Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: catalogdv1alpha1.ReasonSucceeded, Message: "Successfully unpacked and stored content from resolved source", ObservedGeneration: successfulObservedGeneration, @@ -863,22 +865,23 @@ func TestPollingReconcilerUnpack(t *testing.T) { ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ - Ref: "my.org/someimage@sha256:" + oldDigest, - LastSuccessfulPollAttempt: metav1.Time{Time: now.Add(-time.Minute * 5)}, + Ref: "my.org/someimage@sha256:" + oldDigest, }, }, + LastUnpacked: &metav1.Time{}, } for _, mod := range mods { mod(&s) } return s } - successfulStoredCatalogData := func() map[string]storedCatalogData { + successfulStoredCatalogData := func(lastPoll metav1.Time) map[string]storedCatalogData { return map[string]storedCatalogData{ "test-catalog": { observedGeneration: successfulObservedGeneration, unpackResult: source.Result{ - ResolvedSource: successfulUnpackStatus().ResolvedSource, + ResolvedSource: successfulUnpackStatus().ResolvedSource, + LastSuccessfulPollAttempt: lastPoll, }, }, } @@ -899,8 +902,8 @@ func TestPollingReconcilerUnpack(t *testing.T) { Source: catalogdv1alpha1.CatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "my.org/someimage:latest", - PollInterval: &metav1.Duration{Duration: time.Minute * 5}, + Ref: "my.org/someimage:latest", + PollIntervalMinutes: ptr.To(5), }, }, }, @@ -924,7 +927,7 @@ func TestPollingReconcilerUnpack(t *testing.T) { }, Status: successfulUnpackStatus(), }, - storedCatalogData: successfulStoredCatalogData(), + storedCatalogData: successfulStoredCatalogData(metav1.Now()), expectedUnpackRun: false, }, "ClusterCatalog not being resolved the first time, pollInterval mentioned, \"now\" is before next expected poll time, unpack should not run": { @@ -938,14 +941,14 @@ func TestPollingReconcilerUnpack(t *testing.T) { Source: catalogdv1alpha1.CatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "my.org/someimage:latest", - PollInterval: &metav1.Duration{Duration: time.Minute * 7}, + Ref: "my.org/someimage:latest", + PollIntervalMinutes: ptr.To(7), }, }, }, Status: successfulUnpackStatus(), }, - storedCatalogData: successfulStoredCatalogData(), + storedCatalogData: successfulStoredCatalogData(metav1.Now()), expectedUnpackRun: false, }, "ClusterCatalog not being resolved the first time, pollInterval mentioned, \"now\" is after next expected poll time, unpack should run": { @@ -959,14 +962,14 @@ func TestPollingReconcilerUnpack(t *testing.T) { Source: catalogdv1alpha1.CatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "my.org/someimage:latest", - PollInterval: &metav1.Duration{Duration: time.Minute * 3}, + Ref: "my.org/someimage:latest", + PollIntervalMinutes: ptr.To(3), }, }, }, Status: successfulUnpackStatus(), }, - storedCatalogData: successfulStoredCatalogData(), + storedCatalogData: successfulStoredCatalogData(metav1.NewTime(time.Now().Add(-5 * time.Minute))), expectedUnpackRun: true, }, "ClusterCatalog not being resolved the first time, pollInterval mentioned, \"now\" is before next expected poll time, generation changed, unpack should run": { @@ -980,14 +983,14 @@ func TestPollingReconcilerUnpack(t *testing.T) { Source: catalogdv1alpha1.CatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "my.org/someotherimage@sha256:" + newDigest, - PollInterval: &metav1.Duration{Duration: time.Minute * 7}, + Ref: "my.org/someotherimage@sha256:" + newDigest, + PollIntervalMinutes: ptr.To(7), }, }, }, Status: successfulUnpackStatus(), }, - storedCatalogData: successfulStoredCatalogData(), + storedCatalogData: successfulStoredCatalogData(metav1.Now()), expectedUnpackRun: true, }, "ClusterCatalog not being resolved the first time, no stored catalog in cache, unpack should run": { @@ -1001,8 +1004,8 @@ func TestPollingReconcilerUnpack(t *testing.T) { Source: catalogdv1alpha1.CatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "my.org/someotherimage@sha256:" + newDigest, - PollInterval: &metav1.Duration{Duration: time.Minute * 7}, + Ref: "my.org/someotherimage@sha256:" + newDigest, + PollIntervalMinutes: ptr.To(7), }, }, }, @@ -1021,8 +1024,8 @@ func TestPollingReconcilerUnpack(t *testing.T) { Source: catalogdv1alpha1.CatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ImageSource{ - Ref: "my.org/someotherimage@sha256:" + newDigest, - PollInterval: &metav1.Duration{Duration: time.Minute * 7}, + Ref: "my.org/someotherimage@sha256:" + newDigest, + PollIntervalMinutes: ptr.To(7), }, }, }, @@ -1030,7 +1033,7 @@ func TestPollingReconcilerUnpack(t *testing.T) { meta.FindStatusCondition(status.Conditions, catalogdv1alpha1.TypeProgressing).Status = metav1.ConditionTrue }), }, - storedCatalogData: successfulStoredCatalogData(), + storedCatalogData: successfulStoredCatalogData(metav1.Now()), expectedUnpackRun: true, }, } { diff --git a/internal/source/containers_image.go b/internal/source/containers_image.go index 4b240ff4..7f3035f5 100644 --- a/internal/source/containers_image.go +++ b/internal/source/containers_image.go @@ -173,22 +173,18 @@ func successResult(unpackPath string, canonicalRef reference.Canonical, lastUnpa Type: catalogdv1alpha1.SourceTypeImage, Image: &catalogdv1alpha1.ResolvedImageSource{ Ref: canonicalRef.String(), - - // We truncate to the second because metav1.Time is serialized - // as RFC 3339 which only has second-level precision. When we - // use this result in a comparison with what we deserialized - // from the Kubernetes API server, we need it to match. - LastSuccessfulPollAttempt: metav1.NewTime(time.Now().Truncate(time.Second)), }, }, State: StateUnpacked, Message: fmt.Sprintf("unpacked %q successfully", canonicalRef), - // We truncate to the second because metav1.Time is serialized + // We truncate both the unpack time and last successful poll attempt + // to the second because metav1.Time is serialized // as RFC 3339 which only has second-level precision. When we // use this result in a comparison with what we deserialized // from the Kubernetes API server, we need it to match. - UnpackTime: lastUnpacked.Truncate(time.Second), + UnpackTime: lastUnpacked.Truncate(time.Second), + LastSuccessfulPollAttempt: metav1.NewTime(time.Now().Truncate(time.Second)), } } diff --git a/internal/source/unpacker.go b/internal/source/unpacker.go index db6ffc57..7b0f878a 100644 --- a/internal/source/unpacker.go +++ b/internal/source/unpacker.go @@ -5,6 +5,8 @@ import ( "io/fs" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + catalogdv1alpha1 "github.com/operator-framework/catalogd/api/core/v1alpha1" ) @@ -49,6 +51,8 @@ type Result struct { // commit hash rather than a branch or tag. ResolvedSource *catalogdv1alpha1.ResolvedCatalogSource + LastSuccessfulPollAttempt metav1.Time + // State is the current state of unpacking the catalog content. State State diff --git a/test/e2e/unpack_test.go b/test/e2e/unpack_test.go index da20654e..ad0b0b06 100644 --- a/test/e2e/unpack_test.go +++ b/test/e2e/unpack_test.go @@ -71,7 +71,7 @@ var _ = Describe("ClusterCatalog Unpacking", func() { g.Expect(err).ToNot(HaveOccurred()) cond := meta.FindStatusCondition(catalog.Status.Conditions, catalogd.TypeProgressing) g.Expect(cond).ToNot(BeNil()) - g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) g.Expect(cond.Reason).To(Equal(catalogd.ReasonSucceeded)) }).Should(Succeed())