From 8d7de9ad4d8e8e9d882fc2b6cfde1c26c5adcef0 Mon Sep 17 00:00:00 2001 From: Nico Schieder Date: Wed, 27 Aug 2025 16:01:27 +0200 Subject: [PATCH] Add migration from helm to boxcutter revision --- api/v1/clusterextensionrevision_types.go | 45 +++- api/v1/zz_generated.deepcopy.go | 5 - cmd/operator-controller/main.go | 46 +++- ...ramework.io_clusterextensionrevisions.yaml | 6 + .../operator-controller/applier/boxcutter.go | 141 +++++++++++-- .../applier/boxcutter_test.go | 198 +++++++++++++++++- .../clusterextension_controller.go | 65 +++--- .../clusterextensionrevision_controller.go | 51 +++-- internal/shared/util/testutils/artifacts.go | 39 +++- manifests/experimental-e2e.yaml | 6 + manifests/experimental.yaml | 6 + 11 files changed, 529 insertions(+), 79 deletions(-) diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 69802de34f..55fb4e726a 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -44,16 +44,28 @@ const ( // 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"` + // Revision number orders changes over time, must always be previous revision +1. + // // +kubebuilder:validation:Required // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="revision is immutable" Revision int64 `json:"revision"` + // Phases are groups of objects that will be applied at the same time. + // All objects in the a phase will have to pass their probes in order to progress to the next phase. + // // +kubebuilder:validation:Required // +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf.size() == 0", message="phases is immutable" + // +patchMergeKey=name + // +patchStrategy=merge + // +listType=map + // +listMapKey=name Phases []ClusterExtensionRevisionPhase `json:"phases"` + // Previous references previous revisions that objects can be adopted from. + // // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="previous is immutable" Previous []ClusterExtensionRevisionPrevious `json:"previous,omitempty"` } @@ -73,18 +85,47 @@ const ( ClusterExtensionRevisionLifecycleStateArchived ClusterExtensionRevisionLifecycleState = "Archived" ) +// ClusterExtensionRevisionPhase are groups of objects that will be applied at the same time. +// All objects in the a phase will have to pass their probes in order to progress to the next phase. type ClusterExtensionRevisionPhase struct { - Name string `json:"name"` + // Name identifies this phase. + // + // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:Pattern=`^[a-z]([-a-z0-9]*[a-z0-9])?$` + Name string `json:"name"` + // Objects are a list of all the objects within this phase. Objects []ClusterExtensionRevisionObject `json:"objects"` - Slices []string `json:"slices,omitempty"` } +// ClusterExtensionRevisionObject contains an object and settings for it. type ClusterExtensionRevisionObject struct { // +kubebuilder:validation:EmbeddedResource // +kubebuilder:pruning:PreserveUnknownFields Object unstructured.Unstructured `json:"object"` + // CollisionProtection controls whether OLM can adopt and modify objects + // already existing on the cluster or even owned by another controller. + // + // +kubebuilder:default="Prevent" + CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"` } +// CollisionProtection specifies if and how ownership collisions are prevented. +type CollisionProtection string + +const ( + // CollisionProtectionPrevent prevents owner collisions entirely + // by only allowing to work with objects itself has created. + CollisionProtectionPrevent CollisionProtection = "Prevent" + // CollisionProtectionIfNoController allows to patch and override + // objects already present if they are not owned by another controller. + CollisionProtectionIfNoController CollisionProtection = "IfNoController" + // CollisionProtectionNone allows to patch and override objects + // already present and owned by other controllers. + // Be careful! This setting may cause multiple controllers to fight over a resource, + // causing load on the API server and etcd. + CollisionProtectionNone CollisionProtection = "None" +) + type ClusterExtensionRevisionPrevious struct { // +kubebuilder:validation:Required Name string `json:"name"` diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index e9dd7744b3..e13f1532b0 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -424,11 +424,6 @@ func (in *ClusterExtensionRevisionPhase) DeepCopyInto(out *ClusterExtensionRevis (*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. diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 243c7dfaa6..e52e2cb6c7 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -489,22 +489,50 @@ func getCertificateProvider() render.CertificateProvider { func setupBoxcutter(mgr manager.Manager, ceReconciler *controllers.ClusterExtensionReconciler, preflights []applier.Preflight) error { certProvider := getCertificateProvider() + coreClient, err := corev1client.NewForConfig(mgr.GetConfig()) + if err != nil { + return fmt.Errorf("unable to create core client: %w", err) + } + cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), + helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, mgr.GetAPIReader(), cfg.systemNamespace)), + helmclient.ClientNamespaceMapper(func(obj client.Object) (string, error) { + ext := obj.(*ocv1.ClusterExtension) + return ext.Spec.Namespace, nil + }), + ) + if err != nil { + return fmt.Errorf("unable to create helm action config getter: %w", err) + } + + acg, err := action.NewWrappedActionClientGetter(cfgGetter, + helmclient.WithFailureRollbacks(false), + ) + if err != nil { + return fmt.Errorf("unable to create helm action client getter: %w", err) + } + // TODO: add support for preflight checks // TODO: better scheme handling - which types do we want to support? _ = apiextensionsv1.AddToScheme(mgr.GetScheme()) - ceReconciler.Applier = &applier.Boxcutter{ - Client: mgr.GetClient(), + rg := &applier.SimpleRevisionGenerator{ Scheme: mgr.GetScheme(), - RevisionGenerator: &applier.SimpleRevisionGenerator{ - Scheme: mgr.GetScheme(), - BundleRenderer: &applier.RegistryV1BundleRenderer{ - BundleRenderer: registryv1.Renderer, - CertificateProvider: certProvider, - }, + BundleRenderer: &applier.RegistryV1BundleRenderer{ + BundleRenderer: registryv1.Renderer, + CertificateProvider: certProvider, }, - Preflights: preflights, + } + ceReconciler.Applier = &applier.Boxcutter{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + RevisionGenerator: rg, + Preflights: preflights, } ceReconciler.RevisionStatesGetter = &controllers.BoxcutterRevisionStatesGetter{Reader: mgr.GetClient()} + ceReconciler.StorageMigrator = &applier.BoxcutterStorageMigrator{ + Client: mgr.GetClient(), + ActionClientGetter: acg, + RevisionGenerator: rg, + } // Boxcutter const ( 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 index 4eba986335..6947054bcf 100644 --- a/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml +++ b/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml @@ -62,11 +62,17 @@ spec: objects: items: properties: + collisionProtection: + default: Prevent + description: CollisionProtection specifies if and how + ownership collisions are prevented. + type: string object: type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true required: + - collisionProtection - object type: object type: array diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 116e565f8d..74f5574eea 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -11,8 +11,11 @@ import ( "io/fs" "maps" "slices" + "strings" "github.com/davecgh/go-spew/spew" + "helm.sh/helm/v3/pkg/release" + "helm.sh/helm/v3/pkg/storage/driver" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -20,9 +23,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/yaml" + + helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" + "github.com/operator-framework/operator-controller/internal/operator-controller/labels" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" ) @@ -33,6 +40,10 @@ const ( type ClusterExtensionRevisionGenerator interface { GenerateRevision(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1.ClusterExtensionRevision, error) + GenerateRevisionFromHelmRelease( + helmRelease *release.Release, ext *ocv1.ClusterExtension, + objectLabels map[string]string, + ) (*ocv1.ClusterExtensionRevision, error) } type SimpleRevisionGenerator struct { @@ -40,7 +51,47 @@ type SimpleRevisionGenerator struct { BundleRenderer BundleRenderer } -func (r *SimpleRevisionGenerator) GenerateRevision(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1.ClusterExtensionRevision, error) { +func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease( + helmRelease *release.Release, ext *ocv1.ClusterExtension, + objectLabels map[string]string, +) (*ocv1.ClusterExtensionRevision, error) { + docs := splitManifestDocuments(helmRelease.Manifest) + objs := make([]ocv1.ClusterExtensionRevisionObject, 0, len(docs)) + for _, doc := range docs { + obj := unstructured.Unstructured{} + if err := yaml.Unmarshal([]byte(doc), &obj); err != nil { + return nil, err + } + + labels := maps.Clone(obj.GetLabels()) + if labels == nil { + labels = map[string]string{} + } + maps.Copy(labels, objectLabels) + obj.SetLabels(labels) + obj.SetOwnerReferences(nil) // reset OwnerReferences for migration. + + objs = append(objs, ocv1.ClusterExtensionRevisionObject{ + Object: obj, + CollisionProtection: ocv1.CollisionProtectionNone, // allow to adopt objects from previous release + }) + } + + rev := r.buildClusterExtensionRevision(objs, ext, map[string]string{ + labels.BundleNameKey: helmRelease.Labels[labels.BundleNameKey], + labels.PackageNameKey: helmRelease.Labels[labels.PackageNameKey], + labels.BundleVersionKey: helmRelease.Labels[labels.BundleVersionKey], + labels.BundleReferenceKey: helmRelease.Labels[labels.BundleReferenceKey], + }) + rev.Name = fmt.Sprintf("%s-1", ext.Name) + rev.Spec.Revision = 1 + return rev, nil +} + +func (r *SimpleRevisionGenerator) GenerateRevision( + bundleFS fs.FS, ext *ocv1.ClusterExtension, + objectLabels, revisionAnnotations map[string]string, +) (*ocv1.ClusterExtensionRevision, error) { // extract plain manifests plain, err := r.BundleRenderer.Render(bundleFS, ext) if err != nil { @@ -50,14 +101,12 @@ func (r *SimpleRevisionGenerator) GenerateRevision(bundleFS fs.FS, ext *ocv1.Clu // 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) + 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 { @@ -80,18 +129,73 @@ func (r *SimpleRevisionGenerator) GenerateRevision(bundleFS fs.FS, ext *ocv1.Clu revisionAnnotations = map[string]string{} } - // Build desired revision + return r.buildClusterExtensionRevision(objs, ext, revisionAnnotations), nil +} + +func (r *SimpleRevisionGenerator) buildClusterExtensionRevision( + objects []ocv1.ClusterExtensionRevisionObject, + ext *ocv1.ClusterExtension, + annotations map[string]string, +) *ocv1.ClusterExtensionRevision { return &ocv1.ClusterExtensionRevision{ ObjectMeta: metav1.ObjectMeta{ - Annotations: revisionAnnotations, + Annotations: annotations, Labels: map[string]string{ controllers.ClusterExtensionRevisionOwnerLabel: ext.Name, }, }, Spec: ocv1.ClusterExtensionRevisionSpec{ - Phases: PhaseSort(objs), + Phases: PhaseSort(objects), }, - }, nil + } +} + +type BoxcutterStorageMigrator struct { + ActionClientGetter helmclient.ActionClientGetter + RevisionGenerator ClusterExtensionRevisionGenerator + Client boxcutterStorageMigratorClient +} + +type boxcutterStorageMigratorClient interface { + List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error + Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error +} + +func (m *BoxcutterStorageMigrator) Migrate(ctx context.Context, ext *ocv1.ClusterExtension, objectLabels map[string]string) error { + existingRevisionList := ocv1.ClusterExtensionRevisionList{} + if err := m.Client.List(ctx, &existingRevisionList, client.MatchingLabels{ + controllers.ClusterExtensionRevisionOwnerLabel: ext.Name, + }); err != nil { + return fmt.Errorf("listing ClusterExtensionRevisions before attempting migration: %w", err) + } + if len(existingRevisionList.Items) != 0 { + // No migration needed. + return nil + } + + ac, err := m.ActionClientGetter.ActionClientFor(ctx, ext) + if err != nil { + return err + } + + helmRelease, err := ac.Get(ext.GetName()) + if errors.Is(err, driver.ErrReleaseNotFound) { + // no Helm Release -> no prior installation. + return nil + } + if err != nil { + return err + } + + rev, err := m.RevisionGenerator.GenerateRevisionFromHelmRelease(helmRelease, ext, objectLabels) + if err != nil { + return err + } + + if err := m.Client.Create(ctx, rev); err != nil { + return err + } + return nil } type Boxcutter struct { @@ -288,3 +392,16 @@ func (r *RegistryV1BundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExten } return r.BundleRenderer.Render(reg, ext.Spec.Namespace, render.WithTargetNamespaces(watchNamespace), render.WithCertificateProvider(r.CertificateProvider)) } + +func splitManifestDocuments(file string) []string { + //nolint:prealloc + var docs []string + for _, manifest := range strings.Split(file, "\n") { + manifest = strings.TrimSpace(manifest) + if len(manifest) == 0 { + continue + } + docs = append(docs, manifest) + } + return docs +} diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 3169c4fc0f..92d2ea2ffc 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -9,7 +9,10 @@ import ( "testing/fstest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "helm.sh/helm/v3/pkg/release" + "helm.sh/helm/v3/pkg/storage/driver" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -23,6 +26,7 @@ import ( 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/labels" "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" @@ -81,7 +85,88 @@ func Test_RegistryV1BundleRenderer_Render_Failure(t *testing.T) { require.Contains(t, err.Error(), "some-error") } -func Test_SimpleRevisionGenerator_Success(t *testing.T) { +func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) { + g := &applier.SimpleRevisionGenerator{} + + helmRelease := &release.Release{ + Name: "test-123", + Manifest: `{"apiVersion":"v1","kind":"ConfigMap"}` + "\n" + `{"apiVersion":"v1","kind":"Secret"}` + "\n", + Labels: map[string]string{ + labels.BundleNameKey: "my-bundle", + labels.PackageNameKey: "my-package", + labels.BundleVersionKey: "1.2.0", + labels.BundleReferenceKey: "bundle-ref", + }, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-123", + }, + } + + objectLabels := map[string]string{ + "my-label": "my-value", + } + + rev, err := g.GenerateRevisionFromHelmRelease(helmRelease, ext, objectLabels) + require.NoError(t, err) + + assert.Equal(t, &ocv1.ClusterExtensionRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-123-1", + Annotations: map[string]string{ + "olm.operatorframework.io/bundle-name": "my-bundle", + "olm.operatorframework.io/bundle-reference": "bundle-ref", + "olm.operatorframework.io/bundle-version": "1.2.0", + "olm.operatorframework.io/package-name": "my-package", + }, + Labels: map[string]string{ + "olm.operatorframework.io/owner": "test-123", + }, + }, + Spec: ocv1.ClusterExtensionRevisionSpec{ + Revision: 1, + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "deploy", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "my-label": "my-value", + }, + }, + }, + }, + CollisionProtection: ocv1.CollisionProtectionNone, + }, + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "my-label": "my-value", + }, + }, + }, + }, + CollisionProtection: ocv1.CollisionProtectionNone, + }, + }, + }, + }, + }, + }, rev) +} + +func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { var r mockBundleRenderer = func(_ fs.FS, _ *ocv1.ClusterExtension) ([]client.Object, error) { return []client.Object{ &corev1.Service{ @@ -268,7 +353,7 @@ func TestBoxcutter_Apply(t *testing.T) { UID: "test-uid", }, } - defaultDesiredHash := "faaeb52a1cb7c968c96278bc1cd804e50d3ae9faae08807c9279a5e569933ea0" + defaultDesiredHash := "705ada5296ab26f74d94bfa497295a0cbccdb140623bbe704a3506cd1dfba4eb" defaultDesiredRevision := &ocv1.ClusterExtensionRevision{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ext-1", @@ -460,7 +545,7 @@ func TestBoxcutter_Apply(t *testing.T) { assert.Equal(t, "test-ext-2", newRev.Name) assert.Equal(t, int64(2), newRev.Spec.Revision) - assert.Equal(t, "ec8213d4061a75b55cd67a009d9cdeb1bdd6f503d4b3bb7b6cfea3a5233aad43", newRev.Annotations[applier.RevisionHashAnnotation]) + assert.Equal(t, "9d0e48f6830fce1be5f510eb996f2876719fdb8bcffcfe1dfd3fd60e56316424", 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) @@ -525,6 +610,92 @@ func TestBoxcutter_Apply(t *testing.T) { } } +func TestBoxcutterStorageMigrator(t *testing.T) { + t.Run("creates revision", func(t *testing.T) { + brb := &mockBundleRevisionBuilder{} + mag := &mockActionGetter{} + client := &clientMock{} + sm := &applier.BoxcutterStorageMigrator{ + RevisionGenerator: brb, + ActionClientGetter: mag, + Client: client, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + } + + client. + On("List", mock.Anything, mock.AnythingOfType("*v1.ClusterExtensionRevisionList"), mock.Anything). + Return(nil) + client. + On("Create", mock.Anything, mock.AnythingOfType("*v1.ClusterExtensionRevision"), mock.Anything). + Once(). + Return(nil) + + err := sm.Migrate(t.Context(), ext, map[string]string{"my-label": "my-value"}) + require.NoError(t, err) + + client.AssertExpectations(t) + }) + + t.Run("does not create revision when revisions exist", func(t *testing.T) { + brb := &mockBundleRevisionBuilder{} + mag := &mockActionGetter{} + client := &clientMock{} + sm := &applier.BoxcutterStorageMigrator{ + RevisionGenerator: brb, + ActionClientGetter: mag, + Client: client, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + } + + client. + On("List", mock.Anything, mock.AnythingOfType("*v1.ClusterExtensionRevisionList"), mock.Anything). + Run(func(args mock.Arguments) { + list := args.Get(1).(*ocv1.ClusterExtensionRevisionList) + list.Items = []ocv1.ClusterExtensionRevision{ + {}, {}, // Existing revisions. + } + }). + Return(nil) + + err := sm.Migrate(t.Context(), ext, map[string]string{"my-label": "my-value"}) + require.NoError(t, err) + + client.AssertExpectations(t) + }) + + t.Run("does not create revision when no helm release", func(t *testing.T) { + brb := &mockBundleRevisionBuilder{} + mag := &mockActionGetter{ + getClientErr: driver.ErrReleaseNotFound, + } + client := &clientMock{} + sm := &applier.BoxcutterStorageMigrator{ + RevisionGenerator: brb, + ActionClientGetter: mag, + Client: client, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + } + + client. + On("List", mock.Anything, mock.AnythingOfType("*v1.ClusterExtensionRevisionList"), mock.Anything). + Return(nil) + + err := sm.Migrate(t.Context(), ext, map[string]string{"my-label": "my-value"}) + require.NoError(t, err) + + client.AssertExpectations(t) + }) +} + // mockBundleRevisionBuilder is a mock implementation of the ClusterExtensionRevisionGenerator for testing. type mockBundleRevisionBuilder struct { makeRevisionFunc func(bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotation map[string]string) (*ocv1.ClusterExtensionRevision, error) @@ -534,8 +705,29 @@ func (m *mockBundleRevisionBuilder) GenerateRevision(bundleFS fs.FS, ext *ocv1.C return m.makeRevisionFunc(bundleFS, ext, objectLabels, revisionAnnotations) } +func (m *mockBundleRevisionBuilder) GenerateRevisionFromHelmRelease( + helmRelease *release.Release, ext *ocv1.ClusterExtension, + objectLabels map[string]string, +) (*ocv1.ClusterExtensionRevision, error) { + return nil, nil +} + 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) } + +type clientMock struct { + mock.Mock +} + +func (m *clientMock) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + args := m.Called(ctx, list, opts) + return args.Error(0) +} + +func (m *clientMock) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + args := m.Called(ctx, obj, opts) + return args.Error(0) +} diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 49e59a81a7..71520f42e0 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -70,11 +70,16 @@ type ClusterExtensionReconciler struct { ImageCache imageutil.Cache ImagePuller imageutil.Puller + StorageMigrator StorageMigrator Applier Applier RevisionStatesGetter RevisionStatesGetter Finalizers crfinalizer.Finalizers } +type StorageMigrator interface { + Migrate(context.Context, *ocv1.ClusterExtension, map[string]string) error +} + type Applier interface { // Apply applies the content in the provided fs.FS using the configuration of the provided ClusterExtension. // It also takes in a map[string]string to be applied to all applied resources as labels and another @@ -209,6 +214,17 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl return ctrl.Result{}, nil } + objLbls := map[string]string{ + labels.OwnerKindKey: ocv1.ClusterExtensionKind, + labels.OwnerNameKey: ext.GetName(), + } + + if r.StorageMigrator != nil { + if err := r.StorageMigrator.Migrate(ctx, ext, objLbls); err != nil { + return ctrl.Result{}, fmt.Errorf("migrating storage: %w", err) + } + } + l.Info("getting installed bundle") revisionStates, err := r.RevisionStatesGetter.GetRevisionStates(ctx, ext) if err != nil { @@ -275,11 +291,6 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl return ctrl.Result{}, err } - objLbls := map[string]string{ - labels.OwnerKindKey: ocv1.ClusterExtensionKind, - labels.OwnerNameKey: ext.GetName(), - } - storeLbls := map[string]string{ labels.BundleNameKey: resolvedRevisionMetadata.Name, labels.PackageNameKey: resolvedRevisionMetadata.Package, @@ -536,31 +547,37 @@ func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, e return nil, fmt.Errorf("listing revisions: %w", err) } slices.SortFunc(existingRevisionList.Items, func(a, b ocv1.ClusterExtensionRevision) int { - return cmp.Compare(b.Spec.Revision, a.Spec.Revision) + return cmp.Compare(a.Spec.Revision, b.Spec.Revision) }) rs := &RevisionStates{} for _, rev := range existingRevisionList.Items { - if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateActive { - // TODO: the setting of these annotations (happens in boxcutter applier when we pass in "storageLabels") - // is fairly decoupled from this code where we get the annotations back out. We may want to co-locate - // the set/get logic a bit better to make it more maintainable and less likely to get out of sync. - rm := &RevisionMetadata{ - Package: rev.Labels[labels.PackageNameKey], - Image: rev.Annotations[labels.BundleReferenceKey], - BundleMetadata: ocv1.BundleMetadata{ - Name: rev.Annotations[labels.BundleNameKey], - Version: rev.Annotations[labels.BundleVersionKey], - }, - } + switch rev.Spec.LifecycleState { + case ocv1.ClusterExtensionRevisionLifecycleStateActive, + ocv1.ClusterExtensionRevisionLifecycleStatePaused: + default: + // Skip anything not active or paused, which should only be "Archived". + continue + } - if installedCondition := apimeta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded); installedCondition == nil || installedCondition.Status != metav1.ConditionTrue { - rs.RollingOut = append(rs.RollingOut, rm) - } else { - rs.Installed = rm - break - } + // TODO: the setting of these annotations (happens in boxcutter applier when we pass in "storageLabels") + // is fairly decoupled from this code where we get the annotations back out. We may want to co-locate + // the set/get logic a bit better to make it more maintainable and less likely to get out of sync. + rm := &RevisionMetadata{ + Package: rev.Labels[labels.PackageNameKey], + Image: rev.Annotations[labels.BundleReferenceKey], + BundleMetadata: ocv1.BundleMetadata{ + Name: rev.Annotations[labels.BundleNameKey], + Version: rev.Annotations[labels.BundleVersionKey], + }, + } + + if apimeta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) { + rs.Installed = rm + } else { + rs.RollingOut = append(rs.RollingOut, rm) } } + return rs, nil } diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index ed04c5b4fc..025102d67a 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -362,28 +362,6 @@ func (c *ClusterExtensionRevisionReconciler) removeFinalizer(ctx context.Context } func toBoxcutterRevision(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] = rev.Labels[ClusterExtensionRevisionOwnerLabel] - 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{} @@ -421,6 +399,35 @@ func toBoxcutterRevision(rev *ocv1.ClusterExtensionRevision) (*boxcutter.Revisio return false, []string{"not available or not fully updated"} })), } + + 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] = rev.Labels[ClusterExtensionRevisionOwnerLabel] + obj.SetLabels(labels) + + switch specObj.CollisionProtection { + case ocv1.CollisionProtectionIfNoController, ocv1.CollisionProtectionNone: + opts = append(opts, boxcutter.WithObjectReconcileOptions( + &obj, boxcutter.WithCollisionProtection(specObj.CollisionProtection))) + } + + phase.Objects = append(phase.Objects, obj) + } + r.Phases = append(r.Phases, phase) + } + if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStatePaused { opts = append(opts, boxcutter.WithPaused{}) } diff --git a/internal/shared/util/testutils/artifacts.go b/internal/shared/util/testutils/artifacts.go index 485128c834..6bda44b673 100644 --- a/internal/shared/util/testutils/artifacts.go +++ b/internal/shared/util/testutils/artifacts.go @@ -11,13 +11,13 @@ import ( "time" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" kubeclient "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/utils/env" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" ocv1 "github.com/operator-framework/operator-controller/api/v1" ) @@ -25,6 +25,7 @@ import ( // CollectTestArtifacts gets all the artifacts from the test run and saves them to the artifact path. // Currently, it saves: // - clusterextensions +// - clusterextensionrevisions // - pods logs // - deployments // - catalogsources @@ -54,7 +55,7 @@ func CollectTestArtifacts(t *testing.T, artifactName string, c client.Client, cf // get all cluster extensions save them to the artifact path. clusterExtensions := ocv1.ClusterExtensionList{} - if err := c.List(context.Background(), &clusterExtensions, client.InNamespace("")); err != nil { + if err := c.List(context.Background(), &clusterExtensions); err != nil { fmt.Printf("Failed to list cluster extensions: %v", err) } for _, clusterExtension := range clusterExtensions.Items { @@ -69,6 +70,23 @@ func CollectTestArtifacts(t *testing.T, artifactName string, c client.Client, cf } } + // get all cluster extension revisions save them to the artifact path. + clusterExtensionRevisions := ocv1.ClusterExtensionRevisionList{} + if err := c.List(context.Background(), &clusterExtensionRevisions); err != nil { + fmt.Printf("Failed to list cluster extensions: %v", err) + } + for _, cer := range clusterExtensionRevisions.Items { + // Save cluster extension to artifact path + clusterExtensionYaml, err := yaml.Marshal(cer) + if err != nil { + fmt.Printf("Failed to marshal cluster extension: %v", err) + continue + } + if err := os.WriteFile(filepath.Join(artifactPath, cer.Name+"-clusterextensionrevision.yaml"), clusterExtensionYaml, 0600); err != nil { + fmt.Printf("Failed to write cluster extension to file: %v", err) + } + } + // get all catalogsources save them to the artifact path. catalogsources := ocv1.ClusterCatalogList{} if err := c.List(context.Background(), &catalogsources, client.InNamespace("")); err != nil { @@ -117,6 +135,23 @@ func CollectTestArtifacts(t *testing.T, artifactName string, c client.Client, cf } } + // Get secrets in all namespaces + secrets := corev1.SecretList{} + if err := c.List(context.Background(), &secrets, client.InNamespace(namespace.Name)); err != nil { + fmt.Printf("Failed to list secrets %v in namespace: %q", err, namespace.Name) + } + for _, secret := range secrets.Items { + // Save secret to artifact path + secretYaml, err := yaml.Marshal(secret) + if err != nil { + fmt.Printf("Failed to marshal secret: %v", err) + continue + } + if err := os.WriteFile(filepath.Join(namespacedArtifactPath, secret.Name+"-secret.yaml"), secretYaml, 0600); err != nil { + fmt.Printf("Failed to write secret to file: %v", err) + } + } + // Get logs from all pods in all namespaces pods := corev1.PodList{} if err := c.List(context.Background(), &pods, client.InNamespace(namespace.Name)); err != nil { diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 8eb93b967f..9eda450233 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -516,11 +516,17 @@ spec: objects: items: properties: + collisionProtection: + default: Prevent + description: CollisionProtection specifies if and how + ownership collisions are prevented. + type: string object: type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true required: + - collisionProtection - object type: object type: array diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 3f0a48257b..eca953de15 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -516,11 +516,17 @@ spec: objects: items: properties: + collisionProtection: + default: Prevent + description: CollisionProtection specifies if and how + ownership collisions are prevented. + type: string object: type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true required: + - collisionProtection - object type: object type: array