From 8e3683f7b54f8c10e7dc8504fac452457b9d6180 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Tue, 15 Jul 2025 12:20:17 +0200 Subject: [PATCH 01/17] Update boxcutter library to branch with latest k8s and controller-runtime libs Signed-off-by: Per Goncalves da Silva --- go.mod | 4 +++- go.sum | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 7f2e8a9e3..cb0f0baa5 100644 --- a/go.mod +++ b/go.mod @@ -47,6 +47,8 @@ require ( sigs.k8s.io/yaml v1.6.0 ) +replace pkg.package-operator.run/boxcutter => github.com/perdasilva/boxcutter v0.0.0-20250715101157-18ea858f54bd + require ( k8s.io/component-helpers v0.33.2 // indirect k8s.io/kube-openapi v0.0.0-20250610211856-8b98d1ed966a // indirect @@ -178,7 +180,7 @@ require ( github.com/proglottis/gpgme v0.1.4 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.65.0 // indirect - github.com/prometheus/procfs v0.16.1 // indirect + github.com/prometheus/procfs v0.17.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/rubenv/sql-migrate v1.8.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect diff --git a/go.sum b/go.sum index 897a5ba13..7479d509b 100644 --- a/go.sum +++ b/go.sum @@ -400,8 +400,8 @@ github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNw github.com/prometheus/client_model v0.6.2/go.mod h1:y3m2F6Gdpfy6Ut/GBsUqTWZqCUvMVzSfMLjcu6wAwpE= github.com/prometheus/common v0.65.0 h1:QDwzd+G1twt//Kwj/Ww6E9FQq1iVMmODnILtW1t2VzE= github.com/prometheus/common v0.65.0/go.mod h1:0gZns+BLRQ3V6NdaerOhMbwwRbNh9hkGINtQAsP5GS8= -github.com/prometheus/procfs v0.16.1 h1:hZ15bTNuirocR6u0JZ6BAHHmwS1p8B4P6MRqxtzMyRg= -github.com/prometheus/procfs v0.16.1/go.mod h1:teAbpZRB1iIAJYREa1LsoWUXykVXA1KlTmWl8x/U+Is= +github.com/prometheus/procfs v0.17.0 h1:FuLQ+05u4ZI+SS/w9+BWEM2TXiHKsUQ9TADiRH7DuK0= +github.com/prometheus/procfs v0.17.0/go.mod h1:oPQLaDAMRbA+u8H5Pbfq+dl3VDAvHxMUOVhe0wYB2zw= github.com/redis/go-redis/extra/rediscmd/v9 v9.10.0 h1:uTiEyEyfLhkw678n6EulHVto8AkcXVr8zUcBJNZ0ark= github.com/redis/go-redis/extra/rediscmd/v9 v9.10.0/go.mod h1:eFYL/99JvdLP4T9/3FZ5t2pClnv7mMskc+WstTcyVr4= github.com/redis/go-redis/extra/redisotel/v9 v9.10.0 h1:4z7/hCJ9Jft8EBb2tDmK38p2WjyIEJ1ShhhwAhjOCps= From 1b1396cb14441bc25d6b1a39a9f57eb1459ee0be Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:08:22 +0200 Subject: [PATCH 02/17] Update go.mod Signed-off-by: Per Goncalves da Silva --- go.mod | 7 ++++--- go.sum | 10 ++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index cb0f0baa5..ed780c90e 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/cert-manager/cert-manager v1.18.2 github.com/containerd/containerd v1.7.28 github.com/containers/image/v5 v5.36.0 + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc github.com/fsnotify/fsnotify v1.9.0 github.com/go-logr/logr v1.4.3 github.com/golang-jwt/jwt/v5 v5.3.0 @@ -41,6 +42,7 @@ require ( k8s.io/klog/v2 v2.130.1 k8s.io/kubernetes v1.33.2 k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 + pkg.package-operator.run/boxcutter v0.1.2 sigs.k8s.io/controller-runtime v0.21.0 sigs.k8s.io/controller-tools v0.18.0 sigs.k8s.io/crdify v0.4.1-0.20250613143457-398e4483fb58 @@ -89,7 +91,6 @@ require ( github.com/containers/storage v1.59.0 // indirect github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467 // indirect github.com/cyphar/filepath-securejoin v0.4.1 // indirect - github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/distribution/reference v0.6.0 // indirect github.com/docker/cli v28.3.2+incompatible // indirect github.com/docker/distribution v2.8.3+incompatible // indirect @@ -217,7 +218,7 @@ require ( go.opentelemetry.io/otel/trace v1.36.0 // indirect go.opentelemetry.io/proto/otlp v1.7.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect - go.yaml.in/yaml/v3 v3.0.3 // indirect + go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/crypto v0.40.0 // indirect golang.org/x/net v0.42.0 // indirect golang.org/x/oauth2 v0.30.0 // indirect @@ -226,7 +227,7 @@ require ( golang.org/x/text v0.27.0 // indirect golang.org/x/time v0.12.0 // indirect golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated // indirect - gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect + gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect google.golang.org/genproto v0.0.0-20250603155806-513f23925822 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250603155806-513f23925822 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250603155806-513f23925822 // indirect diff --git a/go.sum b/go.sum index 7479d509b..8f3d61d6e 100644 --- a/go.sum +++ b/go.sum @@ -380,6 +380,8 @@ github.com/otiai10/copy v1.14.1 h1:5/7E6qsUMBaH5AnQ0sSLzzTg1oTECmcCmT6lvF45Na8= github.com/otiai10/copy v1.14.1/go.mod h1:oQwrEDDOci3IM8dJF0d8+jnbfPDllW6vUjNc3DoZm9I= github.com/otiai10/mint v1.6.3 h1:87qsV/aw1F5as1eH1zS/yqHY85ANKVMgkDrf9rcxbQs= github.com/otiai10/mint v1.6.3/go.mod h1:MJm72SBthJjz8qhefc4z1PYEieWmy8Bku7CjcAqyUSM= +github.com/perdasilva/boxcutter v0.0.0-20250715101157-18ea858f54bd h1:CEXvUTPMkCMX+0q9cqwaFAl51s71qESI1V2wRK/8xsg= +github.com/perdasilva/boxcutter v0.0.0-20250715101157-18ea858f54bd/go.mod h1:74MfxxOWGkveUl9Iv2/01tg/IULlWnHEaVv2dfR4/b8= github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 h1:Ii+DKncOVM8Cu1Hc+ETb5K+23HdAMvESYE3ZJ5b5cMI= @@ -549,8 +551,8 @@ go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI= go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU= -go.yaml.in/yaml/v3 v3.0.3 h1:bXOww4E/J3f66rav3pX3m8w6jDE4knZjGOw8b5Y6iNE= -go.yaml.in/yaml/v3 v3.0.3/go.mod h1:tBHosrYAkRZjRAOREWbDnBXUf08JOwYq++0QNwQiWzI= +go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= +go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= @@ -676,8 +678,8 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw= -gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= +gomodules.xyz/jsonpatch/v2 v2.5.0 h1:JELs8RLM12qJGXU4u/TO3V25KW8GreMKl9pdkk14RM0= +gomodules.xyz/jsonpatch/v2 v2.5.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= From 63224c10c02a45a7767b693a60f84e9b94b7ce02 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:08:49 +0200 Subject: [PATCH 03/17] Add ClusterExtensionRevisionAPI Signed-off-by: Per Goncalves da Silva --- api/v1/clusterextensionrevision_types.go | 123 +++++++++++++++++ api/v1/zz_generated.deepcopy.go | 166 +++++++++++++++++++++++ 2 files changed, 289 insertions(+) create mode 100644 api/v1/clusterextensionrevision_types.go diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go new file mode 100644 index 000000000..66382730c --- /dev/null +++ b/api/v1/clusterextensionrevision_types.go @@ -0,0 +1,123 @@ +/* +Copyright 2024. + +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. +*/ + +package v1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" +) + +const ClusterExtensionRevisionKind = "ClusterExtensionRevision" + +// ClusterExtensionRevisionSpec defines the desired state of ClusterExtensionRevision. +type ClusterExtensionRevisionSpec struct { + // Specifies the lifecycle state of the ClusterExtensionRevision. + // +kubebuilder:default="Active" + // +kubebuilder:validation:Enum=Active;Paused;Archived + // +kubebuilder:validation:XValidation:rule="oldSelf == 'Active' || oldSelf == 'Paused' || oldSelf == 'Archived' && oldSelf == self", message="can not un-archive" + LifecycleState ClusterExtensionRevisionLifecycleState `json:"lifecycleState,omitempty"` + // +kubebuilder:validation:Required + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="revision is immutable" + Revision int64 `json:"revision"` + // +kubebuilder:validation:Required + // +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf.size() == 0", message="phases is immutable" + Phases []ClusterExtensionRevisionPhase `json:"phases"` + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="previous is immutable" + Previous []ClusterExtensionRevisionPrevious `json:"previous,omitempty"` +} + +// ClusterExtensionRevisionLifecycleState specifies the lifecycle state of the ClusterExtensionRevision. +type ClusterExtensionRevisionLifecycleState string + +const ( + // ClusterExtensionRevisionLifecycleStateActive / "Active" is the default lifecycle state. + ClusterExtensionRevisionLifecycleStateActive ClusterExtensionRevisionLifecycleState = "Active" + // ClusterExtensionRevisionLifecycleStatePaused / "Paused" disables reconciliation of the ClusterExtensionRevision. + // Only Status updates will still propagated, but object changes will not be reconciled. + ClusterExtensionRevisionLifecycleStatePaused ClusterExtensionRevisionLifecycleState = "Paused" + // ClusterExtensionRevisionLifecycleStateArchived / "Archived" disables reconciliation while also "scaling to zero", + // which deletes all objects that are not excluded via the pausedFor property and + // removes itself from the owner list of all other objects previously under management. + ClusterExtensionRevisionLifecycleStateArchived ClusterExtensionRevisionLifecycleState = "Archived" +) + +type ClusterExtensionRevisionPhase struct { + Name string `json:"name"` + Objects []ClusterExtensionRevisionObject `json:"objects"` + Slices []string `json:"slices,omitempty"` +} + +type ClusterExtensionRevisionObject struct { + // +kubebuilder:validation:EmbeddedResource + // +kubebuilder:pruning:PreserveUnknownFields + Object unstructured.Unstructured `json:"object"` +} + +type ClusterExtensionRevisionPrevious struct { + // +kubebuilder:validation:Required + Name string `json:"name"` + // +kubebuilder:validation:Required + UID types.UID `json:"uid"` +} + +// ClusterExtensionRevisionStatus defines the observed state of a ClusterExtensionRevision. +type ClusterExtensionRevisionStatus struct { + // +patchMergeKey=type + // +patchStrategy=merge + // +listType=map + // +listMapKey=type + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:resource:scope=Cluster +// +kubebuilder:subresource:status + +// ClusterExtensionRevision is the Schema for the clusterextensionrevisions API +type ClusterExtensionRevision struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // spec is an optional field that defines the desired state of the ClusterExtension. + // +optional + Spec ClusterExtensionRevisionSpec `json:"spec,omitempty"` + + // status is an optional field that defines the observed state of the ClusterExtension. + // +optional + Status ClusterExtensionRevisionStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// ClusterExtensionRevisionList contains a list of ClusterExtensionRevision +type ClusterExtensionRevisionList struct { + metav1.TypeMeta `json:",inline"` + + // +optional + metav1.ListMeta `json:"metadata,omitempty"` + + // items is a required list of ClusterExtensionRevision objects. + // + // +kubebuilder:validation:Required + Items []ClusterExtensionRevision `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ClusterExtensionRevision{}, &ClusterExtensionRevisionList{}) +} diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 23fcf7d85..94d6e1dc7 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -320,6 +320,172 @@ func (in *ClusterExtensionList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterExtensionRevision) DeepCopyInto(out *ClusterExtensionRevision) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionRevision. +func (in *ClusterExtensionRevision) DeepCopy() *ClusterExtensionRevision { + if in == nil { + return nil + } + out := new(ClusterExtensionRevision) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterExtensionRevision) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterExtensionRevisionList) DeepCopyInto(out *ClusterExtensionRevisionList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ClusterExtensionRevision, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionRevisionList. +func (in *ClusterExtensionRevisionList) DeepCopy() *ClusterExtensionRevisionList { + if in == nil { + return nil + } + out := new(ClusterExtensionRevisionList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterExtensionRevisionList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterExtensionRevisionObject) DeepCopyInto(out *ClusterExtensionRevisionObject) { + *out = *in + in.Object.DeepCopyInto(&out.Object) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionRevisionObject. +func (in *ClusterExtensionRevisionObject) DeepCopy() *ClusterExtensionRevisionObject { + if in == nil { + return nil + } + out := new(ClusterExtensionRevisionObject) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterExtensionRevisionPhase) DeepCopyInto(out *ClusterExtensionRevisionPhase) { + *out = *in + if in.Objects != nil { + in, out := &in.Objects, &out.Objects + *out = make([]ClusterExtensionRevisionObject, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Slices != nil { + in, out := &in.Slices, &out.Slices + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionRevisionPhase. +func (in *ClusterExtensionRevisionPhase) DeepCopy() *ClusterExtensionRevisionPhase { + if in == nil { + return nil + } + out := new(ClusterExtensionRevisionPhase) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterExtensionRevisionPrevious) DeepCopyInto(out *ClusterExtensionRevisionPrevious) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionRevisionPrevious. +func (in *ClusterExtensionRevisionPrevious) DeepCopy() *ClusterExtensionRevisionPrevious { + if in == nil { + return nil + } + out := new(ClusterExtensionRevisionPrevious) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterExtensionRevisionSpec) DeepCopyInto(out *ClusterExtensionRevisionSpec) { + *out = *in + if in.Phases != nil { + in, out := &in.Phases, &out.Phases + *out = make([]ClusterExtensionRevisionPhase, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Previous != nil { + in, out := &in.Previous, &out.Previous + *out = make([]ClusterExtensionRevisionPrevious, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionRevisionSpec. +func (in *ClusterExtensionRevisionSpec) DeepCopy() *ClusterExtensionRevisionSpec { + if in == nil { + return nil + } + out := new(ClusterExtensionRevisionSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterExtensionRevisionStatus) DeepCopyInto(out *ClusterExtensionRevisionStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionRevisionStatus. +func (in *ClusterExtensionRevisionStatus) DeepCopy() *ClusterExtensionRevisionStatus { + if in == nil { + return nil + } + out := new(ClusterExtensionRevisionStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterExtensionSpec) DeepCopyInto(out *ClusterExtensionSpec) { *out = *in From 6430597a0a8c5007f9da79325eab7fe6097dc328 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:09:20 +0200 Subject: [PATCH 04/17] Add BoxcutterRuntime featuregate Signed-off-by: Per Goncalves da Silva --- .../features/boxcutter-runtime/kustomization.yaml | 9 +++++++++ .../boxcutter-runtime/patches/enable-featuregate.yaml | 4 ++++ internal/operator-controller/features/features.go | 8 ++++++++ 3 files changed, 21 insertions(+) create mode 100644 config/components/features/boxcutter-runtime/kustomization.yaml create mode 100644 config/components/features/boxcutter-runtime/patches/enable-featuregate.yaml diff --git a/config/components/features/boxcutter-runtime/kustomization.yaml b/config/components/features/boxcutter-runtime/kustomization.yaml new file mode 100644 index 000000000..d075a1121 --- /dev/null +++ b/config/components/features/boxcutter-runtime/kustomization.yaml @@ -0,0 +1,9 @@ +# DO NOT ADD A NAMESPACE HERE +--- +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component +patches: + - target: + kind: Deployment + name: operator-controller-controller-manager + path: patches/enable-featuregate.yaml diff --git a/config/components/features/boxcutter-runtime/patches/enable-featuregate.yaml b/config/components/features/boxcutter-runtime/patches/enable-featuregate.yaml new file mode 100644 index 000000000..97f8b89be --- /dev/null +++ b/config/components/features/boxcutter-runtime/patches/enable-featuregate.yaml @@ -0,0 +1,4 @@ +# enable Boxcutter runtime feature gate +- op: add + path: /spec/template/spec/containers/0/args/- + value: "--feature-gates=BoxcutterRuntime=true" diff --git a/internal/operator-controller/features/features.go b/internal/operator-controller/features/features.go index 41bad3cf7..1abdf0a18 100644 --- a/internal/operator-controller/features/features.go +++ b/internal/operator-controller/features/features.go @@ -17,6 +17,7 @@ const ( WebhookProviderCertManager featuregate.Feature = "WebhookProviderCertManager" WebhookProviderOpenshiftServiceCA featuregate.Feature = "WebhookProviderOpenshiftServiceCA" HelmChartSupport featuregate.Feature = "HelmChartSupport" + BoxcutterRuntime featuregate.Feature = "BoxcutterRuntime" ) var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -72,6 +73,13 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature PreRelease: featuregate.Alpha, LockToDefault: false, }, + + // BoxcutterRuntime configures OLM to use the Boxcutter runtime for extension lifecycling + BoxcutterRuntime: { + Default: false, + PreRelease: featuregate.Alpha, + LockToDefault: false, + }, } var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate() From ae24b48e469ff8bfa32af86049965446cc272710 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:09:50 +0200 Subject: [PATCH 05/17] Add Boxcutter applier Signed-off-by: Per Goncalves da Silva --- .../operator-controller/applier/boxcutter.go | 239 ++++++++ .../applier/boxcutter_test.go | 536 ++++++++++++++++++ .../rukpak/bundle/source/source_test.go | 48 +- .../rukpak/util/testing/bundlefs.go | 64 +++ 4 files changed, 845 insertions(+), 42 deletions(-) create mode 100644 internal/operator-controller/applier/boxcutter.go create mode 100644 internal/operator-controller/applier/boxcutter_test.go create mode 100644 internal/operator-controller/rukpak/util/testing/bundlefs.go diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go new file mode 100644 index 000000000..fe9461b6c --- /dev/null +++ b/internal/operator-controller/applier/boxcutter.go @@ -0,0 +1,239 @@ +package applier + +import ( + "cmp" + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "hash" + "io/fs" + "maps" + "slices" + + "github.com/davecgh/go-spew/spew" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + 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/rukpak/bundle/source" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" +) + +const ( + RevisionHashAnnotation = "olm.operatorframework.io/hash" +) + +type ClusterExtensionRevisionGenerator interface { + GenerateRevision(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1.ClusterExtensionRevision, error) +} + +type SimpleRevisionGenerator struct { + Scheme *runtime.Scheme + BundleRenderer BundleRenderer +} + +func (r *SimpleRevisionGenerator) GenerateRevision(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1.ClusterExtensionRevision, error) { + // extract plain manifests + plain, err := r.BundleRenderer.Render(bundleFS, ext) + if err != nil { + return nil, err + } + + // objectLabels + objs := make([]ocv1.ClusterExtensionRevisionObject, 0, len(plain)) + for _, obj := range plain { + if len(obj.GetLabels()) > 0 { + labels := maps.Clone(obj.GetLabels()) + if labels == nil { + labels = map[string]string{} + } + maps.Copy(labels, objectLabels) + obj.SetLabels(labels) + } + + gvk, err := apiutil.GVKForObject(obj, r.Scheme) + if err != nil { + return nil, err + } + + unstrObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + if err != nil { + return nil, err + } + unstr := unstructured.Unstructured{Object: unstrObj} + unstr.SetGroupVersionKind(gvk) + + objs = append(objs, ocv1.ClusterExtensionRevisionObject{ + Object: unstr, + }) + } + + // Build desired revision + return &ocv1.ClusterExtensionRevision{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + Labels: map[string]string{ + controllers.ClusterExtensionRevisionOwnerLabel: ext.Name, + }, + }, + Spec: ocv1.ClusterExtensionRevisionSpec{ + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "everything", + Objects: objs, + }, + }, + }, + }, nil +} + +type Boxcutter struct { + Client client.Client + Scheme *runtime.Scheme + RevisionGenerator ClusterExtensionRevisionGenerator +} + +func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, storageLabels map[string]string) ([]client.Object, string, error) { + objs, err := bc.apply(ctx, contentFS, ext, objectLabels, storageLabels) + return objs, "", err +} + +func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, _ map[string]string) ([]client.Object, error) { + // Generate desired revision + desiredRevision, err := bc.RevisionGenerator.GenerateRevision(contentFS, ext, objectLabels) + if err != nil { + return nil, err + } + + // List all existing revisions + existingRevisions, err := bc.getExistingRevisions(ctx, ext.GetName()) + if err != nil { + return nil, err + } + desiredHash := computeSHA256Hash(desiredRevision.Spec.Phases) + + // Sort into current and previous revisions. + var ( + currentRevision *ocv1.ClusterExtensionRevision + ) + if len(existingRevisions) > 0 { + maybeCurrentRevision := existingRevisions[len(existingRevisions)-1] + annotations := maybeCurrentRevision.GetAnnotations() + if annotations != nil { + if revisionHash, ok := annotations[RevisionHashAnnotation]; ok && revisionHash == desiredHash { + currentRevision = &maybeCurrentRevision + } + } + } + + if currentRevision == nil { + // all Revisions are outdated => create a new one. + prevRevisions := existingRevisions + revisionNumber := latestRevisionNumber(prevRevisions) + 1 + + newRevision := desiredRevision + newRevision.Name = fmt.Sprintf("%s-%d", ext.Name, revisionNumber) + if newRevision.GetAnnotations() == nil { + newRevision.Annotations = map[string]string{} + } + newRevision.Annotations[RevisionHashAnnotation] = desiredHash + newRevision.Spec.Revision = revisionNumber + for _, prevRevision := range prevRevisions { + newRevision.Spec.Previous = append(newRevision.Spec.Previous, ocv1.ClusterExtensionRevisionPrevious{ + Name: prevRevision.Name, + UID: prevRevision.UID, + }) + } + + if err := controllerutil.SetControllerReference(ext, newRevision, bc.Scheme); err != nil { + return nil, fmt.Errorf("set ownerref: %w", err) + } + if err := bc.Client.Create(ctx, newRevision); err != nil { + return nil, fmt.Errorf("creating new Revision: %w", err) + } + } + + // TODO: Delete archived previous revisions over a certain revision limit + + // TODO: Read status from revision. + + // Collect objects + var plain []client.Object + for _, phase := range desiredRevision.Spec.Phases { + for _, phaseObject := range phase.Objects { + plain = append(plain, &phaseObject.Object) + } + } + return plain, nil +} + +// getExistingRevisions returns the list of ClusterExtensionRevisions for a ClusterExtension with name extName in revision order (oldest to newest) +func (bc *Boxcutter) getExistingRevisions(ctx context.Context, extName string) ([]ocv1.ClusterExtensionRevision, error) { + existingRevisionList := &ocv1.ClusterExtensionRevisionList{} + if err := bc.Client.List(ctx, existingRevisionList, client.MatchingLabels{ + controllers.ClusterExtensionRevisionOwnerLabel: extName, + }); err != nil { + return nil, fmt.Errorf("listing revisions: %w", err) + } + slices.SortFunc(existingRevisionList.Items, func(a, b ocv1.ClusterExtensionRevision) int { + return cmp.Compare(a.Spec.Revision, b.Spec.Revision) + }) + return existingRevisionList.Items, nil +} + +// computeSHA256Hash returns a sha236 hash value calculated from object. +func computeSHA256Hash(obj any) string { + hasher := sha256.New() + deepHashObject(hasher, obj) + return hex.EncodeToString(hasher.Sum(nil)) +} + +// deepHashObject writes specified object to hash using the spew library +// which follows pointers and prints actual values of the nested objects +// ensuring the hash does not change when a pointer changes. +func deepHashObject(hasher hash.Hash, objectToWrite any) { + hasher.Reset() + + printer := spew.ConfigState{ + Indent: " ", + SortKeys: true, + DisableMethods: true, + SpewKeys: true, + } + if _, err := printer.Fprintf(hasher, "%#v", objectToWrite); err != nil { + panic(err) + } +} + +func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 { + if len(prevRevisions) == 0 { + return 0 + } + return prevRevisions[len(prevRevisions)-1].Spec.Revision +} + +type BundleRenderer interface { + Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) +} + +type RegistryV1BundleRenderer struct { + BundleRenderer render.BundleRenderer +} + +func (r *RegistryV1BundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { + reg, err := source.FromFS(bundleFS).GetBundle() + if err != nil { + return nil, err + } + watchNamespace, err := GetWatchNamespace(ext) + if err != nil { + return nil, err + } + return r.BundleRenderer.Render(reg, ext.Spec.Namespace, render.WithTargetNamespaces(watchNamespace)) +} diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go new file mode 100644 index 000000000..0e237cbd6 --- /dev/null +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -0,0 +1,536 @@ +package applier_test + +import ( + "context" + "errors" + "fmt" + "io/fs" + "testing" + "testing/fstest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + 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" + k8scheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "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/applier" + "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + testutils "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" +) + +func Test_RegistryV1BundleRenderer_Render_Success(t *testing.T) { + expectedObjs := []client.Object{ + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + }, + }, + } + r := applier.RegistryV1BundleRenderer{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + require.Equal(t, []string{""}, opts.TargetNamespaces) + require.Equal(t, "some-namespace", opts.InstallNamespace) + return expectedObjs, nil + }, + }, + }, + } + bundleFS := testutils.NewBundleFS() + + objs, err := r.Render(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "some-namespace", + }, + }) + require.NoError(t, err) + require.Equal(t, expectedObjs, objs) +} + +func Test_RegistryV1BundleRenderer_Render_Failure(t *testing.T) { + var expectedObjs []client.Object + r := applier.RegistryV1BundleRenderer{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + return expectedObjs, fmt.Errorf("some-error") + }, + }, + }, + } + bundleFS := testutils.NewBundleFS() + + objs, err := r.Render(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "some-namespace", + }, + }) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "some-error") +} + +func Test_SimpleRevisionGenerator_Success(t *testing.T) { + var r mockBundleRenderer = func(_ fs.FS, _ *ocv1.ClusterExtension) ([]client.Object, error) { + return []client.Object{ + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + }, + }, + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + }, + }, + }, nil + } + + b := applier.SimpleRevisionGenerator{ + Scheme: k8scheme.Scheme, + BundleRenderer: r, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-extension", + }, + } + + rev, err := b.GenerateRevision(fstest.MapFS{}, ext, map[string]string{}) + require.NoError(t, err) + + t.Log("by checking the olm.operatorframework.io/owner label is set to the name of the ClusterExtension") + require.Equal(t, map[string]string{ + controllers.ClusterExtensionRevisionOwnerLabel: "test-extension", + }, rev.Labels) + t.Log("by checking there are no annotations") + require.Empty(t, rev.Annotations) + t.Log("by checking the revision number is 0") + require.Equal(t, int64(0), rev.Spec.Revision) + t.Log("by checking the rendered objects are present in the correct phases") + require.Equal(t, []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "everything", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "creationTimestamp": nil, + "name": "test-service", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{ + "loadBalancer": map[string]interface{}{}, + }, + }, + }, + }, + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "creationTimestamp": nil, + "name": "test-deployment", + }, + "spec": map[string]interface{}{ + "selector": nil, + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "creationTimestamp": nil, + }, + "spec": map[string]interface{}{ + "containers": nil, + }, + }, + "strategy": map[string]interface{}{}, + }, + "status": map[string]interface{}{}, + }, + }, + }, + }, + }, + }, rev.Spec.Phases) +} + +func Test_SimpleRevisionGenerator_Renderer_Integration(t *testing.T) { + bundleFS := fstest.MapFS{} + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-extension", + }, + } + var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { + t.Log("by checking renderer was called with the correct parameters") + require.Equal(t, bundleFS, b) + require.Equal(t, ext, e) + return nil, nil + } + b := applier.SimpleRevisionGenerator{ + Scheme: k8scheme.Scheme, + BundleRenderer: r, + } + + _, err := b.GenerateRevision(bundleFS, ext, map[string]string{}) + require.NoError(t, err) +} + +func Test_SimpleRevisionGenerator_AppliesObjectLabels(t *testing.T) { + renderedObjs := []client.Object{ + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + Labels: map[string]string{ + "app": "test-obj", + }, + }, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Labels: map[string]string{ + "app": "test-obj", + }, + }, + }, + } + var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { + return renderedObjs, nil + } + b := applier.SimpleRevisionGenerator{ + Scheme: k8scheme.Scheme, + BundleRenderer: r, + } + + rev, err := b.GenerateRevision(fstest.MapFS{}, &ocv1.ClusterExtension{}, map[string]string{ + "some": "value", + }) + require.NoError(t, err) + t.Log("by checking the rendered objects contain the given object labels") + for _, phase := range rev.Spec.Phases { + for _, revObj := range phase.Objects { + require.Equal(t, map[string]string{ + "app": "test-obj", + "some": "value", + }, revObj.Object.GetLabels()) + } + } +} + +func Test_SimpleRevisionGenerator_Failure(t *testing.T) { + var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { + return nil, fmt.Errorf("some-error") + } + b := applier.SimpleRevisionGenerator{ + Scheme: k8scheme.Scheme, + BundleRenderer: r, + } + + rev, err := b.GenerateRevision(fstest.MapFS{}, &ocv1.ClusterExtension{}, map[string]string{}) + require.Nil(t, rev) + t.Log("by checking rendering errors are propagated") + require.Error(t, err) + require.Contains(t, err.Error(), "some-error") +} + +func TestBoxcutter_Apply(t *testing.T) { + testScheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(testScheme)) + + // This is the revision that the mock builder will produce by default. + // We calculate its hash to use in the tests. + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ext", + UID: "test-uid", + }, + } + defaultDesiredHash := "2a3d3548913494df7d4cbaef51cb6c36f6f67399cbfe2dc6a3cc49e4db0083ae" + defaultDesiredRevision := &ocv1.ClusterExtensionRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ext-1", + UID: "rev-uid-1", + Annotations: map[string]string{ + applier.RevisionHashAnnotation: defaultDesiredHash, + }, + Labels: map[string]string{ + controllers.ClusterExtensionRevisionOwnerLabel: ext.Name, + }, + }, + Spec: ocv1.ClusterExtensionRevisionSpec{ + Revision: 1, + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "everything", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + }, + }, + }, + }, + }, + }, + }, + }, + } + + testCases := []struct { + name string + mockBuilder applier.ClusterExtensionRevisionGenerator + existingObjs []client.Object + expectedErr string + validate func(t *testing.T, c client.Client) + expectedObjectsInPhase int + }{ + { + name: "first revision", + mockBuilder: &mockBundleRevisionBuilder{ + makeRevisionFunc: func(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1.ClusterExtensionRevision, error) { + return &ocv1.ClusterExtensionRevision{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + Labels: map[string]string{ + controllers.ClusterExtensionRevisionOwnerLabel: ext.Name, + }, + }, + Spec: ocv1.ClusterExtensionRevisionSpec{ + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "everything", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + }, + }, + }, + }, + }, + }, + }, + }, + }, nil + }, + }, + validate: func(t *testing.T, c client.Client) { + revList := &ocv1.ClusterExtensionRevisionList{} + err := c.List(t.Context(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name}) + require.NoError(t, err) + require.Len(t, revList.Items, 1) + + rev := revList.Items[0] + assert.Equal(t, "test-ext-1", rev.Name) + assert.Equal(t, int64(1), rev.Spec.Revision) + assert.Equal(t, defaultDesiredHash, rev.Annotations[applier.RevisionHashAnnotation]) + assert.Len(t, rev.OwnerReferences, 1) + assert.Equal(t, ext.Name, rev.OwnerReferences[0].Name) + assert.Equal(t, ext.UID, rev.OwnerReferences[0].UID) + }, + expectedObjectsInPhase: 1, + }, + { + name: "no change, revision exists", + mockBuilder: &mockBundleRevisionBuilder{ + makeRevisionFunc: func(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1.ClusterExtensionRevision, error) { + return &ocv1.ClusterExtensionRevision{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + Labels: map[string]string{ + controllers.ClusterExtensionRevisionOwnerLabel: ext.Name, + }, + }, + Spec: ocv1.ClusterExtensionRevisionSpec{ + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "everything", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + }, + }, + }, + }, + }, + }, + }, + }, + }, nil + }, + }, + existingObjs: []client.Object{ + defaultDesiredRevision, + }, + validate: func(t *testing.T, c client.Client) { + revList := &ocv1.ClusterExtensionRevisionList{} + err := c.List(context.Background(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name}) + require.NoError(t, err) + // No new revision should be created + require.Len(t, revList.Items, 1) + assert.Equal(t, "test-ext-1", revList.Items[0].Name) + }, + expectedObjectsInPhase: 1, + }, + { + name: "new revision created when hash differs", + mockBuilder: &mockBundleRevisionBuilder{ + makeRevisionFunc: func(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1.ClusterExtensionRevision, error) { + return &ocv1.ClusterExtensionRevision{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + Labels: map[string]string{ + controllers.ClusterExtensionRevisionOwnerLabel: ext.Name, + }, + }, + Spec: ocv1.ClusterExtensionRevisionSpec{ + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "everything", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "new-secret", + }, + }, + }, + }, + }, + }, + }, + }, + }, nil + }, + }, + existingObjs: []client.Object{ + defaultDesiredRevision, + }, + validate: func(t *testing.T, c client.Client) { + revList := &ocv1.ClusterExtensionRevisionList{} + err := c.List(context.Background(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name}) + require.NoError(t, err) + require.Len(t, revList.Items, 2) + + // Find the new revision (rev 2) + var newRev ocv1.ClusterExtensionRevision + for _, r := range revList.Items { + if r.Spec.Revision == 2 { + newRev = r + break + } + } + require.NotNil(t, newRev) + + assert.Equal(t, "test-ext-2", newRev.Name) + assert.Equal(t, int64(2), newRev.Spec.Revision) + assert.Equal(t, "bc1c7457a476193460747e8223fff9b492f0a2f60057831fb55a88ec8c2387b2", newRev.Annotations[applier.RevisionHashAnnotation]) + require.Len(t, newRev.Spec.Previous, 1) + assert.Equal(t, "test-ext-1", newRev.Spec.Previous[0].Name) + assert.Equal(t, types.UID("rev-uid-1"), newRev.Spec.Previous[0].UID) + }, + expectedObjectsInPhase: 1, + }, + { + name: "error from GenerateRevision", + mockBuilder: &mockBundleRevisionBuilder{ + makeRevisionFunc: func(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1.ClusterExtensionRevision, error) { + return nil, errors.New("render boom") + }, + }, + expectedErr: "render boom", + validate: func(t *testing.T, c client.Client) { + // Ensure no revisions were created + revList := &ocv1.ClusterExtensionRevisionList{} + err := c.List(context.Background(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name}) + require.NoError(t, err) + assert.Empty(t, revList.Items) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup + fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(tc.existingObjs...).Build() + + boxcutter := &applier.Boxcutter{ + Client: fakeClient, + Scheme: testScheme, + RevisionGenerator: tc.mockBuilder, + } + + // We need a dummy fs.FS + testFS := fstest.MapFS{} + + // Execute + objs, _, err := boxcutter.Apply(t.Context(), testFS, ext, nil, nil) + + // Assert + if tc.expectedErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErr) + } else { + require.NoError(t, err) + assert.Len(t, objs, tc.expectedObjectsInPhase) + } + + if tc.validate != nil { + // For the client create error, we need a client that *will* error. + // Since we can't do that easily, we will skip validation for that specific path + // as the state won't be what we expect. + if tc.name != "error from client create" { + tc.validate(t, fakeClient) + } + } + }) + } +} + +// mockBundleRevisionBuilder is a mock implementation of the ClusterExtensionRevisionGenerator for testing. +type mockBundleRevisionBuilder struct { + makeRevisionFunc func(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1.ClusterExtensionRevision, error) +} + +func (m *mockBundleRevisionBuilder) GenerateRevision(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1.ClusterExtensionRevision, error) { + return m.makeRevisionFunc(bundleFS, ext, objectLabels) +} + +type mockBundleRenderer func(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) + +func (f mockBundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { + return f(bundleFS, ext) +} diff --git a/internal/operator-controller/rukpak/bundle/source/source_test.go b/internal/operator-controller/rukpak/bundle/source/source_test.go index 2ee948638..cf7b1cb90 100644 --- a/internal/operator-controller/rukpak/bundle/source/source_test.go +++ b/internal/operator-controller/rukpak/bundle/source/source_test.go @@ -10,14 +10,11 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" ) const ( olmProperties = "olm.properties" - - bundlePathAnnotations = "metadata/annotations.yaml" - bundlePathProperties = "metadata/properties.yaml" - bundlePathCSV = "manifests/csv.yaml" ) func Test_FromBundle_Success(t *testing.T) { @@ -30,7 +27,7 @@ func Test_FromBundle_Success(t *testing.T) { } func Test_FromFS_Success(t *testing.T) { - rv1, err := source.FromFS(newBundleFS()).GetBundle() + rv1, err := source.FromFS(NewBundleFS()).GetBundle() require.NoError(t, err) t.Log("Check package name is correctly taken from metadata/annotations.yaml") @@ -47,16 +44,16 @@ func Test_FromFS_Fails(t *testing.T) { }{ { name: "bundle missing ClusterServiceVersion manifest", - FS: removePaths(newBundleFS(), bundlePathCSV), + FS: removePaths(NewBundleFS(), BundlePathCSV), }, { name: "bundle missing metadata/annotations.yaml", - FS: removePaths(newBundleFS(), bundlePathAnnotations), + FS: removePaths(NewBundleFS(), BundlePathAnnotations), }, { name: "bundle missing metadata/ directory", - FS: removePaths(newBundleFS(), "metadata/"), + FS: removePaths(NewBundleFS(), "metadata/"), }, { name: "bundle missing manifests/ directory", - FS: removePaths(newBundleFS(), "manifests/"), + FS: removePaths(NewBundleFS(), "manifests/"), }, } { t.Run(tt.name, func(t *testing.T) { @@ -66,39 +63,6 @@ func Test_FromFS_Fails(t *testing.T) { } } -func newBundleFS() fstest.MapFS { - annotationsYml := ` -annotations: - operators.operatorframework.io.bundle.mediatype.v1: registry+v1 - operators.operatorframework.io.bundle.package.v1: test -` - - propertiesYml := ` -properties: - - type: "from-file-key" - value: "from-file-value" -` - - csvYml := ` -apiVersion: operators.operatorframework.io/v1alpha1 -kind: ClusterServiceVersion -metadata: - name: test.v1.0.0 - annotations: - olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]' -spec: - installModes: - - type: AllNamespaces - supported: true -` - - return fstest.MapFS{ - bundlePathAnnotations: &fstest.MapFile{Data: []byte(strings.Trim(annotationsYml, "\n"))}, - bundlePathProperties: &fstest.MapFile{Data: []byte(strings.Trim(propertiesYml, "\n"))}, - bundlePathCSV: &fstest.MapFile{Data: []byte(strings.Trim(csvYml, "\n"))}, - } -} - func removePaths(mapFs fstest.MapFS, paths ...string) fstest.MapFS { for k := range mapFs { for _, path := range paths { diff --git a/internal/operator-controller/rukpak/util/testing/bundlefs.go b/internal/operator-controller/rukpak/util/testing/bundlefs.go new file mode 100644 index 000000000..6c5f58837 --- /dev/null +++ b/internal/operator-controller/rukpak/util/testing/bundlefs.go @@ -0,0 +1,64 @@ +package testing + +import ( + "fmt" + "path/filepath" + "strings" + "testing/fstest" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" +) + +const ( + BundlePathAnnotations = "metadata/annotations.yaml" + BundlePathProperties = "metadata/properties.yaml" + BundlePathManifests = "manifests" + BundlePathCSV = BundlePathManifests + "/csv.yaml" +) + +func NewBundleFS() fstest.MapFS { + annotationsYml := ` +annotations: + operators.operatorframework.io.bundle.mediatype.v1: registry+v1 + operators.operatorframework.io.bundle.package.v1: test +` + + propertiesYml := ` +properties: + - type: "from-file-key" + value: "from-file-value" +` + + csvYml := ` +apiVersion: operators.operatorframework.io/v1alpha1 +kind: ClusterServiceVersion +metadata: + name: test.v1.0.0 + annotations: + olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]' +spec: + installModes: + - type: AllNamespaces + supported: true +` + + return fstest.MapFS{ + BundlePathAnnotations: &fstest.MapFile{Data: []byte(strings.Trim(annotationsYml, "\n"))}, + BundlePathProperties: &fstest.MapFile{Data: []byte(strings.Trim(propertiesYml, "\n"))}, + BundlePathCSV: &fstest.MapFile{Data: []byte(strings.Trim(csvYml, "\n"))}, + } +} + +func AddManifest(bundleFS fstest.MapFS, obj client.Object) error { + gvk := obj.GetObjectKind().GroupVersionKind() + manifestName := fmt.Sprintf("%s%s_%s_%s%s.yaml", gvk.Group, gvk.Version, gvk.Kind, obj.GetNamespace(), obj.GetName()) + bytes, err := yaml.Marshal(obj) + if err != nil { + return err + } + bundleFS[filepath.Join(BundlePathManifests, manifestName)] = &fstest.MapFile{ + Data: bytes, + } + return nil +} From 9ae89ac3f2c1f6cad4fad0076906221cbcb11aa6 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:11:52 +0200 Subject: [PATCH 06/17] Add ClusterExtensionRevision controller Signed-off-by: Per Goncalves da Silva --- .../clusterextensionrevision_controller.go | 442 +++++++ ...lusterextensionrevision_controller_test.go | 1021 +++++++++++++++++ .../controllers/suite_test.go | 12 +- 3 files changed, 1471 insertions(+), 4 deletions(-) create mode 100644 internal/operator-controller/controllers/clusterextensionrevision_controller.go create mode 100644 internal/operator-controller/controllers/clusterextensionrevision_controller_test.go diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go new file mode 100644 index 000000000..a6fa2ac16 --- /dev/null +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -0,0 +1,442 @@ +//go:build !standard + +package controllers + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "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/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/discovery" + "k8s.io/utils/ptr" + "pkg.package-operator.run/boxcutter" + "pkg.package-operator.run/boxcutter/machinery" + machinerytypes "pkg.package-operator.run/boxcutter/machinery/types" + "pkg.package-operator.run/boxcutter/managedcache" + "pkg.package-operator.run/boxcutter/ownerhandling" + "pkg.package-operator.run/boxcutter/validation" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/source" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" +) + +const ( + ClusterExtensionRevisionOwnerLabel = "olm.operatorframework.io/owner" + boxcutterSystemPrefixFieldOwner = "olm.operatorframework.io" + clusterExtensionRevisionTeardownFinalizer = "olm.operatorframework.io/teardown" +) + +// ClusterExtensionRevisionReconciler actions individual snapshots of ClusterExtensions, +// as part of the boxcutter integration. +type ClusterExtensionRevisionReconciler struct { + Client client.Client + RevisionManager RevisionManager +} + +type AccessManager interface { + GetWithUser(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) + FreeWithUser(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error + Source(handler.EventHandler, ...predicate.Predicate) source.Source +} + +type RevisionEngine interface { + Teardown(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) + Reconcile(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) +} + +type RevisionManager interface { + GetScopedRevisionEngine(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (RevisionEngine, error) + HandleDeletion(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error + Source(handler.EventHandler, ...predicate.Predicate) source.Source +} + +type OLMRevisionEngineGetter struct { + DiscoveryClient discovery.DiscoveryInterface + Scheme *runtime.Scheme + RestMapper meta.RESTMapper + AccessManager AccessManager +} + +func (r *OLMRevisionEngineGetter) GetScopedRevisionEngine(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (RevisionEngine, error) { + accessor, err := r.AccessManager.GetWithUser(ctx, owner, user, usedFor) + if err != nil { + return nil, fmt.Errorf("get cache: %w", err) + } + return machinery.NewRevisionEngine( + machinery.NewPhaseEngine( + machinery.NewObjectEngine( + r.Scheme, accessor, accessor, + ownerhandling.NewNative(r.Scheme), + machinery.NewComparator(ownerhandling.NewNative(r.Scheme), r.DiscoveryClient, r.Scheme, boxcutterSystemPrefixFieldOwner), + boxcutterSystemPrefixFieldOwner, boxcutterSystemPrefixFieldOwner, + ), + validation.NewClusterPhaseValidator(r.RestMapper, accessor), + ), + validation.NewRevisionValidator(), accessor, + ), nil +} + +func (r *OLMRevisionEngineGetter) HandleDeletion(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return r.AccessManager.FreeWithUser(ctx, owner, user) +} + +func (r *OLMRevisionEngineGetter) Source(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source { + return r.AccessManager.Source(eventHandler, p...) +} + +//+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 + +func (c *ClusterExtensionRevisionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + l := log.FromContext(ctx).WithName("cluster-extension-revision") + ctx = log.IntoContext(ctx, l) + + rev := &ocv1.ClusterExtensionRevision{} + if err := c.Client.Get(ctx, req.NamespacedName, rev); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + l = l.WithValues("key", req.String()) + l.Info("reconcile starting") + defer l.Info("reconcile ending") + + controller, ok := getControllingClusterExtension(rev) + if !ok { + // ClusterExtension revisions can't exist without a ClusterExtension in control. + // This situation can only appear if the ClusterExtension object has been deleted with --cascade=Orphan. + // To not leave unactionable resources on the cluster, we are going to just + // reap the revision reverences and propagate the Orphan deletion. + if rev.DeletionTimestamp.IsZero() { + return ctrl.Result{}, client.IgnoreNotFound( + c.Client.Delete(ctx, rev, client.PropagationPolicy(metav1.DeletePropagationOrphan), client.Preconditions{ + UID: ptr.To(rev.GetUID()), + ResourceVersion: ptr.To(rev.GetResourceVersion()), + }), + ) + } + return ctrl.Result{}, c.removeFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer) + } + + return c.reconcile(ctx, controller, rev) +} + +func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, ce *ocv1.ClusterExtension, rev *ocv1.ClusterExtensionRevision) (ctrl.Result, error) { + l := log.FromContext(ctx) + + revision, opts, previous := toBoxcutterRevision(ce.Name, rev) + + var objects []client.Object + for _, phase := range revision.GetPhases() { + for _, pobj := range phase.GetObjects() { + objects = append(objects, &pobj) + } + } + + // THIS IS STUPID, PLEASE FIX! + // Revisions need individual finalizers on the ClusterExtension to prevent its premature deletion. + if rev.DeletionTimestamp.IsZero() && + rev.Spec.LifecycleState != ocv1.ClusterExtensionRevisionLifecycleStateArchived { + // We can't lookup the complete ClusterExtension when it's already deleted. + // This only works when the controller-manager is not restarted during teardown. + if err := c.Client.Get(ctx, client.ObjectKeyFromObject(ce), ce); err != nil { + return ctrl.Result{}, err + } + } + + re, err := c.RevisionManager.GetScopedRevisionEngine(ctx, ce, rev, objects) + if err != nil { + return ctrl.Result{}, err + } + + if !rev.DeletionTimestamp.IsZero() || + rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived { + // + // Teardown + // + tres, err := re.Teardown(ctx, *revision) + if err != nil { + return ctrl.Result{}, fmt.Errorf("revision teardown: %w", err) + } + + l.Info("teardown report", "report", tres.String()) + + if !tres.IsComplete() { + return ctrl.Result{}, nil + } + if err := c.RevisionManager.HandleDeletion(ctx, ce, rev); err != nil { + return ctrl.Result{}, fmt.Errorf("get cache: %w", err) + } + return ctrl.Result{}, c.removeFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer) + } + + // + // Reconcile + // + if err := c.ensureFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil { + return ctrl.Result{}, err + } + rres, err := re.Reconcile(ctx, *revision, opts...) + if err != nil { + return ctrl.Result{}, fmt.Errorf("revision reconcile: %w", err) + } + l.Info("reconcile report", "report", rres.String()) + + // Retry failing preflight checks with a flat 10s retry. + // TODO: report status, backoff? + if verr := rres.GetValidationError(); verr != nil { + l.Info("preflight error, retrying after 10s", "err", verr.String()) + return ctrl.Result{RequeueAfter: 10 * time.Second}, nil + } + for _, pres := range rres.GetPhases() { + if verr := pres.GetValidationError(); verr != nil { + l.Info("preflight error, retrying after 10s", "err", verr.String()) + return ctrl.Result{RequeueAfter: 10 * time.Second}, nil + } + } + + //nolint:nestif + if rres.IsComplete() { + // Archive other revisions. + for _, a := range previous { + if err := c.Client.Patch(ctx, a, client.RawPatch( + types.MergePatchType, []byte(`{"spec":{"lifecycleState":"Archived"}}`))); err != nil { + return ctrl.Result{}, fmt.Errorf("archive previous Revision: %w", err) + } + } + + // Report status. + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: "Available", + Status: metav1.ConditionTrue, + Reason: "Available", + Message: "Object is available and passes all probes.", + ObservedGeneration: rev.Generation, + }) + if !meta.IsStatusConditionTrue(rev.Status.Conditions, "Succeeded") { + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: "Succeeded", + Status: metav1.ConditionTrue, + Reason: "RolloutSuccess", + Message: "Revision succeeded rolling out.", + ObservedGeneration: rev.Generation, + }) + } + } else { + var probeFailureMsgs []string + for _, pres := range rres.GetPhases() { + if pres.IsComplete() { + continue + } + for _, ores := range pres.GetObjects() { + pr := ores.Probes()[boxcutter.ProgressProbeType] + if pr.Success { + continue + } + + obj := ores.Object() + gvk := obj.GetObjectKind().GroupVersionKind() + probeFailureMsgs = append(probeFailureMsgs, fmt.Sprintf( + "Object %s.%s %s/%s: %v", + gvk.Kind, gvk.GroupVersion().String(), + obj.GetNamespace(), obj.GetName(), strings.Join(pr.Messages, " and "), + )) + break + } + } + if len(probeFailureMsgs) > 0 { + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: "Available", + Status: metav1.ConditionFalse, + Reason: "ProbeFailure", + Message: strings.Join(probeFailureMsgs, "\n"), + ObservedGeneration: rev.Generation, + }) + } else { + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: "Available", + Status: metav1.ConditionFalse, + Reason: "Incomplete", + Message: "Revision has not been rolled out completely.", + ObservedGeneration: rev.Generation, + }) + } + } + if rres.InTransistion() { + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ + Type: "InTransition", + Status: metav1.ConditionTrue, + Reason: "InTransition", + Message: "Rollout in progress.", + ObservedGeneration: rev.Generation, + }) + } else { + meta.RemoveStatusCondition(&rev.Status.Conditions, "InTransition") + } + + return ctrl.Result{}, c.Client.Status().Update(ctx, rev) +} + +func (c *ClusterExtensionRevisionReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For( + &ocv1.ClusterExtensionRevision{}, + builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), + ). + WatchesRawSource( + c.RevisionManager.Source( + handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &ocv1.ClusterExtensionRevision{}), + predicate.ResourceVersionChangedPredicate{}, + ), + ). + Complete(c) +} + +func (c *ClusterExtensionRevisionReconciler) ensureFinalizer( + ctx context.Context, obj client.Object, finalizer string, +) error { + if controllerutil.ContainsFinalizer(obj, finalizer) { + return nil + } + + controllerutil.AddFinalizer(obj, finalizer) + patch := map[string]any{ + "metadata": map[string]any{ + "resourceVersion": obj.GetResourceVersion(), + "finalizers": obj.GetFinalizers(), + }, + } + patchJSON, err := json.Marshal(patch) + if err != nil { + return fmt.Errorf("marshalling patch to remove finalizer: %w", err) + } + if err := c.Client.Patch(ctx, obj, client.RawPatch(types.MergePatchType, patchJSON)); err != nil { + return fmt.Errorf("adding finalizer: %w", err) + } + return nil +} + +func (c *ClusterExtensionRevisionReconciler) removeFinalizer(ctx context.Context, obj client.Object, finalizer string) error { + if !controllerutil.ContainsFinalizer(obj, finalizer) { + return nil + } + + controllerutil.RemoveFinalizer(obj, finalizer) + + patch := map[string]any{ + "metadata": map[string]any{ + "resourceVersion": obj.GetResourceVersion(), + "finalizers": obj.GetFinalizers(), + }, + } + patchJSON, err := json.Marshal(patch) + if err != nil { + return fmt.Errorf("marshalling patch to remove finalizer: %w", err) + } + if err := c.Client.Patch(ctx, obj, client.RawPatch(types.MergePatchType, patchJSON)); err != nil { + return fmt.Errorf("removing finalizer: %w", err) + } + return nil +} + +// getControllingClusterExtension checks the objects ownerreferences for a ClusterExtension +// with the controller flag set to true. +// Returns a ClusterExtension with metadata recovered from the OwnerRef or nil. +func getControllingClusterExtension(obj client.Object) (*ocv1.ClusterExtension, bool) { + for _, v := range obj.GetOwnerReferences() { + if v.Controller != nil && *v.Controller && + v.APIVersion == ocv1.GroupVersion.String() && + v.Kind == "ClusterExtension" { + return &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + UID: v.UID, + Name: v.Name, + }, + }, true + } + } + return nil, false +} + +func toBoxcutterRevision(clusterExtensionName string, rev *ocv1.ClusterExtensionRevision) (*boxcutter.Revision, []boxcutter.RevisionReconcileOption, []client.Object) { + r := &boxcutter.Revision{ + Name: rev.Name, + Owner: rev, + Revision: rev.Spec.Revision, + } + for _, specPhase := range rev.Spec.Phases { + phase := boxcutter.Phase{Name: specPhase.Name} + for _, specObj := range specPhase.Objects { + obj := specObj.Object + + labels := obj.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels[ClusterExtensionRevisionOwnerLabel] = clusterExtensionName + obj.SetLabels(labels) + + phase.Objects = append(phase.Objects, obj) + } + r.Phases = append(r.Phases, phase) + } + + previous := make([]client.Object, 0, len(rev.Spec.Previous)) + for _, specPrevious := range rev.Spec.Previous { + prev := &unstructured.Unstructured{} + prev.SetName(specPrevious.Name) + prev.SetUID(specPrevious.UID) + prev.SetGroupVersionKind(ocv1.GroupVersion.WithKind(ocv1.ClusterExtensionRevisionKind)) + previous = append(previous, prev) + } + + opts := []boxcutter.RevisionReconcileOption{ + boxcutter.WithPreviousOwners(previous), + boxcutter.WithProbe(boxcutter.ProgressProbeType, boxcutter.ProbeFunc(func(obj client.Object) (bool, []string) { + deployGK := schema.GroupKind{ + Group: "apps", Kind: "Deployment", + } + if obj.GetObjectKind().GroupVersionKind().GroupKind() != deployGK { + return true, nil + } + ustrObj := obj.(*unstructured.Unstructured) + depl := &appsv1.Deployment{} + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(ustrObj.Object, depl); err != nil { + return false, []string{err.Error()} + } + + if depl.Status.ObservedGeneration != depl.Generation { + return false, []string{".status.observedGeneration outdated"} + } + for _, cond := range depl.Status.Conditions { + if cond.Type == "Available" && + cond.Status == corev1.ConditionTrue && + depl.Status.UpdatedReplicas == *depl.Spec.Replicas { + return true, nil + } + } + return false, []string{"not available or not fully updated"} + })), + } + if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStatePaused { + opts = append(opts, boxcutter.WithPaused{}) + } + return r, opts, previous +} diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go new file mode 100644 index 000000000..ccc73811c --- /dev/null +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go @@ -0,0 +1,1021 @@ +package controllers_test + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "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/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/discovery" + "pkg.package-operator.run/boxcutter" + "pkg.package-operator.run/boxcutter/machinery" + machinerytypes "pkg.package-operator.run/boxcutter/machinery/types" + "pkg.package-operator.run/boxcutter/managedcache" + "pkg.package-operator.run/boxcutter/validation" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/source" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" +) + +func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *testing.T) { + const ( + clusterExtensionRevisionName = "test-ext-1" + ) + + testScheme := newScheme(t) + + for _, tc := range []struct { + name string + existingObjs func() []client.Object + revisionResult machinery.RevisionResult + validate func(*testing.T, client.Client) + }{ + { + name: "sets teardown finalizer", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + require.Contains(t, rev.Finalizers, "olm.operatorframework.io/teardown") + }, + }, + { + name: "set Available:False:InComplete status condition during rollout when no probe failures are detected", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, "Available") + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, "Incomplete", cond.Reason) + require.Equal(t, "Revision has not been rolled out completely.", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "set Available:False:ProbeFailure condition when probe failures are detected", + revisionResult: mockRevisionResult{ + phases: []machinery.PhaseResult{ + mockPhaseResult{ + name: "somephase", + isComplete: false, + objects: []machinery.ObjectResult{ + mockObjectResult{ + success: true, + probes: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: true, + }, + }, + }, + mockObjectResult{ + success: false, + object: func() client.Object { + obj := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + Namespace: "my-namespace", + }, + } + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) + return obj + }(), + probes: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: false, + Messages: []string{ + "something bad happened", + "something worse happened", + }, + }, + }, + }, + }, + }, + mockPhaseResult{ + name: "someotherphase", + isComplete: false, + objects: []machinery.ObjectResult{ + mockObjectResult{ + success: false, + object: func() client.Object { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-configmap", + Namespace: "my-namespace", + }, + } + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) + return obj + }(), + probes: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: false, + Messages: []string{ + "we have a problem", + }, + }, + }, + }, + }, + }, + }, + }, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, "Available") + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, "ProbeFailure", cond.Reason) + require.Equal(t, "Object Service.v1 my-namespace/my-service: something bad happened and something worse happened\nObject ConfigMap.v1 my-namespace/my-configmap: we have a problem", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "set InTransition:True:InTransition condition while revision is transitioning", + revisionResult: mockRevisionResult{ + inTransition: true, + }, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, "InTransition") + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionTrue, cond.Status) + require.Equal(t, "InTransition", cond.Reason) + require.Equal(t, "Rollout in progress.", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "remove InTransition condition once transition rollout is finished", + revisionResult: mockRevisionResult{ + inTransition: false, + }, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ + Type: "InTransition", + Status: metav1.ConditionTrue, + Reason: "InTransition", + Message: "some message", + ObservedGeneration: 1, + }) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, "InTransition") + require.Nil(t, cond) + }, + }, + { + name: "set Available:True:Available and Succeeded:True:RolloutSuccess conditions on successful revision rollout", + revisionResult: mockRevisionResult{ + isComplete: true, + }, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, "Available") + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionTrue, cond.Status) + require.Equal(t, "Available", cond.Reason) + require.Equal(t, "Object is available and passes all probes.", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + + cond = meta.FindStatusCondition(rev.Status.Conditions, "Succeeded") + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionTrue, cond.Status) + require.Equal(t, "RolloutSuccess", cond.Reason) + require.Equal(t, "Revision succeeded rolling out.", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "archive previous revisions on successful rollout", + revisionResult: mockRevisionResult{ + isComplete: true, + }, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + prevRev1 := newTestClusterExtensionRevision("prev-rev-1") + require.NoError(t, controllerutil.SetControllerReference(ext, prevRev1, testScheme)) + prevRev2 := newTestClusterExtensionRevision("prev-rev-2") + require.NoError(t, controllerutil.SetControllerReference(ext, prevRev2, testScheme)) + rev1 := newTestClusterExtensionRevision("test-ext-1") + rev1.Spec.Previous = []ocv1.ClusterExtensionRevisionPrevious{ + { + Name: "prev-rev-1", + UID: "prev-rev-1", + }, { + Name: "prev-rev-2", + UID: "prev-rev-2", + }, + } + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{ext, prevRev1, prevRev2, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: "prev-rev-1", + }, rev) + require.NoError(t, err) + require.Equal(t, ocv1.ClusterExtensionRevisionLifecycleStateArchived, rev.Spec.LifecycleState) + + err = c.Get(t.Context(), client.ObjectKey{ + Name: "prev-rev-2", + }, rev) + require.NoError(t, err) + require.Equal(t, ocv1.ClusterExtensionRevisionLifecycleStateArchived, rev.Spec.LifecycleState) + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + // create extension and cluster extension + testClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithStatusSubresource(&ocv1.ClusterExtensionRevision{}). + WithObjects(tc.existingObjs()...). + Build() + + // reconcile cluster extension revision + result, err := (&controllers.ClusterExtensionRevisionReconciler{ + Client: testClient, + RevisionManager: mockRevisionManager{ + getScopedRevisionEngine: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) { + return &mockRevisionEngine{ + reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return tc.revisionResult, nil + }, + }, nil + }, + }, + }).Reconcile(t.Context(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: clusterExtensionRevisionName, + }, + }) + + // reconcile cluster extensionr evision + require.Equal(t, ctrl.Result{}, result) + require.NoError(t, err) + + // validate test case + tc.validate(t, testClient) + }) + } +} + +func Test_ClusterExtensionRevisionReconciler_Reconcile_ValidationError_Retries(t *testing.T) { + const ( + clusterExtensionName = "test-ext" + clusterExtensionRevisionName = "test-ext-1" + ) + + testScheme := newScheme(t) + + for _, tc := range []struct { + name string + revisionResult machinery.RevisionResult + }{ + { + name: "retries on revision result validation error", + revisionResult: mockRevisionResult{ + validationError: &validation.RevisionValidationError{ + RevisionName: "test-ext-1", + RevisionNumber: 1, + Phases: []validation.PhaseValidationError{ + { + PhaseName: "everything", + PhaseError: fmt.Errorf("some error"), + Objects: []validation.ObjectValidationError{ + { + ObjectRef: machinerytypes.ObjectRef{ + GroupVersionKind: schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }, + ObjectKey: client.ObjectKey{ + Name: "my-configmap", + Namespace: "my-namespace", + }, + }, + Errors: []error{ + fmt.Errorf("is not a config"), + fmt.Errorf("is not a map"), + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "retries on revision result phase validation error", + revisionResult: mockRevisionResult{ + phases: []machinery.PhaseResult{ + mockPhaseResult{ + validationError: &validation.PhaseValidationError{ + PhaseName: "everything", + PhaseError: fmt.Errorf("some error"), + Objects: []validation.ObjectValidationError{ + { + ObjectRef: machinerytypes.ObjectRef{ + GroupVersionKind: schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }, + ObjectKey: client.ObjectKey{ + Name: "my-configmap", + Namespace: "my-namespace", + }, + }, + Errors: []error{ + fmt.Errorf("is not a config"), + fmt.Errorf("is not a map"), + }, + }, + }, + }, + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + + // create extension and cluster extension + testClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithStatusSubresource(&ocv1.ClusterExtensionRevision{}). + WithObjects(ext, rev1). + Build() + + // reconcile cluster extension revision + result, err := (&controllers.ClusterExtensionRevisionReconciler{ + Client: testClient, + RevisionManager: mockRevisionManager{ + getScopedRevisionEngine: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) { + return &mockRevisionEngine{ + reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return tc.revisionResult, nil + }, + }, nil + }, + }, + }).Reconcile(t.Context(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: clusterExtensionRevisionName, + }, + }) + + // reconcile cluster extensionr evision + require.Equal(t, ctrl.Result{ + RequeueAfter: 10 * time.Second, + }, result) + require.NoError(t, err) + }) + } +} + +func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { + const ( + clusterExtensionRevisionName = "test-ext-1" + ) + + testScheme := newScheme(t) + require.NoError(t, corev1.AddToScheme(testScheme)) + + for _, tc := range []struct { + name string + existingObjs func() []client.Object + revisionResult machinery.RevisionResult + revisionEngineTeardownFn func(*testing.T) func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) + validate func(*testing.T, client.Client) + handleDeletionFn func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error + expectedErr string + }{ + { + name: "teardown finalizer is removed", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + rev1.Finalizers = []string{ + "olm.operatorframework.io/teardown", + } + return []client.Object{rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) + }, + handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return nil + }, + revisionEngineTeardownFn: func(t *testing.T) func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return nil + }, + }, + { + name: "delete with propagation orphan if parent ClusterExtension does not exist", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-configmap", + Namespace: "my-namespace", + }, + Data: map[string]string{ + "some": "data", + }, + } + require.NoError(t, controllerutil.SetControllerReference(rev1, configMap, testScheme)) + return []client.Object{rev1, configMap} + }, + validate: func(t *testing.T, c client.Client) { + t.Log("checking if ClusterExtensionRevision is deleted") + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.Error(t, err) + require.True(t, errors.IsNotFound(err)) + t.Log("checking if ConfigMap still exists") + cm := &corev1.ConfigMap{} + err = c.Get(t.Context(), client.ObjectKey{ + Name: "my-configmap", + Namespace: "my-namespace", + }, cm) + require.NoError(t, err) + require.NotNil(t, cm) + }, + handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return nil + }, + revisionEngineTeardownFn: func(t *testing.T) func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return nil + }, + }, + { + name: "revision is torn down and deleted when deleted", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + rev1.Finalizers = []string{ + "olm.operatorframework.io/teardown", + } + rev1.DeletionTimestamp = &metav1.Time{Time: time.Now()} + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{rev1, ext} + }, + revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return &mockRevisionTeardownResult{ + isComplete: true, + }, nil + } + }, + handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + t.Log("check owner is cluster extension and user is cluster extension revision") + require.Equal(t, "test-ext", owner.GetName()) + require.Equal(t, "test-ext-1", user.GetName()) + return nil + } + }, + validate: func(t *testing.T, c client.Client) { + t.Log("cluster revision is deleted") + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.Error(t, err) + require.True(t, errors.IsNotFound(err)) + }, + }, + { + name: "surfaces tear down errors when deleted", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + rev1.Finalizers = []string{ + "olm.operatorframework.io/teardown", + } + rev1.DeletionTimestamp = &metav1.Time{Time: time.Now()} + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{rev1, ext} + }, + revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return nil, fmt.Errorf("some teardown error") + } + }, + expectedErr: "some teardown error", + validate: func(t *testing.T, c client.Client) { + t.Log("cluster revision is not deleted and still contains finalizer") + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) + }, + handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return nil + }, + }, + { + name: "revision is torn down when in archived state and finalizer is removed", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + rev1.Finalizers = []string{ + "olm.operatorframework.io/teardown", + } + rev1.Spec.LifecycleState = ocv1.ClusterExtensionRevisionLifecycleStateArchived + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{rev1, ext} + }, + revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return &mockRevisionTeardownResult{ + isComplete: true, + }, nil + } + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) + }, + handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + t.Log("check owner is cluster extension and user is cluster extension revision") + require.Equal(t, "test-ext", owner.GetName()) + require.Equal(t, "test-ext-1", user.GetName()) + return nil + } + }, + }, + { + name: "surfaces revision teardown error when in archived state", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) + rev1.Finalizers = []string{ + "olm.operatorframework.io/teardown", + } + rev1.Spec.LifecycleState = ocv1.ClusterExtensionRevisionLifecycleStateArchived + require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + return []client.Object{rev1, ext} + }, + revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return nil, fmt.Errorf("some teardown error") + } + }, + expectedErr: "some teardown error", + validate: func(t *testing.T, c client.Client) { + t.Log("cluster revision is not deleted and still contains finalizer") + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) + }, + handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return nil + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + // create extension and cluster extension + testClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithStatusSubresource(&ocv1.ClusterExtensionRevision{}). + WithObjects(tc.existingObjs()...). + Build() + + // reconcile cluster extension revision + result, err := (&controllers.ClusterExtensionRevisionReconciler{ + Client: testClient, + RevisionManager: mockRevisionManager{ + getScopedRevisionEngine: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) { + return &mockRevisionEngine{ + reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return tc.revisionResult, nil + }, + teardown: tc.revisionEngineTeardownFn(t), + }, nil + }, + handleDeletion: tc.handleDeletionFn(t), + }, + }).Reconcile(t.Context(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: clusterExtensionRevisionName, + }, + }) + + // reconcile cluster extension revision + require.Equal(t, ctrl.Result{}, result) + if tc.expectedErr != "" { + require.Contains(t, err.Error(), tc.expectedErr) + } else { + require.NoError(t, err) + } + + // validate test case + tc.validate(t, testClient) + }) + } +} + +func Test_OLMRevisionGetter_AccessManager_Integration(t *testing.T) { + expectedCtx := t.Context() + expectedOwner := &ocv1.ClusterExtension{} + expectedUser := &ocv1.ClusterExtensionRevision{} + expectedUsedFor := []client.Object{ + &corev1.ConfigMap{}, + &corev1.ServiceAccount{}, + } + expectedHandler := handler.EnqueueRequestsFromMapFunc(nil) + expectedPredicates := []predicate.Predicate{ + predicate.AnnotationChangedPredicate{}, + predicate.GenerationChangedPredicate{}, + } + + discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) + require.NoError(t, err) + + g := controllers.OLMRevisionEngineGetter{ + DiscoveryClient: discoveryClient, + AccessManager: mockAccessManager{ + getWithUser: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) { + require.Equal(t, expectedCtx, ctx) + require.Equal(t, expectedOwner, owner) + require.Equal(t, expectedUser, user) + require.Equal(t, expectedUsedFor, usedFor) + return nil, nil + }, + freeWithUser: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + require.Equal(t, expectedCtx, ctx) + require.Equal(t, expectedOwner, owner) + require.Equal(t, expectedUser, user) + return nil + }, + sourceFn: func(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source { + require.Equal(t, expectedHandler, eventHandler) + require.Equal(t, expectedPredicates, p) + return nil + }, + }, + } + + _, _ = g.GetScopedRevisionEngine(expectedCtx, expectedOwner, expectedUser, expectedUsedFor) + _ = g.HandleDeletion(expectedCtx, expectedOwner, expectedUser) + _ = g.Source(expectedHandler, expectedPredicates...) +} + +func Test_OLMRevisionGetter_AccessManager_Errors(t *testing.T) { + g := controllers.OLMRevisionEngineGetter{ + AccessManager: mockAccessManager{ + getWithUser: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) { + return nil, fmt.Errorf("some error") + }, + freeWithUser: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return fmt.Errorf("some error") + }, + }, + } + + _, err := g.GetScopedRevisionEngine(t.Context(), nil, nil, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "some error") + err = g.HandleDeletion(t.Context(), nil, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "some error") +} + +func newTestClusterExtension() *ocv1.ClusterExtension { + return &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ext", + UID: "test-ext", + }, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "some-namespace", + ServiceAccount: ocv1.ServiceAccountReference{ + Name: "service-account", + }, + Source: ocv1.SourceConfig{ + SourceType: ocv1.SourceTypeCatalog, + Catalog: &ocv1.CatalogFilter{ + PackageName: "some-package", + }, + }, + }, + } +} + +func newTestClusterExtensionRevision(name string) *ocv1.ClusterExtensionRevision { + return &ocv1.ClusterExtensionRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + UID: types.UID(name), + Generation: int64(1), + }, + Spec: ocv1.ClusterExtensionRevisionSpec{ + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "everything", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "data": map[string]interface{}{ + "foo": "bar", + }, + }, + }, + }, + }, + }, + }, + }, + } +} + +type mockAccessManager struct { + getWithUser func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) + freeWithUser func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error + sourceFn func(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source +} + +func (m mockAccessManager) GetWithUser(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) { + return m.getWithUser(ctx, owner, user, usedFor) +} + +func (m mockAccessManager) FreeWithUser(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return m.freeWithUser(ctx, owner, user) +} + +func (m mockAccessManager) Source(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source { + return m.sourceFn(eventHandler, p...) +} + +var _ controllers.AccessManager = &mockAccessManager{} + +type mockRevisionManager struct { + getScopedRevisionEngine func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) + handleDeletion func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error + source func(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source +} + +func (m mockRevisionManager) HandleDeletion(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { + return m.handleDeletion(ctx, owner, user) +} + +func (m mockRevisionManager) Source(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source { + return m.source(eventHandler, p...) +} + +func (m mockRevisionManager) GetScopedRevisionEngine(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) { + return m.getScopedRevisionEngine(ctx, owner, user, usedFor) +} + +type mockRevisionEngine struct { + teardown func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) + reconcile func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) +} + +func (m mockRevisionEngine) Teardown(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return m.teardown(ctx, rev) +} + +func (m mockRevisionEngine) Reconcile(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return m.reconcile(ctx, rev) +} + +type mockRevisionResult struct { + validationError *validation.RevisionValidationError + phases []machinery.PhaseResult + inTransition bool + isComplete bool + hasProgressed bool + string string +} + +func (m mockRevisionResult) GetValidationError() *validation.RevisionValidationError { + return m.validationError +} + +func (m mockRevisionResult) GetPhases() []machinery.PhaseResult { + return m.phases +} + +func (m mockRevisionResult) InTransistion() bool { + return m.inTransition +} + +func (m mockRevisionResult) IsComplete() bool { + return m.isComplete +} + +func (m mockRevisionResult) HasProgressed() bool { + return m.hasProgressed +} + +func (m mockRevisionResult) String() string { + return m.string +} + +type mockPhaseResult struct { + name string + validationError *validation.PhaseValidationError + objects []machinery.ObjectResult + inTransition bool + isComplete bool + hasProgressed bool + string string +} + +func (m mockPhaseResult) GetName() string { + return m.name +} + +func (m mockPhaseResult) GetValidationError() *validation.PhaseValidationError { + return m.validationError +} + +func (m mockPhaseResult) GetObjects() []machinery.ObjectResult { + return m.objects +} + +func (m mockPhaseResult) InTransistion() bool { + return m.inTransition +} + +func (m mockPhaseResult) IsComplete() bool { + return m.isComplete +} + +func (m mockPhaseResult) HasProgressed() bool { + return m.hasProgressed +} + +func (m mockPhaseResult) String() string { + return m.string +} + +type mockObjectResult struct { + action machinery.Action + object machinery.Object + success bool + probes map[string]machinery.ObjectProbeResult + string string +} + +func (m mockObjectResult) Action() machinery.Action { + return m.action +} + +func (m mockObjectResult) Object() machinery.Object { + return m.object +} + +func (m mockObjectResult) Success() bool { + return m.success +} + +func (m mockObjectResult) Probes() map[string]machinery.ObjectProbeResult { + return m.probes +} + +func (m mockObjectResult) String() string { + return m.string +} + +type mockRevisionTeardownResult struct { + phases []machinery.PhaseTeardownResult + isComplete bool + waitingPhaseNames []string + activePhaseName string + phaseIsActive bool + gonePhaseNames []string + string string +} + +func (m mockRevisionTeardownResult) GetPhases() []machinery.PhaseTeardownResult { + return m.phases +} + +func (m mockRevisionTeardownResult) IsComplete() bool { + return m.isComplete +} + +func (m mockRevisionTeardownResult) GetWaitingPhaseNames() []string { + return m.waitingPhaseNames +} + +func (m mockRevisionTeardownResult) GetActivePhaseName() (string, bool) { + return m.activePhaseName, m.phaseIsActive +} + +func (m mockRevisionTeardownResult) GetGonePhaseNames() []string { + return m.gonePhaseNames +} + +func (m mockRevisionTeardownResult) String() string { + return m.string +} diff --git a/internal/operator-controller/controllers/suite_test.go b/internal/operator-controller/controllers/suite_test.go index f287982ec..1af2b1e7d 100644 --- a/internal/operator-controller/controllers/suite_test.go +++ b/internal/operator-controller/controllers/suite_test.go @@ -41,12 +41,16 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" ) +func newScheme(t *testing.T) *apimachineryruntime.Scheme { + sch := apimachineryruntime.NewScheme() + require.NoError(t, ocv1.AddToScheme(sch)) + return sch +} + func newClient(t *testing.T) client.Client { // TODO: this is a live client, which behaves differently than a cache client. // We may want to use a caching client instead to get closer to real behavior. - sch := apimachineryruntime.NewScheme() - require.NoError(t, ocv1.AddToScheme(sch)) - cl, err := client.New(config, client.Options{Scheme: sch}) + cl, err := client.New(config, client.Options{Scheme: newScheme(t)}) require.NoError(t, err) require.NotNil(t, cl) return cl @@ -138,7 +142,7 @@ var ( func TestMain(m *testing.M) { testEnv := &envtest.Environment{ CRDDirectoryPaths: []string{ - filepath.Join("..", "..", "..", "config", "base", "operator-controller", "crd", "standard"), + filepath.Join("..", "..", "..", "config", "base", "operator-controller", "crd", "experimental"), }, ErrorIfCRDPathMissing: true, } From 0b0f5e7c98ecce9dffabab439a60773f37a36491 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:12:35 +0200 Subject: [PATCH 07/17] Add Boxcutter runtime to main Signed-off-by: Per Goncalves da Silva --- cmd/operator-controller/main.go | 136 ++++++++++++++---- .../clusterextension_controller.go | 21 ++- 2 files changed, 128 insertions(+), 29 deletions(-) diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index d426793d4..8ef618b34 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -31,14 +31,19 @@ import ( "github.com/containers/image/v5/types" "github.com/spf13/cobra" rbacv1 "k8s.io/api/rbac/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" k8slabels "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" k8stypes "k8s.io/apimachinery/pkg/types" apimachineryrand "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/client-go/discovery" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" _ "k8s.io/client-go/plugin/pkg/client/auth" + "k8s.io/client-go/rest" "k8s.io/klog/v2" "k8s.io/utils/ptr" + "pkg.package-operator.run/boxcutter/managedcache" ctrl "sigs.k8s.io/controller-runtime" crcache "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/certwatcher" @@ -219,6 +224,12 @@ func run() error { DefaultLabelSelector: k8slabels.Nothing(), } + if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { + cacheOptions.ByObject[&ocv1.ClusterExtensionRevision{}] = crcache.ByObject{ + Label: k8slabels.Everything(), + } + } + saKey, err := sautil.GetServiceAccount() if err != nil { setupLog.Error(err, "Failed to extract serviceaccount from JWT") @@ -272,7 +283,8 @@ func run() error { "Metrics will not be served since the TLS certificate and key file are not provided.") } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + restConfig := ctrl.GetConfigOrDie() + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: scheme.Scheme, Metrics: metricsServerOptions, PprofBindAddress: cfg.pprofAddr, @@ -427,28 +439,38 @@ func run() error { preAuth = authorization.NewRBACPreAuthorizer(mgr.GetClient()) } - // determine if a certificate provider should be set in the bundle renderer and feature support for the provider - // based on the feature flag - var certProvider render.CertificateProvider - var isWebhookSupportEnabled bool - if features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderCertManager) { - certProvider = certproviders.CertManagerCertificateProvider{} - isWebhookSupportEnabled = true - } else if features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderOpenshiftServiceCA) { - certProvider = certproviders.OpenshiftServiceCaCertificateProvider{} - isWebhookSupportEnabled = true - } - - // now initialize the helmApplier, assigning the potentially nil preAuth - helmApplier := &applier.Helm{ - ActionClientGetter: acg, - Preflights: preflights, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{ - BundleRenderer: registryv1.Renderer, - CertificateProvider: certProvider, - IsWebhookSupportEnabled: isWebhookSupportEnabled, - }, - PreAuthorizer: preAuth, + // create applier + var ctrlBuilderOpts []controllers.ControllerBuilderOption + var extApplier controllers.Applier + + if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { + // TODO: add support for preflight checks + // TODO: better scheme handling - which types do we want to support? + _ = apiextensionsv1.AddToScheme(mgr.GetScheme()) + extApplier = &applier.Boxcutter{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + RevisionGenerator: &applier.SimpleRevisionGenerator{ + Scheme: mgr.GetScheme(), + BundleRenderer: &applier.RegistryV1BundleRenderer{ + BundleRenderer: registryv1.Renderer, + }, + }, + } + ctrlBuilderOpts = append(ctrlBuilderOpts, controllers.WithOwns(&ocv1.ClusterExtensionRevision{})) + } else { + // now initialize the helmApplier, assigning the potentially nil preAuth + certProvider := getCertificateProvider() + extApplier = &applier.Helm{ + ActionClientGetter: acg, + Preflights: preflights, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{ + BundleRenderer: registryv1.Renderer, + CertificateProvider: certProvider, + IsWebhookSupportEnabled: certProvider != nil, + }, + PreAuthorizer: preAuth, + } } cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()) @@ -467,15 +489,70 @@ func run() error { Resolver: resolver, ImageCache: imageCache, ImagePuller: imagePuller, - Applier: helmApplier, + Applier: extApplier, InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg}, Finalizers: clusterExtensionFinalizers, Manager: cm, - }).SetupWithManager(mgr); err != nil { + }).SetupWithManager(mgr, ctrlBuilderOpts...); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") return err } + if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { + // Boxcutter + discoveryClient, err := discovery.NewDiscoveryClientForConfig(restConfig) + if err != nil { + setupLog.Error(err, "unable to create discovery client") + return err + } + mapFunc := func(ctx context.Context, ce *ocv1.ClusterExtension, c *rest.Config, o crcache.Options) (*rest.Config, crcache.Options, error) { + saKey := client.ObjectKey{ + Name: ce.Spec.ServiceAccount.Name, + Namespace: ce.Spec.Namespace, + } + saConfig := rest.AnonymousClientConfig(c) + saConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper { + return &authentication.TokenInjectingRoundTripper{ + Tripper: rt, + TokenGetter: tokenGetter, + Key: saKey, + } + }) + + // Cache scoping + req1, err := k8slabels.NewRequirement( + controllers.ClusterExtensionRevisionOwnerLabel, selection.Equals, []string{ce.Name}) + if err != nil { + return nil, o, err + } + o.DefaultLabelSelector = k8slabels.NewSelector().Add(*req1) + + return saConfig, o, nil + } + + accessManager := managedcache.NewObjectBoundAccessManager( + ctrl.Log.WithName("accessmanager"), mapFunc, restConfig, crcache.Options{ + Scheme: mgr.GetScheme(), Mapper: mgr.GetRESTMapper(), + }) + if err := mgr.Add(accessManager); err != nil { + setupLog.Error(err, "unable to register AccessManager") + return err + } + + if err = (&controllers.ClusterExtensionRevisionReconciler{ + Client: cl, + RevisionManager: &controllers.OLMRevisionEngineGetter{ + AccessManager: accessManager, + Scheme: mgr.GetScheme(), + RestMapper: mgr.GetRESTMapper(), + DiscoveryClient: discoveryClient, + }, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") + return err + } + } + if err = (&controllers.ClusterCatalogReconciler{ Client: cl, CatalogCache: catalogClientBackend, @@ -521,6 +598,15 @@ func run() error { return nil } +func getCertificateProvider() render.CertificateProvider { + if features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderCertManager) { + return certproviders.CertManagerCertificateProvider{} + } else if features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderOpenshiftServiceCA) { + return certproviders.OpenshiftServiceCaCertificateProvider{} + } + return nil +} + func main() { if err := operatorControllerCmd.Execute(); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 24824bfd1..94d3b652f 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -413,9 +413,17 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, depreca } } +type ControllerBuilderOption func(builder *ctrl.Builder) + +func WithOwns(obj client.Object) ControllerBuilderOption { + return func(builder *ctrl.Builder) { + builder.Owns(obj) + } +} + // SetupWithManager sets up the controller with the Manager. -func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error { - controller, err := ctrl.NewControllerManagedBy(mgr). +func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager, opts ...ControllerBuilderOption) error { + ctrlBuilder := ctrl.NewControllerManagedBy(mgr). For(&ocv1.ClusterExtension{}). Named("controller-operator-cluster-extension-controller"). Watches(&ocv1.ClusterCatalog{}, @@ -436,8 +444,13 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error { } return true }, - })). - Build(r) + })) + + for _, applyOpt := range opts { + applyOpt(ctrlBuilder) + } + + controller, err := ctrlBuilder.Build(r) if err != nil { return err } From 71e08b50325155b19a208b15fd2ba666c90c0105 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:19:06 +0200 Subject: [PATCH 08/17] Remove ClusterExtensionRevision from crd-docs Signed-off-by: Per Goncalves da Silva --- docs/api-reference/crd-ref-docs-gen-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api-reference/crd-ref-docs-gen-config.yaml b/docs/api-reference/crd-ref-docs-gen-config.yaml index f6394fdf6..c8efa15c1 100644 --- a/docs/api-reference/crd-ref-docs-gen-config.yaml +++ b/docs/api-reference/crd-ref-docs-gen-config.yaml @@ -1,5 +1,5 @@ processor: - ignoreTypes: [] + ignoreTypes: [ClusterExtensionRevision, ClusterExtensionRevisionList] ignoreFields: [] render: From 8e0fa98c90181b846a7d87d9b89a48f1e7607c40 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:20:02 +0200 Subject: [PATCH 09/17] Update hack/tools/update-crds.sh for ClusterExtensionRevision API Signed-off-by: Per Goncalves da Silva --- hack/tools/update-crds.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hack/tools/update-crds.sh b/hack/tools/update-crds.sh index 8627784fe..744d7f09e 100755 --- a/hack/tools/update-crds.sh +++ b/hack/tools/update-crds.sh @@ -7,11 +7,12 @@ set -e # The names of the generated CRDs CE="olm.operatorframework.io_clusterextensions.yaml" CC="olm.operatorframework.io_clustercatalogs.yaml" +CR="olm.operatorframework.io_clusterextensionrevisions.yaml" # order for modules and crds must match # each item in crds must be unique, and should be associated with a module -modules=("operator-controller" "catalogd") -crds=("${CE}" "${CC}") +modules=("operator-controller" "catalogd" "operator-controller") +crds=("${CE}" "${CC}" "${CR}") # Channels must much those in the generator channels=("standard" "experimental") From 975f9c25336aac69bf69d38fdd7cfbcb33a60152 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 23 Jul 2025 14:20:22 +0200 Subject: [PATCH 10/17] Generate manifests Signed-off-by: Per Goncalves da Silva --- .../crd/experimental/kustomization.yaml | 1 + ...ramework.io_clusterextensionrevisions.yaml | 182 ++++++++++++++++ .../rbac/experimental/kustomization.yaml | 4 +- .../rbac/experimental/role.yaml | 16 +- config/samples/olm_v1_clusterextension.yaml | 4 + manifests/experimental-e2e.yaml | 199 +++++++++++++++++- manifests/experimental.yaml | 199 +++++++++++++++++- 7 files changed, 600 insertions(+), 5 deletions(-) create mode 100644 config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml diff --git a/config/base/operator-controller/crd/experimental/kustomization.yaml b/config/base/operator-controller/crd/experimental/kustomization.yaml index 1c4db41af..f0315ce34 100644 --- a/config/base/operator-controller/crd/experimental/kustomization.yaml +++ b/config/base/operator-controller/crd/experimental/kustomization.yaml @@ -1,2 +1,3 @@ resources: - olm.operatorframework.io_clusterextensions.yaml +- olm.operatorframework.io_clusterextensionrevisions.yaml diff --git a/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml b/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml new file mode 100644 index 000000000..4eba98633 --- /dev/null +++ b/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml @@ -0,0 +1,182 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 + olm.operatorframework.io/generator: experimental + name: clusterextensionrevisions.olm.operatorframework.io +spec: + group: olm.operatorframework.io + names: + kind: ClusterExtensionRevision + listKind: ClusterExtensionRevisionList + plural: clusterextensionrevisions + singular: clusterextensionrevision + scope: Cluster + versions: + - name: v1 + schema: + openAPIV3Schema: + description: ClusterExtensionRevision is the Schema for the clusterextensionrevisions + API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: spec is an optional field that defines the desired state + of the ClusterExtension. + properties: + lifecycleState: + default: Active + description: Specifies the lifecycle state of the ClusterExtensionRevision. + enum: + - Active + - Paused + - Archived + type: string + x-kubernetes-validations: + - message: can not un-archive + rule: oldSelf == 'Active' || oldSelf == 'Paused' || oldSelf == 'Archived' + && oldSelf == self + phases: + items: + properties: + name: + type: string + objects: + items: + properties: + object: + type: object + x-kubernetes-embedded-resource: true + x-kubernetes-preserve-unknown-fields: true + required: + - object + type: object + type: array + slices: + items: + type: string + type: array + required: + - name + - objects + type: object + type: array + x-kubernetes-validations: + - message: phases is immutable + rule: self == oldSelf || oldSelf.size() == 0 + previous: + items: + properties: + name: + type: string + uid: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + required: + - name + - uid + type: object + type: array + x-kubernetes-validations: + - message: previous is immutable + rule: self == oldSelf + revision: + format: int64 + type: integer + x-kubernetes-validations: + - message: revision is immutable + rule: self == oldSelf + required: + - phases + - revision + type: object + status: + description: status is an optional field that defines the observed state + of the ClusterExtension. + properties: + conditions: + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/config/base/operator-controller/rbac/experimental/kustomization.yaml b/config/base/operator-controller/rbac/experimental/kustomization.yaml index 52a91a8e1..7d430c538 100644 --- a/config/base/operator-controller/rbac/experimental/kustomization.yaml +++ b/config/base/operator-controller/rbac/experimental/kustomization.yaml @@ -3,5 +3,5 @@ kind: Kustomization namespace: olmv1-system namePrefix: operator-controller- resources: -- ../common -- role.yaml + - ../common + - role.yaml diff --git a/config/base/operator-controller/rbac/experimental/role.yaml b/config/base/operator-controller/rbac/experimental/role.yaml index bb1cbe626..ea0d24fd0 100644 --- a/config/base/operator-controller/rbac/experimental/role.yaml +++ b/config/base/operator-controller/rbac/experimental/role.yaml @@ -27,8 +27,10 @@ rules: - apiGroups: - olm.operatorframework.io resources: - - clusterextensions + - clusterextensionrevisions verbs: + - create + - delete - get - list - patch @@ -37,16 +39,28 @@ rules: - apiGroups: - olm.operatorframework.io resources: + - clusterextensionrevisions/finalizers - clusterextensions/finalizers verbs: - update - apiGroups: - olm.operatorframework.io resources: + - clusterextensionrevisions/status - clusterextensions/status verbs: - patch - update +- apiGroups: + - olm.operatorframework.io + resources: + - clusterextensions + verbs: + - get + - list + - patch + - update + - watch - apiGroups: - rbac.authorization.k8s.io resources: diff --git a/config/samples/olm_v1_clusterextension.yaml b/config/samples/olm_v1_clusterextension.yaml index 4438dfb76..14c8e167e 100644 --- a/config/samples/olm_v1_clusterextension.yaml +++ b/config/samples/olm_v1_clusterextension.yaml @@ -33,6 +33,10 @@ rules: resources: [clusterextensions/finalizers] verbs: [update] resourceNames: [argocd] +# Allow ClusterExtensionRevisions to set blockOwnerDeletion ownerReferences +- apiGroups: [olm.operatorframework.io] + resources: [clusterextensionrevisions/finalizers] + verbs: [update] # Manage ArgoCD CRDs - apiGroups: [apiextensions.k8s.io] resources: [customresourcedefinitions] diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index a91833bd7..72b11364e 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -454,6 +454,189 @@ spec: --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 + olm.operatorframework.io/feature-set: experimental + olm.operatorframework.io/generator: experimental + name: clusterextensionrevisions.olm.operatorframework.io +spec: + group: olm.operatorframework.io + names: + kind: ClusterExtensionRevision + listKind: ClusterExtensionRevisionList + plural: clusterextensionrevisions + singular: clusterextensionrevision + scope: Cluster + versions: + - name: v1 + schema: + openAPIV3Schema: + description: ClusterExtensionRevision is the Schema for the clusterextensionrevisions + API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: spec is an optional field that defines the desired state + of the ClusterExtension. + properties: + lifecycleState: + default: Active + description: Specifies the lifecycle state of the ClusterExtensionRevision. + enum: + - Active + - Paused + - Archived + type: string + x-kubernetes-validations: + - message: can not un-archive + rule: oldSelf == 'Active' || oldSelf == 'Paused' || oldSelf == 'Archived' + && oldSelf == self + phases: + items: + properties: + name: + type: string + objects: + items: + properties: + object: + type: object + x-kubernetes-embedded-resource: true + x-kubernetes-preserve-unknown-fields: true + required: + - object + type: object + type: array + slices: + items: + type: string + type: array + required: + - name + - objects + type: object + type: array + x-kubernetes-validations: + - message: phases is immutable + rule: self == oldSelf || oldSelf.size() == 0 + previous: + items: + properties: + name: + type: string + uid: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + required: + - name + - uid + type: object + type: array + x-kubernetes-validations: + - message: previous is immutable + rule: self == oldSelf + revision: + format: int64 + type: integer + x-kubernetes-validations: + - message: revision is immutable + rule: self == oldSelf + required: + - phases + - revision + type: object + status: + description: status is an optional field that defines the observed state + of the ClusterExtension. + properties: + conditions: + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + type: object + served: true + storage: true + subresources: + status: {} +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition metadata: annotations: controller-gen.kubebuilder.io/version: v0.18.0 @@ -1331,8 +1514,10 @@ rules: - apiGroups: - olm.operatorframework.io resources: - - clusterextensions + - clusterextensionrevisions verbs: + - create + - delete - get - list - patch @@ -1341,16 +1526,28 @@ rules: - apiGroups: - olm.operatorframework.io resources: + - clusterextensionrevisions/finalizers - clusterextensions/finalizers verbs: - update - apiGroups: - olm.operatorframework.io resources: + - clusterextensionrevisions/status - clusterextensions/status verbs: - patch - update +- apiGroups: + - olm.operatorframework.io + resources: + - clusterextensions + verbs: + - get + - list + - patch + - update + - watch - apiGroups: - rbac.authorization.k8s.io resources: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 00dc14153..0f7ebe23c 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -454,6 +454,189 @@ spec: --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 + olm.operatorframework.io/feature-set: experimental + olm.operatorframework.io/generator: experimental + name: clusterextensionrevisions.olm.operatorframework.io +spec: + group: olm.operatorframework.io + names: + kind: ClusterExtensionRevision + listKind: ClusterExtensionRevisionList + plural: clusterextensionrevisions + singular: clusterextensionrevision + scope: Cluster + versions: + - name: v1 + schema: + openAPIV3Schema: + description: ClusterExtensionRevision is the Schema for the clusterextensionrevisions + API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: spec is an optional field that defines the desired state + of the ClusterExtension. + properties: + lifecycleState: + default: Active + description: Specifies the lifecycle state of the ClusterExtensionRevision. + enum: + - Active + - Paused + - Archived + type: string + x-kubernetes-validations: + - message: can not un-archive + rule: oldSelf == 'Active' || oldSelf == 'Paused' || oldSelf == 'Archived' + && oldSelf == self + phases: + items: + properties: + name: + type: string + objects: + items: + properties: + object: + type: object + x-kubernetes-embedded-resource: true + x-kubernetes-preserve-unknown-fields: true + required: + - object + type: object + type: array + slices: + items: + type: string + type: array + required: + - name + - objects + type: object + type: array + x-kubernetes-validations: + - message: phases is immutable + rule: self == oldSelf || oldSelf.size() == 0 + previous: + items: + properties: + name: + type: string + uid: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + required: + - name + - uid + type: object + type: array + x-kubernetes-validations: + - message: previous is immutable + rule: self == oldSelf + revision: + format: int64 + type: integer + x-kubernetes-validations: + - message: revision is immutable + rule: self == oldSelf + required: + - phases + - revision + type: object + status: + description: status is an optional field that defines the observed state + of the ClusterExtension. + properties: + conditions: + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + type: object + served: true + storage: true + subresources: + status: {} +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition metadata: annotations: controller-gen.kubebuilder.io/version: v0.18.0 @@ -1331,8 +1514,10 @@ rules: - apiGroups: - olm.operatorframework.io resources: - - clusterextensions + - clusterextensionrevisions verbs: + - create + - delete - get - list - patch @@ -1341,16 +1526,28 @@ rules: - apiGroups: - olm.operatorframework.io resources: + - clusterextensionrevisions/finalizers - clusterextensions/finalizers verbs: - update - apiGroups: - olm.operatorframework.io resources: + - clusterextensionrevisions/status - clusterextensions/status verbs: - patch - update +- apiGroups: + - olm.operatorframework.io + resources: + - clusterextensions + verbs: + - get + - list + - patch + - update + - watch - apiGroups: - rbac.authorization.k8s.io resources: From 2313eef2e5328d7ab8b30a36d46e4372744078de Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 28 Jul 2025 16:39:52 +0200 Subject: [PATCH 11/17] Remove access manager and dynamic cache Signed-off-by: Per Goncalves da Silva --- cmd/operator-controller/main.go | 69 +++--- .../clusterextensionrevision_controller.go | 108 +-------- ...lusterextensionrevision_controller_test.go | 209 +----------------- 3 files changed, 50 insertions(+), 336 deletions(-) diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 8ef618b34..62aa98ce5 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -34,16 +34,17 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" k8slabels "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" k8stypes "k8s.io/apimachinery/pkg/types" apimachineryrand "k8s.io/apimachinery/pkg/util/rand" "k8s.io/client-go/discovery" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" _ "k8s.io/client-go/plugin/pkg/client/auth" - "k8s.io/client-go/rest" "k8s.io/klog/v2" "k8s.io/utils/ptr" + "pkg.package-operator.run/boxcutter/machinery" "pkg.package-operator.run/boxcutter/managedcache" + "pkg.package-operator.run/boxcutter/ownerhandling" + "pkg.package-operator.run/boxcutter/validation" ctrl "sigs.k8s.io/controller-runtime" crcache "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/certwatcher" @@ -500,55 +501,43 @@ func run() error { if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { // Boxcutter + const ( + boxcutterSystemPrefixFieldOwner = "olm.operatorframework.io" + ) + discoveryClient, err := discovery.NewDiscoveryClientForConfig(restConfig) if err != nil { setupLog.Error(err, "unable to create discovery client") return err } - mapFunc := func(ctx context.Context, ce *ocv1.ClusterExtension, c *rest.Config, o crcache.Options) (*rest.Config, crcache.Options, error) { - saKey := client.ObjectKey{ - Name: ce.Spec.ServiceAccount.Name, - Namespace: ce.Spec.Namespace, - } - saConfig := rest.AnonymousClientConfig(c) - saConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper { - return &authentication.TokenInjectingRoundTripper{ - Tripper: rt, - TokenGetter: tokenGetter, - Key: saKey, - } - }) - // Cache scoping - req1, err := k8slabels.NewRequirement( - controllers.ClusterExtensionRevisionOwnerLabel, selection.Equals, []string{ce.Name}) - if err != nil { - return nil, o, err - } - o.DefaultLabelSelector = k8slabels.NewSelector().Add(*req1) - - return saConfig, o, nil - } - - accessManager := managedcache.NewObjectBoundAccessManager( - ctrl.Log.WithName("accessmanager"), mapFunc, restConfig, crcache.Options{ + trackingCache, err := managedcache.NewTrackingCache( + ctrl.Log.WithName("accessmanager"), + restConfig, + crcache.Options{ Scheme: mgr.GetScheme(), Mapper: mgr.GetRESTMapper(), - }) - if err := mgr.Add(accessManager); err != nil { - setupLog.Error(err, "unable to register AccessManager") - return err + }, + ) + if err != nil { + setupLog.Error(err, "unable to create boxcutter tracking cache") } if err = (&controllers.ClusterExtensionRevisionReconciler{ Client: cl, - RevisionManager: &controllers.OLMRevisionEngineGetter{ - AccessManager: accessManager, - Scheme: mgr.GetScheme(), - RestMapper: mgr.GetRESTMapper(), - DiscoveryClient: discoveryClient, - }, - }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") + RevisionEngine: machinery.NewRevisionEngine( + machinery.NewPhaseEngine( + machinery.NewObjectEngine( + mgr.GetScheme(), trackingCache, mgr.GetClient(), + ownerhandling.NewNative(mgr.GetScheme()), + machinery.NewComparator(ownerhandling.NewNative(mgr.GetScheme()), discoveryClient, mgr.GetScheme(), boxcutterSystemPrefixFieldOwner), + boxcutterSystemPrefixFieldOwner, boxcutterSystemPrefixFieldOwner, + ), + validation.NewClusterPhaseValidator(mgr.GetRESTMapper(), mgr.GetClient()), + ), + validation.NewRevisionValidator(), mgr.GetClient(), + ), + }).SetupWithManager(mgr, trackingCache); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ClusterExtensionRevision") return err } } diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index a6fa2ac16..876a05c80 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -17,14 +17,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/discovery" - "k8s.io/utils/ptr" "pkg.package-operator.run/boxcutter" "pkg.package-operator.run/boxcutter/machinery" machinerytypes "pkg.package-operator.run/boxcutter/machinery/types" - "pkg.package-operator.run/boxcutter/managedcache" - "pkg.package-operator.run/boxcutter/ownerhandling" - "pkg.package-operator.run/boxcutter/validation" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,21 +34,14 @@ import ( const ( ClusterExtensionRevisionOwnerLabel = "olm.operatorframework.io/owner" - boxcutterSystemPrefixFieldOwner = "olm.operatorframework.io" clusterExtensionRevisionTeardownFinalizer = "olm.operatorframework.io/teardown" ) // ClusterExtensionRevisionReconciler actions individual snapshots of ClusterExtensions, // as part of the boxcutter integration. type ClusterExtensionRevisionReconciler struct { - Client client.Client - RevisionManager RevisionManager -} - -type AccessManager interface { - GetWithUser(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) - FreeWithUser(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error - Source(handler.EventHandler, ...predicate.Predicate) source.Source + Client client.Client + RevisionEngine RevisionEngine } type RevisionEngine interface { @@ -61,46 +49,6 @@ type RevisionEngine interface { Reconcile(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) } -type RevisionManager interface { - GetScopedRevisionEngine(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (RevisionEngine, error) - HandleDeletion(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error - Source(handler.EventHandler, ...predicate.Predicate) source.Source -} - -type OLMRevisionEngineGetter struct { - DiscoveryClient discovery.DiscoveryInterface - Scheme *runtime.Scheme - RestMapper meta.RESTMapper - AccessManager AccessManager -} - -func (r *OLMRevisionEngineGetter) GetScopedRevisionEngine(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (RevisionEngine, error) { - accessor, err := r.AccessManager.GetWithUser(ctx, owner, user, usedFor) - if err != nil { - return nil, fmt.Errorf("get cache: %w", err) - } - return machinery.NewRevisionEngine( - machinery.NewPhaseEngine( - machinery.NewObjectEngine( - r.Scheme, accessor, accessor, - ownerhandling.NewNative(r.Scheme), - machinery.NewComparator(ownerhandling.NewNative(r.Scheme), r.DiscoveryClient, r.Scheme, boxcutterSystemPrefixFieldOwner), - boxcutterSystemPrefixFieldOwner, boxcutterSystemPrefixFieldOwner, - ), - validation.NewClusterPhaseValidator(r.RestMapper, accessor), - ), - validation.NewRevisionValidator(), accessor, - ), nil -} - -func (r *OLMRevisionEngineGetter) HandleDeletion(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return r.AccessManager.FreeWithUser(ctx, owner, user) -} - -func (r *OLMRevisionEngineGetter) Source(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source { - return r.AccessManager.Source(eventHandler, p...) -} - //+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 @@ -120,18 +68,7 @@ func (c *ClusterExtensionRevisionReconciler) Reconcile(ctx context.Context, req controller, ok := getControllingClusterExtension(rev) if !ok { - // ClusterExtension revisions can't exist without a ClusterExtension in control. - // This situation can only appear if the ClusterExtension object has been deleted with --cascade=Orphan. - // To not leave unactionable resources on the cluster, we are going to just - // reap the revision reverences and propagate the Orphan deletion. - if rev.DeletionTimestamp.IsZero() { - return ctrl.Result{}, client.IgnoreNotFound( - c.Client.Delete(ctx, rev, client.PropagationPolicy(metav1.DeletePropagationOrphan), client.Preconditions{ - UID: ptr.To(rev.GetUID()), - ResourceVersion: ptr.To(rev.GetResourceVersion()), - }), - ) - } + // TODO: clean up all the deletion logic for the case where orphaned CEV are created for reasons return ctrl.Result{}, c.removeFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer) } @@ -143,47 +80,20 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, ce * revision, opts, previous := toBoxcutterRevision(ce.Name, rev) - var objects []client.Object - for _, phase := range revision.GetPhases() { - for _, pobj := range phase.GetObjects() { - objects = append(objects, &pobj) - } - } - - // THIS IS STUPID, PLEASE FIX! - // Revisions need individual finalizers on the ClusterExtension to prevent its premature deletion. - if rev.DeletionTimestamp.IsZero() && - rev.Spec.LifecycleState != ocv1.ClusterExtensionRevisionLifecycleStateArchived { - // We can't lookup the complete ClusterExtension when it's already deleted. - // This only works when the controller-manager is not restarted during teardown. - if err := c.Client.Get(ctx, client.ObjectKeyFromObject(ce), ce); err != nil { - return ctrl.Result{}, err - } - } - - re, err := c.RevisionManager.GetScopedRevisionEngine(ctx, ce, rev, objects) - if err != nil { - return ctrl.Result{}, err - } - if !rev.DeletionTimestamp.IsZero() || rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived { // // Teardown // - tres, err := re.Teardown(ctx, *revision) + tres, err := c.RevisionEngine.Teardown(ctx, *revision) if err != nil { return ctrl.Result{}, fmt.Errorf("revision teardown: %w", err) } l.Info("teardown report", "report", tres.String()) - if !tres.IsComplete() { return ctrl.Result{}, nil } - if err := c.RevisionManager.HandleDeletion(ctx, ce, rev); err != nil { - return ctrl.Result{}, fmt.Errorf("get cache: %w", err) - } return ctrl.Result{}, c.removeFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer) } @@ -193,7 +103,7 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, ce * if err := c.ensureFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil { return ctrl.Result{}, err } - rres, err := re.Reconcile(ctx, *revision, opts...) + rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...) if err != nil { return ctrl.Result{}, fmt.Errorf("revision reconcile: %w", err) } @@ -294,14 +204,18 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, ce * return ctrl.Result{}, c.Client.Status().Update(ctx, rev) } -func (c *ClusterExtensionRevisionReconciler) SetupWithManager(mgr ctrl.Manager) error { +type Sourcerer interface { + Source(handler handler.EventHandler, predicates ...predicate.Predicate) source.Source +} + +func (c *ClusterExtensionRevisionReconciler) SetupWithManager(mgr ctrl.Manager, sourcerer Sourcerer) error { return ctrl.NewControllerManagedBy(mgr). For( &ocv1.ClusterExtensionRevision{}, builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), ). WatchesRawSource( - c.RevisionManager.Source( + sourcerer.Source( handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &ocv1.ClusterExtensionRevision{}), predicate.ResourceVersionChangedPredicate{}, ), diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go index ccc73811c..d3e24a909 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go @@ -14,19 +14,14 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/discovery" "pkg.package-operator.run/boxcutter" "pkg.package-operator.run/boxcutter/machinery" machinerytypes "pkg.package-operator.run/boxcutter/machinery/types" - "pkg.package-operator.run/boxcutter/managedcache" "pkg.package-operator.run/boxcutter/validation" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/source" ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" @@ -311,13 +306,9 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te // reconcile cluster extension revision result, err := (&controllers.ClusterExtensionRevisionReconciler{ Client: testClient, - RevisionManager: mockRevisionManager{ - getScopedRevisionEngine: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) { - return &mockRevisionEngine{ - reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { - return tc.revisionResult, nil - }, - }, nil + RevisionEngine: &mockRevisionEngine{ + reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return tc.revisionResult, nil }, }, }).Reconcile(t.Context(), ctrl.Request{ @@ -430,13 +421,9 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_ValidationError_Retries(t // reconcile cluster extension revision result, err := (&controllers.ClusterExtensionRevisionReconciler{ Client: testClient, - RevisionManager: mockRevisionManager{ - getScopedRevisionEngine: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) { - return &mockRevisionEngine{ - reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { - return tc.revisionResult, nil - }, - }, nil + RevisionEngine: &mockRevisionEngine{ + reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return tc.revisionResult, nil }, }, }).Reconcile(t.Context(), ctrl.Request{ @@ -468,7 +455,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { revisionResult machinery.RevisionResult revisionEngineTeardownFn func(*testing.T) func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) validate func(*testing.T, client.Client) - handleDeletionFn func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error expectedErr string }{ { @@ -489,50 +475,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { require.NoError(t, err) require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) }, - handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return nil - }, - revisionEngineTeardownFn: func(t *testing.T) func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { - return nil - }, - }, - { - name: "delete with propagation orphan if parent ClusterExtension does not exist", - revisionResult: mockRevisionResult{}, - existingObjs: func() []client.Object { - rev1 := newTestClusterExtensionRevision(clusterExtensionRevisionName) - configMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-configmap", - Namespace: "my-namespace", - }, - Data: map[string]string{ - "some": "data", - }, - } - require.NoError(t, controllerutil.SetControllerReference(rev1, configMap, testScheme)) - return []client.Object{rev1, configMap} - }, - validate: func(t *testing.T, c client.Client) { - t.Log("checking if ClusterExtensionRevision is deleted") - rev := &ocv1.ClusterExtensionRevision{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterExtensionRevisionName, - }, rev) - require.Error(t, err) - require.True(t, errors.IsNotFound(err)) - t.Log("checking if ConfigMap still exists") - cm := &corev1.ConfigMap{} - err = c.Get(t.Context(), client.ObjectKey{ - Name: "my-configmap", - Namespace: "my-namespace", - }, cm) - require.NoError(t, err) - require.NotNil(t, cm) - }, - handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return nil - }, revisionEngineTeardownFn: func(t *testing.T) func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { return nil }, @@ -557,14 +499,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { }, nil } }, - handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - t.Log("check owner is cluster extension and user is cluster extension revision") - require.Equal(t, "test-ext", owner.GetName()) - require.Equal(t, "test-ext-1", user.GetName()) - return nil - } - }, validate: func(t *testing.T, c client.Client) { t.Log("cluster revision is deleted") rev := &ocv1.ClusterExtensionRevision{} @@ -603,9 +537,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { require.NoError(t, err) require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) }, - handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return nil - }, }, { name: "revision is torn down when in archived state and finalizer is removed", @@ -635,14 +566,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { require.NoError(t, err) require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) }, - handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - t.Log("check owner is cluster extension and user is cluster extension revision") - require.Equal(t, "test-ext", owner.GetName()) - require.Equal(t, "test-ext-1", user.GetName()) - return nil - } - }, }, { name: "surfaces revision teardown error when in archived state", @@ -672,9 +595,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { require.NoError(t, err) require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) }, - handleDeletionFn: func(t *testing.T) func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return nil - }, }, } { t.Run(tc.name, func(t *testing.T) { @@ -688,16 +608,11 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { // reconcile cluster extension revision result, err := (&controllers.ClusterExtensionRevisionReconciler{ Client: testClient, - RevisionManager: mockRevisionManager{ - getScopedRevisionEngine: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) { - return &mockRevisionEngine{ - reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { - return tc.revisionResult, nil - }, - teardown: tc.revisionEngineTeardownFn(t), - }, nil + RevisionEngine: &mockRevisionEngine{ + reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return tc.revisionResult, nil }, - handleDeletion: tc.handleDeletionFn(t), + teardown: tc.revisionEngineTeardownFn(t), }, }).Reconcile(t.Context(), ctrl.Request{ NamespacedName: types.NamespacedName{ @@ -719,72 +634,6 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { } } -func Test_OLMRevisionGetter_AccessManager_Integration(t *testing.T) { - expectedCtx := t.Context() - expectedOwner := &ocv1.ClusterExtension{} - expectedUser := &ocv1.ClusterExtensionRevision{} - expectedUsedFor := []client.Object{ - &corev1.ConfigMap{}, - &corev1.ServiceAccount{}, - } - expectedHandler := handler.EnqueueRequestsFromMapFunc(nil) - expectedPredicates := []predicate.Predicate{ - predicate.AnnotationChangedPredicate{}, - predicate.GenerationChangedPredicate{}, - } - - discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) - require.NoError(t, err) - - g := controllers.OLMRevisionEngineGetter{ - DiscoveryClient: discoveryClient, - AccessManager: mockAccessManager{ - getWithUser: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) { - require.Equal(t, expectedCtx, ctx) - require.Equal(t, expectedOwner, owner) - require.Equal(t, expectedUser, user) - require.Equal(t, expectedUsedFor, usedFor) - return nil, nil - }, - freeWithUser: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - require.Equal(t, expectedCtx, ctx) - require.Equal(t, expectedOwner, owner) - require.Equal(t, expectedUser, user) - return nil - }, - sourceFn: func(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source { - require.Equal(t, expectedHandler, eventHandler) - require.Equal(t, expectedPredicates, p) - return nil - }, - }, - } - - _, _ = g.GetScopedRevisionEngine(expectedCtx, expectedOwner, expectedUser, expectedUsedFor) - _ = g.HandleDeletion(expectedCtx, expectedOwner, expectedUser) - _ = g.Source(expectedHandler, expectedPredicates...) -} - -func Test_OLMRevisionGetter_AccessManager_Errors(t *testing.T) { - g := controllers.OLMRevisionEngineGetter{ - AccessManager: mockAccessManager{ - getWithUser: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) { - return nil, fmt.Errorf("some error") - }, - freeWithUser: func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return fmt.Errorf("some error") - }, - }, - } - - _, err := g.GetScopedRevisionEngine(t.Context(), nil, nil, nil) - require.Error(t, err) - require.Contains(t, err.Error(), "some error") - err = g.HandleDeletion(t.Context(), nil, nil) - require.Error(t, err) - require.Contains(t, err.Error(), "some error") -} - func newTestClusterExtension() *ocv1.ClusterExtension { return &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ @@ -836,44 +685,6 @@ func newTestClusterExtensionRevision(name string) *ocv1.ClusterExtensionRevision } } -type mockAccessManager struct { - getWithUser func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) - freeWithUser func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error - sourceFn func(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source -} - -func (m mockAccessManager) GetWithUser(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (managedcache.Accessor, error) { - return m.getWithUser(ctx, owner, user, usedFor) -} - -func (m mockAccessManager) FreeWithUser(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return m.freeWithUser(ctx, owner, user) -} - -func (m mockAccessManager) Source(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source { - return m.sourceFn(eventHandler, p...) -} - -var _ controllers.AccessManager = &mockAccessManager{} - -type mockRevisionManager struct { - getScopedRevisionEngine func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) - handleDeletion func(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error - source func(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source -} - -func (m mockRevisionManager) HandleDeletion(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object) error { - return m.handleDeletion(ctx, owner, user) -} - -func (m mockRevisionManager) Source(eventHandler handler.EventHandler, p ...predicate.Predicate) source.Source { - return m.source(eventHandler, p...) -} - -func (m mockRevisionManager) GetScopedRevisionEngine(ctx context.Context, owner *ocv1.ClusterExtension, user client.Object, usedFor []client.Object) (controllers.RevisionEngine, error) { - return m.getScopedRevisionEngine(ctx, owner, user, usedFor) -} - type mockRevisionEngine struct { teardown func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) reconcile func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) From 3a6304ab121706f688fe02b33fa03214f0dfb847 Mon Sep 17 00:00:00 2001 From: Nico Schieder Date: Thu, 31 Jul 2025 20:30:40 +0200 Subject: [PATCH 12/17] Update boxcutter to v0.3.0, add TrackingCache to Runnables --- cmd/operator-controller/main.go | 5 ++++- go.mod | 16 +++++++--------- go.sum | 11 ++++++----- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 62aa98ce5..02077693f 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -512,7 +512,7 @@ func run() error { } trackingCache, err := managedcache.NewTrackingCache( - ctrl.Log.WithName("accessmanager"), + ctrl.Log.WithName("trackingCache"), restConfig, crcache.Options{ Scheme: mgr.GetScheme(), Mapper: mgr.GetRESTMapper(), @@ -521,6 +521,9 @@ func run() error { if err != nil { setupLog.Error(err, "unable to create boxcutter tracking cache") } + if err := mgr.Add(trackingCache); err != nil { + setupLog.Error(err, "unable to set up tracking cache") + } if err = (&controllers.ClusterExtensionRevisionReconciler{ Client: cl, diff --git a/go.mod b/go.mod index ed780c90e..7e5273c72 100644 --- a/go.mod +++ b/go.mod @@ -32,25 +32,23 @@ require ( golang.org/x/tools v0.35.0 gopkg.in/yaml.v2 v2.4.0 helm.sh/helm/v3 v3.18.4 - k8s.io/api v0.33.2 - k8s.io/apiextensions-apiserver v0.33.2 - k8s.io/apimachinery v0.33.2 + k8s.io/api v0.33.3 + k8s.io/apiextensions-apiserver v0.33.3 + k8s.io/apimachinery v0.33.3 k8s.io/apiserver v0.33.2 k8s.io/cli-runtime v0.33.2 - k8s.io/client-go v0.33.2 + k8s.io/client-go v0.33.3 k8s.io/component-base v0.33.2 k8s.io/klog/v2 v2.130.1 k8s.io/kubernetes v1.33.2 k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 - pkg.package-operator.run/boxcutter v0.1.2 + pkg.package-operator.run/boxcutter v0.3.0 sigs.k8s.io/controller-runtime v0.21.0 sigs.k8s.io/controller-tools v0.18.0 sigs.k8s.io/crdify v0.4.1-0.20250613143457-398e4483fb58 sigs.k8s.io/yaml v1.6.0 ) -replace pkg.package-operator.run/boxcutter => github.com/perdasilva/boxcutter v0.0.0-20250715101157-18ea858f54bd - require ( k8s.io/component-helpers v0.33.2 // indirect k8s.io/kube-openapi v0.0.0-20250610211856-8b98d1ed966a // indirect @@ -104,7 +102,7 @@ require ( github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect github.com/fatih/color v1.18.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect - github.com/fxamacker/cbor/v2 v2.8.0 // indirect + github.com/fxamacker/cbor/v2 v2.9.0 // indirect github.com/go-errors/errors v1.4.2 // indirect github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect github.com/go-git/go-billy/v5 v5.6.2 // indirect @@ -193,7 +191,7 @@ require ( github.com/sirupsen/logrus v1.9.3 // indirect github.com/smallstep/pkcs7 v0.2.1 // indirect github.com/spf13/cast v1.7.1 // indirect - github.com/spf13/pflag v1.0.6 // indirect + github.com/spf13/pflag v1.0.7 // indirect github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6 // indirect github.com/stoewer/go-strcase v1.3.1 // indirect github.com/stretchr/objx v0.5.2 // indirect diff --git a/go.sum b/go.sum index 8f3d61d6e..4f4e50e6b 100644 --- a/go.sum +++ b/go.sum @@ -146,8 +146,8 @@ github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHk github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= -github.com/fxamacker/cbor/v2 v2.8.0 h1:fFtUGXUzXPHTIUdne5+zzMPTfffl3RD5qYnkY40vtxU= -github.com/fxamacker/cbor/v2 v2.8.0/go.mod h1:vM4b+DJCtHn+zz7h3FFp/hDAI9WNWCsZj23V5ytsSxQ= +github.com/fxamacker/cbor/v2 v2.9.0 h1:NpKPmjDBgUfBms6tr6JZkTHtfFGcMKsw3eGcmD/sapM= +github.com/fxamacker/cbor/v2 v2.9.0/go.mod h1:vM4b+DJCtHn+zz7h3FFp/hDAI9WNWCsZj23V5ytsSxQ= github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA= github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI= @@ -380,8 +380,6 @@ github.com/otiai10/copy v1.14.1 h1:5/7E6qsUMBaH5AnQ0sSLzzTg1oTECmcCmT6lvF45Na8= github.com/otiai10/copy v1.14.1/go.mod h1:oQwrEDDOci3IM8dJF0d8+jnbfPDllW6vUjNc3DoZm9I= github.com/otiai10/mint v1.6.3 h1:87qsV/aw1F5as1eH1zS/yqHY85ANKVMgkDrf9rcxbQs= github.com/otiai10/mint v1.6.3/go.mod h1:MJm72SBthJjz8qhefc4z1PYEieWmy8Bku7CjcAqyUSM= -github.com/perdasilva/boxcutter v0.0.0-20250715101157-18ea858f54bd h1:CEXvUTPMkCMX+0q9cqwaFAl51s71qESI1V2wRK/8xsg= -github.com/perdasilva/boxcutter v0.0.0-20250715101157-18ea858f54bd/go.mod h1:74MfxxOWGkveUl9Iv2/01tg/IULlWnHEaVv2dfR4/b8= github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 h1:Ii+DKncOVM8Cu1Hc+ETb5K+23HdAMvESYE3ZJ5b5cMI= @@ -441,8 +439,9 @@ github.com/spf13/cast v1.7.1 h1:cuNEagBQEHWN1FnbGEjCXL2szYEXqfJPbP2HNUaca9Y= github.com/spf13/cast v1.7.1/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo= github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0= -github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.7 h1:vN6T9TfwStFPFM5XzjsvmzZkLuaLX+HS+0SeFLRgU6M= +github.com/spf13/pflag v1.0.7/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6 h1:pnnLyeX7o/5aX8qUQ69P/mLojDqwda8hFOCBTmP/6hw= github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6/go.mod h1:39R/xuhNgVhi+K0/zst4TLrJrVmbm6LVgl4A0+ZFS5M= github.com/stoewer/go-strcase v1.3.1 h1:iS0MdW+kVTxgMoE1LAZyMiYJFKlOzLooE4MxjirtkAs= @@ -761,6 +760,8 @@ k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 h1:hwvWFiBzdWw1FhfY1FooPn3kzWuJ8 k8s.io/utils v0.0.0-20250604170112-4c0f3b243397/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go/v2 v2.6.0 h1:X4ELRsiGkrbeox69+9tzTu492FMUu7zJQW6eJU+I2oc= oras.land/oras-go/v2 v2.6.0/go.mod h1:magiQDfG6H1O9APp+rOsvCPcW1GD2MM7vgnKY0Y+u1o= +pkg.package-operator.run/boxcutter v0.3.0 h1:Pkfu6jKi7cLINCM1PzLz1rManawxPQFEGflSouqzeww= +pkg.package-operator.run/boxcutter v0.3.0/go.mod h1:Q0TEZgWu6nAhTSI4NbvKulp0v/sxHoAVfRCQeLHV9v8= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.33.0 h1:qPrZsv1cwQiFeieFlRqT627fVZ+tyfou/+S5S0H5ua0= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.33.0/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw= sigs.k8s.io/controller-runtime v0.21.0 h1:CYfjpEuicjUecRk+KAeyYh+ouUBn4llGyDYytIGcJS8= From b72f9b2b7c258a4656048386e7b9d0ff3c793d67 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Wed, 30 Jul 2025 20:47:59 -0400 Subject: [PATCH 13/17] boxcutter webhook support Signed-off-by: Joe Lanford --- cmd/operator-controller/main.go | 6 +++--- internal/operator-controller/applier/boxcutter.go | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 02077693f..380f5e61a 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -443,7 +443,7 @@ func run() error { // create applier var ctrlBuilderOpts []controllers.ControllerBuilderOption var extApplier controllers.Applier - + certProvider := getCertificateProvider() if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { // TODO: add support for preflight checks // TODO: better scheme handling - which types do we want to support? @@ -454,14 +454,14 @@ func run() error { RevisionGenerator: &applier.SimpleRevisionGenerator{ Scheme: mgr.GetScheme(), BundleRenderer: &applier.RegistryV1BundleRenderer{ - BundleRenderer: registryv1.Renderer, + BundleRenderer: registryv1.Renderer, + CertificateProvider: certProvider, }, }, } ctrlBuilderOpts = append(ctrlBuilderOpts, controllers.WithOwns(&ocv1.ClusterExtensionRevision{})) } else { // now initialize the helmApplier, assigning the potentially nil preAuth - certProvider := getCertificateProvider() extApplier = &applier.Helm{ ActionClientGetter: acg, Preflights: preflights, diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index fe9461b6c..0b864669c 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -223,7 +223,8 @@ type BundleRenderer interface { } type RegistryV1BundleRenderer struct { - BundleRenderer render.BundleRenderer + BundleRenderer render.BundleRenderer + CertificateProvider render.CertificateProvider } func (r *RegistryV1BundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { @@ -231,9 +232,14 @@ func (r *RegistryV1BundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExten if err != nil { return nil, err } + + if len(reg.CSV.Spec.WebhookDefinitions) > 0 && r.CertificateProvider == nil { + return nil, fmt.Errorf("unsupported bundle: webhookDefinitions are not supported") + } + watchNamespace, err := GetWatchNamespace(ext) if err != nil { return nil, err } - return r.BundleRenderer.Render(reg, ext.Spec.Namespace, render.WithTargetNamespaces(watchNamespace)) + return r.BundleRenderer.Render(reg, ext.Spec.Namespace, render.WithTargetNamespaces(watchNamespace), render.WithCertificateProvider(r.CertificateProvider)) } From 2f49013bce3f2289f287c1eb105b7c3fd939e73e Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 31 Jul 2025 16:06:40 -0400 Subject: [PATCH 14/17] add BoxcutterRuntime feature gate to experimental release Signed-off-by: Joe Lanford --- config/components/base/experimental/kustomization.yaml | 1 + manifests/experimental-e2e.yaml | 1 + manifests/experimental.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/config/components/base/experimental/kustomization.yaml b/config/components/base/experimental/kustomization.yaml index ab4eac1f7..f69e0e973 100644 --- a/config/components/base/experimental/kustomization.yaml +++ b/config/components/base/experimental/kustomization.yaml @@ -16,5 +16,6 @@ components: - ../../features/preflight-permissions - ../../features/apiv1-metas-handler - ../../features/helm-chart +- ../../features/boxcutter-runtime # This one is downstream only, so we shant use it # - ../../features/webhook-provider-openshift-serviceca diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 72b11364e..36987b815 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1968,6 +1968,7 @@ spec: - --feature-gates=SingleOwnNamespaceInstallSupport=true - --feature-gates=PreflightPermissions=true - --feature-gates=HelmChartSupport=true + - --feature-gates=BoxcutterRuntime=true - --catalogd-cas-dir=/var/certs - --pull-cas-dir=/var/certs - --tls-cert=/var/certs/tls.cert diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 0f7ebe23c..37c281043 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1934,6 +1934,7 @@ spec: - --feature-gates=SingleOwnNamespaceInstallSupport=true - --feature-gates=PreflightPermissions=true - --feature-gates=HelmChartSupport=true + - --feature-gates=BoxcutterRuntime=true - --catalogd-cas-dir=/var/certs - --pull-cas-dir=/var/certs - --tls-cert=/var/certs/tls.cert From 294dd8c6e6a271af5d6e5993576ceecc5a841e6d Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 31 Jul 2025 17:29:21 -0400 Subject: [PATCH 15/17] add boxcutter cluster-admin cluster role binding in boxcutter's feature component Signed-off-by: Joe Lanford --- .../boxcutter-runtime/cluster_role_binding.yaml | 12 ++++++++++++ .../features/boxcutter-runtime/kustomization.yaml | 2 ++ manifests/experimental-e2e.yaml | 15 +++++++++++++++ manifests/experimental.yaml | 15 +++++++++++++++ 4 files changed, 44 insertions(+) create mode 100644 config/components/features/boxcutter-runtime/cluster_role_binding.yaml diff --git a/config/components/features/boxcutter-runtime/cluster_role_binding.yaml b/config/components/features/boxcutter-runtime/cluster_role_binding.yaml new file mode 100644 index 000000000..e4a77f41f --- /dev/null +++ b/config/components/features/boxcutter-runtime/cluster_role_binding.yaml @@ -0,0 +1,12 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: operator-controller-boxcutter-cluster-admin +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: cluster-admin +subjects: + - kind: ServiceAccount + name: operator-controller-controller-manager + namespace: olmv1-system \ No newline at end of file diff --git a/config/components/features/boxcutter-runtime/kustomization.yaml b/config/components/features/boxcutter-runtime/kustomization.yaml index d075a1121..bb8922d09 100644 --- a/config/components/features/boxcutter-runtime/kustomization.yaml +++ b/config/components/features/boxcutter-runtime/kustomization.yaml @@ -2,6 +2,8 @@ --- apiVersion: kustomize.config.k8s.io/v1alpha1 kind: Component +resources: + - cluster_role_binding.yaml patches: - target: kind: Deployment diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 36987b815..26e3905f9 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1699,6 +1699,21 @@ subjects: --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding +metadata: + annotations: + olm.operatorframework.io/feature-set: experimental + name: operator-controller-boxcutter-cluster-admin +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: cluster-admin +subjects: +- kind: ServiceAccount + name: operator-controller-controller-manager + namespace: olmv1-system +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding metadata: annotations: olm.operatorframework.io/feature-set: experimental diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 37c281043..1b66bd2b7 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1699,6 +1699,21 @@ subjects: --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding +metadata: + annotations: + olm.operatorframework.io/feature-set: experimental + name: operator-controller-boxcutter-cluster-admin +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: cluster-admin +subjects: +- kind: ServiceAccount + name: operator-controller-controller-manager + namespace: olmv1-system +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding metadata: annotations: olm.operatorframework.io/feature-set: experimental From bfeb343b0b6b3bb40527755d1e367da2920e63c1 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Thu, 31 Jul 2025 14:18:18 -0400 Subject: [PATCH 16/17] Boxcutter Preflight Signed-off-by: Todd Short --- cmd/operator-controller/main.go | 4 +- .../operator-controller/applier/boxcutter.go | 41 ++++++++-- internal/operator-controller/applier/helm.go | 76 +++++++------------ .../operator-controller/applier/helm_test.go | 61 +++++++++------ .../operator-controller/applier/preflight.go | 54 +++++++++++++ .../crdupgradesafety/crdupgradesafety.go | 23 ++---- .../crdupgradesafety/crdupgradesafety_test.go | 11 ++- 7 files changed, 171 insertions(+), 99 deletions(-) create mode 100644 internal/operator-controller/applier/preflight.go diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 380f5e61a..afd0b84c9 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -458,6 +458,7 @@ func run() error { CertificateProvider: certProvider, }, }, + Preflights: preflights, } ctrlBuilderOpts = append(ctrlBuilderOpts, controllers.WithOwns(&ocv1.ClusterExtensionRevision{})) } else { @@ -470,7 +471,8 @@ func run() error { CertificateProvider: certProvider, IsWebhookSupportEnabled: certProvider != nil, }, - PreAuthorizer: preAuth, + PreAuthorizer: preAuth, + HelmReleaseToObjectsConverter: &applier.HelmReleaseToObjectsConverter{}, } } diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 0b864669c..f55c566e9 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -97,6 +97,7 @@ type Boxcutter struct { Client client.Client Scheme *runtime.Scheme RevisionGenerator ClusterExtensionRevisionGenerator + Preflights []Preflight } func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, storageLabels map[string]string) ([]client.Object, string, error) { @@ -104,6 +105,16 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust return objs, "", err } +func (bc *Boxcutter) getObjects(rev *ocv1.ClusterExtensionRevision) []client.Object { + var objs []client.Object + for _, phase := range rev.Spec.Phases { + for _, phaseObject := range phase.Objects { + objs = append(objs, &phaseObject.Object) + } + } + return objs +} + func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, _ map[string]string) ([]client.Object, error) { // Generate desired revision desiredRevision, err := bc.RevisionGenerator.GenerateRevision(contentFS, ext, objectLabels) @@ -122,6 +133,7 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust var ( currentRevision *ocv1.ClusterExtensionRevision ) + state := StateNeedsInstall if len(existingRevisions) > 0 { maybeCurrentRevision := existingRevisions[len(existingRevisions)-1] annotations := maybeCurrentRevision.GetAnnotations() @@ -130,6 +142,27 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust currentRevision = &maybeCurrentRevision } } + state = StateNeedsUpgrade + } + + // Preflights + plainObjs := bc.getObjects(desiredRevision) + for _, preflight := range bc.Preflights { + if shouldSkipPreflight(ctx, preflight, ext, state) { + continue + } + switch state { + case StateNeedsInstall: + err := preflight.Install(ctx, plainObjs) + if err != nil { + return nil, err + } + case StateNeedsUpgrade: + err := preflight.Upgrade(ctx, plainObjs) + if err != nil { + return nil, err + } + } } if currentRevision == nil { @@ -164,13 +197,7 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust // TODO: Read status from revision. // Collect objects - var plain []client.Object - for _, phase := range desiredRevision.Spec.Phases { - for _, phaseObject := range phase.Objects { - plain = append(plain, &phaseObject.Object) - } - } - return plain, nil + return bc.getObjects(desiredRevision), nil } // getExistingRevisions returns the list of ClusterExtensionRevisions for a ClusterExtension with name extName in revision order (oldest to newest) diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index ecfb3fdc2..d47bef437 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -20,7 +20,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" apimachyaml "k8s.io/apimachinery/pkg/util/yaml" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" @@ -28,63 +27,40 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" "github.com/operator-framework/operator-controller/internal/operator-controller/features" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" ) -const ( - StateNeedsInstall string = "NeedsInstall" - StateNeedsUpgrade string = "NeedsUpgrade" - StateUnchanged string = "Unchanged" - StateError string = "Error" - maxHelmReleaseHistory = 10 -) - -// Preflight is a check that should be run before making any changes to the cluster -type Preflight interface { - // Install runs checks that should be successful prior - // to installing the Helm release. It is provided - // a Helm release and returns an error if the - // check is unsuccessful - Install(context.Context, *release.Release) error - - // Upgrade runs checks that should be successful prior - // to upgrading the Helm release. It is provided - // a Helm release and returns an error if the - // check is unsuccessful - Upgrade(context.Context, *release.Release) error -} - type BundleToHelmChartConverter interface { ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) } -type Helm struct { - ActionClientGetter helmclient.ActionClientGetter - Preflights []Preflight - PreAuthorizer authorization.PreAuthorizer - BundleToHelmChartConverter BundleToHelmChartConverter +type HelmReleaseToObjectsConverter struct { + Mock bool } -// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND -// if it is set to enforcement None. -func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.ClusterExtension, state string) bool { - l := log.FromContext(ctx) - hasCRDUpgradeSafety := ext.Spec.Install != nil && ext.Spec.Install.Preflight != nil && ext.Spec.Install.Preflight.CRDUpgradeSafety != nil - _, isCRDUpgradeSafetyInstance := preflight.(*crdupgradesafety.Preflight) +type HelmReleaseToObjectsConverterInterface interface { + GetObjectsFromRelease(rel *release.Release) ([]client.Object, error) +} - if hasCRDUpgradeSafety && isCRDUpgradeSafetyInstance { - if state == StateNeedsInstall || state == StateNeedsUpgrade { - l.Info("crdUpgradeSafety ", "policy", ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement) - } - if ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement == ocv1.CRDUpgradeSafetyEnforcementNone { - // Skip this preflight check because it is of type *crdupgradesafety.Preflight and the CRD Upgrade Safety - // policy is set to None - return true - } +func (h HelmReleaseToObjectsConverter) GetObjectsFromRelease(rel *release.Release) ([]client.Object, error) { + if rel == nil || h.Mock { + return nil, nil + } + + relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name)) + if err != nil { + return nil, fmt.Errorf("parsing release %q objects: %w", rel.Name, err) } - return false + return relObjects, nil +} + +type Helm struct { + ActionClientGetter helmclient.ActionClientGetter + Preflights []Preflight + PreAuthorizer authorization.PreAuthorizer + BundleToHelmChartConverter BundleToHelmChartConverter + HelmReleaseToObjectsConverter HelmReleaseToObjectsConverterInterface } // runPreAuthorizationChecks performs pre-authorization checks for a Helm release @@ -149,6 +125,10 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte if err != nil { return nil, "", fmt.Errorf("failed to get release state using server-side dry-run: %w", err) } + objs, err := h.HelmReleaseToObjectsConverter.GetObjectsFromRelease(desiredRel) + if err != nil { + return nil, state, err + } for _, preflight := range h.Preflights { if shouldSkipPreflight(ctx, preflight, ext, state) { @@ -156,12 +136,12 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte } switch state { case StateNeedsInstall: - err := preflight.Install(ctx, desiredRel) + err := preflight.Install(ctx, objs) if err != nil { return nil, state, err } case StateNeedsUpgrade: - err := preflight.Upgrade(ctx, desiredRel) + err := preflight.Upgrade(ctx, objs) if err != nil { return nil, state, err } diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 89c94df88..2ab2a2d6a 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -48,11 +48,11 @@ func (p *mockPreAuthorizer) PreAuthorize( return p.missingRules, p.returnError } -func (mp *mockPreflight) Install(context.Context, *release.Release) error { +func (mp *mockPreflight) Install(context.Context, []client.Object) error { return mp.installErr } -func (mp *mockPreflight) Upgrade(context.Context, *release.Release) error { +func (mp *mockPreflight) Upgrade(context.Context, []client.Object) error { return mp.upgradeErr } @@ -248,9 +248,10 @@ func TestApply_Installation(t *testing.T) { } mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -266,8 +267,9 @@ func TestApply_Installation(t *testing.T) { installErr: errors.New("failed installing chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -286,8 +288,9 @@ func TestApply_Installation(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -328,10 +331,11 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { } mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + PreAuthorizer: &mockPreAuthorizer{nil, nil}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -408,9 +412,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + PreAuthorizer: &mockPreAuthorizer{nil, nil}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } // Use a ClusterExtension with valid Spec fields. @@ -464,9 +469,10 @@ func TestApply_Upgrade(t *testing.T) { } mockPf := &mockPreflight{upgradeErr: errors.New("failed during upgrade pre-flight check")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -488,7 +494,8 @@ func TestApply_Upgrade(t *testing.T) { mockPf := &mockPreflight{} helmApplier := applier.Helm{ ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -509,9 +516,10 @@ func TestApply_Upgrade(t *testing.T) { } mockPf := &mockPreflight{} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -530,8 +538,9 @@ func TestApply_Upgrade(t *testing.T) { desiredRel: &testDesiredRelease, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -562,6 +571,7 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin return nil, nil }, }, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } testExt := &ocv1.ClusterExtension{ @@ -595,6 +605,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { return nil, nil }, }, + HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, } _, _, _ = helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) diff --git a/internal/operator-controller/applier/preflight.go b/internal/operator-controller/applier/preflight.go new file mode 100644 index 000000000..7b5d91e27 --- /dev/null +++ b/internal/operator-controller/applier/preflight.go @@ -0,0 +1,54 @@ +package applier + +import ( + "context" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety" +) + +const ( + StateNeedsInstall string = "NeedsInstall" + StateNeedsUpgrade string = "NeedsUpgrade" + StateUnchanged string = "Unchanged" + StateError string = "Error" + maxHelmReleaseHistory = 10 +) + +// Preflight is a check that should be run before making any changes to the cluster +type Preflight interface { + // Install runs checks that should be successful prior + // to installing the Helm release. It is provided + // a Helm release and returns an error if the + // check is unsuccessful + Install(context.Context, []client.Object) error + + // Upgrade runs checks that should be successful prior + // to upgrading the Helm release. It is provided + // a Helm release and returns an error if the + // check is unsuccessful + Upgrade(context.Context, []client.Object) error +} + +// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND +// if it is set to enforcement None. +func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.ClusterExtension, state string) bool { + l := log.FromContext(ctx) + hasCRDUpgradeSafety := ext.Spec.Install != nil && ext.Spec.Install.Preflight != nil && ext.Spec.Install.Preflight.CRDUpgradeSafety != nil + _, isCRDUpgradeSafetyInstance := preflight.(*crdupgradesafety.Preflight) + + if hasCRDUpgradeSafety && isCRDUpgradeSafetyInstance { + if state == StateNeedsInstall || state == StateNeedsUpgrade { + l.Info("crdUpgradeSafety ", "policy", ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement) + } + if ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement == ocv1.CRDUpgradeSafetyEnforcementNone { + // Skip this preflight check because it is of type *crdupgradesafety.Preflight and the CRD Upgrade Safety + // policy is set to None + return true + } + } + return false +} diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go index fadc85873..8c3824eca 100644 --- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go @@ -4,19 +4,16 @@ import ( "context" "errors" "fmt" - "strings" - "helm.sh/helm/v3/pkg/release" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/crdify/pkg/config" "sigs.k8s.io/crdify/pkg/runner" "sigs.k8s.io/crdify/pkg/validations" - - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) type Option func(p *Preflight) @@ -53,24 +50,18 @@ func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface return p } -func (p *Preflight) Install(ctx context.Context, rel *release.Release) error { - return p.runPreflight(ctx, rel) +func (p *Preflight) Install(ctx context.Context, objs []client.Object) error { + return p.runPreflight(ctx, objs) } -func (p *Preflight) Upgrade(ctx context.Context, rel *release.Release) error { - return p.runPreflight(ctx, rel) +func (p *Preflight) Upgrade(ctx context.Context, objs []client.Object) error { + return p.runPreflight(ctx, objs) } -func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release) error { - if rel == nil { +func (p *Preflight) runPreflight(ctx context.Context, relObjects []client.Object) error { + if len(relObjects) == 0 { return nil } - - relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name)) - if err != nil { - return fmt.Errorf("parsing release %q objects: %w", rel.Name, err) - } - runner, err := runner.New(p.config, p.registry) if err != nil { return fmt.Errorf("creating CRD validation runner: %w", err) diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go index 73db9673b..ba065dd2b 100644 --- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go @@ -16,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) @@ -186,7 +187,10 @@ func TestInstall(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { preflight := newMockPreflight(getCrdFromManifestFile(t, tc.oldCrdPath), tc.wantCrdGetErr) - err := preflight.Install(context.Background(), tc.release) + objs, err := applier.HelmReleaseToObjectsConverter{}.GetObjectsFromRelease(tc.release) + if err == nil { + err = preflight.Install(context.Background(), objs) + } if len(tc.wantErrMsgs) != 0 { for _, expectedErrMsg := range tc.wantErrMsgs { require.ErrorContains(t, err, expectedErrMsg) @@ -335,7 +339,10 @@ func TestUpgrade(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { preflight := newMockPreflight(getCrdFromManifestFile(t, tc.oldCrdPath), tc.wantCrdGetErr) - err := preflight.Upgrade(context.Background(), tc.release) + objs, err := applier.HelmReleaseToObjectsConverter{}.GetObjectsFromRelease(tc.release) + if err == nil { + err = preflight.Upgrade(context.Background(), objs) + } if len(tc.wantErrMsgs) != 0 { for _, expectedErrMsg := range tc.wantErrMsgs { require.ErrorContains(t, err, expectedErrMsg) From 7ba6e02be6dbccc83e8cc0f73ae70ed6dfa3d447 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 1 Aug 2025 16:22:58 -0400 Subject: [PATCH 17/17] Boxcutter Preflight mock cleanup Signed-off-by: Todd Short --- internal/operator-controller/applier/helm.go | 3 +- .../operator-controller/applier/helm_test.go | 29 ++++++++++++------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index d47bef437..2b411571a 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -36,7 +36,6 @@ type BundleToHelmChartConverter interface { } type HelmReleaseToObjectsConverter struct { - Mock bool } type HelmReleaseToObjectsConverterInterface interface { @@ -44,7 +43,7 @@ type HelmReleaseToObjectsConverterInterface interface { } func (h HelmReleaseToObjectsConverter) GetObjectsFromRelease(rel *release.Release) ([]client.Object, error) { - if rel == nil || h.Mock { + if rel == nil { return nil, nil } diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 2ab2a2d6a..6ebaad530 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -56,6 +56,13 @@ func (mp *mockPreflight) Upgrade(context.Context, []client.Object) error { return mp.upgradeErr } +type mockHelmReleaseToObjectsConverter struct { +} + +func (mockHelmReleaseToObjectsConverter) GetObjectsFromRelease(*release.Release) ([]client.Object, error) { + return nil, nil +} + type mockActionGetter struct { actionClientForErr error getClientErr error @@ -251,7 +258,7 @@ func TestApply_Installation(t *testing.T) { ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -269,7 +276,7 @@ func TestApply_Installation(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -290,7 +297,7 @@ func TestApply_Installation(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -335,7 +342,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { Preflights: []applier.Preflight{mockPf}, PreAuthorizer: &mockPreAuthorizer{nil, nil}, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -415,7 +422,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { ActionClientGetter: mockAcg, PreAuthorizer: &mockPreAuthorizer{nil, nil}, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } // Use a ClusterExtension with valid Spec fields. @@ -472,7 +479,7 @@ func TestApply_Upgrade(t *testing.T) { ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -495,7 +502,7 @@ func TestApply_Upgrade(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -519,7 +526,7 @@ func TestApply_Upgrade(t *testing.T) { ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -540,7 +547,7 @@ func TestApply_Upgrade(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -571,7 +578,7 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin return nil, nil }, }, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } testExt := &ocv1.ClusterExtension{ @@ -605,7 +612,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { return nil, nil }, }, - HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true}, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } _, _, _ = helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)