From 2abab1308a5a0dd08435f7b9542a0911117549d0 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Fri, 4 Sep 2026 11:03:13 -0500 Subject: [PATCH 1/3] implementation of a source-sniffing direct bundle installer Signed-off-by: grokspawn --- api/v1/clusterextension_types.go | 37 +++++- api/v1/zz_generated.deepcopy.go | 20 +++ .../api/v1/clusterextensionspec.go | 1 - applyconfigurations/api/v1/ociimagesource.go | 41 ++++++ applyconfigurations/api/v1/sourceconfig.go | 18 ++- applyconfigurations/internal/internal.go | 9 ++ applyconfigurations/utils.go | 4 +- cmd/operator-controller/main.go | 13 +- docs/api-reference/olmv1-api-reference.md | 21 +++- ...peratorframework.io_clusterextensions.yaml | 50 +++++++- ...peratorframework.io_clusterextensions.yaml | 50 +++++++- .../clusterextension_admission_test.go | 48 +++++++ .../clusterextension_reconcile_steps.go | 41 ++++++ .../controllers/direct_bundle_test.go | 35 ++++++ .../operator-controller/resolve/ociimage.go | 118 ++++++++++++++++++ .../resolve/ociimage_test.go | 70 +++++++++++ .../operator-controller/resolve/resolver.go | 18 +++ manifests/experimental-e2e.yaml | 50 +++++++- manifests/experimental.yaml | 50 +++++++- manifests/standard-e2e.yaml | 50 +++++++- manifests/standard.yaml | 50 +++++++- 21 files changed, 771 insertions(+), 23 deletions(-) create mode 100644 applyconfigurations/api/v1/ociimagesource.go create mode 100644 internal/operator-controller/controllers/direct_bundle_test.go create mode 100644 internal/operator-controller/resolve/ociimage.go create mode 100644 internal/operator-controller/resolve/ociimage_test.go diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 244630b87d..688643c766 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -99,7 +99,6 @@ type ClusterExtensionSpec struct { // source is required and selects the installation source of content for this ClusterExtension. // Set the sourceType field to perform the selection. // - // Catalog is currently the only implemented sourceType. // Setting sourceType to "Catalog" requires the catalog field to also be defined. // // Below is a minimal example of a source definition (in yaml): @@ -142,23 +141,30 @@ type ClusterExtensionSpec struct { ProgressDeadlineMinutes int32 `json:"progressDeadlineMinutes,omitempty"` } -const SourceTypeCatalog = "Catalog" +const ( + SourceTypeCatalog = "Catalog" + SourceTypeOCIImage = "OCIImage" +) // SourceConfig is a discriminated union which selects the installation source. // // +union // +kubebuilder:validation:XValidation:rule="has(self.sourceType) && self.sourceType == 'Catalog' ? has(self.catalog) : !has(self.catalog)",message="catalog is required when sourceType is Catalog, and forbidden otherwise" +// +kubebuilder:validation:XValidation:rule="has(self.sourceType) && self.sourceType == 'OCIImage' ? has(self.ociImage) : !has(self.ociImage)",message="ociImage is required when sourceType is OCIImage, and forbidden otherwise" type SourceConfig struct { // sourceType is required and specifies the type of install source. // - // The only allowed value is "Catalog". + // The allowed values are "Catalog" and "OCIImage". + // + // When set to "OCIImage", the bundle image is used directly. Direct sources do not perform + // dependency resolution and are only supported by the Boxcutter runtime. // // When set to "Catalog", information for determining the appropriate bundle of content to install // is fetched from ClusterCatalog resources on the cluster. // When using the Catalog sourceType, the catalog field must also be set. // // +unionDiscriminator - // +kubebuilder:validation:Enum:="Catalog" + // +kubebuilder:validation:Enum:="Catalog";"OCIImage" // +required SourceType string `json:"sourceType"` @@ -167,6 +173,29 @@ type SourceConfig struct { // // +optional Catalog *CatalogFilter `json:"catalog,omitempty"` + + // ociImage configures a bundle image to install directly. + // They do not provide catalog dependency resolution or upgrade safety. + // + // +optional + OCIImage *OCIImageSource `json:"ociImage,omitempty"` +} + +// OCIImageSource identifies a bundle image to install directly from an OCI registry. +type OCIImageSource struct { + // ref is a Docker-style image reference with a tag or digest. + // + // +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" + // +kubebuilder:validation:XValidation:rule="self.find(\"(\\\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)\") != \"\"",message="a valid image name is required" + // +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" + // +kubebuilder:validation:XValidation:rule="self.find(\"(@.*:)\") == \"\" ? (self.find(\":.*$\") != \"\" ? self.find(\":.*$\").matches(\":[\\\\w][\\\\w.-]*$\") : true) : true",message="tag is invalid" + // +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" + // +kubebuilder:validation:XValidation:rule="self.find(\"(@.*:)\") != \"\" ? self.find(\":.*$\").substring(1).size() >= 32 : true",message="digest is not valid" + // +kubebuilder:validation:XValidation:rule="self.find(\"(@.*:)\") != \"\" ? self.find(\":.*$\").matches(\":[0-9A-Fa-f]*$\") : true",message="digest is not valid" + Ref string `json:"ref"` } // ClusterExtensionInstallConfig is a union which selects the clusterExtension installation config. diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 6b54d21ca4..8d501ccc57 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -643,6 +643,21 @@ func (in *ImageSource) DeepCopy() *ImageSource { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OCIImageSource) DeepCopyInto(out *OCIImageSource) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OCIImageSource. +func (in *OCIImageSource) DeepCopy() *OCIImageSource { + if in == nil { + return nil + } + out := new(OCIImageSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ObjectSelector) DeepCopyInto(out *ObjectSelector) { *out = *in @@ -811,6 +826,11 @@ func (in *SourceConfig) DeepCopyInto(out *SourceConfig) { *out = new(CatalogFilter) (*in).DeepCopyInto(*out) } + if in.OCIImage != nil { + in, out := &in.OCIImage, &out.OCIImage + *out = new(OCIImageSource) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SourceConfig. diff --git a/applyconfigurations/api/v1/clusterextensionspec.go b/applyconfigurations/api/v1/clusterextensionspec.go index 8a99964b12..0003e8cca0 100644 --- a/applyconfigurations/api/v1/clusterextensionspec.go +++ b/applyconfigurations/api/v1/clusterextensionspec.go @@ -65,7 +65,6 @@ type ClusterExtensionSpecApplyConfiguration struct { // source is required and selects the installation source of content for this ClusterExtension. // Set the sourceType field to perform the selection. // - // Catalog is currently the only implemented sourceType. // Setting sourceType to "Catalog" requires the catalog field to also be defined. // // Below is a minimal example of a source definition (in yaml): diff --git a/applyconfigurations/api/v1/ociimagesource.go b/applyconfigurations/api/v1/ociimagesource.go new file mode 100644 index 0000000000..11ee5f265d --- /dev/null +++ b/applyconfigurations/api/v1/ociimagesource.go @@ -0,0 +1,41 @@ +/* +Copyright 2022. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +// Code generated by controller-gen-v0.21. DO NOT EDIT. + +package v1 + +// OCIImageSourceApplyConfiguration represents a declarative configuration of the OCIImageSource type for use +// with apply. +// +// OCIImageSource identifies a bundle image to install directly from an OCI registry. +type OCIImageSourceApplyConfiguration struct { + // ref is a Docker-style image reference with a tag or digest. + Ref *string `json:"ref,omitempty"` +} + +// OCIImageSourceApplyConfiguration constructs a declarative configuration of the OCIImageSource type for use with +// apply. +func OCIImageSource() *OCIImageSourceApplyConfiguration { + return &OCIImageSourceApplyConfiguration{} +} + +// WithRef sets the Ref field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Ref field is set to the value of the last call. +func (b *OCIImageSourceApplyConfiguration) WithRef(value string) *OCIImageSourceApplyConfiguration { + b.Ref = &value + return b +} diff --git a/applyconfigurations/api/v1/sourceconfig.go b/applyconfigurations/api/v1/sourceconfig.go index 13221594a1..4b39793b5f 100644 --- a/applyconfigurations/api/v1/sourceconfig.go +++ b/applyconfigurations/api/v1/sourceconfig.go @@ -13,7 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -// Code generated by controller-gen-v0.20. DO NOT EDIT. +// Code generated by controller-gen-v0.21. DO NOT EDIT. package v1 @@ -24,7 +24,10 @@ package v1 type SourceConfigApplyConfiguration struct { // sourceType is required and specifies the type of install source. // - // The only allowed value is "Catalog". + // The allowed values are "Catalog" and "OCIImage". + // + // When set to "OCIImage", the bundle image is used directly. Direct sources do not perform + // dependency resolution and are only supported by the Boxcutter runtime. // // When set to "Catalog", information for determining the appropriate bundle of content to install // is fetched from ClusterCatalog resources on the cluster. @@ -33,6 +36,9 @@ type SourceConfigApplyConfiguration struct { // catalog configures how information is sourced from a catalog. // It is required when sourceType is "Catalog", and forbidden otherwise. Catalog *CatalogFilterApplyConfiguration `json:"catalog,omitempty"` + // ociImage configures a bundle image to install directly. + // They do not provide catalog dependency resolution or upgrade safety. + OCIImage *OCIImageSourceApplyConfiguration `json:"ociImage,omitempty"` } // SourceConfigApplyConfiguration constructs a declarative configuration of the SourceConfig type for use with @@ -56,3 +62,11 @@ func (b *SourceConfigApplyConfiguration) WithCatalog(value *CatalogFilterApplyCo b.Catalog = value return b } + +// WithOCIImage sets the OCIImage field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the OCIImage field is set to the value of the last call. +func (b *SourceConfigApplyConfiguration) WithOCIImage(value *OCIImageSourceApplyConfiguration) *SourceConfigApplyConfiguration { + b.OCIImage = value + return b +} diff --git a/applyconfigurations/internal/internal.go b/applyconfigurations/internal/internal.go index 13a04520f8..39d16afeaa 100644 --- a/applyconfigurations/internal/internal.go +++ b/applyconfigurations/internal/internal.go @@ -384,6 +384,12 @@ var schemaYAML = typed.YAMLObject(`types: - name: ref type: scalar: string +- name: com.github.operator-framework.operator-controller.api.v1.OCIImageSource + map: + fields: + - name: ref + type: + scalar: string - name: com.github.operator-framework.operator-controller.api.v1.ObjectSelector map: fields: @@ -480,6 +486,9 @@ var schemaYAML = typed.YAMLObject(`types: - name: catalog type: namedType: com.github.operator-framework.operator-controller.api.v1.CatalogFilter + - name: ociImage + type: + namedType: com.github.operator-framework.operator-controller.api.v1.OCIImageSource - name: sourceType type: scalar: string diff --git a/applyconfigurations/utils.go b/applyconfigurations/utils.go index 6a467f96a6..6b09afb643 100644 --- a/applyconfigurations/utils.go +++ b/applyconfigurations/utils.go @@ -13,7 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -// Code generated by controller-gen-v0.20. DO NOT EDIT. +// Code generated by controller-gen-v0.21. DO NOT EDIT. package applyconfigurations @@ -85,6 +85,8 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &apiv1.ObjectSourceRefApplyConfiguration{} case v1.SchemeGroupVersion.WithKind("ObservedPhase"): return &apiv1.ObservedPhaseApplyConfiguration{} + case v1.SchemeGroupVersion.WithKind("OCIImageSource"): + return &apiv1.OCIImageSourceApplyConfiguration{} case v1.SchemeGroupVersion.WithKind("PreflightConfig"): return &apiv1.PreflightConfigApplyConfiguration{} case v1.SchemeGroupVersion.WithKind("ProgressionProbe"): diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 5203db1ac4..8cefb20469 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -435,7 +435,7 @@ func run() error { return catalogclient.BuildHTTPClient(cpwCatalogd) }) - resolver := &resolve.CatalogResolver{ + catalogResolver := &resolve.CatalogResolver{ WalkCatalogsFunc: resolve.CatalogWalker( func(ctx context.Context, option ...client.ListOption) ([]ocv1.ClusterCatalog, error) { var catalogs ocv1.ClusterCatalogList @@ -450,6 +450,15 @@ func run() error { resolve.NoDependencyValidation, }, } + resolver := resolve.MultiResolver{ + ocv1.SourceTypeCatalog: catalogResolver, + } + if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { + resolver.RegisterType(ocv1.SourceTypeOCIImage, &resolve.OCIImageResolver{ + Puller: imagePuller, + Cache: imageCache, + }) + } aeClient, err := apiextensionsv1client.NewForConfig(mgr.GetConfig()) if err != nil { @@ -656,6 +665,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl controllers.HandleFinalizers(c.finalizers), controllers.ValidateClusterExtension( controllers.ServiceAccountDeprecationWarning(), + controllers.DirectBundleRequiresBoxcutter(), ), controllers.MigrateStorage(storageMigrator), controllers.RetrieveRevisionStates(revisionStatesGetter), @@ -745,6 +755,7 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster controllers.HandleFinalizers(c.finalizers), controllers.ValidateClusterExtension( controllers.ServiceAccountDeprecationWarning(), + controllers.DirectBundleRequiresBoxcutter(), ), controllers.RetrieveRevisionStates(revisionStatesGetter), controllers.ResolveBundle(c.resolver, c.mgr.GetClient()), diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index ed8d0657e4..d3c15adbdc 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -360,7 +360,7 @@ _Appears in:_ | --- | --- | --- | --- | | `namespace` _string_ | namespace specifies a Kubernetes namespace.
**Standard channel:** It designates the default namespace where namespace-scoped resources for the extension are applied to the cluster.
Some extensions may contain namespace-scoped resources to be applied in other namespaces.
This namespace must exist.
The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,
and be no longer than 63 characters.
[RFC 1123]: https://tools.ietf.org/html/rfc1123

**Experimental channel:**
It designates the default namespace where namespace-scoped resources for the extension
are applied to.
namespace is optional. When set, it must reference an existing namespace on the cluster.
When omitted, operator-controller resolves and creates a managed namespace from the
bundle's metadata. Whether namespace is set or omitted is fixed at creation time and
cannot be changed afterwards.
The namespace field follows the DNS label standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,
and be no longer than 63 characters.
[RFC 1123]: https://tools.ietf.org/html/rfc1123





| | MaxLength: 63
Required: \{\}
| | `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a deprecated field and is completely ignored.
OLMv1 is a single-tenant system where users with ClusterExtension write access are
effectively delegated cluster-admin trust. The operator-controller runs with
cluster-admin privileges and uses its own service account for all cluster interactions.
Deprecated: serviceAccount is no longer used and will be removed in a future release. | | MinProperties: 1
Optional: \{\}
| -| `source` _[SourceConfig](#sourceconfig)_ | source is required and selects the installation source of content for this ClusterExtension.
Set the sourceType field to perform the selection.
Catalog is currently the only implemented sourceType.
Setting sourceType to "Catalog" requires the catalog field to also be defined.
Below is a minimal example of a source definition (in yaml):
source:
sourceType: Catalog
catalog:
packageName: example-package | | Required: \{\}
| +| `source` _[SourceConfig](#sourceconfig)_ | source is required and selects the installation source of content for this ClusterExtension.
Set the sourceType field to perform the selection.
Setting sourceType to "Catalog" requires the catalog field to also be defined.
Below is a minimal example of a source definition (in yaml):
source:
sourceType: Catalog
catalog:
packageName: example-package | | Required: \{\}
| | `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is optional and configures installation options for the ClusterExtension,
such as the pre-flight check configuration. | | Optional: \{\}
| | `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config is optional and specifies bundle-specific configuration.
Configuration is bundle-specific and a bundle may provide a configuration schema.
When not specified, the default configuration of the resolved bundle is used.
config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide
a configuration schema the bundle is deemed to not be configurable. More information on how
to configure bundles can be found in the OLM documentation associated with your current OLM version.
**Experimental channel:** | | Optional: \{\}
| | `progressDeadlineMinutes` _integer_ | progressDeadlineMinutes is an optional field that defines the maximum period
of time in minutes after which an installation should be considered failed and
require manual intervention. This functionality is disabled when no value
is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).
**Experimental channel:** | | Maximum: 720
Minimum: 10
Optional: \{\}
| @@ -457,6 +457,22 @@ _Appears in:_ | `pollIntervalMinutes` _integer_ | pollIntervalMinutes is an optional field that sets the interval, in minutes, at which the image source is polled for new content.
You cannot specify pollIntervalMinutes when ref is a digest-based reference.
When omitted, the image is not polled for new content. | | Minimum: 1
Optional: \{\}
| +#### OCIImageSource + + + +OCIImageSource identifies a bundle image to install directly from an OCI registry. + + + +_Appears in:_ +- [SourceConfig](#sourceconfig) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `ref` _string_ | ref is a Docker-style image reference with a tag or digest. | | MaxLength: 1000
Required: \{\}
| + + #### ObjectSelector @@ -613,8 +629,9 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `sourceType` _string_ | sourceType is required and specifies the type of install source.
The only allowed value is "Catalog".
When set to "Catalog", information for determining the appropriate bundle of content to install
is fetched from ClusterCatalog resources on the cluster.
When using the Catalog sourceType, the catalog field must also be set. | | Enum: [Catalog]
Required: \{\}
| +| `sourceType` _string_ | sourceType is required and specifies the type of install source.
The allowed values are "Catalog" and "OCIImage".
When set to "OCIImage", the bundle image is used directly. Direct sources do not perform
dependency resolution and are only supported by the Boxcutter runtime.
When set to "Catalog", information for determining the appropriate bundle of content to install
is fetched from ClusterCatalog resources on the cluster.
When using the Catalog sourceType, the catalog field must also be set. | | Enum: [Catalog OCIImage]
Required: \{\}
| | `catalog` _[CatalogFilter](#catalogfilter)_ | catalog configures how information is sourced from a catalog.
It is required when sourceType is "Catalog", and forbidden otherwise. | | Optional: \{\}
| +| `ociImage` _[OCIImageSource](#ociimagesource)_ | ociImage configures a bundle image to install directly.
They do not provide catalog dependency resolution or upgrade safety. | | Optional: \{\}
| #### SourceType diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index d5d6e03b6a..189ed19a58 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -228,7 +228,6 @@ spec: source is required and selects the installation source of content for this ClusterExtension. Set the sourceType field to perform the selection. - Catalog is currently the only implemented sourceType. Setting sourceType to "Catalog" requires the catalog field to also be defined. Below is a minimal example of a source definition (in yaml): @@ -477,17 +476,60 @@ spec: required: - packageName type: object + ociImage: + description: |- + ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + properties: + ref: + description: ref is a Docker-style image reference with a + tag or digest. + maxLength: 1000 + type: string + x-kubernetes-validations: + - message: must start with a valid domain + 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 image name is required + 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 + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").substring(1).size() <= 127 : true) + : true' + - message: tag is invalid + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) + : true' + - message: digest algorithm is not valid + 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 + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + >= 32 : true' + - message: digest is not valid + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + : true' + required: + - ref + type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The only allowed value is "Catalog". + The allowed values are "Catalog" and "OCIImage". + + When set to "OCIImage", the bundle image is used directly. Direct sources do not perform + dependency resolution and are only supported by the Boxcutter runtime. When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog + - OCIImage type: string required: - sourceType @@ -497,6 +539,10 @@ spec: otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' + - message: ociImage is required when sourceType is OCIImage, and forbidden + otherwise + rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? + has(self.ociImage) : !has(self.ociImage)' required: - source type: object diff --git a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml index 954dea621e..d7cb6ca823 100644 --- a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml @@ -175,7 +175,6 @@ spec: source is required and selects the installation source of content for this ClusterExtension. Set the sourceType field to perform the selection. - Catalog is currently the only implemented sourceType. Setting sourceType to "Catalog" requires the catalog field to also be defined. Below is a minimal example of a source definition (in yaml): @@ -424,17 +423,60 @@ spec: required: - packageName type: object + ociImage: + description: |- + ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + properties: + ref: + description: ref is a Docker-style image reference with a + tag or digest. + maxLength: 1000 + type: string + x-kubernetes-validations: + - message: must start with a valid domain + 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 image name is required + 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 + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").substring(1).size() <= 127 : true) + : true' + - message: tag is invalid + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) + : true' + - message: digest algorithm is not valid + 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 + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + >= 32 : true' + - message: digest is not valid + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + : true' + required: + - ref + type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The only allowed value is "Catalog". + The allowed values are "Catalog" and "OCIImage". + + When set to "OCIImage", the bundle image is used directly. Direct sources do not perform + dependency resolution and are only supported by the Boxcutter runtime. When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog + - OCIImage type: string required: - sourceType @@ -444,6 +486,10 @@ spec: otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' + - message: ociImage is required when sourceType is OCIImage, and forbidden + otherwise + rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? + has(self.ociImage) : !has(self.ociImage)' required: - namespace - source diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go index 62aef51b60..ee4af4b66a 100644 --- a/internal/operator-controller/controllers/clusterextension_admission_test.go +++ b/internal/operator-controller/controllers/clusterextension_admission_test.go @@ -76,6 +76,54 @@ func TestClusterExtensionSourceConfig(t *testing.T) { } } +func TestClusterExtensionOCIImageSourceConfig(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + source ocv1.SourceConfig + wantError bool + }{ + { + name: "valid tagged image", + source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: &ocv1.OCIImageSource{Ref: "quay.io/example/operator:latest"}, + }, + }, + { + name: "missing image payload", + source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeOCIImage}, + wantError: true, + }, + { + name: "catalog payload with image source", + source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: &ocv1.OCIImageSource{Ref: "quay.io/example/operator:latest"}, + Catalog: &ocv1.CatalogFilter{PackageName: "example"}, + }, + wantError: true, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cl := newClient(t) + err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ + Source: tc.source, + Namespace: "default", + })) + if tc.wantError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + func TestClusterExtensionAdmissionPackageName(t *testing.T) { tooLongError := "spec.source.catalog.packageName: Too long: may not be more than 253" regexMismatchError := "packageName must be a valid DNS1123 subdomain" diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 9f6dea4ea7..90bac4efc2 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -32,6 +32,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" "github.com/operator-framework/operator-controller/internal/shared/labels" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" @@ -110,6 +111,18 @@ func ServiceAccountDeprecationWarning() ClusterExtensionValidator { } } +// DirectBundleRequiresBoxcutter rejects direct OCI image sources when the +// Boxcutter runtime is unavailable. The Helm runtime has no direct-source +// implementation and must never silently interpret the source as a catalog. +func DirectBundleRequiresBoxcutter() ClusterExtensionValidator { + return func(_ context.Context, ext *ocv1.ClusterExtension) error { + if ext.Spec.Source.SourceType == ocv1.SourceTypeOCIImage && !features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { + return fmt.Errorf("sourceType %q requires the %s feature gate", ocv1.SourceTypeOCIImage, features.BoxcutterRuntime) + } + return nil + } +} + func RetrieveRevisionStates(r RevisionStatesGetter) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { l := log.FromContext(ctx) @@ -148,6 +161,27 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc { return nil, nil } + // Direct OCIImage sources have no catalog metadata, so resolve them + // without running catalog fallback or deprecation handling. + if ext.Spec.Source.SourceType == ocv1.SourceTypeOCIImage { + l.V(1).Info("resolving direct OCI image bundle") + resolvedBundle, resolvedBundleVersion, _, err := r.Resolve(ctx, ext, nil) + if err != nil { + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + return nil, err + } + state.hasCatalogData = false + state.resolvedDeprecation = nil + SetDeprecationStatus(ext, installedBundleName(state.revisionStates), nil, false) + state.resolvedRevisionMetadata = &RevisionMetadata{ + Package: resolvedBundle.Package, + Image: resolvedBundle.Image, + BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion), + } + return nil, nil + } + // Resolve a new bundle from the catalog l.V(1).Info("resolving bundle") var bm *ocv1.BundleMetadata @@ -200,6 +234,13 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc { } } +func installedBundleName(states *RevisionStates) string { + if states != nil && states.Installed != nil { + return states.Installed.Name + } + return "" +} + // handleResolutionError handles the case when bundle resolution fails. // // Decision logic (evaluated in order): diff --git a/internal/operator-controller/controllers/direct_bundle_test.go b/internal/operator-controller/controllers/direct_bundle_test.go new file mode 100644 index 0000000000..080d406e1b --- /dev/null +++ b/internal/operator-controller/controllers/direct_bundle_test.go @@ -0,0 +1,35 @@ +package controllers_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" +) + +func TestDirectBundleRequiresBoxcutter(t *testing.T) { + previous := features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) + t.Cleanup(func() { + _ = features.OperatorControllerFeatureGate.Set(string(features.BoxcutterRuntime) + "=" + boolString(previous)) + }) + + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeOCIImage}}} + validator := controllers.DirectBundleRequiresBoxcutter() + + require.NoError(t, features.OperatorControllerFeatureGate.Set(string(features.BoxcutterRuntime)+"=false")) + require.Error(t, validator(context.Background(), ext)) + + require.NoError(t, features.OperatorControllerFeatureGate.Set(string(features.BoxcutterRuntime)+"=true")) + require.NoError(t, validator(context.Background(), ext)) +} + +func boolString(value bool) string { + if value { + return "true" + } + return "false" +} diff --git a/internal/operator-controller/resolve/ociimage.go b/internal/operator-controller/resolve/ociimage.go new file mode 100644 index 0000000000..ee713d4388 --- /dev/null +++ b/internal/operator-controller/resolve/ociimage.go @@ -0,0 +1,118 @@ +package resolve + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io/fs" + + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" + bundlesource "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" + imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" +) + +// OCIImageResolver resolves a bundle directly from an OCI image. The image is +// unpacked through the shared image cache before its content is inspected. +type OCIImageResolver struct { + Puller imageutil.Puller + Cache imageutil.Cache + Detectors []BundleContentDetector +} + +// BundleContentDetector identifies and loads a supported bundle format from +// already-unpacked image content. +type BundleContentDetector interface { + Detect(fs.FS, string) (*declcfg.Bundle, error) +} + +// RegistryV1ContentDetector loads registry+v1 bundles from their filesystem layout. +type RegistryV1ContentDetector struct{} + +func (RegistryV1ContentDetector) Detect(bundleFS fs.FS, image string) (*declcfg.Bundle, error) { + return bundleFromFS(bundleFS, image) +} + +// Resolve loads a registry+v1 bundle from the direct OCIImage source. Direct +// sources intentionally do not consult catalogs or perform dependency resolution. +func (r *OCIImageResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + if ext.Spec.Source.OCIImage == nil { + return nil, nil, nil, reconcile.TerminalError(fmt.Errorf("OCIImage source is missing ociImage.ref")) + } + if r.Puller == nil || r.Cache == nil { + return nil, nil, nil, fmt.Errorf("direct OCIImage resolver is not configured") + } + + imageFS, canonicalRef, _, err := r.Puller.Pull(ctx, ext.Name, ext.Spec.Source.OCIImage.Ref, r.Cache) + if err != nil { + return nil, nil, nil, fmt.Errorf("failed to pull direct bundle image: %w", err) + } + if canonicalRef == nil { + return nil, nil, nil, fmt.Errorf("direct bundle image pull returned no canonical reference") + } + + bundle, err := r.detect(imageFS, canonicalRef.String()) + if err != nil { + return nil, nil, nil, reconcile.TerminalError(fmt.Errorf("invalid direct bundle image: %w", err)) + } + versionRelease, err := bundleutil.GetVersionAndRelease(*bundle) + if err != nil { + return nil, nil, nil, reconcile.TerminalError(err) + } + return bundle, versionRelease, nil, nil +} + +func (r *OCIImageResolver) detect(bundleFS fs.FS, image string) (*declcfg.Bundle, error) { + detectors := r.Detectors + if len(detectors) == 0 { + detectors = []BundleContentDetector{RegistryV1ContentDetector{}} + } + var errs []error + for _, detector := range detectors { + bundle, err := detector.Detect(bundleFS, image) + if err == nil { + return bundle, nil + } + errs = append(errs, err) + } + return nil, errors.Join(errs...) +} + +func bundleFromFS(bundleFS fs.FS, image string) (*declcfg.Bundle, error) { + registryBundle, err := bundlesource.FromFS(bundleFS).GetBundle() + if err != nil { + return nil, err + } + + bundle := &declcfg.Bundle{ + Name: registryBundle.CSV.Name, + Package: registryBundle.PackageName, + Image: image, + } + propertiesJSON := registryBundle.CSV.Annotations[bundlesource.PropertyOLMProperties] + if propertiesJSON == "" { + return nil, fmt.Errorf("bundle %q has no %q package property", bundle.Name, bundlesource.PropertyOLMProperties) + } + if err := json.Unmarshal([]byte(propertiesJSON), &bundle.Properties); err != nil { + return nil, fmt.Errorf("failed to parse bundle properties: %w", err) + } + if !hasPackageProperty(bundle.Properties) { + return nil, fmt.Errorf("bundle %q has no package property", bundle.Name) + } + return bundle, nil +} + +func hasPackageProperty(properties []property.Property) bool { + for _, p := range properties { + if p.Type == property.TypePackage { + return true + } + } + return false +} diff --git a/internal/operator-controller/resolve/ociimage_test.go b/internal/operator-controller/resolve/ociimage_test.go new file mode 100644 index 0000000000..1e88294c26 --- /dev/null +++ b/internal/operator-controller/resolve/ociimage_test.go @@ -0,0 +1,70 @@ +package resolve + +import ( + "context" + "io/fs" + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.podman.io/image/v5/docker/reference" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" + imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" + csvbuilder "github.com/operator-framework/operator-controller/internal/testing/bundle/csv" + bundlefs "github.com/operator-framework/operator-controller/internal/testing/bundle/fs" +) + +func TestOCIImageResolverResolve(t *testing.T) { + ref := "quay.io/example/operator@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + bundleFS := bundlefs.Builder(). + WithPackageName("example-operator"). + WithCSV(csvbuilder.Builder().WithName("example-operator.v1.2.3").WithAnnotations(map[string]string{ + source.PropertyOLMProperties: `[{"type":"olm.package","value":{"packageName":"example-operator","version":"1.2.3"}}]`, + }).Build()). + Build() + + resolver := &OCIImageResolver{Puller: fakePuller{fs: bundleFS, ref: ref}, Cache: fakeCache{}} + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: &ocv1.OCIImageSource{Ref: ref}, + }}} + + bundle, version, deprecation, err := resolver.Resolve(context.Background(), ext, nil) + require.NoError(t, err) + require.Equal(t, "example-operator.v1.2.3", bundle.Name) + require.Equal(t, "example-operator", bundle.Package) + require.Equal(t, ref, bundle.Image) + require.Equal(t, "1.2.3", version.Version.String()) + require.Nil(t, deprecation) +} + +func TestOCIImageResolverRejectsInvalidBundle(t *testing.T) { + ref := "quay.io/example/operator@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + resolver := &OCIImageResolver{Puller: fakePuller{fs: bundlefs.Builder().Build(), ref: ref}, Cache: fakeCache{}} + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: &ocv1.OCIImageSource{Ref: ref}, + }}} + + _, _, _, err := resolver.Resolve(context.Background(), ext, nil) + require.Error(t, err) + require.ErrorIs(t, err, reconcile.TerminalError(nil)) +} + +type fakePuller struct { + fs fs.FS + ref string +} + +func (p fakePuller) Pull(context.Context, string, string, imageutil.Cache) (fs.FS, reference.Canonical, time.Time, error) { + canonical, err := reference.ParseNormalizedNamed(p.ref) + if err != nil { + return nil, nil, time.Time{}, err + } + return p.fs, canonical.(reference.Canonical), time.Time{}, nil +} + +type fakeCache struct{ imageutil.Cache } diff --git a/internal/operator-controller/resolve/resolver.go b/internal/operator-controller/resolve/resolver.go index ef7543b5c8..7ec8d69edb 100644 --- a/internal/operator-controller/resolve/resolver.go +++ b/internal/operator-controller/resolve/resolver.go @@ -2,6 +2,7 @@ package resolve import ( "context" + "fmt" "github.com/operator-framework/operator-registry/alpha/declcfg" @@ -17,3 +18,20 @@ type Func func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle func (f Func) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { return f(ctx, ext, installedBundle) } + +// MultiResolver dispatches bundle resolution by ClusterExtension source type. +type MultiResolver map[string]Resolver + +// RegisterType associates a source type with its resolver. +func (m MultiResolver) RegisterType(sourceType string, resolver Resolver) { + m[sourceType] = resolver +} + +// Resolve dispatches to the resolver selected by the ClusterExtension source type. +func (m MultiResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + resolver, ok := m[ext.Spec.Source.SourceType] + if !ok { + return nil, nil, nil, fmt.Errorf("no resolver for source type %q", ext.Spec.Source.SourceType) + } + return resolver.Resolve(ctx, ext, installedBundle) +} diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 683d3969df..8805cda987 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -842,7 +842,6 @@ spec: source is required and selects the installation source of content for this ClusterExtension. Set the sourceType field to perform the selection. - Catalog is currently the only implemented sourceType. Setting sourceType to "Catalog" requires the catalog field to also be defined. Below is a minimal example of a source definition (in yaml): @@ -1091,17 +1090,60 @@ spec: required: - packageName type: object + ociImage: + description: |- + ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + properties: + ref: + description: ref is a Docker-style image reference with a + tag or digest. + maxLength: 1000 + type: string + x-kubernetes-validations: + - message: must start with a valid domain + 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 image name is required + 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 + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").substring(1).size() <= 127 : true) + : true' + - message: tag is invalid + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) + : true' + - message: digest algorithm is not valid + 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 + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + >= 32 : true' + - message: digest is not valid + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + : true' + required: + - ref + type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The only allowed value is "Catalog". + The allowed values are "Catalog" and "OCIImage". + + When set to "OCIImage", the bundle image is used directly. Direct sources do not perform + dependency resolution and are only supported by the Boxcutter runtime. When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog + - OCIImage type: string required: - sourceType @@ -1111,6 +1153,10 @@ spec: otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' + - message: ociImage is required when sourceType is OCIImage, and forbidden + otherwise + rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? + has(self.ociImage) : !has(self.ociImage)' required: - source type: object diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 1d13fa0c87..c1f14eb73f 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -803,7 +803,6 @@ spec: source is required and selects the installation source of content for this ClusterExtension. Set the sourceType field to perform the selection. - Catalog is currently the only implemented sourceType. Setting sourceType to "Catalog" requires the catalog field to also be defined. Below is a minimal example of a source definition (in yaml): @@ -1052,17 +1051,60 @@ spec: required: - packageName type: object + ociImage: + description: |- + ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + properties: + ref: + description: ref is a Docker-style image reference with a + tag or digest. + maxLength: 1000 + type: string + x-kubernetes-validations: + - message: must start with a valid domain + 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 image name is required + 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 + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").substring(1).size() <= 127 : true) + : true' + - message: tag is invalid + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) + : true' + - message: digest algorithm is not valid + 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 + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + >= 32 : true' + - message: digest is not valid + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + : true' + required: + - ref + type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The only allowed value is "Catalog". + The allowed values are "Catalog" and "OCIImage". + + When set to "OCIImage", the bundle image is used directly. Direct sources do not perform + dependency resolution and are only supported by the Boxcutter runtime. When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog + - OCIImage type: string required: - sourceType @@ -1072,6 +1114,10 @@ spec: otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' + - message: ociImage is required when sourceType is OCIImage, and forbidden + otherwise + rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? + has(self.ociImage) : !has(self.ociImage)' required: - source type: object diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 28dca6563d..a0c8344d20 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -789,7 +789,6 @@ spec: source is required and selects the installation source of content for this ClusterExtension. Set the sourceType field to perform the selection. - Catalog is currently the only implemented sourceType. Setting sourceType to "Catalog" requires the catalog field to also be defined. Below is a minimal example of a source definition (in yaml): @@ -1038,17 +1037,60 @@ spec: required: - packageName type: object + ociImage: + description: |- + ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + properties: + ref: + description: ref is a Docker-style image reference with a + tag or digest. + maxLength: 1000 + type: string + x-kubernetes-validations: + - message: must start with a valid domain + 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 image name is required + 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 + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").substring(1).size() <= 127 : true) + : true' + - message: tag is invalid + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) + : true' + - message: digest algorithm is not valid + 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 + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + >= 32 : true' + - message: digest is not valid + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + : true' + required: + - ref + type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The only allowed value is "Catalog". + The allowed values are "Catalog" and "OCIImage". + + When set to "OCIImage", the bundle image is used directly. Direct sources do not perform + dependency resolution and are only supported by the Boxcutter runtime. When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog + - OCIImage type: string required: - sourceType @@ -1058,6 +1100,10 @@ spec: otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' + - message: ociImage is required when sourceType is OCIImage, and forbidden + otherwise + rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? + has(self.ociImage) : !has(self.ociImage)' required: - namespace - source diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 71c7677772..035322245e 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -750,7 +750,6 @@ spec: source is required and selects the installation source of content for this ClusterExtension. Set the sourceType field to perform the selection. - Catalog is currently the only implemented sourceType. Setting sourceType to "Catalog" requires the catalog field to also be defined. Below is a minimal example of a source definition (in yaml): @@ -999,17 +998,60 @@ spec: required: - packageName type: object + ociImage: + description: |- + ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + properties: + ref: + description: ref is a Docker-style image reference with a + tag or digest. + maxLength: 1000 + type: string + x-kubernetes-validations: + - message: must start with a valid domain + 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 image name is required + 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 + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").substring(1).size() <= 127 : true) + : true' + - message: tag is invalid + rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != + "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) + : true' + - message: digest algorithm is not valid + 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 + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + >= 32 : true' + - message: digest is not valid + rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + : true' + required: + - ref + type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The only allowed value is "Catalog". + The allowed values are "Catalog" and "OCIImage". + + When set to "OCIImage", the bundle image is used directly. Direct sources do not perform + dependency resolution and are only supported by the Boxcutter runtime. When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog + - OCIImage type: string required: - sourceType @@ -1019,6 +1061,10 @@ spec: otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' + - message: ociImage is required when sourceType is OCIImage, and forbidden + otherwise + rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? + has(self.ociImage) : !has(self.ociImage)' required: - namespace - source From 2f8bd128b25cc62c059e8956eaca5ceacae55e7a Mon Sep 17 00:00:00 2001 From: grokspawn Date: Tue, 8 Sep 2026 14:08:32 -0500 Subject: [PATCH 2/3] review updates Signed-off-by: grokspawn --- api/v1/clusterextension_types.go | 79 ++++++++++++--- api/v1/validation_test.go | 2 +- api/v1/zz_generated.deepcopy.go | 12 +-- applyconfigurations/api/v1/ociimagesource.go | 37 ++++++- applyconfigurations/api/v1/sourceconfig.go | 17 ++++ cmd/operator-controller/main.go | 4 +- docs/api-reference/olmv1-api-reference.md | 11 ++- ...peratorframework.io_clusterextensions.yaml | 97 ++++++++++++++----- ...peratorframework.io_clusterextensions.yaml | 51 +--------- ...usterobjectset_controller_internal_test.go | 2 +- .../clusterobjectset_controller_test.go | 10 +- .../clusterextension_admission_test.go | 45 +++++---- .../clusterextension_controller.go | 2 +- .../clusterextension_controller_test.go | 76 +++++++-------- .../clusterextension_reconcile_steps.go | 68 ++++++------- .../controllers/direct_bundle_test.go | 28 +++++- .../operator-controller/resolve/catalog.go | 2 +- .../resolve/catalog_test.go | 8 +- .../operator-controller/resolve/ociimage.go | 67 ++++++------- .../resolve/ociimage_test.go | 23 ++++- .../operator-controller/resolve/resolver.go | 17 ++++ manifests/experimental-e2e.yaml | 97 ++++++++++++++----- manifests/experimental.yaml | 97 ++++++++++++++----- manifests/standard-e2e.yaml | 51 +--------- manifests/standard.yaml | 51 +--------- .../extension_developer_test.go | 2 +- 26 files changed, 541 insertions(+), 415 deletions(-) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 688643c766..54da4b9f69 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -149,11 +149,20 @@ const ( // SourceConfig is a discriminated union which selects the installation source. // // +union -// +kubebuilder:validation:XValidation:rule="has(self.sourceType) && self.sourceType == 'Catalog' ? has(self.catalog) : !has(self.catalog)",message="catalog is required when sourceType is Catalog, and forbidden otherwise" -// +kubebuilder:validation:XValidation:rule="has(self.sourceType) && self.sourceType == 'OCIImage' ? has(self.ociImage) : !has(self.ociImage)",message="ociImage is required when sourceType is OCIImage, and forbidden otherwise" +// +kubebuilder:validation:XValidation:rule="has(self.sourceType) && self.sourceType == 'Catalog' ? self.catalog.size() != 0 : self.catalog.size() == 0",message="catalog is required when sourceType is Catalog, and forbidden otherwise" +// type SourceConfig struct { // sourceType is required and specifies the type of install source. // + // + // The allowed value is "Catalog". + // + // When set to "Catalog", information for determining the appropriate bundle of content to install + // is fetched from ClusterCatalog resources on the cluster. + // When using the Catalog sourceType, the catalog field must also be set. + // + // + // // The allowed values are "Catalog" and "OCIImage". // // When set to "OCIImage", the bundle image is used directly. Direct sources do not perform @@ -162,9 +171,11 @@ type SourceConfig struct { // When set to "Catalog", information for determining the appropriate bundle of content to install // is fetched from ClusterCatalog resources on the cluster. // When using the Catalog sourceType, the catalog field must also be set. + // // // +unionDiscriminator - // +kubebuilder:validation:Enum:="Catalog";"OCIImage" + // +kubebuilder:validation:Enum:="Catalog" + // // +required SourceType string `json:"sourceType"` @@ -172,29 +183,67 @@ type SourceConfig struct { // It is required when sourceType is "Catalog", and forbidden otherwise. // // +optional - Catalog *CatalogFilter `json:"catalog,omitempty"` + Catalog CatalogFilter `json:"catalog,omitzero"` // ociImage configures a bundle image to install directly. + // // They do not provide catalog dependency resolution or upgrade safety. - // + // + // // +optional - OCIImage *OCIImageSource `json:"ociImage,omitempty"` + OCIImage OCIImageSource `json:"ociImage,omitzero"` } // OCIImageSource identifies a bundle image to install directly from an OCI registry. +// +kubebuilder:validation:MinProperties:=1 type OCIImageSource struct { - // ref is a Docker-style image reference with a tag or digest. + // ref is a required field that defines the reference to a container image for a registry+v1 bundle. + // It cannot be more than 1000 characters. + // + // A reference has 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" // // +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" - // +kubebuilder:validation:XValidation:rule="self.find(\"(\\\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)\") != \"\"",message="a valid image name is required" - // +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" - // +kubebuilder:validation:XValidation:rule="self.find(\"(@.*:)\") == \"\" ? (self.find(\":.*$\") != \"\" ? self.find(\":.*$\").matches(\":[\\\\w][\\\\w.-]*$\") : true) : true",message="tag is invalid" - // +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" - // +kubebuilder:validation:XValidation:rule="self.find(\"(@.*:)\") != \"\" ? self.find(\":.*$\").substring(1).size() >= 32 : true",message="digest is not valid" - // +kubebuilder:validation:XValidation:rule="self.find(\"(@.*:)\") != \"\" ? self.find(\":.*$\").matches(\":[0-9A-Fa-f]*$\") : true",message="digest is not valid" + // +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"` } diff --git a/api/v1/validation_test.go b/api/v1/validation_test.go index 2c77c17663..3fa4d2293f 100644 --- a/api/v1/validation_test.go +++ b/api/v1/validation_test.go @@ -27,7 +27,7 @@ func TestValidate(t *testing.T) { s.Namespace = "ns" s.Source = SourceConfig{ SourceType: SourceTypeCatalog, - Catalog: &CatalogFilter{ + Catalog: CatalogFilter{ PackageName: "test", }, } diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 8d501ccc57..4428ecf196 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -821,16 +821,8 @@ func (in *ServiceAccountReference) DeepCopy() *ServiceAccountReference { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SourceConfig) DeepCopyInto(out *SourceConfig) { *out = *in - if in.Catalog != nil { - in, out := &in.Catalog, &out.Catalog - *out = new(CatalogFilter) - (*in).DeepCopyInto(*out) - } - if in.OCIImage != nil { - in, out := &in.OCIImage, &out.OCIImage - *out = new(OCIImageSource) - **out = **in - } + in.Catalog.DeepCopyInto(&out.Catalog) + out.OCIImage = in.OCIImage } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SourceConfig. diff --git a/applyconfigurations/api/v1/ociimagesource.go b/applyconfigurations/api/v1/ociimagesource.go index 11ee5f265d..9dd6b213c9 100644 --- a/applyconfigurations/api/v1/ociimagesource.go +++ b/applyconfigurations/api/v1/ociimagesource.go @@ -22,7 +22,42 @@ package v1 // // OCIImageSource identifies a bundle image to install directly from an OCI registry. type OCIImageSourceApplyConfiguration struct { - // ref is a Docker-style image reference with a tag or digest. + // ref is a required field that defines the reference to a container image for a registry+v1 bundle. + // It cannot be more than 1000 characters. + // + // A reference has 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" Ref *string `json:"ref,omitempty"` } diff --git a/applyconfigurations/api/v1/sourceconfig.go b/applyconfigurations/api/v1/sourceconfig.go index 4b39793b5f..cc9d47d976 100644 --- a/applyconfigurations/api/v1/sourceconfig.go +++ b/applyconfigurations/api/v1/sourceconfig.go @@ -21,9 +21,20 @@ package v1 // with apply. // // SourceConfig is a discriminated union which selects the installation source. +// +// type SourceConfigApplyConfiguration struct { // sourceType is required and specifies the type of install source. // + // + // The allowed value is "Catalog". + // + // When set to "Catalog", information for determining the appropriate bundle of content to install + // is fetched from ClusterCatalog resources on the cluster. + // When using the Catalog sourceType, the catalog field must also be set. + // + // + // // The allowed values are "Catalog" and "OCIImage". // // When set to "OCIImage", the bundle image is used directly. Direct sources do not perform @@ -32,12 +43,18 @@ type SourceConfigApplyConfiguration struct { // When set to "Catalog", information for determining the appropriate bundle of content to install // is fetched from ClusterCatalog resources on the cluster. // When using the Catalog sourceType, the catalog field must also be set. + // + // + // SourceType *string `json:"sourceType,omitempty"` // catalog configures how information is sourced from a catalog. // It is required when sourceType is "Catalog", and forbidden otherwise. Catalog *CatalogFilterApplyConfiguration `json:"catalog,omitempty"` // ociImage configures a bundle image to install directly. + // // They do not provide catalog dependency resolution or upgrade safety. + // + // OCIImage *OCIImageSourceApplyConfiguration `json:"ociImage,omitempty"` } diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 8cefb20469..b22e031446 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -665,7 +665,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl controllers.HandleFinalizers(c.finalizers), controllers.ValidateClusterExtension( controllers.ServiceAccountDeprecationWarning(), - controllers.DirectBundleRequiresBoxcutter(), + controllers.ValidateDirectBundle(), ), controllers.MigrateStorage(storageMigrator), controllers.RetrieveRevisionStates(revisionStatesGetter), @@ -755,7 +755,7 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster controllers.HandleFinalizers(c.finalizers), controllers.ValidateClusterExtension( controllers.ServiceAccountDeprecationWarning(), - controllers.DirectBundleRequiresBoxcutter(), + controllers.ValidateDirectBundle(), ), controllers.RetrieveRevisionStates(revisionStatesGetter), controllers.ResolveBundle(c.resolver, c.mgr.GetClient()), diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index d3c15adbdc..03f88e5c71 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -463,14 +463,15 @@ _Appears in:_ OCIImageSource identifies a bundle image to install directly from an OCI registry. - +_Validation:_ +- MinProperties: 1 _Appears in:_ - [SourceConfig](#sourceconfig) | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `ref` _string_ | ref is a Docker-style image reference with a tag or digest. | | MaxLength: 1000
Required: \{\}
| +| `ref` _string_ | ref is a required field that defines the reference to a container image for a registry+v1 bundle.
It cannot be more than 1000 characters.
A reference has 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
Required: \{\}
| #### ObjectSelector @@ -624,14 +625,16 @@ SourceConfig is a discriminated union which selects the installation source. + + _Appears in:_ - [ClusterExtensionSpec](#clusterextensionspec) | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `sourceType` _string_ | sourceType is required and specifies the type of install source.
The allowed values are "Catalog" and "OCIImage".
When set to "OCIImage", the bundle image is used directly. Direct sources do not perform
dependency resolution and are only supported by the Boxcutter runtime.
When set to "Catalog", information for determining the appropriate bundle of content to install
is fetched from ClusterCatalog resources on the cluster.
When using the Catalog sourceType, the catalog field must also be set. | | Enum: [Catalog OCIImage]
Required: \{\}
| +| `sourceType` _string_ | sourceType is required and specifies the type of install source.
**Standard channel:**
The allowed value is "Catalog".
When set to "Catalog", information for determining the appropriate bundle of content to install
is fetched from ClusterCatalog resources on the cluster.
When using the Catalog sourceType, the catalog field must also be set.

**Experimental channel:**
The allowed values are "Catalog" and "OCIImage".
When set to "OCIImage", the bundle image is used directly. Direct sources do not perform
dependency resolution and are only supported by the Boxcutter runtime.
When set to "Catalog", information for determining the appropriate bundle of content to install
is fetched from ClusterCatalog resources on the cluster.
When using the Catalog sourceType, the catalog field must also be set.

| | Enum: [Catalog]
Required: \{\}
| | `catalog` _[CatalogFilter](#catalogfilter)_ | catalog configures how information is sourced from a catalog.
It is required when sourceType is "Catalog", and forbidden otherwise. | | Optional: \{\}
| -| `ociImage` _[OCIImageSource](#ociimagesource)_ | ociImage configures a bundle image to install directly.
They do not provide catalog dependency resolution or upgrade safety. | | Optional: \{\}
| +| `ociImage` _[OCIImageSource](#ociimagesource)_ | ociImage configures a bundle image to install directly.
**Experimental channel:**
They do not provide catalog dependency resolution or upgrade safety.

**Experimental channel:** | | MinProperties: 1
Optional: \{\}
| #### SourceType diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 189ed19a58..90e646e577 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -479,38 +479,87 @@ spec: ociImage: description: |- ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + minProperties: 1 properties: ref: - description: ref is a Docker-style image reference with a - tag or digest. + description: |- + ref is a required field that defines the reference to a container image for a registry+v1 bundle. + It cannot be more than 1000 characters. + + A reference has 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 - 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 image name is required - rule: self.find("(\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)") + - 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(":.*$") != + rule: self.find('(@.*:)') != "" || self.find(':.*$') != "" - - message: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").substring(1).size() <= 127 : true) + - 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: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) - : true' - - message: digest algorithm is not valid - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + - 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 @@ -538,11 +587,7 @@ spec: - message: catalog is required when sourceType is Catalog, and forbidden otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? - has(self.catalog) : !has(self.catalog)' - - message: ociImage is required when sourceType is OCIImage, and forbidden - otherwise - rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? - has(self.ociImage) : !has(self.ociImage)' + self.catalog.size() != 0 : self.catalog.size() == 0' required: - source type: object diff --git a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml index d7cb6ca823..56ed498771 100644 --- a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml @@ -423,60 +423,17 @@ spec: required: - packageName type: object - ociImage: - description: |- - ociImage configures a bundle image to install directly. - They do not provide catalog dependency resolution or upgrade safety. - properties: - ref: - description: ref is a Docker-style image reference with a - tag or digest. - maxLength: 1000 - type: string - x-kubernetes-validations: - - message: must start with a valid domain - 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 image name is required - 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 - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").substring(1).size() <= 127 : true) - : true' - - message: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) - : true' - - message: digest algorithm is not valid - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() - >= 32 : true' - - message: digest is not valid - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") - : true' - required: - - ref - type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The allowed values are "Catalog" and "OCIImage". - - When set to "OCIImage", the bundle image is used directly. Direct sources do not perform - dependency resolution and are only supported by the Boxcutter runtime. + The allowed value is "Catalog". When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog - - OCIImage type: string required: - sourceType @@ -485,11 +442,7 @@ spec: - message: catalog is required when sourceType is Catalog, and forbidden otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? - has(self.catalog) : !has(self.catalog)' - - message: ociImage is required when sourceType is OCIImage, and forbidden - otherwise - rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? - has(self.ociImage) : !has(self.ociImage)' + self.catalog.size() != 0 : self.catalog.size() == 0' required: - namespace - source diff --git a/internal/object-controller/controllers/clusterobjectset_controller_internal_test.go b/internal/object-controller/controllers/clusterobjectset_controller_internal_test.go index 4d67856176..f8403843f6 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_internal_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_internal_test.go @@ -221,7 +221,7 @@ func newTestClusterExtensionInternal() *ocv1.ClusterExtension { Namespace: "some-namespace", Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "my-package", }, }, diff --git a/internal/object-controller/controllers/clusterobjectset_controller_test.go b/internal/object-controller/controllers/clusterobjectset_controller_test.go index f5ebd7c441..df37e66dc0 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_test.go @@ -1194,7 +1194,7 @@ func newTestClusterExtension() *ocv1.ClusterExtension { Namespace: "some-namespace", Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeCatalog, - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "some-package", }, }, @@ -1410,7 +1410,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ForeignRevisionCollision(t *testi Namespace: "ns-a", Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeCatalog, - Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}, + Catalog: ocv1.CatalogFilter{PackageName: "pkg"}, }, }, } @@ -1420,7 +1420,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ForeignRevisionCollision(t *testi Namespace: "ns-b", Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeCatalog, - Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}, + Catalog: ocv1.CatalogFilter{PackageName: "pkg"}, }, }, } @@ -1476,7 +1476,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ForeignRevisionCollision(t *testi Namespace: "ns-a", Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeCatalog, - Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}, + Catalog: ocv1.CatalogFilter{PackageName: "pkg"}, }, }, } @@ -1532,7 +1532,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ForeignRevisionCollision(t *testi Namespace: "ns-b", Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeCatalog, - Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}, + Catalog: ocv1.CatalogFilter{PackageName: "pkg"}, }, }, } diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go index ee4af4b66a..73360c494e 100644 --- a/internal/operator-controller/controllers/clusterextension_admission_test.go +++ b/internal/operator-controller/controllers/clusterextension_admission_test.go @@ -42,7 +42,7 @@ func TestClusterExtensionSourceConfig(t *testing.T) { err = cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: tc.sourceType, - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-package", }, }, @@ -87,22 +87,29 @@ func TestClusterExtensionOCIImageSourceConfig(t *testing.T) { name: "valid tagged image", source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeOCIImage, - OCIImage: &ocv1.OCIImageSource{Ref: "quay.io/example/operator:latest"}, + OCIImage: ocv1.OCIImageSource{Ref: "quay.io/example/operator:latest"}, }, }, { - name: "missing image payload", - source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeOCIImage}, - wantError: true, + name: "valid tagged image with registry port", + source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: ocv1.OCIImageSource{Ref: "quay.io:5000/example/operator:latest"}, + }, + }, + { + name: "valid digested image with registry port", + source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: ocv1.OCIImageSource{Ref: "quay.io:5000/example/operator@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, + }, }, { - name: "catalog payload with image source", + name: "uppercase repository segment", source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeOCIImage, - OCIImage: &ocv1.OCIImageSource{Ref: "quay.io/example/operator:latest"}, - Catalog: &ocv1.CatalogFilter{PackageName: "example"}, + OCIImage: ocv1.OCIImageSource{Ref: "quay.io/example/Operator:latest"}, }, - wantError: true, }, } @@ -133,7 +140,7 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) { pkgName string errMsg string }{ - {"no package name", "", regexMismatchError}, + {"no package name", "", "catalog is required when sourceType is Catalog"}, {"long package name", strings.Repeat("x", 254), tooLongError}, {"leading digits with hypens", "0my-1package-9name", ""}, {"trailing digits with hypens", "my0-package1-name9", ""}, @@ -162,7 +169,7 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) { err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: tc.pkgName, }, }, @@ -256,7 +263,7 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) { err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "package", Version: tc.version, }, @@ -308,7 +315,7 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) { err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "package", Channels: tc.channels, }, @@ -359,7 +366,7 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) { err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "package", }, }, @@ -380,7 +387,7 @@ func TestClusterExtensionAdmissionNamespaceImmutability(t *testing.T) { return ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "package", }, }, @@ -540,7 +547,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) { err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "package", }, }, @@ -570,7 +577,7 @@ func TestClusterExtensionAdmissionServiceAccountLifecycle(t *testing.T) { return ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{PackageName: "package"}, + Catalog: ocv1.CatalogFilter{PackageName: "package"}, }, Namespace: "default", } @@ -667,7 +674,7 @@ func TestClusterExtensionAdmissionInstall(t *testing.T) { err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "package", }, }, @@ -716,7 +723,7 @@ func Test_ClusterExtensionAdmissionInlineConfig(t *testing.T) { err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "package", }, }, diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 4bbb48506c..3693da1db6 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -370,7 +370,7 @@ type deprecationInfo struct { func buildDeprecationInfo(ext *ocv1.ClusterExtension, installedBundleName string, deprecation *declcfg.Deprecation) deprecationInfo { info := deprecationInfo{BundleStatus: metav1.ConditionUnknown} channelSet := sets.New[string]() - if ext.Spec.Source.Catalog != nil { + if ext.Spec.Source.Catalog.PackageName != "" { channelSet.Insert(ext.Spec.Source.Catalog.Channels...) } diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index 8275b4f828..7906f9af3f 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -104,7 +104,7 @@ func TestClusterExtensionShortCircuitsReconcileDuringDeletion(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, }, }, @@ -142,7 +142,7 @@ func TestClusterExtensionResolutionFails(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, }, }, @@ -205,7 +205,7 @@ func TestClusterExtensionResolutionFailsWithDeprecationData(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{PackageName: pkgName}, + Catalog: ocv1.CatalogFilter{PackageName: pkgName}, }, Namespace: "default", }, @@ -293,7 +293,7 @@ func TestClusterExtensionUpgradeShowsInstalledBundleDeprecation(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{PackageName: pkgName}, + Catalog: ocv1.CatalogFilter{PackageName: pkgName}, }, Namespace: "default", }, @@ -392,7 +392,7 @@ func TestClusterExtensionUpgradeFromDeprecatedBundleClearsDeprecation(t *testing Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{PackageName: pkgName}, + Catalog: ocv1.CatalogFilter{PackageName: pkgName}, }, Namespace: "default", }, @@ -486,7 +486,7 @@ func TestClusterExtensionResolutionFailsWithoutCatalogDeprecationData(t *testing Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{PackageName: pkgName}, + Catalog: ocv1.CatalogFilter{PackageName: pkgName}, }, Namespace: "default", }, @@ -555,7 +555,7 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, Version: pkgVer, Channels: []string{pkgChan}, @@ -676,7 +676,7 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, Version: pkgVer, Channels: []string{pkgChan}, @@ -783,7 +783,7 @@ func TestClusterExtensionBoxcutterApplierFailsDoesNotLeakDeprecationErrors(t *te Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "prometheus", Version: "1.0.0", Channels: []string{"beta"}, @@ -941,7 +941,7 @@ func TestValidateClusterExtension(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-package", }, }, @@ -1025,7 +1025,7 @@ func TestValidateInstallNamespace(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{PackageName: "test-package"}, + Catalog: ocv1.CatalogFilter{PackageName: "test-package"}, }, Namespace: tt.specNamespace, ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck // deprecated field used in test @@ -1121,7 +1121,7 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, Version: pkgVer, Channels: []string{pkgChan}, @@ -1200,7 +1200,7 @@ func TestClusterExtensionManagerFailed(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, Version: pkgVer, Channels: []string{pkgChan}, @@ -1273,7 +1273,7 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) { Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeCatalog, - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, Version: pkgVer, Channels: []string{pkgChan}, @@ -1346,7 +1346,7 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, Version: pkgVer, Channels: []string{pkgChan}, @@ -1428,7 +1428,7 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkgName, Version: pkgVer, Channels: []string{pkgChan}, @@ -1737,7 +1737,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{}, + Catalog: ocv1.CatalogFilter{}, }, }, Status: ocv1.ClusterExtensionStatus{ @@ -1751,7 +1751,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{}, + Catalog: ocv1.CatalogFilter{}, }, }, Status: ocv1.ClusterExtensionStatus{ @@ -1809,7 +1809,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"nondeprecated"}, }, }, @@ -1825,7 +1825,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"nondeprecated"}, }, }, @@ -1886,7 +1886,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel"}, }, }, @@ -1902,7 +1902,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel"}, }, }, @@ -1964,7 +1964,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel"}, }, }, @@ -1980,7 +1980,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel"}, }, }, @@ -2055,7 +2055,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel"}, }, }, @@ -2071,7 +2071,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel"}, }, }, @@ -2140,7 +2140,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel"}, }, }, @@ -2156,7 +2156,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel"}, }, }, @@ -2224,7 +2224,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel", "anotherbadchannel"}, }, }, @@ -2240,7 +2240,7 @@ func TestSetDeprecationStatus(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ Channels: []string{"badchannel", "anotherbadchannel"}, }, }, @@ -2670,7 +2670,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-pkg", // No version - should fall back }, @@ -2751,7 +2751,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-pkg", Version: "1.0.1", // Requesting upgrade }, @@ -2822,7 +2822,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-pkg", // No version - auto-update to latest }, @@ -2921,7 +2921,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-pkg", // No version specified }, @@ -2968,7 +2968,7 @@ func TestCheckCatalogsExist(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-pkg", }, }, @@ -2988,7 +2988,7 @@ func TestCheckCatalogsExist(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-pkg", Selector: nil, // No selector }, @@ -3009,7 +3009,7 @@ func TestCheckCatalogsExist(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-pkg", Selector: &metav1.LabelSelector{}, // Empty selector (matches everything) }, @@ -3030,7 +3030,7 @@ func TestCheckCatalogsExist(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "test-pkg", Selector: &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 90bac4efc2..de368ae05d 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -111,14 +111,21 @@ func ServiceAccountDeprecationWarning() ClusterExtensionValidator { } } -// DirectBundleRequiresBoxcutter rejects direct OCI image sources when the -// Boxcutter runtime is unavailable. The Helm runtime has no direct-source -// implementation and must never silently interpret the source as a catalog. -func DirectBundleRequiresBoxcutter() ClusterExtensionValidator { +// ValidateDirectBundle fails if the Boxcutter runtime is unavailable or if the configuration conflicts with direct bundle installation +func ValidateDirectBundle() ClusterExtensionValidator { return func(_ context.Context, ext *ocv1.ClusterExtension) error { + if ext.Spec.Source.SourceType != ocv1.SourceTypeOCIImage { + return nil + } if ext.Spec.Source.SourceType == ocv1.SourceTypeOCIImage && !features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { return fmt.Errorf("sourceType %q requires the %s feature gate", ocv1.SourceTypeOCIImage, features.BoxcutterRuntime) } + if ext.Spec.Source.OCIImage.Ref == "" { + return fmt.Errorf("sourceType %q requires ociImage.ref", ocv1.SourceTypeOCIImage) + } + if ext.Spec.Source.Catalog.PackageName != "" { + return fmt.Errorf("sourceType %q forbids source.catalog", ocv1.SourceTypeOCIImage) + } return nil } } @@ -152,37 +159,11 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc { // If already rolling out, use existing revision and set deprecation to Unknown (no catalog check) if len(state.revisionStates.RollingOut) > 0 { - installedBundleName := "" - if state.revisionStates.Installed != nil { - installedBundleName = state.revisionStates.Installed.Name - } - SetDeprecationStatus(ext, installedBundleName, nil, false) - state.resolvedRevisionMetadata = state.revisionStates.RollingOut[0] - return nil, nil - } - - // Direct OCIImage sources have no catalog metadata, so resolve them - // without running catalog fallback or deprecation handling. - if ext.Spec.Source.SourceType == ocv1.SourceTypeOCIImage { - l.V(1).Info("resolving direct OCI image bundle") - resolvedBundle, resolvedBundleVersion, _, err := r.Resolve(ctx, ext, nil) - if err != nil { - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) - return nil, err - } - state.hasCatalogData = false - state.resolvedDeprecation = nil SetDeprecationStatus(ext, installedBundleName(state.revisionStates), nil, false) - state.resolvedRevisionMetadata = &RevisionMetadata{ - Package: resolvedBundle.Package, - Image: resolvedBundle.Image, - BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion), - } + state.resolvedRevisionMetadata = state.revisionStates.RollingOut[0] return nil, nil } - // Resolve a new bundle from the catalog l.V(1).Info("resolving bundle") var bm *ocv1.BundleMetadata if state.revisionStates.Installed != nil { @@ -192,10 +173,7 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc { // Get the installed bundle name for deprecation status. // BundleDeprecated should reflect what's currently running, not what we're trying to install. - installedBundleName := "" - if state.revisionStates.Installed != nil { - installedBundleName = state.revisionStates.Installed.Name - } + installedBundleName := installedBundleName(state.revisionStates) // Set deprecation status based on resolution results: // - If resolution succeeds: hasCatalogData=true, deprecation shows catalog data (nil=not deprecated) @@ -211,11 +189,19 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc { // the deprecation status to unknown? Or perhaps we somehow combine the deprecation information from // all catalogs? This needs a follow-up discussion and PR. hasCatalogData := err == nil || resolvedDeprecation != nil + if behavior, ok := r.(resolve.ResolverBehavior); ok { + hasCatalogData = behavior.HasCatalogData(ext) + } state.resolvedDeprecation = resolvedDeprecation state.hasCatalogData = hasCatalogData SetDeprecationStatus(ext, installedBundleName, resolvedDeprecation, hasCatalogData) if err != nil { + if behavior, ok := r.(resolve.ResolverBehavior); ok && !behavior.ShouldFallbackOnError(ext) { + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + return nil, err + } return handleResolutionError(ctx, c, state, ext, err) } @@ -268,7 +254,7 @@ func handleResolutionError(ctx context.Context, c client.Client, state *reconcil // Check if the spec is requesting a specific version that differs from installed specVersion := "" - if ext.Spec.Source.Catalog != nil { + if ext.Spec.Source.Catalog.PackageName != "" { specVersion = ext.Spec.Source.Catalog.Version } installedVersion := state.revisionStates.Installed.Version @@ -291,7 +277,7 @@ func handleResolutionError(ctx context.Context, c client.Client, state *reconcil if catalogCheckErr != nil { msg := fmt.Sprintf("failed to resolve bundle: %v", err) var catalogName string - if ext.Spec.Source.Catalog != nil { + if ext.Spec.Source.Catalog.PackageName != "" { catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) } l.Error(catalogCheckErr, "error checking if ClusterCatalogs exist, will retry resolution", @@ -309,7 +295,7 @@ func handleResolutionError(ctx context.Context, c client.Client, state *reconcil // Retry resolution instead of falling back msg := fmt.Sprintf("failed to resolve bundle, retrying: %v", err) var catalogName string - if ext.Spec.Source.Catalog != nil { + if ext.Spec.Source.Catalog.PackageName != "" { catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) } l.Error(err, "resolution failed but matching ClusterCatalogs exist - retrying instead of falling back", @@ -325,7 +311,7 @@ func handleResolutionError(ctx context.Context, c client.Client, state *reconcil // The controller watches ClusterCatalog resources, so when ClusterCatalogs become available again, // a reconcile will be triggered automatically, allowing the extension to upgrade. var catalogName string - if ext.Spec.Source.Catalog != nil { + if ext.Spec.Source.Catalog.PackageName != "" { catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) } l.Info("matching ClusterCatalogs unavailable or deleted - falling back to installed bundle to maintain workload", @@ -354,7 +340,7 @@ func getCatalogNameFromSelector(selector *metav1.LabelSelector) string { // getPackageName safely extracts the package name from the extension spec. // Returns empty string if Catalog source is nil. func getPackageName(ext *ocv1.ClusterExtension) string { - if ext.Spec.Source.Catalog == nil { + if ext.Spec.Source.Catalog.PackageName == "" { return "" } return ext.Spec.Source.Catalog.PackageName @@ -368,7 +354,7 @@ func CheckCatalogsExist(ctx context.Context, c client.Client, ext *ocv1.ClusterE var catalogList *ocv1.ClusterCatalogList var listErr error - if ext.Spec.Source.Catalog == nil || ext.Spec.Source.Catalog.Selector == nil { + if ext.Spec.Source.Catalog.PackageName == "" || ext.Spec.Source.Catalog.Selector == nil { // No selector means all ClusterCatalogs match - check if any ClusterCatalogs exist at all catalogList = &ocv1.ClusterCatalogList{} listErr = c.List(ctx, catalogList, client.Limit(1)) diff --git a/internal/operator-controller/controllers/direct_bundle_test.go b/internal/operator-controller/controllers/direct_bundle_test.go index 080d406e1b..fe48278d62 100644 --- a/internal/operator-controller/controllers/direct_bundle_test.go +++ b/internal/operator-controller/controllers/direct_bundle_test.go @@ -1,4 +1,4 @@ -package controllers_test +package controllers import ( "context" @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/require" ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" "github.com/operator-framework/operator-controller/internal/operator-controller/features" ) @@ -17,8 +16,11 @@ func TestDirectBundleRequiresBoxcutter(t *testing.T) { _ = features.OperatorControllerFeatureGate.Set(string(features.BoxcutterRuntime) + "=" + boolString(previous)) }) - ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeOCIImage}}} - validator := controllers.DirectBundleRequiresBoxcutter() + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: ocv1.OCIImageSource{Ref: "quay.io/example/operator:latest"}, + }}} + validator := ValidateDirectBundle() require.NoError(t, features.OperatorControllerFeatureGate.Set(string(features.BoxcutterRuntime)+"=false")) require.Error(t, validator(context.Background(), ext)) @@ -27,6 +29,24 @@ func TestDirectBundleRequiresBoxcutter(t *testing.T) { require.NoError(t, validator(context.Background(), ext)) } +func TestValidateDirectBundleSource(t *testing.T) { + validator := ValidateDirectBundle() + + t.Run("requires reference", func(t *testing.T) { + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeOCIImage}}} + require.Error(t, validator(context.Background(), ext)) + }) + + t.Run("rejects catalog payload", func(t *testing.T) { + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: ocv1.OCIImageSource{Ref: "quay.io/example/operator:latest"}, + Catalog: ocv1.CatalogFilter{PackageName: "example"}, + }}} + require.Error(t, validator(context.Background(), ext)) + }) +} + func boolString(value bool) string { if value { return "true" diff --git a/internal/operator-controller/resolve/catalog.go b/internal/operator-controller/resolve/catalog.go index 98a34b5ca4..1cd9bd894a 100644 --- a/internal/operator-controller/resolve/catalog.go +++ b/internal/operator-controller/resolve/catalog.go @@ -47,7 +47,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio // unless overridden, default to selecting all bundles var selector = labels.Everything() var err error - if ext.Spec.Source.Catalog != nil { + if ext.Spec.Source.Catalog.PackageName != "" { selector, err = metav1.LabelSelectorAsSelector(ext.Spec.Source.Catalog.Selector) if err != nil { return nil, nil, nil, fmt.Errorf("desired catalog selector is invalid: %w", err) diff --git a/internal/operator-controller/resolve/catalog_test.go b/internal/operator-controller/resolve/catalog_test.go index 2c6ab5c5b6..2ffd72de4d 100644 --- a/internal/operator-controller/resolve/catalog_test.go +++ b/internal/operator-controller/resolve/catalog_test.go @@ -588,7 +588,7 @@ func buildFooClusterExtension(pkg string, channels []string, version string, upg Namespace: "default", Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: pkg, Version: version, Channels: channels, @@ -704,7 +704,7 @@ func TestInvalidClusterExtensionCatalogMatchExpressions(t *testing.T) { }, Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "foo", Selector: &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ @@ -736,7 +736,7 @@ func TestInvalidClusterExtensionCatalogMatchLabelsName(t *testing.T) { }, Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "foo", Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{"": "value"}, @@ -762,7 +762,7 @@ func TestInvalidClusterExtensionCatalogMatchLabelsValue(t *testing.T) { }, Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: "foo", Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{"name": "&value"}, diff --git a/internal/operator-controller/resolve/ociimage.go b/internal/operator-controller/resolve/ociimage.go index ee713d4388..015e2d962f 100644 --- a/internal/operator-controller/resolve/ociimage.go +++ b/internal/operator-controller/resolve/ociimage.go @@ -3,7 +3,6 @@ package resolve import ( "context" "encoding/json" - "errors" "fmt" "io/fs" @@ -21,28 +20,14 @@ import ( // OCIImageResolver resolves a bundle directly from an OCI image. The image is // unpacked through the shared image cache before its content is inspected. type OCIImageResolver struct { - Puller imageutil.Puller - Cache imageutil.Cache - Detectors []BundleContentDetector -} - -// BundleContentDetector identifies and loads a supported bundle format from -// already-unpacked image content. -type BundleContentDetector interface { - Detect(fs.FS, string) (*declcfg.Bundle, error) -} - -// RegistryV1ContentDetector loads registry+v1 bundles from their filesystem layout. -type RegistryV1ContentDetector struct{} - -func (RegistryV1ContentDetector) Detect(bundleFS fs.FS, image string) (*declcfg.Bundle, error) { - return bundleFromFS(bundleFS, image) + Puller imageutil.Puller + Cache imageutil.Cache } // Resolve loads a registry+v1 bundle from the direct OCIImage source. Direct // sources intentionally do not consult catalogs or perform dependency resolution. func (r *OCIImageResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { - if ext.Spec.Source.OCIImage == nil { + if ext.Spec.Source.OCIImage.Ref == "" { return nil, nil, nil, reconcile.TerminalError(fmt.Errorf("OCIImage source is missing ociImage.ref")) } if r.Puller == nil || r.Cache == nil { @@ -57,7 +42,7 @@ func (r *OCIImageResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensi return nil, nil, nil, fmt.Errorf("direct bundle image pull returned no canonical reference") } - bundle, err := r.detect(imageFS, canonicalRef.String()) + bundle, err := bundleFromFS(imageFS, canonicalRef.String()) if err != nil { return nil, nil, nil, reconcile.TerminalError(fmt.Errorf("invalid direct bundle image: %w", err)) } @@ -68,22 +53,6 @@ func (r *OCIImageResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensi return bundle, versionRelease, nil, nil } -func (r *OCIImageResolver) detect(bundleFS fs.FS, image string) (*declcfg.Bundle, error) { - detectors := r.Detectors - if len(detectors) == 0 { - detectors = []BundleContentDetector{RegistryV1ContentDetector{}} - } - var errs []error - for _, detector := range detectors { - bundle, err := detector.Detect(bundleFS, image) - if err == nil { - return bundle, nil - } - errs = append(errs, err) - } - return nil, errors.Join(errs...) -} - func bundleFromFS(bundleFS fs.FS, image string) (*declcfg.Bundle, error) { registryBundle, err := bundlesource.FromFS(bundleFS).GetBundle() if err != nil { @@ -102,17 +71,35 @@ func bundleFromFS(bundleFS fs.FS, image string) (*declcfg.Bundle, error) { if err := json.Unmarshal([]byte(propertiesJSON), &bundle.Properties); err != nil { return nil, fmt.Errorf("failed to parse bundle properties: %w", err) } - if !hasPackageProperty(bundle.Properties) { - return nil, fmt.Errorf("bundle %q has no package property", bundle.Name) + if err := validatePackageProperty(bundle.Properties, registryBundle.PackageName); err != nil { + return nil, err } return bundle, nil } -func hasPackageProperty(properties []property.Property) bool { +func validatePackageProperty(properties []property.Property, expectedPackageName string) error { + var packageProperties []property.Property for _, p := range properties { if p.Type == property.TypePackage { - return true + packageProperties = append(packageProperties, p) } } - return false + if len(packageProperties) != 1 { + return fmt.Errorf("expected exactly one %q package property, found %d", property.TypePackage, len(packageProperties)) + } + + var packageData struct { + PackageName string `json:"packageName"` + Version string `json:"version"` + } + if err := json.Unmarshal(packageProperties[0].Value, &packageData); err != nil { + return fmt.Errorf("failed to parse %q package property: %w", property.TypePackage, err) + } + if packageData.PackageName == "" || packageData.PackageName != expectedPackageName { + return fmt.Errorf("package property name %q does not match bundle package name %q", packageData.PackageName, expectedPackageName) + } + if packageData.Version == "" { + return fmt.Errorf("package property for %q has no version", expectedPackageName) + } + return nil } diff --git a/internal/operator-controller/resolve/ociimage_test.go b/internal/operator-controller/resolve/ociimage_test.go index 1e88294c26..1f2770bb90 100644 --- a/internal/operator-controller/resolve/ociimage_test.go +++ b/internal/operator-controller/resolve/ociimage_test.go @@ -29,7 +29,7 @@ func TestOCIImageResolverResolve(t *testing.T) { resolver := &OCIImageResolver{Puller: fakePuller{fs: bundleFS, ref: ref}, Cache: fakeCache{}} ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeOCIImage, - OCIImage: &ocv1.OCIImageSource{Ref: ref}, + OCIImage: ocv1.OCIImageSource{Ref: ref}, }}} bundle, version, deprecation, err := resolver.Resolve(context.Background(), ext, nil) @@ -46,7 +46,7 @@ func TestOCIImageResolverRejectsInvalidBundle(t *testing.T) { resolver := &OCIImageResolver{Puller: fakePuller{fs: bundlefs.Builder().Build(), ref: ref}, Cache: fakeCache{}} ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeOCIImage, - OCIImage: &ocv1.OCIImageSource{Ref: ref}, + OCIImage: ocv1.OCIImageSource{Ref: ref}, }}} _, _, _, err := resolver.Resolve(context.Background(), ext, nil) @@ -54,6 +54,25 @@ func TestOCIImageResolverRejectsInvalidBundle(t *testing.T) { require.ErrorIs(t, err, reconcile.TerminalError(nil)) } +func TestOCIImageResolverRejectsMismatchedPackageProperty(t *testing.T) { + ref := "quay.io/example/operator@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + bundleFS := bundlefs.Builder(). + WithPackageName("example-operator"). + WithCSV(csvbuilder.Builder().WithName("example-operator.v1.2.3").WithAnnotations(map[string]string{ + source.PropertyOLMProperties: `[{"type":"olm.package","value":{"packageName":"other-operator","version":"1.2.3"}}]`, + }).Build()). + Build() + resolver := &OCIImageResolver{Puller: fakePuller{fs: bundleFS, ref: ref}, Cache: fakeCache{}} + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeOCIImage, + OCIImage: ocv1.OCIImageSource{Ref: ref}, + }}} + + _, _, _, err := resolver.Resolve(context.Background(), ext, nil) + require.ErrorIs(t, err, reconcile.TerminalError(nil)) + require.ErrorContains(t, err, "does not match bundle package name") +} + type fakePuller struct { fs fs.FS ref string diff --git a/internal/operator-controller/resolve/resolver.go b/internal/operator-controller/resolve/resolver.go index 7ec8d69edb..aac42615f8 100644 --- a/internal/operator-controller/resolve/resolver.go +++ b/internal/operator-controller/resolve/resolver.go @@ -13,6 +13,13 @@ type Resolver interface { Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) } +// ResolverBehavior describes source-specific reconciliation behavior that +// cannot be inferred from a resolved bundle alone. +type ResolverBehavior interface { + HasCatalogData(*ocv1.ClusterExtension) bool + ShouldFallbackOnError(*ocv1.ClusterExtension) bool +} + type Func func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) func (f Func) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { @@ -35,3 +42,13 @@ func (m MultiResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, } return resolver.Resolve(ctx, ext, installedBundle) } + +// HasCatalogData reports whether the selected source can provide catalog metadata. +func (m MultiResolver) HasCatalogData(ext *ocv1.ClusterExtension) bool { + return ext.Spec.Source.SourceType == ocv1.SourceTypeCatalog +} + +// ShouldFallbackOnError reports whether catalog fallback behavior is valid for the source. +func (m MultiResolver) ShouldFallbackOnError(ext *ocv1.ClusterExtension) bool { + return ext.Spec.Source.SourceType == ocv1.SourceTypeCatalog +} diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 8805cda987..12eab919a0 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1093,38 +1093,87 @@ spec: ociImage: description: |- ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + minProperties: 1 properties: ref: - description: ref is a Docker-style image reference with a - tag or digest. + description: |- + ref is a required field that defines the reference to a container image for a registry+v1 bundle. + It cannot be more than 1000 characters. + + A reference has 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 - 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 image name is required - rule: self.find("(\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)") + - 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(":.*$") != + rule: self.find('(@.*:)') != "" || self.find(':.*$') != "" - - message: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").substring(1).size() <= 127 : true) - : true' - - message: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) - : true' - - message: digest algorithm is not valid - rule: 'self.find("(@.*:)") != "" ? self.find("(@.*:)").matches("(@[A-Za-z][A-Za-z0-9]*([-_+.][A-Za-z][A-Za-z0-9]*)*[:])") + - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + - 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 @@ -1152,11 +1201,7 @@ spec: - message: catalog is required when sourceType is Catalog, and forbidden otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? - has(self.catalog) : !has(self.catalog)' - - message: ociImage is required when sourceType is OCIImage, and forbidden - otherwise - rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? - has(self.ociImage) : !has(self.ociImage)' + self.catalog.size() != 0 : self.catalog.size() == 0' required: - source type: object diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index c1f14eb73f..1fcfba52b2 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1054,38 +1054,87 @@ spec: ociImage: description: |- ociImage configures a bundle image to install directly. + They do not provide catalog dependency resolution or upgrade safety. + minProperties: 1 properties: ref: - description: ref is a Docker-style image reference with a - tag or digest. + description: |- + ref is a required field that defines the reference to a container image for a registry+v1 bundle. + It cannot be more than 1000 characters. + + A reference has 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 - 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 image name is required - rule: self.find("(\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?((\\/[a-z0-9]+((([._]|__|[-]*)[a-z0-9]+)+)?)+)?)") + - 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(":.*$") != + rule: self.find('(@.*:)') != "" || self.find(':.*$') != "" - - message: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").substring(1).size() <= 127 : true) - : true' - - message: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) - : true' - - message: digest algorithm is not valid - rule: 'self.find("(@.*:)") != "" ? self.find("(@.*:)").matches("(@[A-Za-z][A-Za-z0-9]*([-_+.][A-Za-z][A-Za-z0-9]*)*[:])") + - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() + - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") + - 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 @@ -1113,11 +1162,7 @@ spec: - message: catalog is required when sourceType is Catalog, and forbidden otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? - has(self.catalog) : !has(self.catalog)' - - message: ociImage is required when sourceType is OCIImage, and forbidden - otherwise - rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? - has(self.ociImage) : !has(self.ociImage)' + self.catalog.size() != 0 : self.catalog.size() == 0' required: - source type: object diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index a0c8344d20..c493c96206 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -1037,60 +1037,17 @@ spec: required: - packageName type: object - ociImage: - description: |- - ociImage configures a bundle image to install directly. - They do not provide catalog dependency resolution or upgrade safety. - properties: - ref: - description: ref is a Docker-style image reference with a - tag or digest. - maxLength: 1000 - type: string - x-kubernetes-validations: - - message: must start with a valid domain - 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 image name is required - 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 - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").substring(1).size() <= 127 : true) - : true' - - message: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) - : true' - - message: digest algorithm is not valid - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() - >= 32 : true' - - message: digest is not valid - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") - : true' - required: - - ref - type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The allowed values are "Catalog" and "OCIImage". - - When set to "OCIImage", the bundle image is used directly. Direct sources do not perform - dependency resolution and are only supported by the Boxcutter runtime. + The allowed value is "Catalog". When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog - - OCIImage type: string required: - sourceType @@ -1099,11 +1056,7 @@ spec: - message: catalog is required when sourceType is Catalog, and forbidden otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? - has(self.catalog) : !has(self.catalog)' - - message: ociImage is required when sourceType is OCIImage, and forbidden - otherwise - rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? - has(self.ociImage) : !has(self.ociImage)' + self.catalog.size() != 0 : self.catalog.size() == 0' required: - namespace - source diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 035322245e..c85bebbae2 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -998,60 +998,17 @@ spec: required: - packageName type: object - ociImage: - description: |- - ociImage configures a bundle image to install directly. - They do not provide catalog dependency resolution or upgrade safety. - properties: - ref: - description: ref is a Docker-style image reference with a - tag or digest. - maxLength: 1000 - type: string - x-kubernetes-validations: - - message: must start with a valid domain - 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 image name is required - 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 - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").substring(1).size() <= 127 : true) - : true' - - message: tag is invalid - rule: 'self.find("(@.*:)") == "" ? (self.find(":.*$") != - "" ? self.find(":.*$").matches(":[\\w][\\w.-]*$") : true) - : true' - - message: digest algorithm is not valid - 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 - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").substring(1).size() - >= 32 : true' - - message: digest is not valid - rule: 'self.find("(@.*:)") != "" ? self.find(":.*$").matches(":[0-9A-Fa-f]*$") - : true' - required: - - ref - type: object sourceType: description: |- sourceType is required and specifies the type of install source. - The allowed values are "Catalog" and "OCIImage". - - When set to "OCIImage", the bundle image is used directly. Direct sources do not perform - dependency resolution and are only supported by the Boxcutter runtime. + The allowed value is "Catalog". When set to "Catalog", information for determining the appropriate bundle of content to install is fetched from ClusterCatalog resources on the cluster. When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog - - OCIImage type: string required: - sourceType @@ -1060,11 +1017,7 @@ spec: - message: catalog is required when sourceType is Catalog, and forbidden otherwise rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? - has(self.catalog) : !has(self.catalog)' - - message: ociImage is required when sourceType is OCIImage, and forbidden - otherwise - rule: 'has(self.sourceType) && self.sourceType == ''OCIImage'' ? - has(self.ociImage) : !has(self.ociImage)' + self.catalog.size() != 0 : self.catalog.size() == 0' required: - namespace - source diff --git a/test/extension-developer-e2e/extension_developer_test.go b/test/extension-developer-e2e/extension_developer_test.go index 9b4626880f..bf1ab611c3 100644 --- a/test/extension-developer-e2e/extension_developer_test.go +++ b/test/extension-developer-e2e/extension_developer_test.go @@ -146,7 +146,7 @@ func TestExtensionDeveloper(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Source: ocv1.SourceConfig{ SourceType: "Catalog", - Catalog: &ocv1.CatalogFilter{ + Catalog: ocv1.CatalogFilter{ PackageName: regPkgName, }, }, From e0332518c9b3054ec00897b7a5b386959e063424 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Wed, 30 Sep 2026 15:53:15 -0500 Subject: [PATCH 3/3] package name/package-property name alignment enforcement in bundle parser Signed-off-by: grokspawn --- .../applier/boxcutter_test.go | 9 +-- .../applier/provider_test.go | 5 +- .../operator-controller/resolve/ociimage.go | 34 -------- .../rukpak/bundle/source/source.go | 47 ++++++++++- .../rukpak/bundle/source/source_test.go | 78 ++++++++++++++++++- internal/testing/bundle/fs/bundlefs.go | 59 ++++++++++++-- 6 files changed, 182 insertions(+), 50 deletions(-) diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index f42ed763f1..28b1c70e59 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -302,10 +302,10 @@ func Test_SimpleRevisionGenerator_GenerateRevision_BundleAnnotations(t *testing. t.Log("by checking bundle properties are added to the revision annotations") require.NotNil(t, rev.Annotations) - require.JSONEq(t, `[{"type":"olm.bundle.property","value":"some-value"},{"type":"olm.another.bundle.property","value":"some-other-value"}]`, rev.Annotations["olm.properties"]) + require.JSONEq(t, `[{"type":"olm.bundle.property","value":"some-value"},{"type":"olm.another.bundle.property","value":"some-other-value"},{"type":"olm.package","value":{"packageName":"test-package","version":"1.0.0"}}]`, rev.Annotations["olm.properties"]) }) - t.Run("olm.properties should not be present if there are no bundle properties", func(t *testing.T) { + t.Run("olm.properties contains the required package property when no custom properties exist", func(t *testing.T) { bundleFS := bundlefs.Builder(). WithPackageName("test-package"). WithCSV(bundlecsv.Builder().WithName("test-csv").Build()). @@ -314,9 +314,8 @@ func Test_SimpleRevisionGenerator_GenerateRevision_BundleAnnotations(t *testing. rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{}) require.NoError(t, err) - t.Log("by checking olm.properties is not present in the revision annotations") - _, ok := rev.Annotations["olm.properties"] - require.False(t, ok, "olm.properties should not be present in the revision annotations") + t.Log("by checking olm.properties contains only the required package property") + require.JSONEq(t, `[{"type":"olm.package","value":{"packageName":"test-package","version":"1.0.0"}}]`, rev.Annotations["olm.properties"]) }) t.Run("csv annotations are not added to the revision annotations", func(t *testing.T) { diff --git a/internal/operator-controller/applier/provider_test.go b/internal/operator-controller/applier/provider_test.go index d1b26faf54..70781f116e 100644 --- a/internal/operator-controller/applier/provider_test.go +++ b/internal/operator-controller/applier/provider_test.go @@ -901,7 +901,10 @@ func Test_RegistryV1HelmChartProvider_Chart(t *testing.T) { require.NotNil(t, chart.Metadata) t.Log("Check Chart metadata contains CSV annotations") - require.Equal(t, map[string]string{"foo": "bar"}, chart.Metadata.Annotations) + require.Equal(t, map[string]string{ + "foo": "bar", + "olm.properties": `[{"type":"olm.package","value":{"packageName":"test","version":"1.0.0"}}]`, + }, chart.Metadata.Annotations) t.Log("Check Chart templates have the same number of resources generated by the renderer") require.Len(t, chart.Templates, 1) diff --git a/internal/operator-controller/resolve/ociimage.go b/internal/operator-controller/resolve/ociimage.go index 015e2d962f..fa147e382a 100644 --- a/internal/operator-controller/resolve/ociimage.go +++ b/internal/operator-controller/resolve/ociimage.go @@ -9,7 +9,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" @@ -65,41 +64,8 @@ func bundleFromFS(bundleFS fs.FS, image string) (*declcfg.Bundle, error) { Image: image, } propertiesJSON := registryBundle.CSV.Annotations[bundlesource.PropertyOLMProperties] - if propertiesJSON == "" { - return nil, fmt.Errorf("bundle %q has no %q package property", bundle.Name, bundlesource.PropertyOLMProperties) - } if err := json.Unmarshal([]byte(propertiesJSON), &bundle.Properties); err != nil { return nil, fmt.Errorf("failed to parse bundle properties: %w", err) } - if err := validatePackageProperty(bundle.Properties, registryBundle.PackageName); err != nil { - return nil, err - } return bundle, nil } - -func validatePackageProperty(properties []property.Property, expectedPackageName string) error { - var packageProperties []property.Property - for _, p := range properties { - if p.Type == property.TypePackage { - packageProperties = append(packageProperties, p) - } - } - if len(packageProperties) != 1 { - return fmt.Errorf("expected exactly one %q package property, found %d", property.TypePackage, len(packageProperties)) - } - - var packageData struct { - PackageName string `json:"packageName"` - Version string `json:"version"` - } - if err := json.Unmarshal(packageProperties[0].Value, &packageData); err != nil { - return fmt.Errorf("failed to parse %q package property: %w", property.TypePackage, err) - } - if packageData.PackageName == "" || packageData.PackageName != expectedPackageName { - return fmt.Errorf("package property name %q does not match bundle package name %q", packageData.PackageName, expectedPackageName) - } - if packageData.Version == "" { - return fmt.Errorf("package property for %q has no version", expectedPackageName) - } - return nil -} diff --git a/internal/operator-controller/rukpak/bundle/source/source.go b/internal/operator-controller/rukpak/bundle/source/source.go index 49ed0c32bf..6ab948b05f 100644 --- a/internal/operator-controller/rukpak/bundle/source/source.go +++ b/internal/operator-controller/rukpak/bundle/source/source.go @@ -25,6 +25,7 @@ const ( ) type BundleSource interface { + // GetBundle returns a structurally parsed registry+v1 bundle with a valid package property. GetBundle() (bundle.RegistryV1, error) } @@ -36,7 +37,11 @@ type RegistryV1Properties struct { type identitySource bundle.RegistryV1 func (r identitySource) GetBundle() (bundle.RegistryV1, error) { - return bundle.RegistryV1(r), nil + reg := bundle.RegistryV1(r) + if err := validatePackageProperty(reg); err != nil { + return reg, err + } + return reg, nil } func FromBundle(rv1 bundle.RegistryV1) BundleSource { @@ -133,10 +138,50 @@ func (f fsBundleSource) GetBundle() (bundle.RegistryV1, error) { if err := copyMetadataPropertiesToCSV(®.CSV, f.FS); err != nil { return reg, err } + if err := validatePackageProperty(reg); err != nil { + return reg, err + } return reg, nil } +// validatePackageProperty ensures the package property agrees with the package +// name from metadata/annotations.yaml. Property data may come from both the CSV +// annotation and metadata/properties.yaml, so require exactly one package entry. +func validatePackageProperty(reg bundle.RegistryV1) error { + propertiesJSON := reg.CSV.Annotations[PropertyOLMProperties] + if propertiesJSON == "" { + return fmt.Errorf("bundle %q has no %q package property", reg.CSV.Name, PropertyOLMProperties) + } + + var properties []property.Property + if err := json.Unmarshal([]byte(propertiesJSON), &properties); err != nil { + return fmt.Errorf("failed to parse bundle properties: %w", err) + } + + var packageProperties []property.Property + for _, p := range properties { + if p.Type == property.TypePackage { + packageProperties = append(packageProperties, p) + } + } + if len(packageProperties) != 1 { + return fmt.Errorf("expected exactly one %q package property, found %d", property.TypePackage, len(packageProperties)) + } + + var packageData property.Package + if err := json.Unmarshal(packageProperties[0].Value, &packageData); err != nil { + return fmt.Errorf("failed to parse %q package property: %w", property.TypePackage, err) + } + if packageData.PackageName == "" || packageData.PackageName != reg.PackageName { + return fmt.Errorf("package property name %q does not match bundle package name %q", packageData.PackageName, reg.PackageName) + } + if packageData.Version == "" { + return fmt.Errorf("package property for %q has no version", reg.PackageName) + } + return nil +} + // copyMetadataPropertiesToCSV copies properties from `metadata/propeties.yaml` (in the filesystem fsys) into // the CSV's `.metadata.annotations['olm.properties']` value, preserving any properties that are already // present in the annotations. diff --git a/internal/operator-controller/rukpak/bundle/source/source_test.go b/internal/operator-controller/rukpak/bundle/source/source_test.go index 1a2ab4698f..23bc139a5c 100644 --- a/internal/operator-controller/rukpak/bundle/source/source_test.go +++ b/internal/operator-controller/rukpak/bundle/source/source_test.go @@ -24,12 +24,27 @@ const ( func Test_FromBundle_Success(t *testing.T) { expectedBundle := bundle.RegistryV1{ PackageName: "my-package", + CSV: csv.Builder().WithAnnotations(map[string]string{ + olmProperties: `[{"type":"olm.package","value":{"packageName":"my-package","version":"1.0.0"}}]`, + }).Build(), } b, err := source.FromBundle(expectedBundle).GetBundle() require.NoError(t, err) require.Equal(t, expectedBundle, b) } +func Test_FromBundle_RejectsMismatchedPackageProperty(t *testing.T) { + reg := bundle.RegistryV1{ + PackageName: "expected-package", + CSV: csv.Builder().WithAnnotations(map[string]string{ + olmProperties: `[{"type":"olm.package","value":{"packageName":"other-package","version":"1.0.0"}}]`, + }).Build(), + } + + _, err := source.FromBundle(reg).GetBundle() + require.ErrorContains(t, err, "does not match bundle package name") +} + func Test_FromFS_Success(t *testing.T) { bundleFS := bundlefs.Builder(). WithPackageName("test"). @@ -37,7 +52,7 @@ func Test_FromFS_Success(t *testing.T) { WithBundleResource("csv.yaml", ptr.To(csv.Builder(). WithName("test.v1.0.0"). WithAnnotations(map[string]string{ - "olm.properties": `[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]`, + olmProperties: `[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"},{"type":"olm.package","value":{"packageName":"test","version":"1.0.0"}}]`, }). WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build())). Build() @@ -49,7 +64,66 @@ func Test_FromFS_Success(t *testing.T) { require.Equal(t, "test", rv1.PackageName) t.Log("Check metadata/properties.yaml is merged into csv.annotations[olm.properties]") - require.JSONEq(t, `[{"type":"from-csv-annotations-key","value":"from-csv-annotations-value"},{"type":"from-file-key","value":"from-file-value"}]`, rv1.CSV.Annotations[olmProperties]) + require.JSONEq(t, `[{"type":"from-csv-annotations-key","value":"from-csv-annotations-value"},{"type":"olm.package","value":{"packageName":"test","version":"1.0.0"}},{"type":"from-file-key","value":"from-file-value"}]`, rv1.CSV.Annotations[olmProperties]) +} + +func Test_FromFS_RejectsInvalidPackageProperties(t *testing.T) { + testCases := []struct { + name string + propertiesJSON string + wantError string + }{ + { + name: "missing olm.properties", + wantError: `has no "olm.properties" package property`, + }, + { + name: "missing package property", + propertiesJSON: `[{"type":"example.property","value":"value"}]`, + wantError: `expected exactly one "olm.package" package property, found 0`, + }, + { + name: "duplicate package properties", + propertiesJSON: `[{"type":"olm.package","value":{"packageName":"test","version":"1.0.0"}},` + + `{"type":"olm.package","value":{"packageName":"test","version":"1.0.1"}}]`, + wantError: `expected exactly one "olm.package" package property, found 2`, + }, + { + name: "mismatched package name", + propertiesJSON: `[{"type":"olm.package","value":{"packageName":"other-package","version":"1.0.0"}}]`, + wantError: "does not match bundle package name", + }, + { + name: "missing package version", + propertiesJSON: `[{"type":"olm.package","value":{"packageName":"test"}}]`, + wantError: `package property for "test" has no version`, + }, + { + name: "malformed package value", + propertiesJSON: `[{"type":"olm.package","value":"not-an-object"}]`, + wantError: `failed to parse "olm.package" package property`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + annotations := map[string]string{} + if tc.propertiesJSON != "" { + annotations[olmProperties] = tc.propertiesJSON + } + bundleFS := bundlefs.Builder(). + WithPackageName("test"). + WithoutDefaultPackageProperty(). + WithBundleResource("csv.yaml", ptr.To(csv.Builder(). + WithName("test.v1.0.0"). + WithAnnotations(annotations). + Build())). + Build() + + _, err := source.FromFS(bundleFS).GetBundle() + require.ErrorContains(t, err, tc.wantError) + }) + } } func Test_FromFS_Fails(t *testing.T) { diff --git a/internal/testing/bundle/fs/bundlefs.go b/internal/testing/bundle/fs/bundlefs.go index 9f5b699865..3db0d2d95f 100644 --- a/internal/testing/bundle/fs/bundlefs.go +++ b/internal/testing/bundle/fs/bundlefs.go @@ -1,6 +1,7 @@ package fs import ( + "encoding/json" "fmt" "path/filepath" "strings" @@ -28,6 +29,7 @@ type BundleFSBuilder interface { WithChannels(channels ...string) BundleFSBuilder WithDefaultChannel(channel string) BundleFSBuilder WithBundleProperty(propertyType string, value string) BundleFSBuilder + WithoutDefaultPackageProperty() BundleFSBuilder WithBundleResource(resourceName string, resource client.Object) BundleFSBuilder WithCSV(csv v1alpha1.ClusterServiceVersion) BundleFSBuilder Build() fstest.MapFS @@ -35,17 +37,19 @@ type BundleFSBuilder interface { // bundleFSBuilder builds a registry+v1 bundle filesystem type bundleFSBuilder struct { - annotations *registry.Annotations - properties []property.Property - resources map[string]client.Object + annotations *registry.Annotations + properties []property.Property + resources map[string]client.Object + withoutDefaultPackageProperty bool } func Builder() BundleFSBuilder { return &bundleFSBuilder{} } -// WithPackageName is an option for NewBundleFS used to set the package name annotation in the -// bundle filesystem metadata/annotations.yaml file +// WithPackageName sets the package name annotation in metadata/annotations.yaml. +// When a CSV is present and no olm.package property is supplied, Build adds a +// matching package property so the resulting bundle satisfies the parser contract. func (b *bundleFSBuilder) WithPackageName(packageName string) BundleFSBuilder { if b.annotations == nil { b.annotations = ®istry.Annotations{} @@ -54,6 +58,13 @@ func (b *bundleFSBuilder) WithPackageName(packageName string) BundleFSBuilder { return b } +// WithoutDefaultPackageProperty disables the default olm.package property added +// when a bundle has a package name but no package property of its own. +func (b *bundleFSBuilder) WithoutDefaultPackageProperty() BundleFSBuilder { + b.withoutDefaultPackageProperty = true + return b +} + // WithChannels is an option for NewBundleFS used to set the channels annotation in the // bundle filesystem metadata/annotations.yaml file func (b *bundleFSBuilder) WithChannels(channels ...string) BundleFSBuilder { @@ -108,6 +119,19 @@ func (b *bundleFSBuilder) WithCSV(csv v1alpha1.ClusterServiceVersion) BundleFSBu // By default, an empty registry+v1 bundle filesystem will be returned func (b *bundleFSBuilder) Build() fstest.MapFS { bundleFS := fstest.MapFS{} + properties := append([]property.Property(nil), b.properties...) + _, hasCSV := b.resources["csv.yaml"] + if hasCSV && b.annotations != nil && b.annotations.PackageName != "" && !b.withoutDefaultPackageProperty && + !hasPackageProperty(properties) && !b.csvHasPackageProperty() { + packageJSON, err := json.Marshal(property.Package{ + PackageName: b.annotations.PackageName, + Version: "1.0.0", + }) + if err != nil { + panic(fmt.Errorf("error building package property: %w", err)) + } + properties = append(properties, property.Property{Type: property.TypePackage, Value: packageJSON}) + } // Add annotations metadata if b.annotations != nil { @@ -121,9 +145,9 @@ func (b *bundleFSBuilder) Build() fstest.MapFS { } // Add property metadata - if len(b.properties) > 0 { + if len(properties) > 0 { propertiesYml, err := yaml.Marshal(source.RegistryV1Properties{ - Properties: b.properties, + Properties: properties, }) if err != nil { panic(fmt.Errorf("error building bundle fs: %w", err)) @@ -143,3 +167,24 @@ func (b *bundleFSBuilder) Build() fstest.MapFS { return bundleFS } + +func hasPackageProperty(properties []property.Property) bool { + for _, p := range properties { + if p.Type == property.TypePackage { + return true + } + } + return false +} + +func (b *bundleFSBuilder) csvHasPackageProperty() bool { + csv, ok := b.resources["csv.yaml"].(*v1alpha1.ClusterServiceVersion) + if !ok || csv.Annotations == nil { + return false + } + var properties []property.Property + if err := json.Unmarshal([]byte(csv.Annotations[source.PropertyOLMProperties]), &properties); err != nil { + return false + } + return hasPackageProperty(properties) +}