From 8d43ef46e20b20cc310a67d89fa3e7b1f3f994f7 Mon Sep 17 00:00:00 2001 From: Predrag Knezevic Date: Fri, 20 Mar 2026 12:59:57 +0100 Subject: [PATCH 1/3] Externalize CER phase objects into Secret refs Add support for storing ClusterExtensionRevision phase objects in content-addressable immutable Secrets instead of inline in the CER spec. This removes the etcd object size limit as a constraint on bundle size. API changes: - Add ObjectSourceRef type with name, namespace, and key fields - Make ClusterExtensionRevisionObject.Object optional (omitzero) - Add optional Ref field with XValidation ensuring exactly one is set - Add RefResolutionFailed condition reason - Add RevisionNameKey label for ref Secret association Applier (boxcutter.go): - Add SecretPacker to bin-pack serialized objects into Secrets with gzip compression for objects exceeding 800KiB - Add createExternalizedRevision with crash-safe three-step sequence: create Secrets, create CER with refs, patch ownerReferences - Externalize desiredRevision before SSA comparison so the patch compares refs-vs-refs instead of inline-vs-refs - Add ensureSecretOwnerReferences for crash recovery - Pass SystemNamespace to Boxcutter from main.go CER controller: - Add resolveObjectRef to fetch and decompress objects from Secrets - Handle ref resolution in buildBoxcutterPhases - Add RBAC for Secret get/list/watch E2e tests: - Add scenario verifying refs, immutability, labels, and ownerRefs - Add step definitions for ref Secret validation - Fix listExtensionRevisionResources and ClusterExtensionRevisionObjectsNotFoundOrNotOwned to resolve refs Co-Authored-By: Claude Opus 4.6 --- api/v1/clusterextensionrevision_types.go | 58 +- api/v1/clusterextensionrevision_types_test.go | 71 ++ api/v1/zz_generated.deepcopy.go | 16 + .../api/v1/clusterextensionrevisionobject.go | 19 +- applyconfigurations/api/v1/objectsourceref.go | 64 ++ applyconfigurations/utils.go | 2 + cmd/operator-controller/main.go | 1 + docs/api-reference/olmv1-api-reference.md | 2 + docs/concepts/large-bundle-support.md | 726 ++++++++++++++++++ ...ramework.io_clusterextensionrevisions.yaml | 40 +- .../operator-controller/applier/boxcutter.go | 264 ++++++- .../applier/boxcutter_test.go | 4 + .../applier/externalize_test.go | 91 +++ .../applier/secretpacker.go | 187 +++++ .../applier/secretpacker_test.go | 206 +++++ .../clusterextensionrevision_controller.go | 55 +- .../controllers/resolve_ref_test.go | 284 +++++++ internal/operator-controller/labels/labels.go | 6 + manifests/experimental-e2e.yaml | 40 +- manifests/experimental.yaml | 40 +- test/e2e/features/install.feature | 29 + test/e2e/steps/steps.go | 281 ++++++- 22 files changed, 2434 insertions(+), 52 deletions(-) create mode 100644 applyconfigurations/api/v1/objectsourceref.go create mode 100644 docs/concepts/large-bundle-support.md create mode 100644 internal/operator-controller/applier/externalize_test.go create mode 100644 internal/operator-controller/applier/secretpacker.go create mode 100644 internal/operator-controller/applier/secretpacker_test.go create mode 100644 internal/operator-controller/controllers/resolve_ref_test.go diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 208d96d9ff..bfdadf20a9 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -30,12 +30,13 @@ const ( ClusterExtensionRevisionTypeSucceeded = "Succeeded" // Condition Reasons - ClusterExtensionRevisionReasonArchived = "Archived" - ClusterExtensionRevisionReasonBlocked = "Blocked" - ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure" - ClusterExtensionRevisionReasonProbesSucceeded = "ProbesSucceeded" - ClusterExtensionRevisionReasonReconciling = "Reconciling" - ClusterExtensionRevisionReasonRetrying = "Retrying" + ClusterExtensionRevisionReasonArchived = "Archived" + ClusterExtensionRevisionReasonBlocked = "Blocked" + ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure" + ClusterExtensionRevisionReasonProbesSucceeded = "ProbesSucceeded" + ClusterExtensionRevisionReasonReconciling = "Reconciling" + ClusterExtensionRevisionReasonRefResolutionFailed = "RefResolutionFailed" + ClusterExtensionRevisionReasonRetrying = "Retrying" ) // ClusterExtensionRevisionSpec defines the desired state of ClusterExtensionRevision. @@ -392,14 +393,29 @@ type ClusterExtensionRevisionPhase struct { // ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part // of a phase, along with its collision protection settings. +// +// Exactly one of object or ref must be set. +// +// +kubebuilder:validation:XValidation:rule="has(self.object) != has(self.ref)",message="exactly one of object or ref must be set" type ClusterExtensionRevisionObject struct { - // object is a required embedded Kubernetes object to be applied. + // object is an optional embedded Kubernetes object to be applied. + // + // Exactly one of object or ref must be set. // // This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. // // +kubebuilder:validation:EmbeddedResource // +kubebuilder:pruning:PreserveUnknownFields - Object unstructured.Unstructured `json:"object"` + // +optional + Object unstructured.Unstructured `json:"object,omitzero"` + + // ref is an optional reference to a Secret that holds the serialized + // object manifest. + // + // Exactly one of object or ref must be set. + // + // +optional + Ref ObjectSourceRef `json:"ref,omitzero"` // collisionProtection controls whether the operator can adopt and modify objects // that already exist on the cluster. @@ -425,6 +441,32 @@ type ClusterExtensionRevisionObject struct { CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"` } +// ObjectSourceRef references content within a Secret that contains a +// serialized object manifest. +type ObjectSourceRef struct { + // name is the name of the referenced Secret. + // + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + Name string `json:"name"` + + // namespace is the namespace of the referenced Secret. + // + // +optional + // +kubebuilder:validation:MaxLength=63 + Namespace string `json:"namespace,omitempty"` + + // key is the data key within the referenced Secret containing the + // object manifest content. The value at this key must be a + // JSON-serialized Kubernetes object manifest. + // + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + Key string `json:"key"` +} + // CollisionProtection specifies if and how ownership collisions are prevented. type CollisionProtection string diff --git a/api/v1/clusterextensionrevision_types_test.go b/api/v1/clusterextensionrevision_types_test.go index 75a1a6cc3b..e1412e0e81 100644 --- a/api/v1/clusterextensionrevision_types_test.go +++ b/api/v1/clusterextensionrevision_types_test.go @@ -272,6 +272,77 @@ func TestClusterExtensionRevisionValidity(t *testing.T) { }, valid: true, }, + "object with inline object is valid": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + Phases: []ClusterExtensionRevisionPhase{ + { + Name: "deploy", + Objects: []ClusterExtensionRevisionObject{ + { + Object: configMap(), + }, + }, + }, + }, + }, + valid: true, + }, + "object with ref is valid": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + Phases: []ClusterExtensionRevisionPhase{ + { + Name: "deploy", + Objects: []ClusterExtensionRevisionObject{ + { + Ref: ObjectSourceRef{Name: "my-secret", Key: "my-key"}, + }, + }, + }, + }, + }, + valid: true, + }, + "object with both object and ref is invalid": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + Phases: []ClusterExtensionRevisionPhase{ + { + Name: "deploy", + Objects: []ClusterExtensionRevisionObject{ + { + Object: configMap(), + Ref: ObjectSourceRef{Name: "my-secret", Key: "my-key"}, + }, + }, + }, + }, + }, + valid: false, + }, + "object with neither object nor ref is invalid": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + Phases: []ClusterExtensionRevisionPhase{ + { + Name: "deploy", + Objects: []ClusterExtensionRevisionObject{ + {}, + }, + }, + }, + }, + valid: false, + }, } { t.Run(name, func(t *testing.T) { cer := &ClusterExtensionRevision{ diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 9c801ca773..4b345e65fc 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -420,6 +420,7 @@ func (in *ClusterExtensionRevisionList) DeepCopyObject() runtime.Object { func (in *ClusterExtensionRevisionObject) DeepCopyInto(out *ClusterExtensionRevisionObject) { *out = *in in.Object.DeepCopyInto(&out.Object) + out.Ref = in.Ref } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionRevisionObject. @@ -648,6 +649,21 @@ func (in *ObjectSelector) DeepCopy() *ObjectSelector { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ObjectSourceRef) DeepCopyInto(out *ObjectSourceRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ObjectSourceRef. +func (in *ObjectSourceRef) DeepCopy() *ObjectSourceRef { + if in == nil { + return nil + } + out := new(ObjectSourceRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PreflightConfig) DeepCopyInto(out *PreflightConfig) { *out = *in diff --git a/applyconfigurations/api/v1/clusterextensionrevisionobject.go b/applyconfigurations/api/v1/clusterextensionrevisionobject.go index fc482bf884..7962712e47 100644 --- a/applyconfigurations/api/v1/clusterextensionrevisionobject.go +++ b/applyconfigurations/api/v1/clusterextensionrevisionobject.go @@ -27,11 +27,20 @@ import ( // // ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part // of a phase, along with its collision protection settings. +// +// Exactly one of object or ref must be set. type ClusterExtensionRevisionObjectApplyConfiguration struct { - // object is a required embedded Kubernetes object to be applied. + // object is an optional embedded Kubernetes object to be applied. + // + // Exactly one of object or ref must be set. // // This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. Object *unstructured.Unstructured `json:"object,omitempty"` + // ref is an optional reference to a Secret that holds the serialized + // object manifest. + // + // Exactly one of object or ref must be set. + Ref *ObjectSourceRefApplyConfiguration `json:"ref,omitempty"` // collisionProtection controls whether the operator can adopt and modify objects // that already exist on the cluster. // @@ -67,6 +76,14 @@ func (b *ClusterExtensionRevisionObjectApplyConfiguration) WithObject(value unst return b } +// 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 *ClusterExtensionRevisionObjectApplyConfiguration) WithRef(value *ObjectSourceRefApplyConfiguration) *ClusterExtensionRevisionObjectApplyConfiguration { + b.Ref = value + return b +} + // WithCollisionProtection sets the CollisionProtection 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 CollisionProtection field is set to the value of the last call. diff --git a/applyconfigurations/api/v1/objectsourceref.go b/applyconfigurations/api/v1/objectsourceref.go new file mode 100644 index 0000000000..06dfa7f8c6 --- /dev/null +++ b/applyconfigurations/api/v1/objectsourceref.go @@ -0,0 +1,64 @@ +/* +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.20. DO NOT EDIT. + +package v1 + +// ObjectSourceRefApplyConfiguration represents a declarative configuration of the ObjectSourceRef type for use +// with apply. +// +// ObjectSourceRef references content within a Secret that contains a +// serialized object manifest. +type ObjectSourceRefApplyConfiguration struct { + // name is the name of the referenced Secret. + Name *string `json:"name,omitempty"` + // namespace is the namespace of the referenced Secret. + Namespace *string `json:"namespace,omitempty"` + // key is the data key within the referenced Secret containing the + // object manifest content. The value at this key must be a + // JSON-serialized Kubernetes object manifest. + Key *string `json:"key,omitempty"` +} + +// ObjectSourceRefApplyConfiguration constructs a declarative configuration of the ObjectSourceRef type for use with +// apply. +func ObjectSourceRef() *ObjectSourceRefApplyConfiguration { + return &ObjectSourceRefApplyConfiguration{} +} + +// WithName sets the Name 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 Name field is set to the value of the last call. +func (b *ObjectSourceRefApplyConfiguration) WithName(value string) *ObjectSourceRefApplyConfiguration { + b.Name = &value + return b +} + +// WithNamespace sets the Namespace 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 Namespace field is set to the value of the last call. +func (b *ObjectSourceRefApplyConfiguration) WithNamespace(value string) *ObjectSourceRefApplyConfiguration { + b.Namespace = &value + return b +} + +// WithKey sets the Key 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 Key field is set to the value of the last call. +func (b *ObjectSourceRefApplyConfiguration) WithKey(value string) *ObjectSourceRefApplyConfiguration { + b.Key = &value + return b +} diff --git a/applyconfigurations/utils.go b/applyconfigurations/utils.go index d8e2bb9da3..aec0ecb0a1 100644 --- a/applyconfigurations/utils.go +++ b/applyconfigurations/utils.go @@ -81,6 +81,8 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &apiv1.ImageSourceApplyConfiguration{} case v1.SchemeGroupVersion.WithKind("ObjectSelector"): return &apiv1.ObjectSelectorApplyConfiguration{} + case v1.SchemeGroupVersion.WithKind("ObjectSourceRef"): + return &apiv1.ObjectSourceRefApplyConfiguration{} 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 f905d105dd..0ba3b545e4 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -634,6 +634,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl Preflights: c.preflights, PreAuthorizer: preAuth, FieldOwner: fieldOwner, + SystemNamespace: cfg.systemNamespace, } revisionStatesGetter := &controllers.BoxcutterRevisionStatesGetter{Reader: c.mgr.GetClient()} storageMigrator := &applier.BoxcutterStorageMigrator{ diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 29731f2cd0..97a8e5d3b0 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -475,6 +475,8 @@ _Appears in:_ | `label` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#labelselector-v1-meta)_ | label is the label selector definition.
Required when type is "Label".
A probe using a Label selector will be executed against every object matching the labels or expressions; you must use care
when using this type of selector. For example, if multiple Kind objects are selected via labels then the probe is
likely to fail because the values of different Kind objects rarely share the same schema.
The LabelSelector field uses the following Kubernetes format:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#LabelSelector
Requires exactly one of matchLabels or matchExpressions.
| | Optional: \{\}
| + + #### PreflightConfig diff --git a/docs/concepts/large-bundle-support.md b/docs/concepts/large-bundle-support.md new file mode 100644 index 0000000000..033222b8f3 --- /dev/null +++ b/docs/concepts/large-bundle-support.md @@ -0,0 +1,726 @@ +# Design: Large Bundle Support + +## Need + +ClusterExtensionRevision (CER) objects embed full Kubernetes manifests inline in +`.spec.phases[].objects[].object`. With up to 20 phases and 50 objects per phase, +the serialized CER can approach or exceed the etcd maximum object size of +1.5 MiB. Large operators shipping many CRDs, Deployments, RBAC rules, and +webhook configurations are likely to hit this limit. + +When the limit is exceeded, the API server rejects the CER and the extension +cannot be installed or upgraded. Today there is no mitigation path other than +reducing the number of objects in the bundle. + +The phases data is immutable after creation, write-once/read-many, and only +consumed by the revision reconciler — making it a good candidate for +externalization. + +This document presents two approaches for solving this problem. Per-object +content references is the preferred approach; externalized phases to Secret +chains is presented as an alternative. + +--- + +## Approach: Per-Object Content References + +Externalize all objects by default. Add an optional `ref` field to +`ClusterExtensionRevisionObject` that points to the object content stored in a +Secret. Exactly one of `object` or `ref` must be set. The system uses `ref` for +all objects it creates; users who manually craft CERs may use either `object` or +`ref`. + +### API change + +Add a new `ObjectSourceRef` type and a `ref` field to +`ClusterExtensionRevisionObject`. Exactly one of `object` or `ref` must be set, +enforced via CEL validation. Both fields are immutable (inherited from the +immutability of phases). + +```go +type ClusterExtensionRevisionObject struct { + // object is an optional embedded Kubernetes object to be applied. + // Exactly one of object or ref must be set. + // + // +kubebuilder:validation:EmbeddedResource + // +kubebuilder:pruning:PreserveUnknownFields + // +optional + Object *unstructured.Unstructured `json:"object,omitempty"` + + // ref is an optional reference to a Secret that holds the serialized + // object manifest. + // Exactly one of object or ref must be set. + // + // +optional + Ref *ObjectSourceRef `json:"ref,omitempty"` + + // collisionProtection controls whether the operator can adopt and modify + // objects that already exist on the cluster. + // + // +optional + // +kubebuilder:validation:Enum=Prevent;IfNoController;None + CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"` +} +``` + +CEL validation on `ClusterExtensionRevisionObject`: + +``` +rule: "has(self.object) != has(self.ref)" +message: "exactly one of object or ref must be set" +``` + +#### ObjectSourceRef + +```go +// ObjectSourceRef references content within a Secret that contains a +// serialized object manifest. +type ObjectSourceRef struct { + // name is the name of the referenced Secret. + // + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + Name string `json:"name"` + + // namespace is the namespace of the referenced Secret. + // + // +optional + // +kubebuilder:validation:MaxLength=63 + Namespace string `json:"namespace,omitempty"` + + // key is the data key within the referenced Secret containing the + // object manifest content. The value at this key must be a + // JSON-serialized Kubernetes object manifest. + // + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + Key string `json:"key"` +} +``` + +### Content format + +The content at the referenced key is a JSON-serialized Kubernetes manifest — the +same structure currently used inline in the `object` field. + +Content may optionally be gzip-compressed. The reconciler auto-detects +compression by inspecting the first two bytes of the content: gzip streams +always start with the magic bytes `\x1f\x8b`. If detected, the content is +decompressed before JSON deserialization. Otherwise, the content is treated as +plain JSON. This makes compression transparent — no additional API fields or +annotations are needed, and producers can choose per-key whether to compress. + +Kubernetes manifests are highly repetitive structured text and typically achieve +5-10x size reduction with gzip, following the same pattern used by Helm for +release storage. + +To inspect an uncompressed referenced object stored in a Secret: + +```sh +kubectl get secret -o jsonpath='{.data.}' | base64 -d | jq . +``` + +To inspect a gzip-compressed referenced object stored in a Secret: + +```sh +kubectl get secret -o jsonpath='{.data.}' | base64 -d | gunzip | jq . +``` + +### Referenced resource conventions + +The CER API does not enforce any particular structure or metadata on the +referenced Secret — the `ref` field is a plain pointer. The reconciler only +requires that the Secret exists and that the key resolves to valid JSON content +(optionally gzip-compressed). Everything else is a convention that the system +follows when it creates referenced Secrets, and that other producers should +follow for consistency and safe lifecycle management. + +Recommended conventions: + +1. **Immutability**: Secrets should set `immutable: true`. Because CER phases + are immutable, the content backing a ref should not change after creation. + Mutable referenced Secrets are not rejected, but modifying them after the + CER is created leads to undefined behavior. + +2. **Owner references**: Referenced Secrets should carry an ownerReference to + the CER so that Kubernetes garbage collection removes them when the CER is + deleted: + ```yaml + ownerReferences: + - apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtensionRevision + name: + uid: + controller: true + ``` + Without an ownerReference, the producer is responsible for cleaning up the + Secret when the CER is deleted. The reconciler does not delete referenced + Secrets itself. + +3. **Revision label**: A label identifying the owning revision aids discovery, + debugging, and bulk cleanup: + ``` + olm.operatorframework.io/revision-name: + ``` + This enables fetching all referenced Secrets for a revision with a single + list call: + ```sh + kubectl get secrets -l olm.operatorframework.io/revision-name=my-extension-1 + ``` + +Multiple externalized objects may share a single Secret by using different keys. +This reduces the number of Secrets created. + +### Controller implementation + +The ClusterExtension controller externalizes all objects by default. When the +Boxcutter applier creates a CER, every object entry uses `ref` pointing to the +object content stored in a Secret. Secrets are created in the system namespace +(`olmv1-system` by default). This keeps CERs uniformly small regardless of +bundle size. + +Users who manually craft CERs may use either inline `object` or `ref` pointing +to their own Secrets. Inline `object` is convenient for development, testing, or +extensions with very few small objects. Users who prefer to manage their own +externalized storage can create Secrets and use `ref` directly. + +### Example + +A system-created CER with all objects externalized: + +```yaml +apiVersion: olm.operatorframework.io/v1 +kind: ClusterExtensionRevision +metadata: + name: my-extension-1 +spec: + revision: 1 + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: rbac + objects: + - ref: + name: my-extension-1-rbac + namespace: olmv1-system + key: service-account + - ref: + name: my-extension-1-rbac + namespace: olmv1-system + key: cluster-role + - name: crds + objects: + - ref: + name: my-extension-1-crds + namespace: olmv1-system + key: my-crd + - name: deploy + objects: + - ref: + name: my-extension-1-deploy + namespace: olmv1-system + key: deployment +--- +apiVersion: v1 +kind: Secret +metadata: + name: my-extension-1-rbac + namespace: olmv1-system + labels: + olm.operatorframework.io/revision-name: my-extension-1 + ownerReferences: + - apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtensionRevision + name: my-extension-1 + uid: + controller: true +immutable: true +data: + service-account: + cluster-role: +--- +apiVersion: v1 +kind: Secret +metadata: + name: my-extension-1-crds + namespace: olmv1-system + labels: + olm.operatorframework.io/revision-name: my-extension-1 + ownerReferences: + - apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtensionRevision + name: my-extension-1 + uid: + controller: true +immutable: true +data: + my-crd: +--- +apiVersion: v1 +kind: Secret +metadata: + name: my-extension-1-deploy + namespace: olmv1-system + labels: + olm.operatorframework.io/revision-name: my-extension-1 + ownerReferences: + - apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtensionRevision + name: my-extension-1 + uid: + controller: true +immutable: true +data: + deployment: +``` + +A user-crafted CER with inline objects: + +```yaml +apiVersion: olm.operatorframework.io/v1 +kind: ClusterExtensionRevision +metadata: + name: my-extension-1 +spec: + revision: 1 + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: deploy + objects: + - object: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: my-operator + namespace: my-ns + spec: { ... } +``` + +### Packing strategy + +The ClusterExtension controller packs externalized objects into Secrets in the +system namespace (`olmv1-system` by default): + +1. Iterate over all objects across all phases in order. For each object, + serialize it as JSON, compute its content hash + (SHA-256, [base64url](https://datatracker.ietf.org/doc/html/rfc4648#section-5)-encoded + without padding — 43 characters), and store it in the current Secret using + the hash as the data key. The corresponding `ref.key` is set to the hash. + Using the content hash as key ensures that it is a stable, deterministic + identifier tied to the exact content — if the content changes, the hash + changes, which in turn changes the key in the CER, guaranteeing that any + content mutation is visible as a CER spec change and triggers a new + revision. +2. When adding an object would push the current Secret beyond 900 KiB (leaving + headroom for base64 overhead and metadata), finalize it and start a new one. + Objects from different phases may share the same Secret. +3. If a single serialized object exceeds the Secret size limit on its own, + creation fails with a clear error. + +Multiple Secrets are independent — each is referenced directly by a `ref` in +the CER. There is no linked-list chaining between them. + +### Crash-safe creation sequence + +The creation sequence ensures that by the time the CER exists and is visible to +the CER reconciler, all referenced Secrets are already present. This avoids +spurious "not found" errors and unnecessary retry loops. + +ownerReferences require the parent's `uid`, which is only assigned by the API +server at creation time. Since we must create the Secrets before the CER, they +are initially created without ownerReferences. The Secret `immutable` flag only +protects `.data` and `.stringData` — metadata (including ownerReferences) can +still be patched after creation. + +``` +Step 1: Create Secret(s) with revision label, no ownerReference + | + | crash here → Orphaned Secrets exist with no owner. + | ClusterExtension controller detects them on + | next reconciliation by listing Secrets with + | the revision label and checking whether the + | corresponding CER exists. If not, deletes them. + v +Step 2: Create CER with refs pointing to the Secrets from step 1 + | + | crash here → CER exists, Secrets exist, refs resolve. + | CER reconciler can proceed normally. + | Secrets have no ownerReferences yet. + | ClusterExtension controller retries step 3. + v +Step 3: Patch ownerReferences onto Secrets (using CER uid) + | + | crash here → Some Secrets have ownerRefs, some don't. + | ClusterExtension controller retries patching + | the remaining Secrets on next reconciliation. + v + Done — CER has refs, all Secrets exist with owner refs. +``` + +Key properties: +- **No reconciler churn**: Referenced Secrets exist before the CER is created. + The CER reconciler never encounters missing Secrets during normal operation. +- **Orphan cleanup**: Secrets created in step 1 carry the revision label + (`olm.operatorframework.io/revision-name`). If a crash leaves Secrets without + a corresponding CER, the ClusterExtension controller detects and deletes them + on its next reconciliation. +- **Idempotent retry**: Secrets are immutable in data. Re-creation of an + existing Secret returns AlreadyExists and is skipped. ownerReference patching + is idempotent — patching an already-set ownerReference is a no-op. +- **Refs are truly immutable**: Set at CER creation, never modified (inherited + from phase immutability). + +### CER reconciler behavior + +When processing a CER phase: +- For each object entry in the phase: + - If `object` is set, use it directly (current behavior, unchanged). + - If `ref` is set, fetch the referenced Secret, read the value at the + specified `key`, and JSON-deserialize into an + `unstructured.Unstructured`. +- The resolved object is used identically to an inline object for the remainder + of reconciliation — collision protection inheritance, owner labeling, and + rollout semantics are unchanged. + +Under normal operation, referenced Secrets are guaranteed to exist before the +CER is created (see [Crash-safe creation sequence](#crash-safe-creation-sequence)). +If a referenced Secret or key is not found — indicating an inconsistent state +caused by external modification or a partially completed creation sequence — +the reconciler sets a terminal error condition on the CER. + +Secrets are fetched using the typed client served from the informer cache. + +--- + +## Alternative: Externalized Phases to Secret Chains + +All phases are serialized as a single JSON array, gzip-compressed into one blob, +and stored under a single `phases` data key in a Secret. If the compressed blob +exceeds ~900 KiB, it is split into fixed-size byte chunks across multiple +Secrets linked via `.nextSecretRef` keys. + +### API change + +Keep the existing `phases` field and add a new `phasesRef` field. Only one of the +two may be set, enforced via CEL validation. `phasesRef` points to the first +Secret in a chain of one or more Secrets containing the phase data. + +```go +type PhasesRef struct { + SecretRef SecretRef `json:"secretRef"` +} + +type SecretRef struct { + Name string `json:"name"` + Namespace string `json:"namespace"` +} +``` + +The reconciler follows `.nextSecretRef` data keys from Secret to Secret until a +Secret has no `.nextSecretRef`. + +Both fields are immutable once set. `phases` and `phasesRef` are mutually +exclusive. + +### Secret type and naming convention + +Secrets use a dedicated type `olm.operatorframework.io/revision-phase-data` to +distinguish them from user-created Secrets and enable easy identification. + +Secret names are derived deterministically from the CER name and a content hash. +The hash is the first 16 hex characters of the SHA-256 digest of the phases +serialized to JSON (before gzip compression). Computing the hash from the JSON +serialization rather than the gzip output makes it deterministic regardless of +gzip implementation details. + +| Secret | Name | +|----------|-------------------------------| +| First | `-` | +| Second | `--1` | +| Third | `--2` | +| Nth | `--` | + +The hash is computed from the phases JSON before any Secrets are created, so +`phasesRef.secretRef.name` is known at CER creation time and can be set +immediately. + +### Secret labeling + +All Secrets in a chain carry a common label identifying the owning revision: + +``` +olm.operatorframework.io/revision-name: +``` + +This allows all phase data Secrets for a given revision to be fetched with a +single list call: + +```sh +kubectl get secrets -l olm.operatorframework.io/revision-name=my-extension-1 +``` + +This is useful for debugging, auditing, and bulk cleanup. It also provides an +efficient alternative to following the `.nextSecretRef` chain when all Secrets +need to be loaded at once. + +### Secret structure + +Each Secret has a single `phases` data key holding its chunk of the gzip blob. +If more Secrets follow, a `.nextSecretRef` key holds the name of the next Secret +in the chain. The dot prefix clearly distinguishes it from the `phases` key. + +**Single Secret (all data fits in one):** + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: my-extension-1-a1b2c3d4e5f67890 + labels: + olm.operatorframework.io/revision-name: my-extension-1 + ownerReferences: + - apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtensionRevision + name: my-extension-1 + uid: + controller: true +immutable: true +type: olm.operatorframework.io/revision-phase-data +data: + phases: +``` + +**Multiple Secrets (chunked):** + +```yaml +# Secret my-extension-1-a1b2c3d4e5f67890 — chunk 0 +apiVersion: v1 +kind: Secret +metadata: + name: my-extension-1-a1b2c3d4e5f67890 + labels: + olm.operatorframework.io/revision-name: my-extension-1 + ownerReferences: + - apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtensionRevision + name: my-extension-1 + uid: + controller: true +immutable: true +type: olm.operatorframework.io/revision-phase-data +data: + phases: + .nextSecretRef: +--- +# Secret my-extension-1-a1b2c3d4e5f67890-1 — chunk 1 +apiVersion: v1 +kind: Secret +metadata: + name: my-extension-1-a1b2c3d4e5f67890-1 + labels: + olm.operatorframework.io/revision-name: my-extension-1 + ownerReferences: + - apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtensionRevision + name: my-extension-1 + uid: + controller: true +immutable: true +type: olm.operatorframework.io/revision-phase-data +data: + phases: + .nextSecretRef: +--- +# Secret my-extension-1-a1b2c3d4e5f67890-2 — chunk 2 (last) +apiVersion: v1 +kind: Secret +metadata: + name: my-extension-1-a1b2c3d4e5f67890-2 + labels: + olm.operatorframework.io/revision-name: my-extension-1 + ownerReferences: + - apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtensionRevision + name: my-extension-1 + uid: + controller: true +immutable: true +type: olm.operatorframework.io/revision-phase-data +data: + phases: +``` + +The last Secret has no `.nextSecretRef`. The reconciler follows the chain until +it encounters a Secret without `.nextSecretRef`. + +### Compression + +All phases are compressed together as a single gzip stream. Kubernetes manifests +are highly repetitive structured text and typically achieve 5-10x compression +with gzip. Compressing all phases together rather than individually exploits +cross-phase redundancy (shared labels, annotations, namespace references, etc.) +for better compression ratios. + +This significantly reduces the number of Secrets needed and makes it likely that +most extensions fit in a single Secret. To inspect phase data: + +```sh +# single Secret +kubectl get secret -o jsonpath='{.data.phases}' | base64 -d | gunzip | jq . +``` + +This follows the same pattern used by Helm, which gzip-compresses release data +before storing in Secrets. + +### Chunking strategy + +The entire gzip blob is split at 900 KiB byte boundaries (leaving headroom for +base64 overhead and metadata). This is a simple byte-level split — phases are +never individually split because the chunking operates on the raw gzip stream, +not on individual phase boundaries. + +If the total compressed size after chunking would require an unreasonable number +of Secrets, creation fails with a clear error. + +### Crash-safe creation sequence + +Because the hash is computed from the phases JSON before any Secrets are created, +`phasesRef` can be set at CER creation time: + +``` +Step 1: Create CER with phasesRef.secretRef.name = - + | + | crash here → CER exists but Secrets do not. + | Reconciler retries Secret creation. + v +Step 2: Create Secrets with ownerReferences pointing to the CER + | + | crash here → Partial set of Secrets exists, all with + | ownerReferences. Not orphaned — GC'd if + | CER is deleted. Existing immutable Secrets + | are skipped (AlreadyExists), missing ones + | are created on next attempt. + v + Done — CER has phasesRef, all Secrets exist with owner refs. +``` + +Key properties: +- **No orphaned Secrets**: Every Secret carries an ownerReference to the CER. + Kubernetes garbage collection removes them if the CER is deleted at any point. +- **Idempotent retry**: Secrets are immutable. Re-creation of an existing Secret + returns AlreadyExists and is skipped. Missing Secrets are created on retry. +- **phasesRef is truly immutable**: Set at CER creation, never modified. + +### CER reconciler behavior + +When reconciling a CER: +- If `phases` is set, use it directly (current behavior, unchanged). +- If `phasesRef` is set: + 1. Fetch the Secret at `phasesRef.secretRef.name` in + `phasesRef.secretRef.namespace`. + 2. Read `data.phases` (base64-decoded), append to a byte buffer. + 3. If `data[.nextSecretRef]` exists, fetch the named Secret and repeat from + step 2. + 4. Gunzip the concatenated buffer. + 5. JSON-deserialize into `[]ClusterExtensionRevisionPhase`. + 6. Use identically to inline phases. + If a Secret is not yet available, return a retryable error. +- If neither is set, skip reconciliation (invalid state). + +The reconstructed phase list is used identically to inline phases for the +remainder of the reconciliation — ordering, collision protection inheritance, and +rollout semantics are unchanged. + +--- + +## Comparison + +| Dimension | Per-Object Content References | Externalized Phases to Secret Chains | +|---|-------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------| +| **Granularity** | Per-object — each object has its own `ref` | Per-phase-set — all phases externalized as a single blob | +| **API complexity** | New `ref` field on existing object struct; new `ObjectSourceRef` type | New top-level `phasesRef` field; new `PhasesRef` and `SecretRef` types | +| **Reconciler complexity** | Secrets are served from cache; no ordering dependency between fetches | Chain traversal — follow `.nextSecretRef` links, concatenate chunks, gunzip, deserialize | +| **Compression** | Optional per-object gzip, auto-detected via magic bytes; each object compressed independently | Single gzip stream across all phases; exploits cross-phase redundancy for better ratios | +| **Number of Secrets** | One or more, typically one | Typically one Secret for all phases; multiple only when compressed blob exceeds 900 KiB | +| **Crash safety** | 3-step: Secrets → CER → patch ownerRefs; orphan cleanup via revision label | 2-step: CER → Secrets with ownerRefs; simpler but reconciler may see missing Secrets temporarily | +| **Flexibility** | Mixed inline/ref per object within the same phase is possible | All-or-nothing — either all phases inline or all externalized | +| **Storage efficiency** | Per-object compression misses cross-object redundancy; potentially more Secrets created in edge cases | Better compression from cross-phase redundancy; fewer Secrets | +| **Resource type** | Secret only | Secret only (with dedicated type) | +| **Phases structure** | Unchanged — phases array preserved as-is; only individual objects gain a new resolution path | Replaced at the top level — phases field swapped for phasesRef | +| **Content addressability** | Content hash as Secret data key — key changes when content changes | Content hash embedded in Secret name — detects changes without fetching contents | + +--- + +## Non-goals + +- **Migration of existing inline objects/phases**: Existing CERs using inline + `object` or `phases` fields continue to work as-is. There is no automatic + migration to `ref` or `phasesRef`. + +- **System-managed lifecycle for system-created resources**: The OLM creates + and owns the referenced Secrets it produces (setting ownerReferences and + immutability). Users who manually craft CERs with `ref` are responsible for + the lifecycle of their own referenced Secrets. + +- **Cross-revision deduplication**: Objects or phases that are identical between + successive revisions are not shared or deduplicated. Each revision gets its + own set of referenced Secrets. Deduplication adds complexity with minimal + storage benefit given that archived revisions are eventually deleted. + +- **Lazy loading or streaming**: The reconciler loads all objects or phases into + memory at the start of processing. Per-object streaming or lazy loading is not + in scope. + +- **Application-level encryption beyond Kubernetes defaults**: Referenced + Secrets and phase data Secrets inherit whatever encryption-at-rest + configuration is applied to Secrets at the cluster level. Application-level + encryption of content is not in scope. + +--- + +## Other Alternatives Considered + +- **Increase etcd max-request-bytes**: Raises the per-object limit at the etcd + level. This is an operational burden on cluster administrators, is not portable + across clusters, degrades etcd performance for all workloads, and only shifts + the ceiling rather than removing it. + +- **Custom Resource for phase/object storage**: A dedicated CRD (e.g., + `ClusterExtensionRevisionPhaseData`) would provide schema validation and + a typed API. However, it introduces a new CRD to manage, is subject to the + same etcd size limit, and the phase data is opaque to the API server anyway + (embedded `unstructured.Unstructured` objects). Secrets are simpler and + sufficient. + +- **External storage (OCI registry, S3, PVC)**: Eliminates Kubernetes size + limits entirely but introduces external dependencies, availability concerns, + authentication complexity, and a fundamentally different failure domain. + Over-engineered for the expected data sizes (low single-digit MiBs). + + +## Recommendation + +**Per-object content references** is the recommended approach for the following +reasons: + +1. **Phases structure preserved**: The phases array in the CER spec remains + unchanged. Only individual object entries gain a new resolution path via + `ref`. This is a smaller, more targeted API change compared to replacing the + entire phases field with `phasesRef`. + +2. **Granular flexibility**: Inline `object` and `ref` can be mixed within the + same phase. Users who manually craft CERs can choose either approach + per-object. The externalized phases approach is all-or-nothing. + +3. **Uniform code path**: By externalizing all objects by default, the system + follows a single code path regardless of bundle size. There is no need for + size-based heuristics to decide when to externalize. + +The tradeoff is that per-object references create potentially more Secrets for very large bundles and miss +cross-object compression opportunities. In practice, this is acceptable: the +additional Secrets are small, immutable, garbage-collected via ownerReferences, +and the slight storage overhead is outweighed by the simpler reconciler logic +and greater flexibility. diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml index 491f8327d7..a9f44c4d8d 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml @@ -162,6 +162,8 @@ spec: description: |- ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part of a phase, along with its collision protection settings. + + Exactly one of object or ref must be set. properties: collisionProtection: description: |- @@ -190,15 +192,47 @@ spec: type: string object: description: |- - object is a required embedded Kubernetes object to be applied. + object is an optional embedded Kubernetes object to be applied. + + Exactly one of object or ref must be set. This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true - required: - - object + ref: + description: |- + ref is an optional reference to a Secret that holds the serialized + object manifest. + + Exactly one of object or ref must be set. + properties: + key: + description: |- + key is the data key within the referenced Secret containing the + object manifest content. The value at this key must be a + JSON-serialized Kubernetes object manifest. + maxLength: 253 + minLength: 1 + type: string + name: + description: name is the name of the referenced Secret. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: namespace is the namespace of the referenced + Secret. + maxLength: 63 + type: string + required: + - key + - name + type: object type: object + x-kubernetes-validations: + - message: exactly one of object or ref must be set + rule: has(self.object) != has(self.ref) maxItems: 50 type: array required: diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 8f2aa28f05..d6692a99dd 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -14,15 +14,18 @@ import ( "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/cli-runtime/pkg/printers" metav1ac "k8s.io/client-go/applyconfigurations/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/log" @@ -446,17 +449,7 @@ type Boxcutter struct { Preflights []Preflight PreAuthorizer authorization.PreAuthorizer FieldOwner string -} - -// apply applies the revision object using server-side apply. PreAuthorization checks are performed -// to ensure the user has sufficient permissions to manage the revision and its resources. -func (bc *Boxcutter) apply(ctx context.Context, user user.Info, rev *ocv1ac.ClusterExtensionRevisionApplyConfiguration) error { - // Run auth preflight checks - if err := bc.runPreAuthorizationChecks(ctx, user, rev); err != nil { - return err - } - - return bc.Client.Apply(ctx, rev, client.FieldOwner(bc.FieldOwner), client.ForceOwnership) + SystemNamespace string } func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (bool, string, error) { @@ -503,24 +496,41 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust currentRevision := &ocv1.ClusterExtensionRevision{} state := StateNeedsInstall - // check if we can update the current revision. if len(existingRevisions) > 0 { - // try first to update the current revision. currentRevision = &existingRevisions[len(existingRevisions)-1] desiredRevision.Spec.WithRevision(currentRevision.Spec.Revision) desiredRevision.WithName(currentRevision.Name) - err := bc.apply(ctx, getUserInfo(ext), desiredRevision) + // Save inline objects before externalization (needed for preflights + createExternalizedRevision) + savedInline := saveInlineObjects(desiredRevision) + + // Externalize with CURRENT revision name so refs match existing CER + phases := extractPhasesForPacking(desiredRevision.Spec.Phases) + packer := &SecretPacker{ + RevisionName: currentRevision.Name, + OwnerName: ext.Name, + SystemNamespace: bc.SystemNamespace, + } + packResult, err := packer.Pack(phases) + if err != nil { + return false, "", fmt.Errorf("packing for SSA comparison: %w", err) + } + replaceInlineWithRefs(desiredRevision, packResult) + + // SSA patch (refs-vs-refs). Skip pre-auth — just checking for changes. + // createExternalizedRevision runs its own pre-auth if upgrade is needed. + err = bc.Client.Apply(ctx, desiredRevision, client.FieldOwner(bc.FieldOwner), client.ForceOwnership) + + // Restore inline objects for preflights + createExternalizedRevision + restoreInlineObjects(desiredRevision, savedInline) + switch { case apierrors.IsInvalid(err): - // We could not update the current revision due to trying to update an immutable field. - // Therefore, we need to create a new revision. state = StateNeedsUpgrade case err == nil: - // inplace patch was successful, no changes in phases state = StateUnchanged default: - return false, "", fmt.Errorf("patching %s Revision: %w", *desiredRevision.GetName(), err) + return false, "", fmt.Errorf("patching %s Revision: %w", currentRevision.Name, err) } } @@ -549,23 +559,71 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust } if state != StateUnchanged { - // need to create new revision - prevRevisions := existingRevisions - revisionNumber := latestRevisionNumber(prevRevisions) + 1 + if err := bc.createExternalizedRevision(ctx, ext, desiredRevision, existingRevisions); err != nil { + return false, "", err + } + } else if currentRevision.Name != "" { + // In-place patch succeeded. Ensure any existing ref Secrets have ownerReferences + // (crash recovery for Step 3 failures). + if err := bc.ensureSecretOwnerReferences(ctx, currentRevision); err != nil { + return false, "", fmt.Errorf("ensuring ownerReferences on ref Secrets: %w", err) + } + } + + return true, "", nil +} - desiredRevision.WithName(fmt.Sprintf("%s-%d", ext.Name, revisionNumber)) - desiredRevision.Spec.WithRevision(revisionNumber) +// createExternalizedRevision creates a new CER with all objects externalized to Secrets. +// It follows a crash-safe three-step sequence: create Secrets, create CER, patch ownerRefs. +func (bc *Boxcutter) createExternalizedRevision(ctx context.Context, ext *ocv1.ClusterExtension, desiredRevision *ocv1ac.ClusterExtensionRevisionApplyConfiguration, existingRevisions []ocv1.ClusterExtensionRevision) error { + prevRevisions := existingRevisions + revisionNumber := latestRevisionNumber(prevRevisions) + 1 - if err = bc.garbageCollectOldRevisions(ctx, prevRevisions); err != nil { - return false, "", fmt.Errorf("garbage collecting old revisions: %w", err) - } + revisionName := fmt.Sprintf("%s-%d", ext.Name, revisionNumber) + desiredRevision.WithName(revisionName) + desiredRevision.Spec.WithRevision(revisionNumber) + + if err := bc.garbageCollectOldRevisions(ctx, prevRevisions); err != nil { + return fmt.Errorf("garbage collecting old revisions: %w", err) + } - if err := bc.apply(ctx, getUserInfo(ext), desiredRevision); err != nil { - return false, "", fmt.Errorf("creating new Revision: %w", err) + // Run pre-authorization on the inline revision (before replacing objects with refs) + if err := bc.runPreAuthorizationChecks(ctx, getUserInfo(ext), desiredRevision); err != nil { + return fmt.Errorf("creating new Revision: %w", err) + } + + // Externalize: pack inline objects into Secrets and replace with refs + phases := extractPhasesForPacking(desiredRevision.Spec.Phases) + packer := &SecretPacker{ + RevisionName: revisionName, + OwnerName: ext.Name, + SystemNamespace: bc.SystemNamespace, + } + packResult, err := packer.Pack(phases) + if err != nil { + return fmt.Errorf("packing objects into Secrets: %w", err) + } + replaceInlineWithRefs(desiredRevision, packResult) + + // Step 1: Create Secrets (skip AlreadyExists) + for i := range packResult.Secrets { + if err := bc.Client.Create(ctx, &packResult.Secrets[i]); err != nil { + if !apierrors.IsAlreadyExists(err) { + return fmt.Errorf("creating ref Secret %q: %w", packResult.Secrets[i].Name, err) + } } } - return true, "", nil + // Step 2: Create CER with refs via SSA (pre-auth already ran above) + if err := bc.Client.Apply(ctx, desiredRevision, client.FieldOwner(bc.FieldOwner), client.ForceOwnership); err != nil { + return fmt.Errorf("creating new Revision: %w", err) + } + + // Step 3: Patch ownerReferences onto Secrets using the CER's UID + if err := bc.patchSecretOwnerReferences(ctx, desiredRevision, packResult.Secrets); err != nil { + return fmt.Errorf("patching ownerReferences on ref Secrets: %w", err) + } + return nil } // runPreAuthorizationChecks runs PreAuthorization checks if the PreAuthorizer is set. An error will be returned if @@ -699,3 +757,151 @@ func mergeStringMaps(m1, m2 map[string]string) map[string]string { maps.Copy(merged, m2) return merged } + +// saveInlineObjects saves Object pointers from each phase/object position. +func saveInlineObjects(rev *ocv1ac.ClusterExtensionRevisionApplyConfiguration) [][]*unstructured.Unstructured { + saved := make([][]*unstructured.Unstructured, len(rev.Spec.Phases)) + for i, p := range rev.Spec.Phases { + saved[i] = make([]*unstructured.Unstructured, len(p.Objects)) + for j, o := range p.Objects { + saved[i][j] = o.Object + } + } + return saved +} + +// restoreInlineObjects restores saved inline objects and clears refs. +func restoreInlineObjects(rev *ocv1ac.ClusterExtensionRevisionApplyConfiguration, saved [][]*unstructured.Unstructured) { + for i := range saved { + for j := range saved[i] { + rev.Spec.Phases[i].Objects[j].Object = saved[i][j] + rev.Spec.Phases[i].Objects[j].Ref = nil + } + } +} + +// extractPhasesForPacking converts apply configuration phases to API types for SecretPacker. +func extractPhasesForPacking(phases []ocv1ac.ClusterExtensionRevisionPhaseApplyConfiguration) []ocv1.ClusterExtensionRevisionPhase { + result := make([]ocv1.ClusterExtensionRevisionPhase, 0, len(phases)) + for _, p := range phases { + phase := ocv1.ClusterExtensionRevisionPhase{} + if p.Name != nil { + phase.Name = *p.Name + } + phase.Objects = make([]ocv1.ClusterExtensionRevisionObject, 0, len(p.Objects)) + for _, o := range p.Objects { + obj := ocv1.ClusterExtensionRevisionObject{} + if o.Object != nil { + obj.Object = *o.Object + } + if o.CollisionProtection != nil { + obj.CollisionProtection = *o.CollisionProtection + } + phase.Objects = append(phase.Objects, obj) + } + result = append(result, phase) + } + return result +} + +// replaceInlineWithRefs replaces inline objects in the apply configuration with refs from the pack result. +func replaceInlineWithRefs(rev *ocv1ac.ClusterExtensionRevisionApplyConfiguration, pack *PackResult) { + if rev.Spec == nil { + return + } + for phaseIdx := range rev.Spec.Phases { + for objIdx := range rev.Spec.Phases[phaseIdx].Objects { + ref, ok := pack.Refs[[2]int{phaseIdx, objIdx}] + if !ok { + continue + } + rev.Spec.Phases[phaseIdx].Objects[objIdx].Object = nil + rev.Spec.Phases[phaseIdx].Objects[objIdx].Ref = ocv1ac.ObjectSourceRef(). + WithName(ref.Name). + WithNamespace(ref.Namespace). + WithKey(ref.Key) + } + } +} + +// patchSecretOwnerReferences fetches the CER to get its UID, then patches ownerReferences onto all Secrets. +func (bc *Boxcutter) patchSecretOwnerReferences(ctx context.Context, rev *ocv1ac.ClusterExtensionRevisionApplyConfiguration, secrets []corev1.Secret) error { + if len(secrets) == 0 { + return nil + } + + // Fetch the CER to get its UID + cer := &ocv1.ClusterExtensionRevision{} + if err := bc.Client.Get(ctx, client.ObjectKey{Name: *rev.GetName()}, cer); err != nil { + return fmt.Errorf("getting CER %q for ownerReference: %w", *rev.GetName(), err) + } + + return bc.patchOwnerRefsOnSecrets(ctx, cer.Name, cer.UID, secrets) +} + +// ensureSecretOwnerReferences checks referenced Secrets on an existing CER and patches missing ownerReferences. +// This handles crash recovery when Step 3 (patching ownerRefs) failed on a previous reconciliation. +func (bc *Boxcutter) ensureSecretOwnerReferences(ctx context.Context, cer *ocv1.ClusterExtensionRevision) error { + // List Secrets with the revision-name label + secretList := &corev1.SecretList{} + if err := bc.Client.List(ctx, secretList, + client.InNamespace(bc.SystemNamespace), + client.MatchingLabels{labels.RevisionNameKey: cer.Name}, + ); err != nil { + return fmt.Errorf("listing ref Secrets for revision %q: %w", cer.Name, err) + } + + var needsPatch []corev1.Secret + for _, s := range secretList.Items { + hasOwnerRef := false + for _, ref := range s.OwnerReferences { + if ref.UID == cer.UID { + hasOwnerRef = true + break + } + } + if !hasOwnerRef { + needsPatch = append(needsPatch, s) + } + } + + if len(needsPatch) == 0 { + return nil + } + + return bc.patchOwnerRefsOnSecrets(ctx, cer.Name, cer.UID, needsPatch) +} + +// patchOwnerRefsOnSecrets patches ownerReferences onto the given Secrets, pointing to the CER. +func (bc *Boxcutter) patchOwnerRefsOnSecrets(ctx context.Context, cerName string, cerUID types.UID, secrets []corev1.Secret) error { + ownerRef := metav1.OwnerReference{ + APIVersion: ocv1.GroupVersion.String(), + Kind: ocv1.ClusterExtensionRevisionKind, + Name: cerName, + UID: cerUID, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + } + + for i := range secrets { + s := &secrets[i] + // Check if ownerRef already set + alreadySet := false + for _, ref := range s.OwnerReferences { + if ref.UID == cerUID { + alreadySet = true + break + } + } + if alreadySet { + continue + } + + patch := client.MergeFrom(s.DeepCopy()) + s.OwnerReferences = append(s.OwnerReferences, ownerRef) + if err := bc.Client.Patch(ctx, s, patch); err != nil { + return fmt.Errorf("patching ownerReference on Secret %s/%s: %w", s.Namespace, s.Name, err) + } + } + return nil +} diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index f1bc569045..8b50dbc031 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -524,6 +524,7 @@ func Test_SimpleRevisionGenerator_Failure(t *testing.T) { func TestBoxcutter_Apply(t *testing.T) { testScheme := runtime.NewScheme() require.NoError(t, ocv1.AddToScheme(testScheme)) + require.NoError(t, corev1.AddToScheme(testScheme)) // This is the revision that the mock builder will produce by default. // We calculate its hash to use in the tests. @@ -1060,6 +1061,7 @@ func TestBoxcutter_Apply(t *testing.T) { Scheme: testScheme, RevisionGenerator: tc.mockBuilder, FieldOwner: "test-owner", + SystemNamespace: "olmv1-system", } // We need a dummy fs.FS @@ -1103,6 +1105,7 @@ func TestBoxcutter_Apply(t *testing.T) { func Test_PreAuthorizer_Integration(t *testing.T) { testScheme := runtime.NewScheme() require.NoError(t, ocv1.AddToScheme(testScheme)) + require.NoError(t, corev1.AddToScheme(testScheme)) // This is the revision that the mock builder will produce by default. // We calculate its hash to use in the tests. @@ -1273,6 +1276,7 @@ func Test_PreAuthorizer_Integration(t *testing.T) { FieldOwner: "test-owner", RevisionGenerator: dummyGenerator, PreAuthorizer: tc.preAuthorizer(t), + SystemNamespace: "olmv1-system", } completed, status, err := boxcutter.Apply(t.Context(), dummyBundleFs, ext, nil, revisionAnnotations) if tc.validate != nil { diff --git a/internal/operator-controller/applier/externalize_test.go b/internal/operator-controller/applier/externalize_test.go new file mode 100644 index 0000000000..7e4cfcfd06 --- /dev/null +++ b/internal/operator-controller/applier/externalize_test.go @@ -0,0 +1,91 @@ +package applier + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + ocv1ac "github.com/operator-framework/operator-controller/applyconfigurations/api/v1" +) + +func TestExtractPhasesForPacking(t *testing.T) { + obj := unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{"name": "cm1"}, + }} + + cp := ocv1.CollisionProtectionPrevent + acPhases := []ocv1ac.ClusterExtensionRevisionPhaseApplyConfiguration{ + *ocv1ac.ClusterExtensionRevisionPhase().WithName("deploy").WithObjects( + ocv1ac.ClusterExtensionRevisionObject().WithObject(obj).WithCollisionProtection(cp), + ), + } + + result := extractPhasesForPacking(acPhases) + + require.Len(t, result, 1) + assert.Equal(t, "deploy", result[0].Name) + require.Len(t, result[0].Objects, 1) + assert.Equal(t, "ConfigMap", result[0].Objects[0].Object.GetKind()) + assert.Equal(t, ocv1.CollisionProtectionPrevent, result[0].Objects[0].CollisionProtection) +} + +func TestReplaceInlineWithRefs(t *testing.T) { + obj1 := unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{"name": "cm1"}, + }} + obj2 := unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{"name": "cm2"}, + }} + + rev := ocv1ac.ClusterExtensionRevision("test-rev"). + WithSpec(ocv1ac.ClusterExtensionRevisionSpec(). + WithPhases( + ocv1ac.ClusterExtensionRevisionPhase().WithName("deploy").WithObjects( + ocv1ac.ClusterExtensionRevisionObject().WithObject(obj1), + ocv1ac.ClusterExtensionRevisionObject().WithObject(obj2), + ), + ), + ) + + pack := &PackResult{ + Refs: map[[2]int]ocv1.ObjectSourceRef{ + {0, 0}: {Name: "secret-1", Namespace: "olmv1-system", Key: "key-a"}, + {0, 1}: {Name: "secret-1", Namespace: "olmv1-system", Key: "key-b"}, + }, + } + + replaceInlineWithRefs(rev, pack) + + require.Len(t, rev.Spec.Phases, 1) + require.Len(t, rev.Spec.Phases[0].Objects, 2) + + // Object should be nil, Ref should be set + assert.Nil(t, rev.Spec.Phases[0].Objects[0].Object) + require.NotNil(t, rev.Spec.Phases[0].Objects[0].Ref) + assert.Equal(t, "secret-1", *rev.Spec.Phases[0].Objects[0].Ref.Name) + assert.Equal(t, "olmv1-system", *rev.Spec.Phases[0].Objects[0].Ref.Namespace) + assert.Equal(t, "key-a", *rev.Spec.Phases[0].Objects[0].Ref.Key) + + assert.Nil(t, rev.Spec.Phases[0].Objects[1].Object) + require.NotNil(t, rev.Spec.Phases[0].Objects[1].Ref) + assert.Equal(t, "secret-1", *rev.Spec.Phases[0].Objects[1].Ref.Name) + assert.Equal(t, "key-b", *rev.Spec.Phases[0].Objects[1].Ref.Key) +} + +func TestReplaceInlineWithRefs_NilSpec(t *testing.T) { + rev := ocv1ac.ClusterExtensionRevision("test-rev") + pack := &PackResult{ + Refs: map[[2]int]ocv1.ObjectSourceRef{}, + } + // Should not panic + replaceInlineWithRefs(rev, pack) +} diff --git a/internal/operator-controller/applier/secretpacker.go b/internal/operator-controller/applier/secretpacker.go new file mode 100644 index 0000000000..1e7a705e2c --- /dev/null +++ b/internal/operator-controller/applier/secretpacker.go @@ -0,0 +1,187 @@ +package applier + +import ( + "bytes" + "compress/gzip" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "fmt" + "sort" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/labels" +) + +const ( + // maxSecretDataSize is the target maximum for Secret .data size + // before starting a new Secret. 900 KiB leaves headroom for + // base64 overhead and metadata within etcd's 1.5 MiB limit. + maxSecretDataSize = 900 * 1024 + + // gzipThreshold is the size above which individual objects are + // gzip-compressed before being stored in a Secret. + gzipThreshold = 800 * 1024 +) + +// SecretPacker packs serialized objects from CER phases into one or more +// immutable Secrets. +type SecretPacker struct { + RevisionName string + OwnerName string + SystemNamespace string +} + +// PackResult holds the packed Secrets and the ref entries that should +// replace inline objects in the CER phases. +type PackResult struct { + // Secrets to be created before the CER. + Secrets []corev1.Secret + // Refs maps (phaseIndex, objectIndex) to the ObjectSourceRef + // that should replace the inline object in the CER. + Refs map[[2]int]ocv1.ObjectSourceRef +} + +// Pack takes CER phases with inline objects and produces: +// 1. A set of immutable Secrets containing the serialized objects +// 2. A mapping from (phaseIdx, objIdx) to the corresponding ObjectSourceRef +func (p *SecretPacker) Pack(phases []ocv1.ClusterExtensionRevisionPhase) (*PackResult, error) { + result := &PackResult{ + Refs: make(map[[2]int]ocv1.ObjectSourceRef), + } + + // pendingRefs tracks which refs belong to the current (not-yet-finalized) Secret. + // Each entry records the ref's position key and the data key within the Secret. + type pendingRef struct { + pos [2]int + key string + } + + var ( + currentData = make(map[string][]byte) + currentSize int + currentPending []pendingRef + ) + + finalizeCurrent := func() { + if len(currentData) == 0 { + return + } + secret := p.newSecret(currentData) + // Back-fill refs for all objects assigned to this Secret. + for _, pr := range currentPending { + result.Refs[pr.pos] = ocv1.ObjectSourceRef{ + Name: secret.Name, + Namespace: p.SystemNamespace, + Key: pr.key, + } + } + result.Secrets = append(result.Secrets, secret) + currentData = make(map[string][]byte) + currentSize = 0 + currentPending = nil + } + + for phaseIdx, phase := range phases { + for objIdx, obj := range phase.Objects { + if obj.Object.Object == nil { + continue // skip ref-only entries + } + + data, err := json.Marshal(obj.Object.Object) + if err != nil { + return nil, fmt.Errorf("serializing object in phase %d index %d: %w", phaseIdx, objIdx, err) + } + + // Gzip large objects. + if len(data) > gzipThreshold { + compressed, cErr := gzipData(data) + if cErr != nil { + return nil, fmt.Errorf("compressing object in phase %d index %d: %w", phaseIdx, objIdx, cErr) + } + data = compressed + } + + if len(data) > maxSecretDataSize { + return nil, fmt.Errorf( + "object in phase %d index %d exceeds maximum Secret data size (%d bytes > %d bytes) even after compression", + phaseIdx, objIdx, len(data), maxSecretDataSize, + ) + } + + key := contentHash(data) + + // If adding this entry would exceed the limit, finalize the current Secret. + if currentSize+len(data) > maxSecretDataSize && len(currentData) > 0 { + finalizeCurrent() + } + + currentData[key] = data + currentSize += len(data) + currentPending = append(currentPending, pendingRef{pos: [2]int{phaseIdx, objIdx}, key: key}) + } + } + finalizeCurrent() + + return result, nil +} + +func (p *SecretPacker) newSecret(data map[string][]byte) corev1.Secret { + return corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: p.secretNameFromData(data), + Namespace: p.SystemNamespace, + Labels: map[string]string{ + labels.RevisionNameKey: p.RevisionName, + labels.OwnerNameKey: p.OwnerName, + }, + }, + Immutable: ptr.To(true), + Data: data, + } +} + +// secretNameFromData computes a content-addressable Secret name from the +// data entries. The name is "-" where hash is the +// first 16 hex characters of the SHA-256 digest of the sorted +// concatenated keys and values. +func (p *SecretPacker) secretNameFromData(data map[string][]byte) string { + h := sha256.New() + // Sort keys for determinism. + keys := make([]string, 0, len(data)) + for k := range data { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + h.Write([]byte(k)) + h.Write(data[k]) + } + return fmt.Sprintf("%s-%x", p.RevisionName, h.Sum(nil)[:8]) +} + +// contentHash returns a base64url-encoded (no padding) SHA-256 hash of data. +// The result is 43 characters long. +func contentHash(data []byte) string { + h := sha256.Sum256(data) + return base64.RawURLEncoding.EncodeToString(h[:]) +} + +func gzipData(data []byte) ([]byte, error) { + var buf bytes.Buffer + w, err := gzip.NewWriterLevel(&buf, gzip.BestCompression) + if err != nil { + return nil, err + } + if _, err := w.Write(data); err != nil { + return nil, err + } + if err := w.Close(); err != nil { + return nil, err + } + return buf.Bytes(), nil +} diff --git a/internal/operator-controller/applier/secretpacker_test.go b/internal/operator-controller/applier/secretpacker_test.go new file mode 100644 index 0000000000..7ac5d08b01 --- /dev/null +++ b/internal/operator-controller/applier/secretpacker_test.go @@ -0,0 +1,206 @@ +package applier + +import ( + "bytes" + "compress/gzip" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/labels" +) + +func TestSecretPacker_Pack(t *testing.T) { + packer := SecretPacker{ + RevisionName: "my-ext-3", + OwnerName: "my-ext", + SystemNamespace: "olmv1-system", + } + + t.Run("empty phases produce no Secrets", func(t *testing.T) { + result, err := packer.Pack(nil) + require.NoError(t, err) + assert.Empty(t, result.Secrets) + assert.Empty(t, result.Refs) + }) + + t.Run("single object packs into one Secret", func(t *testing.T) { + phases := []ocv1.ClusterExtensionRevisionPhase{{ + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{{ + Object: testConfigMap("test-cm", "default"), + }}, + }} + + result, err := packer.Pack(phases) + require.NoError(t, err) + + require.Len(t, result.Secrets, 1) + assert.True(t, strings.HasPrefix(result.Secrets[0].Name, "my-ext-3-"), "Secret name should be content-addressable with revision prefix") + assert.Equal(t, "olmv1-system", result.Secrets[0].Namespace) + assert.True(t, *result.Secrets[0].Immutable) + assert.Equal(t, "my-ext-3", result.Secrets[0].Labels[labels.RevisionNameKey]) + assert.Equal(t, "my-ext", result.Secrets[0].Labels[labels.OwnerNameKey]) + + ref, ok := result.Refs[[2]int{0, 0}] + require.True(t, ok) + assert.Equal(t, result.Secrets[0].Name, ref.Name) + assert.Equal(t, "olmv1-system", ref.Namespace) + + // Verify the key is a valid base64url hash (43 chars). + assert.Len(t, ref.Key, 43) + + // Verify the data at the key deserializes back to the same object. + data, ok := result.Secrets[0].Data[ref.Key] + require.True(t, ok) + var roundtrip map[string]interface{} + require.NoError(t, json.Unmarshal(data, &roundtrip)) + assert.Equal(t, "ConfigMap", roundtrip["kind"]) + }) + + t.Run("multiple objects fit in one Secret", func(t *testing.T) { + phases := []ocv1.ClusterExtensionRevisionPhase{{ + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{ + {Object: testConfigMap("cm-1", "default")}, + {Object: testConfigMap("cm-2", "default")}, + }, + }} + + result, err := packer.Pack(phases) + require.NoError(t, err) + require.Len(t, result.Secrets, 1) + assert.Len(t, result.Secrets[0].Data, 2) + assert.Len(t, result.Refs, 2) + }) + + t.Run("objects across multiple phases", func(t *testing.T) { + phases := []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "crds", + Objects: []ocv1.ClusterExtensionRevisionObject{{Object: testConfigMap("crd-1", "")}}, + }, + { + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{{Object: testConfigMap("deploy-1", "ns")}}, + }, + } + + result, err := packer.Pack(phases) + require.NoError(t, err) + + // Both objects fit in one Secret. + require.Len(t, result.Secrets, 1) + + // Refs point correctly to each phase/object. + ref0, ok := result.Refs[[2]int{0, 0}] + require.True(t, ok) + ref1, ok := result.Refs[[2]int{1, 0}] + require.True(t, ok) + assert.Equal(t, ref0.Name, ref1.Name) // same Secret + assert.NotEqual(t, ref0.Key, ref1.Key) // different keys + }) + + t.Run("deterministic: same input produces same output", func(t *testing.T) { + phases := []ocv1.ClusterExtensionRevisionPhase{{ + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{ + {Object: testConfigMap("cm-1", "default")}, + }, + }} + + result1, err := packer.Pack(phases) + require.NoError(t, err) + result2, err := packer.Pack(phases) + require.NoError(t, err) + + require.Len(t, result1.Secrets, 1) + require.Len(t, result2.Secrets, 1) + assert.Equal(t, result1.Refs, result2.Refs) + }) + + t.Run("skips ref-only objects", func(t *testing.T) { + phases := []ocv1.ClusterExtensionRevisionPhase{{ + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{ + {Ref: ocv1.ObjectSourceRef{Name: "existing-secret", Namespace: "ns", Key: "somekey"}}, + }, + }} + + result, err := packer.Pack(phases) + require.NoError(t, err) + assert.Empty(t, result.Secrets) + assert.Empty(t, result.Refs) + }) + + t.Run("large object gets gzipped", func(t *testing.T) { + // Create a large ConfigMap with data exceeding gzipThreshold (800 KiB). + largeObj := testConfigMap("large-cm", "default") + largeObj.Object["data"] = map[string]interface{}{ + // Repetitive data compresses well with gzip. + "bigkey": strings.Repeat("a", gzipThreshold+1), + } + + phases := []ocv1.ClusterExtensionRevisionPhase{{ + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{{Object: largeObj}}, + }} + + result, err := packer.Pack(phases) + require.NoError(t, err) + require.Len(t, result.Secrets, 1) + + ref := result.Refs[[2]int{0, 0}] + data := result.Secrets[0].Data[ref.Key] + + // Verify the stored data is gzip-compressed (magic bytes 0x1f 0x8b). + require.GreaterOrEqual(t, len(data), 2) + assert.Equal(t, byte(0x1f), data[0]) + assert.Equal(t, byte(0x8b), data[1]) + + // Verify we can decompress it. + reader, err := gzip.NewReader(bytes.NewReader(data)) + require.NoError(t, err) + defer reader.Close() + }) + + t.Run("key is SHA-256 base64url", func(t *testing.T) { + obj := testConfigMap("test-cm", "default") + rawData, err := json.Marshal(obj.Object) + require.NoError(t, err) + + expectedHash := sha256.Sum256(rawData) + expectedKey := base64.RawURLEncoding.EncodeToString(expectedHash[:]) + + phases := []ocv1.ClusterExtensionRevisionPhase{{ + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{{Object: obj}}, + }} + result, err := packer.Pack(phases) + require.NoError(t, err) + + ref := result.Refs[[2]int{0, 0}] + assert.Equal(t, expectedKey, ref.Key) + }) +} + +func testConfigMap(name, namespace string) unstructured.Unstructured { + obj := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": name, + }, + } + if namespace != "" { + obj["metadata"].(map[string]interface{})["namespace"] = namespace + } + return unstructured.Unstructured{Object: obj} +} diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index 2e9d2fce65..bd03ca2b6c 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -3,10 +3,13 @@ package controllers import ( + "bytes" + "compress/gzip" "context" "encoding/json" "errors" "fmt" + "io" "strings" "time" @@ -16,6 +19,7 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" @@ -63,6 +67,7 @@ type trackingCache interface { //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions,verbs=get;list;watch;update;patch;create;delete //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions/status,verbs=update;patch //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions/finalizers,verbs=update +//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch func (c *ClusterExtensionRevisionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { l := log.FromContext(ctx).WithName("cluster-extension-revision") @@ -488,7 +493,19 @@ func (c *ClusterExtensionRevisionReconciler) buildBoxcutterPhases(ctx context.Co for _, specPhase := range cer.Spec.Phases { objs := make([]client.Object, 0) for _, specObj := range specPhase.Objects { - obj := specObj.Object.DeepCopy() + var obj *unstructured.Unstructured + switch { + case specObj.Object.Object != nil: + obj = specObj.Object.DeepCopy() + case specObj.Ref.Name != "": + resolved, err := c.resolveObjectRef(ctx, specObj.Ref) + if err != nil { + return nil, nil, fmt.Errorf("resolving ref in phase %q: %w", specPhase.Name, err) + } + obj = resolved + default: + return nil, nil, fmt.Errorf("object in phase %q has neither object nor ref", specPhase.Name) + } objLabels := obj.GetLabels() if objLabels == nil { @@ -510,6 +527,42 @@ func (c *ClusterExtensionRevisionReconciler) buildBoxcutterPhases(ctx context.Co return phases, opts, nil } +// resolveObjectRef fetches the referenced Secret, reads the value at the specified key, +// auto-detects gzip compression, and deserializes into an unstructured.Unstructured. +func (c *ClusterExtensionRevisionReconciler) resolveObjectRef(ctx context.Context, ref ocv1.ObjectSourceRef) (*unstructured.Unstructured, error) { + secret := &corev1.Secret{} + key := client.ObjectKey{Name: ref.Name, Namespace: ref.Namespace} + if err := c.Client.Get(ctx, key, secret); err != nil { + return nil, fmt.Errorf("getting Secret %s/%s: %w", ref.Namespace, ref.Name, err) + } + + data, ok := secret.Data[ref.Key] + if !ok { + return nil, fmt.Errorf("key %q not found in Secret %s/%s", ref.Key, ref.Namespace, ref.Name) + } + + // Auto-detect gzip compression (magic bytes 0x1f 0x8b) + if len(data) >= 2 && data[0] == 0x1f && data[1] == 0x8b { + reader, err := gzip.NewReader(bytes.NewReader(data)) + if err != nil { + return nil, fmt.Errorf("creating gzip reader for key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) + } + defer reader.Close() + decompressed, err := io.ReadAll(reader) + if err != nil { + return nil, fmt.Errorf("decompressing key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) + } + data = decompressed + } + + obj := &unstructured.Unstructured{} + if err := json.Unmarshal(data, &obj.Object); err != nil { + return nil, fmt.Errorf("unmarshaling object from key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) + } + + return obj, nil +} + // EffectiveCollisionProtection resolves the collision protection value using // the inheritance hierarchy: object > phase > spec > default ("Prevent"). func EffectiveCollisionProtection(cp ...ocv1.CollisionProtection) ocv1.CollisionProtection { diff --git a/internal/operator-controller/controllers/resolve_ref_test.go b/internal/operator-controller/controllers/resolve_ref_test.go new file mode 100644 index 0000000000..eab8c51a1b --- /dev/null +++ b/internal/operator-controller/controllers/resolve_ref_test.go @@ -0,0 +1,284 @@ +package controllers_test + +import ( + "bytes" + "compress/gzip" + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apimachineryruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clocktesting "k8s.io/utils/clock/testing" + "pkg.package-operator.run/boxcutter/machinery" + machinerytypes "pkg.package-operator.run/boxcutter/machinery/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + 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/labels" +) + +func newSchemeWithCoreV1(t *testing.T) *apimachineryruntime.Scheme { + t.Helper() + sch := apimachineryruntime.NewScheme() + require.NoError(t, ocv1.AddToScheme(sch)) + require.NoError(t, corev1.AddToScheme(sch)) + return sch +} + +func TestResolveObjectRef_PlainJSON(t *testing.T) { + testScheme := newSchemeWithCoreV1(t) + + cmObj := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + "namespace": "default", + }, + } + cmData, err := json.Marshal(cmObj) + require.NoError(t, err) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "olmv1-system", + }, + Data: map[string][]byte{ + "my-key": cmData, + }, + } + + cer := newRefTestCER("ref-plain-1", ocv1.ObjectSourceRef{ + Name: "test-secret", + Namespace: "olmv1-system", + Key: "my-key", + }) + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(secret, cer). + WithStatusSubresource(&ocv1.ClusterExtensionRevision{}). + Build() + + mockEngine := &mockRevisionEngine{ + reconcile: func(_ context.Context, _ machinerytypes.Revision, _ ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return mockRevisionResult{}, nil + }, + } + reconciler := &controllers.ClusterExtensionRevisionReconciler{ + Client: fakeClient, + RevisionEngineFactory: &mockRevisionEngineFactory{engine: mockEngine}, + TrackingCache: &mockTrackingCache{client: fakeClient}, + Clock: clocktesting.NewFakeClock(metav1.Now().Time), + } + + _, err = reconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: cer.Name}, + }) + require.NoError(t, err) +} + +func TestResolveObjectRef_GzipCompressed(t *testing.T) { + testScheme := newSchemeWithCoreV1(t) + + cmObj := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + "namespace": "default", + }, + } + cmData, err := json.Marshal(cmObj) + require.NoError(t, err) + + var buf bytes.Buffer + w, err := gzip.NewWriterLevel(&buf, gzip.BestCompression) + require.NoError(t, err) + _, err = w.Write(cmData) + require.NoError(t, err) + require.NoError(t, w.Close()) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret-gz", + Namespace: "olmv1-system", + }, + Data: map[string][]byte{ + "my-key": buf.Bytes(), + }, + } + + cer := newRefTestCER("ref-gzip-1", ocv1.ObjectSourceRef{ + Name: "test-secret-gz", + Namespace: "olmv1-system", + Key: "my-key", + }) + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(secret, cer). + WithStatusSubresource(&ocv1.ClusterExtensionRevision{}). + Build() + + mockEngine := &mockRevisionEngine{ + reconcile: func(_ context.Context, _ machinerytypes.Revision, _ ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return mockRevisionResult{}, nil + }, + } + reconciler := &controllers.ClusterExtensionRevisionReconciler{ + Client: fakeClient, + RevisionEngineFactory: &mockRevisionEngineFactory{engine: mockEngine}, + TrackingCache: &mockTrackingCache{client: fakeClient}, + Clock: clocktesting.NewFakeClock(metav1.Now().Time), + } + + _, err = reconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: cer.Name}, + }) + require.NoError(t, err) +} + +func TestResolveObjectRef_SecretNotFound(t *testing.T) { + testScheme := newSchemeWithCoreV1(t) + + cer := newRefTestCER("ref-notfound-1", ocv1.ObjectSourceRef{ + Name: "nonexistent-secret", + Namespace: "olmv1-system", + Key: "my-key", + }) + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(cer). + WithStatusSubresource(&ocv1.ClusterExtensionRevision{}). + Build() + + reconciler := &controllers.ClusterExtensionRevisionReconciler{ + Client: fakeClient, + RevisionEngineFactory: &mockRevisionEngineFactory{engine: &mockRevisionEngine{}}, + TrackingCache: &mockTrackingCache{client: fakeClient}, + Clock: clocktesting.NewFakeClock(metav1.Now().Time), + } + + _, err := reconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: cer.Name}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "resolving ref") +} + +func TestResolveObjectRef_KeyNotFound(t *testing.T) { + testScheme := newSchemeWithCoreV1(t) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret-nokey", + Namespace: "olmv1-system", + }, + Data: map[string][]byte{ + "other-key": []byte("{}"), + }, + } + + cer := newRefTestCER("ref-nokey-1", ocv1.ObjectSourceRef{ + Name: "test-secret-nokey", + Namespace: "olmv1-system", + Key: "missing-key", + }) + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(secret, cer). + WithStatusSubresource(&ocv1.ClusterExtensionRevision{}). + Build() + + reconciler := &controllers.ClusterExtensionRevisionReconciler{ + Client: fakeClient, + RevisionEngineFactory: &mockRevisionEngineFactory{engine: &mockRevisionEngine{}}, + TrackingCache: &mockTrackingCache{client: fakeClient}, + Clock: clocktesting.NewFakeClock(metav1.Now().Time), + } + + _, err := reconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: cer.Name}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "key") +} + +func TestResolveObjectRef_InvalidJSON(t *testing.T) { + testScheme := newSchemeWithCoreV1(t) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret-invalid", + Namespace: "olmv1-system", + }, + Data: map[string][]byte{ + "my-key": []byte("not-valid-json"), + }, + } + + cer := newRefTestCER("ref-invalid-1", ocv1.ObjectSourceRef{ + Name: "test-secret-invalid", + Namespace: "olmv1-system", + Key: "my-key", + }) + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(secret, cer). + WithStatusSubresource(&ocv1.ClusterExtensionRevision{}). + Build() + + reconciler := &controllers.ClusterExtensionRevisionReconciler{ + Client: fakeClient, + RevisionEngineFactory: &mockRevisionEngineFactory{engine: &mockRevisionEngine{}}, + TrackingCache: &mockTrackingCache{client: fakeClient}, + Clock: clocktesting.NewFakeClock(metav1.Now().Time), + } + + _, err := reconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: cer.Name}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "unmarshal") +} + +func newRefTestCER(name string, ref ocv1.ObjectSourceRef) *ocv1.ClusterExtensionRevision { + cer := &ocv1.ClusterExtensionRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + UID: types.UID(name), + Labels: map[string]string{ + labels.OwnerNameKey: "test-ext", + }, + }, + Spec: ocv1.ClusterExtensionRevisionSpec{ + LifecycleState: ocv1.ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + CollisionProtection: ocv1.CollisionProtectionPrevent, + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Ref: ref, + }, + }, + }, + }, + }, + } + cer.SetGroupVersionKind(ocv1.GroupVersion.WithKind("ClusterExtensionRevision")) + return cer +} diff --git a/internal/operator-controller/labels/labels.go b/internal/operator-controller/labels/labels.go index 16f45ecbb3..9020dec658 100644 --- a/internal/operator-controller/labels/labels.go +++ b/internal/operator-controller/labels/labels.go @@ -40,6 +40,12 @@ const ( // ClusterExtensionRevision operations is preserved. ServiceAccountNamespaceKey = "olm.operatorframework.io/service-account-namespace" + // RevisionNameKey is the label key used to record the name of the + // ClusterExtensionRevision that owns or references a resource (e.g. a + // ref Secret). It enables efficient listing of all resources associated + // with a specific revision. + RevisionNameKey = "olm.operatorframework.io/revision-name" + // MigratedFromHelmKey is the label key used to mark ClusterExtensionRevisions // that were created during migration from Helm releases. This label is used // to distinguish migrated revisions from those created by normal Boxcutter operation. diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 55a4e157c5..f4a3b7121e 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -774,6 +774,8 @@ spec: description: |- ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part of a phase, along with its collision protection settings. + + Exactly one of object or ref must be set. properties: collisionProtection: description: |- @@ -802,15 +804,47 @@ spec: type: string object: description: |- - object is a required embedded Kubernetes object to be applied. + object is an optional embedded Kubernetes object to be applied. + + Exactly one of object or ref must be set. This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true - required: - - object + ref: + description: |- + ref is an optional reference to a Secret that holds the serialized + object manifest. + + Exactly one of object or ref must be set. + properties: + key: + description: |- + key is the data key within the referenced Secret containing the + object manifest content. The value at this key must be a + JSON-serialized Kubernetes object manifest. + maxLength: 253 + minLength: 1 + type: string + name: + description: name is the name of the referenced Secret. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: namespace is the namespace of the referenced + Secret. + maxLength: 63 + type: string + required: + - key + - name + type: object type: object + x-kubernetes-validations: + - message: exactly one of object or ref must be set + rule: has(self.object) != has(self.ref) maxItems: 50 type: array required: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 8604a7db8d..70218c59d4 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -735,6 +735,8 @@ spec: description: |- ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part of a phase, along with its collision protection settings. + + Exactly one of object or ref must be set. properties: collisionProtection: description: |- @@ -763,15 +765,47 @@ spec: type: string object: description: |- - object is a required embedded Kubernetes object to be applied. + object is an optional embedded Kubernetes object to be applied. + + Exactly one of object or ref must be set. This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true - required: - - object + ref: + description: |- + ref is an optional reference to a Secret that holds the serialized + object manifest. + + Exactly one of object or ref must be set. + properties: + key: + description: |- + key is the data key within the referenced Secret containing the + object manifest content. The value at this key must be a + JSON-serialized Kubernetes object manifest. + maxLength: 253 + minLength: 1 + type: string + name: + description: name is the name of the referenced Secret. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: namespace is the namespace of the referenced + Secret. + maxLength: 63 + type: string + required: + - key + - name + type: object type: object + x-kubernetes-validations: + - message: exactly one of object or ref must be set + rule: has(self.object) != has(self.ref) maxItems: 50 type: array required: diff --git a/test/e2e/features/install.feature b/test/e2e/features/install.feature index ce3fb34302..1c89e75b1c 100644 --- a/test/e2e/features/install.feature +++ b/test/e2e/features/install.feature @@ -501,6 +501,35 @@ Feature: Install ClusterExtension And ClusterExtensionRevision "${NAME}-1" has label "olm.operatorframework.io/owner-kind" with value "ClusterExtension" And ClusterExtensionRevision "${NAME}-1" has label "olm.operatorframework.io/owner-name" with value "${NAME}" + @BoxcutterRuntime + Scenario: ClusterExtensionRevision objects are externalized to immutable Secrets + When ClusterExtension is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtension + metadata: + name: ${NAME} + spec: + namespace: ${TEST_NAMESPACE} + serviceAccount: + name: olm-sa + source: + sourceType: Catalog + catalog: + packageName: test + version: 1.2.0 + selector: + matchLabels: + "olm.operatorframework.io/metadata.name": test-catalog + """ + Then ClusterExtension is rolled out + And ClusterExtension is available + And ClusterExtensionRevision "${NAME}-1" phase objects use refs + And ClusterExtensionRevision "${NAME}-1" ref Secrets exist in "olmv1-system" namespace + And ClusterExtensionRevision "${NAME}-1" ref Secrets are immutable + And ClusterExtensionRevision "${NAME}-1" ref Secrets are labeled with revision and owner + And ClusterExtensionRevision "${NAME}-1" ref Secrets have ownerReference to the revision + @DeploymentConfig Scenario: deploymentConfig nodeSelector is applied to the operator deployment When ClusterExtension is applied diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index 57cfc864ed..a856e60f9c 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -95,6 +95,11 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" contains annotation "([^"]+)" with value$`, ClusterExtensionRevisionHasAnnotationWithValue) sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" has label "([^"]+)" with value "([^"]+)"$`, ClusterExtensionRevisionHasLabelWithValue) sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" phase objects are not found or not owned by the revision$`, ClusterExtensionRevisionObjectsNotFoundOrNotOwned) + sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" phase objects use refs$`, ClusterExtensionRevisionPhaseObjectsUseRefs) + sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" ref Secrets exist in "([^"]+)" namespace$`, ClusterExtensionRevisionRefSecretsExist) + sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" ref Secrets are immutable$`, ClusterExtensionRevisionRefSecretsAreImmutable) + sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" ref Secrets are labeled with revision and owner$`, ClusterExtensionRevisionRefSecretsLabeled) + sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" ref Secrets have ownerReference to the revision$`, ClusterExtensionRevisionRefSecretsHaveOwnerRef) sc.Step(`^(?i)resource "([^"]+)" is installed$`, ResourceAvailable) sc.Step(`^(?i)resource "([^"]+)" is available$`, ResourceAvailable) @@ -630,9 +635,21 @@ func ClusterExtensionRevisionObjectsNotFoundOrNotOwned(ctx context.Context, revi // For each object in each phase, verify it either doesn't exist or // doesn't have the CER in its ownerReferences - for _, phase := range rev.Spec.Phases { - for _, phaseObj := range phase.Objects { - obj := phaseObj.Object + for i, phase := range rev.Spec.Phases { + for j, phaseObj := range phase.Objects { + var obj *unstructured.Unstructured + switch { + case phaseObj.Ref.Name != "": + resolved, err := resolveObjectRef(phaseObj.Ref) + if err != nil { + return fmt.Errorf("resolving ref in phase %q object %d: %w", phase.Name, j, err) + } + obj = resolved + case len(phaseObj.Object.Object) > 0: + obj = &phaseObj.Object + default: + return fmt.Errorf("clusterextensionrevision %q phase %d object %d has neither ref nor inline object", revisionName, i, j) + } kind := obj.GetKind() name := obj.GetName() namespace := obj.GetNamespace() @@ -678,6 +695,218 @@ func ClusterExtensionRevisionObjectsNotFoundOrNotOwned(ctx context.Context, revi return nil } +// ClusterExtensionRevisionPhaseObjectsUseRefs verifies that every object in every phase of the named +// ClusterExtensionRevision uses a ref (not an inline object). Polls with timeout. +func ClusterExtensionRevisionPhaseObjectsUseRefs(ctx context.Context, revisionName string) error { + sc := scenarioCtx(ctx) + revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc) + + waitFor(ctx, func() bool { + obj, err := getResource("clusterextensionrevision", revisionName, "") + if err != nil { + return false + } + phases, ok, _ := unstructured.NestedSlice(obj.Object, "spec", "phases") + if !ok || len(phases) == 0 { + return false + } + for _, p := range phases { + phase, ok := p.(map[string]interface{}) + if !ok { + return false + } + objects, ok, _ := unstructured.NestedSlice(phase, "objects") + if !ok || len(objects) == 0 { + return false + } + for _, o := range objects { + obj, ok := o.(map[string]interface{}) + if !ok { + return false + } + ref, refOK, _ := unstructured.NestedMap(obj, "ref") + if !refOK || len(ref) == 0 { + logger.V(1).Info("object does not use ref", "revision", revisionName) + return false + } + name, _, _ := unstructured.NestedString(ref, "name") + if name == "" { + logger.V(1).Info("ref has empty name", "revision", revisionName) + return false + } + } + } + return true + }) + return nil +} + +// ClusterExtensionRevisionRefSecretsExist verifies that all Secrets referenced by the named +// ClusterExtensionRevision's phase objects exist in the given namespace. Polls with timeout. +func ClusterExtensionRevisionRefSecretsExist(ctx context.Context, revisionName, namespace string) error { + sc := scenarioCtx(ctx) + revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc) + namespace = substituteScenarioVars(strings.TrimSpace(namespace), sc) + + secretNames, err := collectRefSecretNames(ctx, revisionName) + if err != nil { + return err + } + + for _, name := range secretNames { + waitFor(ctx, func() bool { + _, err := getResource("secret", name, namespace) + return err == nil + }) + } + return nil +} + +// ClusterExtensionRevisionRefSecretsAreImmutable verifies that all ref Secrets for the named +// ClusterExtensionRevision are immutable. Polls with timeout. +func ClusterExtensionRevisionRefSecretsAreImmutable(ctx context.Context, revisionName string) error { + sc := scenarioCtx(ctx) + revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc) + + secrets, err := listRefSecrets(ctx, revisionName) + if err != nil { + return err + } + if len(secrets) == 0 { + return fmt.Errorf("no ref Secrets found for revision %q", revisionName) + } + for _, s := range secrets { + if s.Immutable == nil || !*s.Immutable { + return fmt.Errorf("ref Secret %s/%s is not immutable", s.Namespace, s.Name) + } + } + return nil +} + +// ClusterExtensionRevisionRefSecretsLabeled verifies that all ref Secrets for the named +// ClusterExtensionRevision have the expected revision-name and owner-name labels. +func ClusterExtensionRevisionRefSecretsLabeled(ctx context.Context, revisionName string) error { + sc := scenarioCtx(ctx) + revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc) + + secrets, err := listRefSecrets(ctx, revisionName) + if err != nil { + return err + } + if len(secrets) == 0 { + return fmt.Errorf("no ref Secrets found for revision %q", revisionName) + } + + // Get the owner name from the CER's own labels. + cerObj, err := getResource("clusterextensionrevision", revisionName, "") + if err != nil { + return fmt.Errorf("getting CER %q: %w", revisionName, err) + } + expectedOwner := cerObj.GetLabels()["olm.operatorframework.io/owner-name"] + + for _, s := range secrets { + revLabel := s.Labels["olm.operatorframework.io/revision-name"] + if revLabel != revisionName { + return fmt.Errorf("secret %s/%s has revision-name label %q, expected %q", s.Namespace, s.Name, revLabel, revisionName) + } + ownerLabel := s.Labels["olm.operatorframework.io/owner-name"] + if expectedOwner != "" && ownerLabel != expectedOwner { + return fmt.Errorf("secret %s/%s has owner-name label %q, expected %q", s.Namespace, s.Name, ownerLabel, expectedOwner) + } + } + return nil +} + +// ClusterExtensionRevisionRefSecretsHaveOwnerRef verifies that all ref Secrets for the named +// ClusterExtensionRevision have an ownerReference pointing to the CER with controller=true. +func ClusterExtensionRevisionRefSecretsHaveOwnerRef(ctx context.Context, revisionName string) error { + sc := scenarioCtx(ctx) + revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc) + + cerObj, err := getResource("clusterextensionrevision", revisionName, "") + if err != nil { + return fmt.Errorf("getting CER %q: %w", revisionName, err) + } + cerUID := cerObj.GetUID() + + secrets, err := listRefSecrets(ctx, revisionName) + if err != nil { + return err + } + if len(secrets) == 0 { + return fmt.Errorf("no ref Secrets found for revision %q", revisionName) + } + + for _, s := range secrets { + found := false + for _, ref := range s.OwnerReferences { + if ref.Kind == ocv1.ClusterExtensionRevisionKind && ref.Name == revisionName && ref.UID == cerUID { + if ref.Controller == nil || !*ref.Controller { + return fmt.Errorf("secret %s/%s has ownerReference to CER but controller is not true", s.Namespace, s.Name) + } + found = true + break + } + } + if !found { + return fmt.Errorf("secret %s/%s does not have ownerReference to CER %q (uid %s)", s.Namespace, s.Name, revisionName, cerUID) + } + } + return nil +} + +// collectRefSecretNames returns the unique set of Secret names referenced by the CER's phase objects. +func collectRefSecretNames(ctx context.Context, revisionName string) ([]string, error) { + var names []string + seen := sets.New[string]() + + var obj *unstructured.Unstructured + waitFor(ctx, func() bool { + var err error + obj, err = getResource("clusterextensionrevision", revisionName, "") + return err == nil + }) + + phases, _, _ := unstructured.NestedSlice(obj.Object, "spec", "phases") + for _, p := range phases { + phase, ok := p.(map[string]interface{}) + if !ok { + continue + } + objects, _, _ := unstructured.NestedSlice(phase, "objects") + for _, o := range objects { + phaseObj, ok := o.(map[string]interface{}) + if !ok { + continue + } + name, _, _ := unstructured.NestedString(phaseObj, "ref", "name") + if name != "" && !seen.Has(name) { + seen.Insert(name) + names = append(names, name) + } + } + } + if len(names) == 0 { + return nil, fmt.Errorf("no ref Secret names found in CER %q", revisionName) + } + return names, nil +} + +// listRefSecrets lists all Secrets in the OLM namespace that have the revision-name label +// matching the given revision name. +func listRefSecrets(_ context.Context, revisionName string) ([]corev1.Secret, error) { + out, err := k8sClient("get", "secrets", "-n", olmNamespace, + "-l", "olm.operatorframework.io/revision-name="+revisionName, "-o", "json") + if err != nil { + return nil, fmt.Errorf("listing ref Secrets for revision %q: %w", revisionName, err) + } + var secretList corev1.SecretList + if err := json.Unmarshal([]byte(out), &secretList); err != nil { + return nil, fmt.Errorf("unmarshalling Secret list: %w", err) + } + return secretList.Items, nil +} + // ResourceAvailable waits for the specified resource (kind/name format) to exist in the test namespace. Polls with timeout. func ResourceAvailable(ctx context.Context, resource string) error { sc := scenarioCtx(ctx) @@ -1325,13 +1554,57 @@ func listExtensionRevisionResources(extName string) ([]client.Object, error) { for i := range rev.Spec.Phases { phase := &rev.Spec.Phases[i] for j := range phase.Objects { - objs = append(objs, &phase.Objects[j].Object) + specObj := &phase.Objects[j] + switch { + case specObj.Ref.Name != "": + resolved, err := resolveObjectRef(specObj.Ref) + if err != nil { + return nil, fmt.Errorf("resolving ref in phase %q object %d: %w", phase.Name, j, err) + } + objs = append(objs, resolved) + case len(specObj.Object.Object) > 0: + objs = append(objs, &specObj.Object) + } } } return objs, nil } +// resolveObjectRef fetches an object from a Secret ref using kubectl. +func resolveObjectRef(ref ocv1.ObjectSourceRef) (*unstructured.Unstructured, error) { + out, err := k8sClient("get", "secret", ref.Name, "-n", ref.Namespace, "-o", "json") + if err != nil { + return nil, fmt.Errorf("getting Secret %s/%s: %w", ref.Namespace, ref.Name, err) + } + var secret corev1.Secret + if err := json.Unmarshal([]byte(out), &secret); err != nil { + return nil, fmt.Errorf("unmarshaling Secret %s/%s: %w", ref.Namespace, ref.Name, err) + } + data, ok := secret.Data[ref.Key] + if !ok { + return nil, fmt.Errorf("key %q not found in Secret %s/%s", ref.Key, ref.Namespace, ref.Name) + } + // Auto-detect gzip compression (magic bytes 0x1f 0x8b) + if len(data) >= 2 && data[0] == 0x1f && data[1] == 0x8b { + reader, err := gzip.NewReader(bytes.NewReader(data)) + if err != nil { + return nil, fmt.Errorf("creating gzip reader for key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) + } + defer reader.Close() + decompressed, err := io.ReadAll(reader) + if err != nil { + return nil, fmt.Errorf("decompressing key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) + } + data = decompressed + } + obj := &unstructured.Unstructured{} + if err := json.Unmarshal(data, &obj.Object); err != nil { + return nil, fmt.Errorf("unmarshaling object from key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) + } + return obj, nil +} + // latestActiveRevisionForExtension returns the latest active revision for the extension called extName func latestActiveRevisionForExtension(extName string) (*ocv1.ClusterExtensionRevision, error) { out, err := k8sClient("get", "clusterextensionrevisions", "-l", fmt.Sprintf("olm.operatorframework.io/owner-name=%s", extName), "-o", "json") From 7bb4725a8ac617cdd572b0f178292db7c771dd69 Mon Sep 17 00:00:00 2001 From: Predrag Knezevic Date: Thu, 26 Mar 2026 13:26:12 +0100 Subject: [PATCH 2/3] Address PR #2595 review feedback - Fix duplicate key size inflation in SecretPacker by only incrementing size for new content hash keys - Add io.LimitReader (10 MiB cap) for gzip decompression to prevent gzip bombs in controller and e2e helpers - Add doc comment clarifying ObjectSourceRef.Namespace defaults to OLM system namespace during ref resolution - Fix docs: orphan cleanup uses ownerReference GC, ref resolution failures are retried (not terminal) - Remove unused ClusterExtensionRevisionReasonRefResolutionFailed constant - Add default error branch in e2e listExtensionRevisionResources for objects missing both ref and inline content Co-Authored-By: Claude Opus 4.6 --- api/v1/clusterextensionrevision_types.go | 14 ++++++------ applyconfigurations/api/v1/objectsourceref.go | 1 + docs/concepts/large-bundle-support.md | 13 +++++++---- ...ramework.io_clusterextensionrevisions.yaml | 5 +++-- .../applier/secretpacker.go | 14 +++++++----- .../applier/secretpacker_test.go | 22 +++++++++++++++++++ .../clusterextensionrevision_controller.go | 7 +++++- manifests/experimental-e2e.yaml | 5 +++-- manifests/experimental.yaml | 5 +++-- test/e2e/steps/steps.go | 9 +++++++- 10 files changed, 70 insertions(+), 25 deletions(-) diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index bfdadf20a9..33d3de0386 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -30,13 +30,12 @@ const ( ClusterExtensionRevisionTypeSucceeded = "Succeeded" // Condition Reasons - ClusterExtensionRevisionReasonArchived = "Archived" - ClusterExtensionRevisionReasonBlocked = "Blocked" - ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure" - ClusterExtensionRevisionReasonProbesSucceeded = "ProbesSucceeded" - ClusterExtensionRevisionReasonReconciling = "Reconciling" - ClusterExtensionRevisionReasonRefResolutionFailed = "RefResolutionFailed" - ClusterExtensionRevisionReasonRetrying = "Retrying" + ClusterExtensionRevisionReasonArchived = "Archived" + ClusterExtensionRevisionReasonBlocked = "Blocked" + ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure" + ClusterExtensionRevisionReasonProbesSucceeded = "ProbesSucceeded" + ClusterExtensionRevisionReasonReconciling = "Reconciling" + ClusterExtensionRevisionReasonRetrying = "Retrying" ) // ClusterExtensionRevisionSpec defines the desired state of ClusterExtensionRevision. @@ -452,6 +451,7 @@ type ObjectSourceRef struct { Name string `json:"name"` // namespace is the namespace of the referenced Secret. + // When empty, defaults to the OLM system namespace during ref resolution. // // +optional // +kubebuilder:validation:MaxLength=63 diff --git a/applyconfigurations/api/v1/objectsourceref.go b/applyconfigurations/api/v1/objectsourceref.go index 06dfa7f8c6..718775f0c7 100644 --- a/applyconfigurations/api/v1/objectsourceref.go +++ b/applyconfigurations/api/v1/objectsourceref.go @@ -26,6 +26,7 @@ type ObjectSourceRefApplyConfiguration struct { // name is the name of the referenced Secret. Name *string `json:"name,omitempty"` // namespace is the namespace of the referenced Secret. + // When empty, defaults to the OLM system namespace during ref resolution. Namespace *string `json:"namespace,omitempty"` // key is the data key within the referenced Secret containing the // object manifest content. The value at this key must be a diff --git a/docs/concepts/large-bundle-support.md b/docs/concepts/large-bundle-support.md index 033222b8f3..055ec628cb 100644 --- a/docs/concepts/large-bundle-support.md +++ b/docs/concepts/large-bundle-support.md @@ -364,9 +364,12 @@ Key properties: - **No reconciler churn**: Referenced Secrets exist before the CER is created. The CER reconciler never encounters missing Secrets during normal operation. - **Orphan cleanup**: Secrets created in step 1 carry the revision label - (`olm.operatorframework.io/revision-name`). If a crash leaves Secrets without - a corresponding CER, the ClusterExtension controller detects and deletes them - on its next reconciliation. + (`olm.operatorframework.io/revision-name`). Once the CER is created in step 2 + and ownerReferences are patched in step 3, Kubernetes garbage collection + automatically removes the Secrets when the CER is deleted. If a crash occurs + between steps 1 and 2, the ClusterExtension controller detects orphaned + Secrets (those with the revision label but no corresponding CER) and deletes + them on its next reconciliation. - **Idempotent retry**: Secrets are immutable in data. Re-creation of an existing Secret returns AlreadyExists and is skipped. ownerReference patching is idempotent — patching an already-set ownerReference is a no-op. @@ -389,7 +392,9 @@ Under normal operation, referenced Secrets are guaranteed to exist before the CER is created (see [Crash-safe creation sequence](#crash-safe-creation-sequence)). If a referenced Secret or key is not found — indicating an inconsistent state caused by external modification or a partially completed creation sequence — -the reconciler sets a terminal error condition on the CER. +the reconciler returns a retryable error, allowing the controller to retry on +subsequent reconciliation attempts. This handles transient issues such as +informer cache lag after Secret creation. Secrets are fetched using the typed client served from the informer cache. diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml index a9f44c4d8d..83f989cb9d 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml @@ -221,8 +221,9 @@ spec: minLength: 1 type: string namespace: - description: namespace is the namespace of the referenced - Secret. + description: |- + namespace is the namespace of the referenced Secret. + When empty, defaults to the OLM system namespace during ref resolution. maxLength: 63 type: string required: diff --git a/internal/operator-controller/applier/secretpacker.go b/internal/operator-controller/applier/secretpacker.go index 1e7a705e2c..7107d382d4 100644 --- a/internal/operator-controller/applier/secretpacker.go +++ b/internal/operator-controller/applier/secretpacker.go @@ -115,13 +115,15 @@ func (p *SecretPacker) Pack(phases []ocv1.ClusterExtensionRevisionPhase) (*PackR key := contentHash(data) - // If adding this entry would exceed the limit, finalize the current Secret. - if currentSize+len(data) > maxSecretDataSize && len(currentData) > 0 { - finalizeCurrent() + // Only add data and increment size for new keys. Duplicate content + // (same hash) reuses the existing entry without inflating the size. + if _, exists := currentData[key]; !exists { + if currentSize+len(data) > maxSecretDataSize && len(currentData) > 0 { + finalizeCurrent() + } + currentData[key] = data + currentSize += len(data) } - - currentData[key] = data - currentSize += len(data) currentPending = append(currentPending, pendingRef{pos: [2]int{phaseIdx, objIdx}, key: key}) } } diff --git a/internal/operator-controller/applier/secretpacker_test.go b/internal/operator-controller/applier/secretpacker_test.go index 7ac5d08b01..b6e44d53d4 100644 --- a/internal/operator-controller/applier/secretpacker_test.go +++ b/internal/operator-controller/applier/secretpacker_test.go @@ -171,6 +171,28 @@ func TestSecretPacker_Pack(t *testing.T) { defer reader.Close() }) + t.Run("duplicate content objects share key and do not inflate size", func(t *testing.T) { + // Two identical objects should produce the same content hash key. + // The second occurrence must not double-count the size. + phases := []ocv1.ClusterExtensionRevisionPhase{{ + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{ + {Object: testConfigMap("same-cm", "default")}, + {Object: testConfigMap("same-cm", "default")}, + }, + }} + + result, err := packer.Pack(phases) + require.NoError(t, err) + require.Len(t, result.Secrets, 1) + // Only one data entry despite two objects. + assert.Len(t, result.Secrets[0].Data, 1) + // Both positions get refs. + assert.Len(t, result.Refs, 2) + // Both refs point to the same key. + assert.Equal(t, result.Refs[[2]int{0, 0}].Key, result.Refs[[2]int{0, 1}].Key) + }) + t.Run("key is SHA-256 base64url", func(t *testing.T) { obj := testConfigMap("test-cm", "default") rawData, err := json.Marshal(obj.Object) diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index bd03ca2b6c..e74d60e58d 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -548,10 +548,15 @@ func (c *ClusterExtensionRevisionReconciler) resolveObjectRef(ctx context.Contex return nil, fmt.Errorf("creating gzip reader for key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) } defer reader.Close() - decompressed, err := io.ReadAll(reader) + const maxDecompressedSize = 10 * 1024 * 1024 // 10 MiB + limited := io.LimitReader(reader, maxDecompressedSize+1) + decompressed, err := io.ReadAll(limited) if err != nil { return nil, fmt.Errorf("decompressing key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) } + if len(decompressed) > maxDecompressedSize { + return nil, fmt.Errorf("decompressed data for key %q in Secret %s/%s exceeds maximum size (%d bytes)", ref.Key, ref.Namespace, ref.Name, maxDecompressedSize) + } data = decompressed } diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index f4a3b7121e..86af261153 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -833,8 +833,9 @@ spec: minLength: 1 type: string namespace: - description: namespace is the namespace of the referenced - Secret. + description: |- + namespace is the namespace of the referenced Secret. + When empty, defaults to the OLM system namespace during ref resolution. maxLength: 63 type: string required: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 70218c59d4..5a31d93419 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -794,8 +794,9 @@ spec: minLength: 1 type: string namespace: - description: namespace is the namespace of the referenced - Secret. + description: |- + namespace is the namespace of the referenced Secret. + When empty, defaults to the OLM system namespace during ref resolution. maxLength: 63 type: string required: diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index a856e60f9c..e473341046 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -1564,6 +1564,8 @@ func listExtensionRevisionResources(extName string) ([]client.Object, error) { objs = append(objs, resolved) case len(specObj.Object.Object) > 0: objs = append(objs, &specObj.Object) + default: + return nil, fmt.Errorf("object %d in phase %q has neither object nor ref", j, phase.Name) } } } @@ -1592,10 +1594,15 @@ func resolveObjectRef(ref ocv1.ObjectSourceRef) (*unstructured.Unstructured, err return nil, fmt.Errorf("creating gzip reader for key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) } defer reader.Close() - decompressed, err := io.ReadAll(reader) + const maxDecompressedSize = 10 * 1024 * 1024 // 10 MiB + limited := io.LimitReader(reader, maxDecompressedSize+1) + decompressed, err := io.ReadAll(limited) if err != nil { return nil, fmt.Errorf("decompressing key %q in Secret %s/%s: %w", ref.Key, ref.Namespace, ref.Name, err) } + if len(decompressed) > maxDecompressedSize { + return nil, fmt.Errorf("decompressed data for key %q in Secret %s/%s exceeds maximum size (%d bytes)", ref.Key, ref.Namespace, ref.Name, maxDecompressedSize) + } data = decompressed } obj := &unstructured.Unstructured{} From 0db238a1e0f356f44b4f4d740fad5472485bc228 Mon Sep 17 00:00:00 2001 From: Predrag Knezevic Date: Thu, 26 Mar 2026 13:39:59 +0100 Subject: [PATCH 3/3] Change gzipThreshold from 800 KiB to 900 KiB Co-Authored-By: Claude Opus 4.6 --- internal/operator-controller/applier/secretpacker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/operator-controller/applier/secretpacker.go b/internal/operator-controller/applier/secretpacker.go index 7107d382d4..7e09d77510 100644 --- a/internal/operator-controller/applier/secretpacker.go +++ b/internal/operator-controller/applier/secretpacker.go @@ -25,7 +25,7 @@ const ( // gzipThreshold is the size above which individual objects are // gzip-compressed before being stored in a Secret. - gzipThreshold = 800 * 1024 + gzipThreshold = 900 * 1024 ) // SecretPacker packs serialized objects from CER phases into one or more