THREESCALE 11949 - New application is created without deleting old application when AccountCR reference changes

apis/capabilities/v1beta1/application_types.go

@@ -41,6 +41,7 @@ type ApplicationSpec struct {
 	// Important: Run "make" to regenerate code after modifying this file
 
 	// AccountCRName name of account custom resource under which the application will be created
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="AccountCR reference is immutable once set"
 	AccountCR *corev1.LocalObjectReference `json:"accountCR"`
 
 	// ProductCRName of product custom resource from which the application plan will be used

config/crd/bases/capabilities.3scale.net_applications.yaml

@@ -51,6 +51,9 @@ spec:
                     type: string
                 type: object
                 x-kubernetes-map-type: atomic
+                x-kubernetes-validations:
+                - message: AccountCR reference is immutable once set
+                  rule: self == oldSelf
               applicationPlanName:
                 description: ApplicationPlanName name of application plan that the
                   application will use

controllers/capabilities/application_controller_test.go

@@ -82,7 +82,7 @@ func TestApplicationReconciler_Reconcile(t *testing.T) {
 		name               string
 		application        *capabilitiesv1beta1.Application
 		account            *capabilitiesv1beta1.DeveloperAccount
-		product            *capabilitiesv1beta1.Product
+		product            []*capabilitiesv1beta1.Product
 		httpHandlerOptions []mocks.ApplicationAPIHandlerOpt
 		testBody           func(t *testing.T, reconciler *reconcilers.BaseReconciler, req reconcile.Request)
 	}{
@@ -105,7 +105,7 @@ func TestApplicationReconciler_Reconcile(t *testing.T) {
 					Description:         "test",
 				},
 			},
-			product: getProductCR(),
+			product: []*capabilitiesv1beta1.Product{getProductCR()},
 			testBody: func(t *testing.T, r *reconcilers.BaseReconciler, req reconcile.Request) {
 				applicationReconciler := ApplicationReconciler{BaseReconciler: r}
 				_, err := applicationReconciler.Reconcile(context.Background(), req)
@@ -208,7 +208,7 @@ func TestApplicationReconciler_Reconcile(t *testing.T) {
 					},
 				},
 			},
