Skip to content

Commit fd87d09

Browse files
committed
more review updates
Signed-off-by: grokspawn <jordan@nimblewidget.com>
1 parent 2f8bd12 commit fd87d09

12 files changed

Lines changed: 28 additions & 69 deletions

File tree

‎api/v1/clusterextension_types.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ type ClusterExtensionSpec struct {
100100
// Set the sourceType field to perform the selection.
101101
//
102102
// Setting sourceType to "Catalog" requires the catalog field to also be defined.
103+
// <opcon:experimental:description>
104+
// Setting sourceType to "OCIImage" requires the ociImage field to also be defined.
105+
// </opcon:experimental:description>
103106
//
104107
// Below is a minimal example of a source definition (in yaml):
105108
//
@@ -149,13 +152,13 @@ const (
149152
// SourceConfig is a discriminated union which selects the installation source.
150153
//
151154
// +union
152-
// +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"
153-
// <opcon:experimental:validation:XValidation:rule="has(self.sourceType) && self.sourceType == 'OCIImage' ? self.ociImage.size() != 0 : self.ociImage.size() == 0",message="ociImage is required when sourceType is OCIImage, and forbidden otherwise">
155+
// +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"
156+
// <opcon:experimental: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">
154157
type SourceConfig struct {
155158
// sourceType is required and specifies the type of install source.
156159
//
157160
// <opcon:standard:description>
158-
// The allowed value is "Catalog".
161+
// The only allowed value is "Catalog".
159162
//
160163
// When set to "Catalog", information for determining the appropriate bundle of content to install
161164
// is fetched from ClusterCatalog resources on the cluster.

‎applyconfigurations/api/v1/clusterextensionspec.go‎

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

‎applyconfigurations/api/v1/sourceconfig.go‎

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

‎docs/api-reference/olmv1-api-reference.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ _Appears in:_
360360
| --- | --- | --- | --- |
361361
| `namespace` _string_ | namespace specifies a Kubernetes namespace.<br />**Standard channel:** It designates the default namespace where namespace-scoped resources for the extension are applied to the cluster.<br />Some extensions may contain namespace-scoped resources to be applied in other namespaces.<br />This namespace must exist.<br />The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123].<br />It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,<br />and be no longer than 63 characters.<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123<br /><br />**Experimental channel:** <br />It designates the default namespace where namespace-scoped resources for the extension<br />are applied to.<br />namespace is optional. When set, it must reference an existing namespace on the cluster.<br />When omitted, operator-controller resolves and creates a managed namespace from the<br />bundle's metadata. Whether namespace is set or omitted is fixed at creation time and<br />cannot be changed afterwards.<br />The namespace field follows the DNS label standard as defined in [RFC 1123].<br />It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,<br />and be no longer than 63 characters.<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123<br /><br /><br /><br /><br /><br /> | | MaxLength: 63 <br />Required: \{\} <br /> |
362362
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a deprecated field and is completely ignored.<br />OLMv1 is a single-tenant system where users with ClusterExtension write access are<br />effectively delegated cluster-admin trust. The operator-controller runs with<br />cluster-admin privileges and uses its own service account for all cluster interactions.<br />Deprecated: serviceAccount is no longer used and will be removed in a future release. | | MinProperties: 1 <br />Optional: \{\} <br /> |
363-
| `source` _[SourceConfig](#sourceconfig)_ | source is required and selects the installation source of content for this ClusterExtension.<br />Set the sourceType field to perform the selection.<br />Setting sourceType to "Catalog" requires the catalog field to also be defined.<br />Below is a minimal example of a source definition (in yaml):<br />source:<br /> sourceType: Catalog<br /> catalog:<br /> packageName: example-package | | Required: \{\} <br /> |
363+
| `source` _[SourceConfig](#sourceconfig)_ | source is required and selects the installation source of content for this ClusterExtension.<br />Set the sourceType field to perform the selection.<br />Setting sourceType to "Catalog" requires the catalog field to also be defined.<br />**Experimental channel:** <br />Setting sourceType to "OCIImage" requires the ociImage field to also be defined.<br /><br />Below is a minimal example of a source definition (in yaml):<br />source:<br /> sourceType: Catalog<br /> catalog:<br /> packageName: example-package | | Required: \{\} <br /> |
364364
| `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is optional and configures installation options for the ClusterExtension,<br />such as the pre-flight check configuration. | | Optional: \{\} <br /> |
365365
| `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config is optional and specifies bundle-specific configuration.<br />Configuration is bundle-specific and a bundle may provide a configuration schema.<br />When not specified, the default configuration of the resolved bundle is used.<br />config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide<br />a configuration schema the bundle is deemed to not be configurable. More information on how<br />to configure bundles can be found in the OLM documentation associated with your current OLM version.<br />**Experimental channel:** | | Optional: \{\} <br /> |
366366
| `progressDeadlineMinutes` _integer_ | progressDeadlineMinutes is an optional field that defines the maximum period<br />of time in minutes after which an installation should be considered failed and<br />require manual intervention. This functionality is disabled when no value<br />is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).<br />**Experimental channel:** | | Maximum: 720 <br />Minimum: 10 <br />Optional: \{\} <br /> |
@@ -632,7 +632,7 @@ _Appears in:_
632632

633633
| Field | Description | Default | Validation |
634634
| --- | --- | --- | --- |
635-
| `sourceType` _string_ | sourceType is required and specifies the type of install source.<br />**Standard channel:** <br />The allowed value is "Catalog".<br />When set to "Catalog", information for determining the appropriate bundle of content to install<br />is fetched from ClusterCatalog resources on the cluster.<br />When using the Catalog sourceType, the catalog field must also be set.<br /><br />**Experimental channel:** <br />The allowed values are "Catalog" and "OCIImage".<br />When set to "OCIImage", the bundle image is used directly. Direct sources do not perform<br />dependency resolution and are only supported by the Boxcutter runtime.<br />When set to "Catalog", information for determining the appropriate bundle of content to install<br />is fetched from ClusterCatalog resources on the cluster.<br />When using the Catalog sourceType, the catalog field must also be set.<br /><br /> | | Enum: [Catalog] <br />Required: \{\} <br /> |
635+
| `sourceType` _string_ | sourceType is required and specifies the type of install source.<br />**Standard channel:** <br />The only allowed value is "Catalog".<br />When set to "Catalog", information for determining the appropriate bundle of content to install<br />is fetched from ClusterCatalog resources on the cluster.<br />When using the Catalog sourceType, the catalog field must also be set.<br /><br />**Experimental channel:** <br />The allowed values are "Catalog" and "OCIImage".<br />When set to "OCIImage", the bundle image is used directly. Direct sources do not perform<br />dependency resolution and are only supported by the Boxcutter runtime.<br />When set to "Catalog", information for determining the appropriate bundle of content to install<br />is fetched from ClusterCatalog resources on the cluster.<br />When using the Catalog sourceType, the catalog field must also be set.<br /><br /> | | Enum: [Catalog] <br />Required: \{\} <br /> |
636636
| `catalog` _[CatalogFilter](#catalogfilter)_ | catalog configures how information is sourced from a catalog.<br />It is required when sourceType is "Catalog", and forbidden otherwise. | | Optional: \{\} <br /> |
637637
| `ociImage` _[OCIImageSource](#ociimagesource)_ | ociImage configures a bundle image to install directly.<br />**Experimental channel:** <br />They do not provide catalog dependency resolution or upgrade safety.<br /><br />**Experimental channel:** | | MinProperties: 1 <br />Optional: \{\} <br /> |
638638

‎helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ spec:
230230
231231
Setting sourceType to "Catalog" requires the catalog field to also be defined.
232232
233+
Setting sourceType to "OCIImage" requires the ociImage field to also be defined.
234+
233235
Below is a minimal example of a source definition (in yaml):
234236
235237
source:
@@ -587,7 +589,7 @@ spec:
587589
- message: catalog is required when sourceType is Catalog, and forbidden
588590
otherwise
589591
rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ?
590-
self.catalog.size() != 0 : self.catalog.size() == 0'
592+
has(self.catalog) : !has(self.catalog)'
591593
required:
592594
- source
593595
type: object

‎helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ spec:
427427
description: |-
428428
sourceType is required and specifies the type of install source.
429429
430-
The allowed value is "Catalog".
430+
The only allowed value is "Catalog".
431431
432432
When set to "Catalog", information for determining the appropriate bundle of content to install
433433
is fetched from ClusterCatalog resources on the cluster.
@@ -442,7 +442,7 @@ spec:
442442
- message: catalog is required when sourceType is Catalog, and forbidden
443443
otherwise
444444
rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ?
445-
self.catalog.size() != 0 : self.catalog.size() == 0'
445+
has(self.catalog) : !has(self.catalog)'
446446
required:
447447
- namespace
448448
- source

‎internal/operator-controller/resolve/ociimage.go‎

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"sigs.k8s.io/controller-runtime/pkg/reconcile"
1010

1111
"github.com/operator-framework/operator-registry/alpha/declcfg"
12-
"github.com/operator-framework/operator-registry/alpha/property"
1312

1413
ocv1 "github.com/operator-framework/operator-controller/api/v1"
1514
"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
@@ -65,41 +64,8 @@ func bundleFromFS(bundleFS fs.FS, image string) (*declcfg.Bundle, error) {
6564
Image: image,
6665
}
6766
propertiesJSON := registryBundle.CSV.Annotations[bundlesource.PropertyOLMProperties]
68-
if propertiesJSON == "" {
69-
return nil, fmt.Errorf("bundle %q has no %q package property", bundle.Name, bundlesource.PropertyOLMProperties)
70-
}
7167
if err := json.Unmarshal([]byte(propertiesJSON), &bundle.Properties); err != nil {
7268
return nil, fmt.Errorf("failed to parse bundle properties: %w", err)
7369
}
74-
if err := validatePackageProperty(bundle.Properties, registryBundle.PackageName); err != nil {
75-
return nil, err
76-
}
7770
return bundle, nil
7871
}
79-
80-
func validatePackageProperty(properties []property.Property, expectedPackageName string) error {
81-
var packageProperties []property.Property
82-
for _, p := range properties {
83-
if p.Type == property.TypePackage {
84-
packageProperties = append(packageProperties, p)
85-
}
86-
}
87-
if len(packageProperties) != 1 {
88-
return fmt.Errorf("expected exactly one %q package property, found %d", property.TypePackage, len(packageProperties))
89-
}
90-
91-
var packageData struct {
92-
PackageName string `json:"packageName"`
93-
Version string `json:"version"`
94-
}
95-
if err := json.Unmarshal(packageProperties[0].Value, &packageData); err != nil {
96-
return fmt.Errorf("failed to parse %q package property: %w", property.TypePackage, err)
97-
}
98-
if packageData.PackageName == "" || packageData.PackageName != expectedPackageName {
99-
return fmt.Errorf("package property name %q does not match bundle package name %q", packageData.PackageName, expectedPackageName)
100-
}
101-
if packageData.Version == "" {
102-
return fmt.Errorf("package property for %q has no version", expectedPackageName)
103-
}
104-
return nil
105-
}

‎internal/operator-controller/resolve/ociimage_test.go‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,25 +54,6 @@ func TestOCIImageResolverRejectsInvalidBundle(t *testing.T) {
5454
require.ErrorIs(t, err, reconcile.TerminalError(nil))
5555
}
5656

57-
func TestOCIImageResolverRejectsMismatchedPackageProperty(t *testing.T) {
58-
ref := "quay.io/example/operator@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
59-
bundleFS := bundlefs.Builder().
60-
WithPackageName("example-operator").
61-
WithCSV(csvbuilder.Builder().WithName("example-operator.v1.2.3").WithAnnotations(map[string]string{
62-
source.PropertyOLMProperties: `[{"type":"olm.package","value":{"packageName":"other-operator","version":"1.2.3"}}]`,
63-
}).Build()).
64-
Build()
65-
resolver := &OCIImageResolver{Puller: fakePuller{fs: bundleFS, ref: ref}, Cache: fakeCache{}}
66-
ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{
67-
SourceType: ocv1.SourceTypeOCIImage,
68-
OCIImage: ocv1.OCIImageSource{Ref: ref},
69-
}}}
70-
71-
_, _, _, err := resolver.Resolve(context.Background(), ext, nil)
72-
require.ErrorIs(t, err, reconcile.TerminalError(nil))
73-
require.ErrorContains(t, err, "does not match bundle package name")
74-
}
75-
7657
type fakePuller struct {
7758
fs fs.FS
7859
ref string

‎manifests/experimental-e2e.yaml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,8 @@ spec:
844844
845845
Setting sourceType to "Catalog" requires the catalog field to also be defined.
846846
847+
Setting sourceType to "OCIImage" requires the ociImage field to also be defined.
848+
847849
Below is a minimal example of a source definition (in yaml):
848850
849851
source:
@@ -1201,7 +1203,7 @@ spec:
12011203
- message: catalog is required when sourceType is Catalog, and forbidden
12021204
otherwise
12031205
rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ?
1204-
self.catalog.size() != 0 : self.catalog.size() == 0'
1206+
has(self.catalog) : !has(self.catalog)'
12051207
required:
12061208
- source
12071209
type: object

‎manifests/experimental.yaml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,8 @@ spec:
805805
806806
Setting sourceType to "Catalog" requires the catalog field to also be defined.
807807
808+
Setting sourceType to "OCIImage" requires the ociImage field to also be defined.
809+
808810
Below is a minimal example of a source definition (in yaml):
809811
810812
source:
@@ -1162,7 +1164,7 @@ spec:
11621164
- message: catalog is required when sourceType is Catalog, and forbidden
11631165
otherwise
11641166
rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ?
1165-
self.catalog.size() != 0 : self.catalog.size() == 0'
1167+
has(self.catalog) : !has(self.catalog)'
11661168
required:
11671169
- source
11681170
type: object

0 commit comments

Comments
 (0)