diff --git a/controllers/dynamic_resource_request_controller.go b/controllers/dynamic_resource_request_controller.go index 25d47f76..98751399 100644 --- a/controllers/dynamic_resource_request_controller.go +++ b/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{} diff --git a/controllers/promise_controller.go b/controllers/promise_controller.go index 840e5dcf..794b09b4 100644 --- a/controllers/promise_controller.go +++ b/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 diff --git a/lib/migrations/conditions.go b/lib/migrations/conditions.go new file mode 100644 index 00000000..1307b76d --- /dev/null +++ b/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 +} diff --git a/lib/migrations/conditions_test.go b/lib/migrations/conditions_test.go new file mode 100644 index 00000000..388f2153 --- /dev/null +++ b/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")) + }) + }) + +}) diff --git a/lib/migrations/migrations_suite_test.go b/lib/migrations/migrations_suite_test.go new file mode 100644 index 00000000..24d48840 --- /dev/null +++ b/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") +})