-			product: getProductCR(),
+			product: []*capabilitiesv1beta1.Product{getProductCR()},
 			httpHandlerOptions: []mocks.ApplicationAPIHandlerOpt{
 				mocks.WithService(3, getApplicationPlanListByProductJson()),
 				mocks.WithAccount(3, &client.ApplicationList{Applications: []client.ApplicationElem{
@@ -250,7 +250,7 @@ func TestApplicationReconciler_Reconcile(t *testing.T) {
 				},
 			},
 			account: getApplicationDeveloperAccount(),
-			product: getProductCR(),
+			product: []*capabilitiesv1beta1.Product{getProductCR()},
 			httpHandlerOptions: []mocks.ApplicationAPIHandlerOpt{
 				mocks.WithService(3, getApplicationPlanListByProductJson()),
 				mocks.WithAccount(3, &client.ApplicationList{Applications: []client.ApplicationElem{
@@ -297,8 +297,9 @@ func TestApplicationReconciler_Reconcile(t *testing.T) {
 				},
 			},
 			account: getApplicationDeveloperAccount(),
-			product: getProductCR(),
+			product: []*capabilitiesv1beta1.Product{getProductCR()},
 			httpHandlerOptions: []mocks.ApplicationAPIHandlerOpt{
+				mocks.WithInitAppID(3),
 				mocks.WithService(3, getApplicationPlanListByProductJson()),
 				mocks.WithAccount(3, &client.ApplicationList{Applications: []client.ApplicationElem{
 					{Application: *getApplicationJson("live")},
@@ -357,7 +358,7 @@ func TestApplicationReconciler_Reconcile(t *testing.T) {
 				},
 			},
 			account: getApplicationDeveloperAccount(),
-			product: getProductCR(),
+			product: []*capabilitiesv1beta1.Product{getProductCR()},
 			httpHandlerOptions: []mocks.ApplicationAPIHandlerOpt{
 				mocks.WithService(3, getApplicationPlanListByProductJson()),
 				mocks.WithAccount(3, &client.ApplicationList{Applications: []client.ApplicationElem{
@@ -397,6 +398,111 @@ func TestApplicationReconciler_Reconcile(t *testing.T) {
 				require.Equal(t, err.Error(), "applications.capabilities.3scale.net \"test\" not found")
 			},
 		},
+		{
+			name: "Update Product reference successful",
+			application: &capabilitiesv1beta1.Application{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: "test",
+				},
+				Spec: capabilitiesv1beta1.ApplicationSpec{
+					AccountCR: &corev1.LocalObjectReference{
+						Name: "test",
+					},
+					ProductCR: &corev1.LocalObjectReference{
+						Name: "test",
+					},
+					Name:                "test",
+					Description:         "test",
+					ApplicationPlanName: "test",
+				},
+			},
+			account: getApplicationDeveloperAccount(),
+			product: []*capabilitiesv1beta1.Product{
+				getProductCR(),
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test2",
+						Namespace: "test",
+					},
+					Spec: capabilitiesv1beta1.ProductSpec{
+						Name:        "test2",
+						SystemName:  "test2",
+						Description: "test2",
+					},
+					Status: capabilitiesv1beta1.ProductStatus{
+						ID:                  ptr.To(int64(4)),
+						ProviderAccountHost: "some string",
+						ObservedGeneration:  1,
+						Conditions: common.Conditions{common.Condition{
+							Type:   capabilitiesv1beta1.ProductSyncedConditionType,
+							Status: corev1.ConditionTrue,
+						}},
+					},
+				},
+			},
+			httpHandlerOptions: []mocks.ApplicationAPIHandlerOpt{
+				mocks.WithInitAppID(3),
+				mocks.WithService(3, getApplicationPlanListByProductJson()),
+				mocks.WithService(4, getApplicationPlanListByProductJson()),
+				mocks.WithAccount(3, &client.ApplicationList{Applications: []client.ApplicationElem{
+					{Application: *getApplicationJson("live")},
+				}}),
+			},
+			testBody: func(t *testing.T, r *reconcilers.BaseReconciler, req reconcile.Request) {
+				ctx := context.Background()
+				applicationReconciler := ApplicationReconciler{BaseReconciler: r}
+				_, err := applicationReconciler.Reconcile(ctx, req)
+				require.NoError(t, err)
+
+				t.Log("verifying the Application gets finalizers assigned")
+				var application capabilitiesv1beta1.Application
+				require.NoError(t, r.Client().Get(ctx, req.NamespacedName, &application))
+				require.ElementsMatch(t, application.GetFinalizers(), []string{
+					applicationFinalizer,
+				})
+
+				// TODO: check owner reference
+
+				// need to trigger the Reconcile again because the first one only updated the finalizers
+				_, err = applicationReconciler.Reconcile(ctx, req)
+				require.NoError(t, err, "reconciliation returned an error")
+				// need to trigger the Reconcile again because the previous updated the Status
+				_, err = applicationReconciler.Reconcile(ctx, req)
+				require.NoError(t, err, "reconciliation returned an error")
+
+				var currentApplication capabilitiesv1beta1.Application
+				require.NoError(t, r.Client().Get(context.Background(), req.NamespacedName, &currentApplication))
+
+				// Check status ID
+				require.Equal(t, currentApplication.Status.ID, ptr.To(int64(3)))
+				// check annotation
+				require.Equal(t, currentApplication.Annotations[applicationIdAnnotation], "3")
+				// Check condition
+				condition := currentApplication.Status.Conditions.GetCondition(capabilitiesv1beta1.ApplicationReadyConditionType)
+				require.Equal(t, corev1.ConditionTrue, condition.Status)
+
+				// Update productReference
+				currentApplication.Spec.ProductCR.Name = "test2"
+				require.NoError(t, r.Client().Update(context.Background(), &currentApplication))
+
+				_, err = applicationReconciler.Reconcile(ctx, req)
+				require.NoError(t, err, "reconciliation returned an error")
+
+				// AppID should increase to 4
+				require.NoError(t, r.Client().Get(context.Background(), req.NamespacedName, &currentApplication))
+				require.Equal(t, currentApplication.Status.ID, ptr.To(int64(4)))
+				condition = currentApplication.Status.Conditions.GetCondition(capabilitiesv1beta1.ApplicationReadyConditionType)
+				require.Equal(t, corev1.ConditionTrue, condition.Status)
+
+				// Reconcile one more time and check annotation
+				_, err = applicationReconciler.Reconcile(ctx, req)
+				require.NoError(t, err, "reconciliation returned an error")
+				require.NoError(t, r.Client().Get(context.Background(), req.NamespacedName, &currentApplication))
+				require.Equal(t, currentApplication.Annotations[applicationIdAnnotation], "4")
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -409,7 +515,9 @@ func TestApplicationReconciler_Reconcile(t *testing.T) {
 				objectsToAdd = append(objectsToAdd, tc.account)
 			}
 			if tc.product != nil {
-				objectsToAdd = append(objectsToAdd, tc.product)
+				for _, product := range tc.product {
+					objectsToAdd = append(objectsToAdd, product)
+				}
 			}
 
 			if tc.httpHandlerOptions != nil {

controllers/capabilities/application_threescale_reconciler.go

@@ -118,6 +118,27 @@ func (t *ApplicationThreescaleReconciler) syncApplication(_ any) error {
 		if err != nil {
 			return fmt.Errorf("error sync application [%s]: %w", t.applicationResource.Spec.Name, err)
 		}
+		application = &a
+		t.applicationResource.Status.ID = &application.ID
+	} else if application.ServiceID != t.productID {
+		// Product reference has changed
+		err := t.threescaleAPIClient.DeleteApplication(t.accountID, application.ID)
+		if err != nil {
+			return fmt.Errorf("failed to delete application [%s] id:[%d] productID:[%d] - err: %w", t.applicationResource.Spec.Name, application.ID, application.ServiceID, err)
+		}
+
+		// Recreate the application
+		params := threescaleapi.Params{
+			"name":        t.applicationResource.Spec.Name,
+			"description": t.applicationResource.Spec.Description,
+		}
+
+		// Application doesn't exist yet - create it
+		a, err := t.threescaleAPIClient.CreateApplication(t.accountID, plan.Element.ID, t.applicationResource.Spec.Name, params)
+		if err != nil {
+			return fmt.Errorf("reconcile3scaleApplication application [%s]: %w", t.applicationResource.Spec.Name, err)
+		}
+
 		application = &a
 		t.applicationResource.Status.ID = &application.ID
 	}

controllers/capabilities/mocks/application_api_handler.go

@@ -12,9 +12,10 @@ import (
 )
 
 type ApplicationAPIHandler struct {
-	mux      *http.ServeMux
-	services map[int64]*client.ApplicationPlanJSONList
-	accounts map[int64]*client.ApplicationList
+	mux        *http.ServeMux
+	appIDCount int64
+	services   map[int64]*client.ApplicationPlanJSONList
+	accounts   map[int64]*client.ApplicationList
 }
 
 type ApplicationAPIHandlerOpt func(h *ApplicationAPIHandler)
@@ -31,6 +32,12 @@ func WithAccount(account int64, applications *client.ApplicationList) Applicatio
 	}
 }
 
+func WithInitAppID(appID int64) ApplicationAPIHandlerOpt {
+	return func(m *ApplicationAPIHandler) {
+		m.appIDCount = appID
+	}
+}
+
 func NewApplicationAPIHandler(opts ...ApplicationAPIHandlerOpt) *ApplicationAPIHandler {
 	handler := &ApplicationAPIHandler{
 		accounts: make(map[int64]*client.ApplicationList),
@@ -139,7 +146,7 @@ func (m *ApplicationAPIHandler) applicationHandler(w http.ResponseWriter, r *htt
 		}
 
 		application := client.Application{
-			ID:            3,
+			ID:            m.appIDCount,
 			State:         "live",
 			UserAccountID: accountID,
 			ServiceID:     serviceID,
@@ -148,6 +155,9 @@ func (m *ApplicationAPIHandler) applicationHandler(w http.ResponseWriter, r *htt
 			Description:   r.FormValue("description"),
 		}
 
+		// Increase appIDCount
+		m.appIDCount++
+
 		elem := client.ApplicationElem{Application: application}
 
 		applications.Applications = append(applications.Applications, elem)

doc/operator-application-capabilities.md

@@ -1645,6 +1645,7 @@ Notes:
 
 * 3scale applications belong to some DeveloperAccount account.
 * 3scale applications are linked directly to a product and applicationPlan
+* Once set, you cannot modify the accountCR references. If you need to change the account reference, create a new ApplicationCR instead.
 
 Consider we have the following product which is connected to a backend
 ```yaml