From 92beec6ba5d31adabaf992920a5e74582cd58720 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wilson=20J=C3=BAnior?= <wilsonpjunior@gmail.com>
Date: Mon, 6 Sep 2021 11:12:11 -0300
Subject: [PATCH] Add validation to prevent to unset creationOnly flavor

---
 internal/pkg/rpaas/k8s.go      | 33 +++++++++++++++++++++++++--------
 internal/pkg/rpaas/k8s_test.go | 25 ++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/internal/pkg/rpaas/k8s.go b/internal/pkg/rpaas/k8s.go
index 1625e87e7..5355329ba 100644
--- a/internal/pkg/rpaas/k8s.go
+++ b/internal/pkg/rpaas/k8s.go
@@ -269,14 +269,15 @@ func (m *k8sRpaasManager) CreateInstance(ctx context.Context, args CreateArgs) e
 }
 
 func (m *k8sRpaasManager) UpdateInstance(ctx context.Context, instanceName string, args UpdateInstanceArgs) error {
-	if err := m.validateUpdateInstanceArgs(ctx, args); err != nil {
+	instance, err := m.GetInstance(ctx, instanceName)
+	if err != nil {
 		return err
 	}
 
-	instance, err := m.GetInstance(ctx, instanceName)
-	if err != nil {
+	if err := m.validateUpdateInstanceArgs(ctx, instance, args); err != nil {
 		return err
 	}
+
 	originalInstance := instance.DeepCopy()
 	if args.Plan != "" && args.Plan != instance.Spec.PlanName {
 		plan, err := m.getPlan(ctx, args.Plan)
@@ -1317,22 +1318,22 @@ func (m *k8sRpaasManager) validateCreate(ctx context.Context, args CreateArgs) e
 		return ConflictError{Msg: fmt.Sprintf("rpaas instance named %q already exists", args.Name)}
 	}
 
-	if err = m.validateFlavors(ctx, args.Flavors(), true); err != nil {
+	if err = m.validateFlavors(ctx, nil, args.Flavors()); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (m *k8sRpaasManager) validateUpdateInstanceArgs(ctx context.Context, args UpdateInstanceArgs) error {
-	if err := m.validateFlavors(ctx, args.Flavors(), false); err != nil {
+func (m *k8sRpaasManager) validateUpdateInstanceArgs(ctx context.Context, instance *v1alpha1.RpaasInstance, args UpdateInstanceArgs) error {
+	if err := m.validateFlavors(ctx, instance, args.Flavors()); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (m *k8sRpaasManager) validateFlavors(ctx context.Context, flavors []string, creation bool) error {
+func (m *k8sRpaasManager) validateFlavors(ctx context.Context, instance *v1alpha1.RpaasInstance, flavors []string) error {
 	encountered := map[string]bool{}
 	allFlavors, err := m.getFlavors(ctx)
 	if err != nil {
@@ -1345,7 +1346,7 @@ func (m *k8sRpaasManager) validateFlavors(ctx context.Context, flavors []string,
 			return ValidationError{Msg: fmt.Sprintf("flavor %q not found", f)}
 		}
 
-		if flavorObj.Spec.CreationOnly && !creation {
+		if flavorObj.Spec.CreationOnly && instance != nil {
 			return ValidationError{Msg: fmt.Sprintf("flavor %q can used only in the creation of instance", f)}
 		}
 
@@ -1356,6 +1357,22 @@ func (m *k8sRpaasManager) validateFlavors(ctx context.Context, flavors []string,
 		encountered[f] = true
 	}
 
+	if instance == nil {
+		return nil
+	}
+
+	for _, f := range instance.Spec.Flavors {
+		flavorObj := m.selectFlavor(ctx, allFlavors, f)
+
+		if flavorObj == nil {
+			continue
+		}
+
+		if flavorObj.Spec.CreationOnly && !encountered[f] {
+			return ValidationError{Msg: fmt.Sprintf("flavor %q can unset, cause it is a creation only flavor", f)}
+		}
+	}
+
 	return nil
 }
 
diff --git a/internal/pkg/rpaas/k8s_test.go b/internal/pkg/rpaas/k8s_test.go
index 513fa030d..6917bffe6 100644
--- a/internal/pkg/rpaas/k8s_test.go
+++ b/internal/pkg/rpaas/k8s_test.go
@@ -3607,6 +3607,12 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
 	}
 	instance1.Spec.PlanName = "plan1"
 
+	instance2 := newEmptyRpaasInstance()
+	instance2.Name = "instance2"
+	instance2.Labels = labelsForRpaasInstance(instance1.Name)
+	instance2.Spec.PlanName = "plan1"
+	instance2.Spec.Flavors = []string{"feature-create-only"}
+
 	podLabels := mergeMap(instance1.Labels, map[string]string{"pod-label-1": "v1"})
 
 	instance1.Spec.PodTemplate = nginxv1alpha1.NginxPodTemplateSpec{
@@ -3647,7 +3653,7 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
 		},
 	}
 
-	resources := []runtime.Object{instance1, plan1, plan2, creationOnlyFlavor}
+	resources := []runtime.Object{instance1, instance2, plan1, plan2, creationOnlyFlavor}
 
 	tests := []struct {
 		name      string
@@ -3685,6 +3691,23 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
 				assert.Equal(t, `flavor "feature-create-only" can used only in the creation of instance`, err.Error())
 			},
 		},
+		{
+			name:     "when tries to remove a creationOnly flavor",
+			instance: "instance2",
+			args: UpdateInstanceArgs{
+				Description: "Another description",
+				Plan:        "plan2",
+				Tags:        []string{},
+				Team:        "team-two",
+				Parameters: map[string]interface{}{
+					"lb-name": "my-instance.example",
+				},
+			},
+			assertion: func(t *testing.T, err error, instance *v1alpha1.RpaasInstance) {
+				require.Error(t, err)
+				assert.Equal(t, `flavor "feature-create-only" can unset, cause it is a creation only flavor`, err.Error())
+			},
+		},
 		{
 			name:     "when successfully updating an instance",
 			instance: "instance1",