fix: Nil pointer when unmanaging module with Ignore CustomResourceP…

…olicy (#2233)

* fix: Nil ptr when unmanaging module with Ignore policy

* white box testing only...

* fix linting

* update coverage

* extend unmanaging e2e test

internal/declarative/v2/reconciler.go

@@ -7,7 +7,6 @@ import (
 	"time"
 
 	apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/cli-runtime/pkg/resource"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -52,14 +51,15 @@ func NewFromManager(mgr manager.Manager, requeueIntervals queue.RequeueIntervals
 	reconciler.RequeueIntervals = requeueIntervals
 	reconciler.specResolver = specResolver
 	reconciler.manifestClient = manifestAPIClient
-	reconciler.managedLabelRemovalService = labelsremoval.NewManagedLabelRemovalService(manifestAPIClient)
+	reconciler.managedLabelRemovalService = labelsremoval.NewManagedByLabelRemovalService(manifestAPIClient)
 	reconciler.Options = DefaultOptions().Apply(WithManager(mgr)).Apply(options...)
 	return reconciler
 }
 
-type ManagedLabelRemoval interface {
-	RemoveManagedLabel(ctx context.Context,
-		manifest *v1beta2.Manifest, skrClient client.Client, defaultCR *unstructured.Unstructured,
+type ManagedByLabelRemoval interface {
+	RemoveManagedByLabel(ctx context.Context,
+		manifest *v1beta2.Manifest,
+		skrClient client.Client,
 	) error
 }
 
@@ -77,7 +77,7 @@ type Reconciler struct {
 	MandatoryModuleMetrics     *metrics.MandatoryModulesMetrics
 	specResolver               SpecResolver
 	manifestClient             ManifestAPIClient
-	managedLabelRemovalService ManagedLabelRemoval
+	managedLabelRemovalService ManagedByLabelRemoval
 }
 
 //nolint:funlen,cyclop,gocognit // Declarative pkg will be removed soon
@@ -123,7 +123,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			return r.cleanupManifest(ctx, manifest, manifestStatus, metrics.ManifestUnmanagedUpdate, nil)
 		}
 
-		if controllerutil.ContainsFinalizer(manifest, labelsremoval.LabelRemovalFinalizer) {
+		if controllerutil.ContainsFinalizer(manifest, finalizer.LabelRemovalFinalizer) {
 			return r.handleLabelsRemovalFinalizer(ctx, skrClient, manifest)
 		}
 
@@ -222,16 +222,11 @@ func recordMandatoryModuleState(manifest *v1beta2.Manifest, r *Reconciler) {
 func (r *Reconciler) handleLabelsRemovalFinalizer(ctx context.Context, skrClient client.Client,
 	manifest *v1beta2.Manifest,
 ) (ctrl.Result, error) {
-	defaultCR, err := modulecr.NewClient(skrClient).GetCR(ctx, manifest)
+	err := r.managedLabelRemovalService.RemoveManagedByLabel(ctx, manifest, skrClient)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 
-	if err := r.managedLabelRemovalService.RemoveManagedLabel(ctx, manifest, skrClient,
-		defaultCR); err != nil {
-		return ctrl.Result{}, err
-	}
-
 	r.ManifestMetrics.RecordRequeueReason(metrics.ManifestResourcesLabelRemoval, queue.IntendedRequeue)
 	return ctrl.Result{Requeue: true}, nil
 }

internal/manifest/finalizer/handler.go

@@ -10,7 +10,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/kyma-project/lifecycle-manager/api/v1beta2"
-	"github.com/kyma-project/lifecycle-manager/internal/manifest/labelsremoval"
 	"github.com/kyma-project/lifecycle-manager/pkg/util"
 )
 
@@ -19,11 +18,12 @@ var ErrRequeueRequired = errors.New("requeue required")
 const (
 	DefaultFinalizer               = "declarative.kyma-project.io/finalizer"
 	CustomResourceManagerFinalizer = "resource.kyma-project.io/finalizer"
+	LabelRemovalFinalizer          = "label-removal-finalizer"
 )
 
 // RemoveRequiredFinalizers removes preconfigured finalizers, but not include CustomResourceManagerFinalizer.
 func RemoveRequiredFinalizers(manifest *v1beta2.Manifest) bool {
-	finalizersToRemove := []string{DefaultFinalizer, labelsremoval.LabelRemovalFinalizer}
+	finalizersToRemove := []string{DefaultFinalizer, LabelRemovalFinalizer}
 
 	finalizerRemoved := false
 	for _, f := range finalizersToRemove {
@@ -46,7 +46,7 @@ func RemoveAllFinalizers(manifest *v1beta2.Manifest) bool {
 
 func FinalizersUpdateRequired(manifest *v1beta2.Manifest) bool {
 	defaultFinalizerAdded := controllerutil.AddFinalizer(manifest, DefaultFinalizer)
-	labelRemovalFinalizerAdded := controllerutil.AddFinalizer(manifest, labelsremoval.LabelRemovalFinalizer)
+	labelRemovalFinalizerAdded := controllerutil.AddFinalizer(manifest, LabelRemovalFinalizer)
 	return defaultFinalizerAdded || labelRemovalFinalizerAdded
 }
 

internal/manifest/labelsremoval/labels_removal.go

@@ -2,6 +2,7 @@ package labelsremoval
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -11,37 +12,43 @@ 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/finalizer"
+	"github.com/kyma-project/lifecycle-manager/internal/manifest/modulecr"
 )
 
-const LabelRemovalFinalizer = "label-removal-finalizer"
-
 type ManifestAPIClient interface {
 	UpdateManifest(ctx context.Context, manifest *v1beta2.Manifest) error
 }
 
-type ManagedLabelRemovalService struct {
+type ManagedByLabelRemovalService struct {
 	manifestClient ManifestAPIClient
 }
 
-func NewManagedLabelRemovalService(manifestClient ManifestAPIClient) *ManagedLabelRemovalService {
-	return &ManagedLabelRemovalService{
+func NewManagedByLabelRemovalService(manifestClient ManifestAPIClient) *ManagedByLabelRemovalService {
+	return &ManagedByLabelRemovalService{
 		manifestClient: manifestClient,
 	}
 }
 
-func (l *ManagedLabelRemovalService) RemoveManagedLabel(ctx context.Context,
-	manifest *v1beta2.Manifest, skrClient client.Client, defaultCR *unstructured.Unstructured,
+func (l *ManagedByLabelRemovalService) RemoveManagedByLabel(ctx context.Context,
+	manifest *v1beta2.Manifest,
+	skrClient client.Client,
 ) error {
-	if err := HandleLabelsRemovalFromResources(ctx, manifest, skrClient, defaultCR); err != nil {
-		return err
+	resourcesError := removeFromSyncedResources(ctx, manifest, skrClient)
+	defaultCRError := removeFromDefaultCR(ctx, manifest, skrClient)
+
+	if resourcesError != nil || defaultCRError != nil {
+		return fmt.Errorf("failed to remove %s label from one or more resources: %w",
+			shared.ManagedBy,
+			errors.Join(resourcesError, defaultCRError))
 	}
 
-	controllerutil.RemoveFinalizer(manifest, LabelRemovalFinalizer)
+	controllerutil.RemoveFinalizer(manifest, finalizer.LabelRemovalFinalizer)
 	return l.manifestClient.UpdateManifest(ctx, manifest)
 }
 
-func HandleLabelsRemovalFromResources(ctx context.Context, manifestCR *v1beta2.Manifest,
-	skrClient client.Client, defaultCR *unstructured.Unstructured,
+func removeFromSyncedResources(ctx context.Context, manifestCR *v1beta2.Manifest,
+	skrClient client.Client,
 ) error {
 	for _, res := range manifestCR.Status.Synced {
 		objectKey := client.ObjectKey{
@@ -54,19 +61,33 @@ func HandleLabelsRemovalFromResources(ctx context.Context, manifestCR *v1beta2.M
 			return fmt.Errorf("failed to get resource, %w", err)
 		}
 
-		if IsManagedLabelRemoved(obj) {
-			if err := skrClient.Update(ctx, obj); err != nil {
-				return fmt.Errorf("failed to update object: %w", err)
-			}
+		if err := removeFromObject(ctx, obj, skrClient); err != nil {
+			return err
 		}
 	}
 
-	if defaultCR == nil {
+	return nil
+}
+
+func removeFromDefaultCR(ctx context.Context,
+	manifest *v1beta2.Manifest,
+	skrClient client.Client,
+) error {
+	if manifest.Spec.Resource == nil {
 		return nil
 	}
 
-	if IsManagedLabelRemoved(defaultCR) {
-		if err := skrClient.Update(ctx, defaultCR); err != nil {
+	defaultCR, err := modulecr.NewClient(skrClient).GetCR(ctx, manifest)
+	if err != nil {
+		return fmt.Errorf("failed to get default CR, %w", err)
+	}
+
+	return removeFromObject(ctx, defaultCR, skrClient)
+}
+
+func removeFromObject(ctx context.Context, obj *unstructured.Unstructured, skrClient client.Client) error {
+	if removeManagedLabel(obj) {
+		if err := skrClient.Update(ctx, obj); err != nil {
 			return fmt.Errorf("failed to update object: %w", err)
 		}
 	}
@@ -87,7 +108,7 @@ func constructResource(resource shared.Resource) *unstructured.Unstructured {
 	return obj
 }
 
-func IsManagedLabelRemoved(resource *unstructured.Unstructured) bool {
+func removeManagedLabel(resource *unstructured.Unstructured) bool {
 	labels := resource.GetLabels()
 	_, managedByLabelExists := labels[shared.ManagedBy]
 	if managedByLabelExists {