chore: Remove deprecated labels from Promise (#283)

Co-authored-by: Chunyi Lyu <[email protected]>

controllers/dynamic_resource_request_controller.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/syntasso/kratix/api/v1alpha1"
+	"github.com/syntasso/kratix/lib/migrations"
 	"github.com/syntasso/kratix/lib/resourceutil"
 	"github.com/syntasso/kratix/lib/workflow"
 
@@ -96,6 +97,15 @@ func (r *DynamicResourceRequestController) Reconcile(ctx context.Context, req ct
 		return defaultRequeue, nil
 	}
 
+	requeue, err := migrations.RemoveDeprecatedConditions(ctx, r.Client, rr, logger)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	if requeue != nil {
+		return *requeue, nil
+	}
+
 	resourceLabels := rr.GetLabels()
 	if resourceLabels == nil {
 		resourceLabels = map[string]string{}

controllers/promise_controller.go

@@ -24,6 +24,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/syntasso/kratix/lib/migrations"
 	"github.com/syntasso/kratix/lib/objectutil"
 
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -150,6 +151,20 @@ func (r *PromiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return r.deletePromise(opts, promise)
 	}
 
+	usPromise, err := promise.ToUnstructured()
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("Error converting Promise to Unstructured: %w", err)
+	}
+
+	requeue, err := migrations.RemoveDeprecatedConditions(ctx, r.Client, usPromise, logger)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	if requeue != nil {
+		return *requeue, nil
+	}
+
 	if value, found := promise.Labels[v1alpha1.PromiseVersionLabel]; found {
 		if promise.Status.Version != value {
 			promise.Status.Version = value
@@ -173,7 +188,7 @@ func (r *PromiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	}
 
 	//TODO handle removing finalizer
-	requeue, err := ensurePromiseDeleteWorkflowFinalizer(opts, promise, promise.HasPipeline(v1alpha1.WorkflowTypePromise, v1alpha1.WorkflowActionDelete))
+	requeue, err = ensurePromiseDeleteWorkflowFinalizer(opts, promise, promise.HasPipeline(v1alpha1.WorkflowTypePromise, v1alpha1.WorkflowActionDelete))
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -219,7 +234,7 @@ func (r *PromiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return addFinalizers(opts, promise, []string{dependenciesCleanupFinalizer})
 	}
 
-	ctrlResult, err := r.reconcileDependenciesAndPromiseWorkflows(opts, promise)
+	ctrlResult, err := r.reconcileDependenciesAndPromiseWorkflows(opts, promise, usPromise)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -404,7 +419,7 @@ func (r *PromiseReconciler) generateStatusAndMarkRequirements(ctx context.Contex
 	return promiseCondition, requirements
 }
 
-func (r *PromiseReconciler) reconcileDependenciesAndPromiseWorkflows(o opts, promise *v1alpha1.Promise) (*ctrl.Result, error) {
+func (r *PromiseReconciler) reconcileDependenciesAndPromiseWorkflows(o opts, promise *v1alpha1.Promise, unstructuredPromise *unstructured.Unstructured) (*ctrl.Result, error) {
 	if len(promise.Spec.Dependencies) > 0 {
 		o.logger.Info("Applying static dependencies for Promise", "promise", promise.GetName())
 		if err := r.applyWorkForStaticDependencies(o, promise); err != nil {
@@ -444,11 +459,6 @@ func (r *PromiseReconciler) reconcileDependenciesAndPromiseWorkflows(o opts, pro
 		}
 	}
 
-	unstructuredPromise, err := promise.ToUnstructured()
-	if err != nil {
-		return nil, err
-	}
-
 	pipelineResources, err := promise.GeneratePromisePipelines(v1alpha1.WorkflowActionConfigure, o.logger)
 	if err != nil {
 		return nil, err

lib/migrations/conditions.go

@@ -0,0 +1,55 @@
+package migrations
+
+import (
+	"context"
+
+	"github.com/go-logr/logr"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+const deprecatedPipelineCompletedCondition = "PipelineCompleted"
+
+func RemoveDeprecatedConditions(ctx context.Context, k8sClient client.Client, obj *unstructured.Unstructured, logger logr.Logger) (*ctrl.Result, error) {
+	conditions, found, err := unstructured.NestedSlice(obj.Object, "status", "conditions")
+	if err != nil {
+		logger.Error(err, "failed to get conditions on object, skipping deprecation fix", "name", obj.GetName(), "kind", obj.GetKind())
+		return nil, nil
+	}
+
+	if !found {
+		logger.Info("deprecated conditions on object not found, skipping deprecation fix", "name", obj.GetName(), "kind", obj.GetKind())
+		return nil, nil
+	}
+
+	newConditions := filterOutCondition(conditions, deprecatedPipelineCompletedCondition)
+	if len(newConditions) == len(conditions) {
+		return nil, nil
+	}
+
+	logger.Info("Removing deprecated condition", "conditionType", deprecatedPipelineCompletedCondition)
+	if err = unstructured.SetNestedSlice(obj.Object, newConditions, "status", "conditions"); err != nil {
+		logger.Error(err, "failed to remove deprecated conditions on object", "name", obj.GetName(), "kind", obj.GetKind())
+		return nil, err
+	}
+
+	if err = k8sClient.Status().Update(ctx, obj); err != nil {
+		logger.Error(err, "failed to update resource to remove deprecated condition")
+		return nil, err
+	}
+
+	return &ctrl.Result{Requeue: true}, nil
+}
+
+func filterOutCondition(conditions []interface{}, conditionType string) []interface{} {
+	var newConditions []interface{}
+	for _, cond := range conditions {
+		if conditionMap, ok := cond.(map[string]interface{}); ok {
+			if conditionMap["type"] != conditionType {
+				newConditions = append(newConditions, cond)
+			}
+		}
+	}
+	return newConditions
+}

lib/migrations/conditions_test.go

@@ -0,0 +1,98 @@
+package migrations_test
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/syntasso/kratix/api/v1alpha1"
+	"github.com/syntasso/kratix/lib/migrations"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+var _ = Describe("Conditions", func() {
+	When("The condition is present", func() {
+		It("should remove deprecated condition", func() {
+			gvk := v1alpha1.GroupVersion.Group + "/" + v1alpha1.GroupVersion.Version
+			promise := &v1alpha1.Promise{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: gvk,
+					Kind:       "Promise",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-promise",
+				},
+				Status: v1alpha1.PromiseStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   "PipelineCompleted",
+							Status: metav1.ConditionTrue,
+						},
+						{
+							Type:   "ConfigureWorkflowCompleted",
+							Status: metav1.ConditionTrue,
+						},
+					},
+				},
+			}
+
+			Expect(fakeK8sClient.Create(ctx, promise)).To(Succeed())
+			promise = &v1alpha1.Promise{}
+			Expect(fakeK8sClient.Get(ctx, client.ObjectKey{Name: "test-promise"}, promise)).To(Succeed())
+
+			usPromise, err := promise.ToUnstructured()
+			Expect(err).ToNot(HaveOccurred())
+			usPromise.SetGroupVersionKind(v1alpha1.GroupVersion.WithKind("Promise"))
+
+			requeue, err := migrations.RemoveDeprecatedConditions(ctx, fakeK8sClient, usPromise, logger)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(requeue).NotTo(BeNil())
+			Expect(requeue.Requeue).To(BeTrue())
+
+			promise = &v1alpha1.Promise{}
+			Expect(fakeK8sClient.Get(ctx, client.ObjectKey{Name: "test-promise"}, promise)).To(Succeed())
+			Expect(promise.Status.Conditions).To(HaveLen(1))
+			Expect(promise.Status.Conditions[0].Type).To(Equal("ConfigureWorkflowCompleted"))
+		})
+	})
+
+	When("The condition is not present", func() {
+		It("should no-op", func() {
+
+			gvk := v1alpha1.GroupVersion.Group + "/" + v1alpha1.GroupVersion.Version
+			promise := &v1alpha1.Promise{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: gvk,
+					Kind:       "Promise",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-promise",
+				},
+				Status: v1alpha1.PromiseStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   "ConfigureWorkflowCompleted",
+							Status: metav1.ConditionTrue,
+						},
+					},
+				},
+			}
+
+			Expect(fakeK8sClient.Create(ctx, promise)).To(Succeed())
+			promise = &v1alpha1.Promise{}
+			Expect(fakeK8sClient.Get(ctx, client.ObjectKey{Name: "test-promise"}, promise)).To(Succeed())
+
+			usPromise, err := promise.ToUnstructured()
+			Expect(err).ToNot(HaveOccurred())
+			usPromise.SetGroupVersionKind(v1alpha1.GroupVersion.WithKind("Promise"))
+
+			requeue, err := migrations.RemoveDeprecatedConditions(ctx, fakeK8sClient, usPromise, logger)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(requeue).To(BeNil())
+			promise = &v1alpha1.Promise{}
+			Expect(fakeK8sClient.Get(ctx, client.ObjectKey{Name: "test-promise"}, promise)).To(Succeed())
+			Expect(promise.Status.Conditions).To(HaveLen(1))
+			Expect(promise.Status.Conditions[0].Type).To(Equal("ConfigureWorkflowCompleted"))
+		})
+	})
+
+})

lib/migrations/migrations_suite_test.go

@@ -0,0 +1,47 @@
+package migrations_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/go-logr/logr"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/syntasso/kratix/api/v1alpha1"
+	"k8s.io/client-go/kubernetes/scheme"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+)
+
+var (
+	fakeK8sClient client.Client
+	ctx           = context.TODO()
+	logger        logr.Logger
+)
+
+func TestManager(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Migration Suite")
+}
+
+var _ = BeforeSuite(func() {
+	err := v1alpha1.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
+	//+kubebuilder:scaffold:scheme
+
+	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
+
+	fakeK8sClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithStatusSubresource(
+		&v1alpha1.Promise{},
+	).Build()
+})
+
+var _ = BeforeEach(func() {
+	fakeK8sClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithStatusSubresource(
+		&v1alpha1.Promise{},
+	).Build()
+	logger = ctrl.Log.WithName("manager")
+})