diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 0acc79ef1c2..97ea381db0c 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -61,6 +61,7 @@ func NewFromManager(mgr manager.Manager, reconciler.RequeueIntervals = requeueIntervals reconciler.specResolver = specResolver reconciler.manifestClient = manifestClient + reconciler.managedLabelRemovalService = labelsremoval.NewManagedLabelRemovalService(manifestClient) reconciler.Options = DefaultOptions().Apply(WithManager(mgr)).Apply(options...) return reconciler } @@ -68,10 +69,11 @@ func NewFromManager(mgr manager.Manager, type Reconciler struct { queue.RequeueIntervals *Options - ManifestMetrics *metrics.ManifestMetrics - MandatoryModuleMetrics *metrics.MandatoryModulesMetrics - specResolver SpecResolver - manifestClient manifestclient.ManifestClient + ManifestMetrics *metrics.ManifestMetrics + MandatoryModuleMetrics *metrics.MandatoryModulesMetrics + specResolver SpecResolver + manifestClient manifestclient.ManifestClient + managedLabelRemovalService *labelsremoval.ManagedLabelRemovalService } const waitingForResourcesMsg = "waiting for resources to become ready" @@ -250,8 +252,9 @@ func (r *Reconciler) handleLabelsRemovalFinalizer(ctx context.Context, skrClient if err != nil { return ctrl.Result{}, err } - if err := labelsremoval.HandleLabelsRemovalFinalizerForUnmanagedModule(ctx, manifest, skrClient, - r.manifestClient, defaultCR); err != nil { + + if err := r.managedLabelRemovalService.HandleLabelsRemovalFinalizerForUnmanagedModule(ctx, manifest, skrClient, + defaultCR); err != nil { return ctrl.Result{}, err } diff --git a/internal/manifest/labelsremoval/labels_removal.go b/internal/manifest/labelsremoval/labels_removal.go index 3a49d61337f..42f83ec881a 100644 --- a/internal/manifest/labelsremoval/labels_removal.go +++ b/internal/manifest/labelsremoval/labels_removal.go @@ -16,16 +16,25 @@ import ( const LabelRemovalFinalizer = "label-removal-finalizer" -func HandleLabelsRemovalFinalizerForUnmanagedModule(ctx context.Context, - manifest *v1beta2.Manifest, skrClient client.Client, manifestClnt manifestclient.ManifestClient, - defaultCR *unstructured.Unstructured, +type ManagedLabelRemovalService struct { + manifestClient manifestclient.ManifestClient +} + +func NewManagedLabelRemovalService(manifestClient manifestclient.ManifestClient) *ManagedLabelRemovalService { + return &ManagedLabelRemovalService{ + manifestClient: manifestClient, + } +} + +func (l *ManagedLabelRemovalService) HandleLabelsRemovalFinalizerForUnmanagedModule(ctx context.Context, + manifest *v1beta2.Manifest, skrClient client.Client, defaultCR *unstructured.Unstructured, ) error { if err := HandleLabelsRemovalFromResources(ctx, manifest, skrClient, defaultCR); err != nil { return err } controllerutil.RemoveFinalizer(manifest, LabelRemovalFinalizer) - return manifestClnt.UpdateManifest(ctx, manifest) + return l.manifestClient.UpdateManifest(ctx, manifest) } func HandleLabelsRemovalFromResources(ctx context.Context, manifestCR *v1beta2.Manifest, @@ -36,19 +45,13 @@ func HandleLabelsRemovalFromResources(ctx context.Context, manifestCR *v1beta2.M Name: res.Name, Namespace: res.Namespace, } - gvk := schema.GroupVersionKind{ - Group: res.Group, - Version: res.Version, - Kind: res.Kind, - } - obj := &unstructured.Unstructured{} - obj.SetGroupVersionKind(gvk) + obj := ConstructResource(res) if err := skrClient.Get(ctx, objectKey, obj); err != nil { return fmt.Errorf("failed to get resource, %w", err) } - if NeedsUpdateAfterLabelRemoval(obj) { + if IsManagedLabelRemoved(obj) { if err := skrClient.Update(ctx, obj); err != nil { return fmt.Errorf("failed to update object: %w", err) } @@ -59,7 +62,7 @@ func HandleLabelsRemovalFromResources(ctx context.Context, manifestCR *v1beta2.M return nil } - if NeedsUpdateAfterLabelRemoval(defaultCR) { + if IsManagedLabelRemoved(defaultCR) { if err := skrClient.Update(ctx, defaultCR); err != nil { return fmt.Errorf("failed to update object: %w", err) } @@ -68,7 +71,20 @@ func HandleLabelsRemovalFromResources(ctx context.Context, manifestCR *v1beta2.M return nil } -func NeedsUpdateAfterLabelRemoval(resource *unstructured.Unstructured) bool { +func ConstructResource(resource shared.Resource) *unstructured.Unstructured { + gvk := schema.GroupVersionKind{ + Group: resource.Group, + Version: resource.Version, + Kind: resource.Kind, + } + + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + + return obj +} + +func IsManagedLabelRemoved(resource *unstructured.Unstructured) bool { labels := resource.GetLabels() _, managedByLabelExists := labels[shared.ManagedBy] if managedByLabelExists { diff --git a/internal/manifest/labelsremoval/labels_removal_test.go b/internal/manifest/labelsremoval/labels_removal_test.go index ff4ffe0702e..0b3b73e8fae 100644 --- a/internal/manifest/labelsremoval/labels_removal_test.go +++ b/internal/manifest/labelsremoval/labels_removal_test.go @@ -15,13 +15,15 @@ import ( "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal/manifest/labelsremoval" + "github.com/kyma-project/lifecycle-manager/internal/manifest/manifestclient" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" ) func Test_needsUpdateAfterLabelRemoval_WhenLabelsAreEmpty(t *testing.T) { emptyLabels := map[string]string{} res := &unstructured.Unstructured{} res.SetLabels(emptyLabels) - actual := labelsremoval.NeedsUpdateAfterLabelRemoval(res) + actual := labelsremoval.IsManagedLabelRemoved(res) require.False(t, actual) require.Equal(t, emptyLabels, res.GetLabels()) @@ -34,7 +36,7 @@ func Test_needsUpdateAfterLabelRemoval_WhenManagedByLabel(t *testing.T) { expectedLabels := map[string]string{} res := &unstructured.Unstructured{} res.SetLabels(labels) - actual := labelsremoval.NeedsUpdateAfterLabelRemoval(res) + actual := labelsremoval.IsManagedLabelRemoved(res) require.True(t, actual) require.Equal(t, expectedLabels, res.GetLabels()) @@ -47,30 +49,29 @@ func Test_handleLabelsRemovalFromResources_WhenManifestResourcesHaveLabels(t *te Kind: "TestKind", } - manifest := &v1beta2.Manifest{ - Status: shared.Status{ - Synced: []shared.Resource{ - { - Name: "test-resource-1", - Namespace: "test-1", - GroupVersionKind: apimetav1.GroupVersionKind{ - Group: gvk.Group, - Version: gvk.Version, - Kind: gvk.Kind, - }, + status := shared.Status{ + Synced: []shared.Resource{ + { + Name: "test-resource-1", + Namespace: "test-1", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, }, - { - Name: "test-resource-2", - Namespace: "test-2", - GroupVersionKind: apimetav1.GroupVersionKind{ - Group: gvk.Group, - Version: gvk.Version, - Kind: gvk.Kind, - }, + }, + { + Name: "test-resource-2", + Namespace: "test-2", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, }, }, }, } + manifest := builder.NewManifestBuilder().WithStatus(status).Build() objs := []client.Object{ &unstructured.Unstructured{ @@ -123,11 +124,7 @@ func Test_handleLabelsRemovalFromResources_WhenManifestResourcesHaveLabels(t *te } func Test_handleLabelsRemovalFromResources_WhenManifestResourcesAreNilAndNoDefaultCR(t *testing.T) { - manifest := &v1beta2.Manifest{ - Status: shared.Status{ - Synced: []shared.Resource{}, - }, - } + manifest := builder.NewManifestBuilder().Build() scheme := machineryruntime.NewScheme() err := v1beta2.AddToScheme(scheme) @@ -147,21 +144,20 @@ func Test_handleLabelsRemovalFromResources_WhenManifestResourcesAndDefaultCRHave Kind: "TestKind", } - manifest := &v1beta2.Manifest{ - Status: shared.Status{ - Synced: []shared.Resource{ - { - Name: "test-resource-1", - Namespace: "test-1", - GroupVersionKind: apimetav1.GroupVersionKind{ - Group: gvk.Group, - Version: gvk.Version, - Kind: gvk.Kind, - }, + status := shared.Status{ + Synced: []shared.Resource{ + { + Name: "test-resource-1", + Namespace: "test-1", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, }, }, }, } + manifest := builder.NewManifestBuilder().WithStatus(status).Build() objs := []client.Object{ &unstructured.Unstructured{ @@ -206,8 +202,118 @@ func Test_handleLabelsRemovalFromResources_WhenManifestResourcesAndDefaultCRHave require.NoError(t, err) require.Empty(t, firstObj.GetLabels()) - err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: "default-cr", Namespace: "default-ns"}, - defaultCR) require.NoError(t, err) require.Empty(t, defaultCR.GetLabels()) } + +func Test_HandleLabelsRemovalFinalizerForUnmanagedModule_WhenErrorIsReturned(t *testing.T) { + scheme := machineryruntime.NewScheme() + err := v1beta2.AddToScheme(scheme) + require.NoError(t, err) + + gvk := schema.GroupVersionKind{ + Group: "test-group", + Version: "v1", + Kind: "TestKind", + } + status := shared.Status{ + Synced: []shared.Resource{ + { + Name: "test-resource-1", + Namespace: "test-1", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }, + }, + }, + } + manifest := builder.NewManifestBuilder().WithStatus(status).Build() + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + manifestClnt := manifestclient.NewManifestClient(nil, fakeClient) + svc := labelsremoval.NewManagedLabelRemovalService(manifestClnt) + + err = svc.HandleLabelsRemovalFinalizerForUnmanagedModule(context.TODO(), manifest, fakeClient, nil) + require.ErrorContains(t, err, "failed to get resource") +} + +func Test_HandleLabelsRemovalFinalizerForUnmanagedModule_WhenFinalizerIsRemoved(t *testing.T) { + scheme := machineryruntime.NewScheme() + err := v1beta2.AddToScheme(scheme) + require.NoError(t, err) + + gvk := schema.GroupVersionKind{ + Group: "test-group", + Version: "v1", + Kind: "TestKind", + } + status := shared.Status{ + Synced: []shared.Resource{ + { + Name: "test-resource-1", + Namespace: "test-1", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + }, + }, + }, + } + finalizers := []string{"label-removal-finalizer"} + manifest := builder.NewManifestBuilder().WithFinalizers(finalizers).WithStatus(status).Build() + + objs := []client.Object{ + &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "version": gvk.Version, + }, + }, + } + objs[0].SetName("test-resource-1") + objs[0].SetNamespace("test-1") + objs[0].SetLabels(map[string]string{ + "operator.kyma-project.io/managed-by": "kyma", + }) + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(manifest).WithObjects(objs...).Build() + + manifestClnt := manifestclient.NewManifestClient(nil, fakeClient) + svc := labelsremoval.NewManagedLabelRemovalService(manifestClnt) + + err = svc.HandleLabelsRemovalFinalizerForUnmanagedModule(context.TODO(), manifest, fakeClient, nil) + require.NoError(t, err) + + err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: manifest.GetName(), Namespace: manifest.GetNamespace()}, + manifest) + require.NoError(t, err) + require.Empty(t, manifest.GetFinalizers()) +} + +func Test_ConstructResource_ReturnsExpected(t *testing.T) { + resource := shared.Resource{ + Name: "test-resource", + Namespace: "test-ns", + GroupVersionKind: apimetav1.GroupVersionKind{ + Group: "test-group", + Version: "v1", + Kind: "TestKind", + }, + } + + actual := labelsremoval.ConstructResource(resource) + + expected := &unstructured.Unstructured{} + expected.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "test-group", + Version: "v1", + Kind: "TestKind", + }) + + require.Equal(t, expected, actual) +} diff --git a/pkg/testutils/builder/manifest.go b/pkg/testutils/builder/manifest.go index 25ca91f2941..e7a49626898 100644 --- a/pkg/testutils/builder/manifest.go +++ b/pkg/testutils/builder/manifest.go @@ -85,6 +85,12 @@ func (mb ManifestBuilder) WithStatus(status shared.Status) ManifestBuilder { return mb } +// WithFinalizers sets v1beta2.Manifest.Finalizers. +func (mb ManifestBuilder) WithFinalizers(finalizers []string) ManifestBuilder { + mb.manifest.Finalizers = finalizers + return mb +} + // Build returns the built v1beta2.Manifest. func (mb ManifestBuilder) Build() *v1beta2.Manifest { return mb.manifest diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index 64b5638fa4e..13890a0e8a4 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -6,7 +6,7 @@ packages: internal/manifest/statecheck: 71 internal/manifest/filemutex: 100 internal/manifest: 10 - internal/manifest/labelsremoval: 75 + internal/manifest/labelsremoval: 100 internal/istio: 63.3 internal/pkg/resources: 91.7 internal/remote: 6.4