fixes based on feedback/updates for create/delete/update logic

pkg/controller/mobilesecurityservice/controller.go

@@ -18,11 +18,11 @@ import (
 )
 
 const (
-	CONFIGMAP     = "ConfigMap"
-	DEEPLOYMENT   = "Deployment"
-	SERVICE       = "Service"
-	ROUTE         = "Route"
-	SERVERSERVICE = "Server Service"
+	CONFIGMAP           = "ConfigMap"
+	DEEPLOYMENT         = "Deployment"
+	PROXY_SERVICE       = "Proxy Service"
+	ROUTE               = "Route"
+	APPLICATION_SERVICE = "Application Service"
 )
 
 var log = logf.Log.WithName("controller_mobilesecurityservice")
@@ -115,22 +115,22 @@ func (r *ReconcileMobileSecurityService) create(instance *mobilesecurityservicev
 	return reconcile.Result{Requeue: true}, nil
 }
 
-// buildFactory will return the resource according to the kind defined
-func (r *ReconcileMobileSecurityService) buildFactory(reqLogger logr.Logger, instance *mobilesecurityservicev1alpha1.MobileSecurityService, kind string) (runtime.Object, error) {
-	reqLogger.Info("Check "+kind, "into the namespace", instance.Namespace)
-	switch kind {
+// buildFactory will return the resource according to the resource defined
+func (r *ReconcileMobileSecurityService) buildFactory(reqLogger logr.Logger, instance *mobilesecurityservicev1alpha1.MobileSecurityService, resource string) (runtime.Object, error) {
+	reqLogger.Info("Check "+resource, "into the namespace", instance.Namespace)
+	switch resource {
 	case CONFIGMAP:
 		return r.buildConfigMap(instance), nil
 	case DEEPLOYMENT:
 		return r.buildDeployment(instance), nil
-	case SERVICE:
-		return r.buildService(instance), nil
-	case SERVERSERVICE:
-		return r.buildServerService(instance), nil
+	case PROXY_SERVICE:
+		return r.buildProxyService(instance), nil
+	case APPLICATION_SERVICE:
+		return r.buildApplicationService(instance), nil
 	case ROUTE:
 		return r.buildRoute(instance), nil
 	default:
-		msg := "Failed to recognize type of object" + kind + " into the Namespace " + instance.Namespace
+		msg := "Failed to recognize type of object" + resource + " into the Namespace " + instance.Namespace
 		panic(msg)
 	}
 }
@@ -190,13 +190,13 @@ func (r *ReconcileMobileSecurityService) Reconcile(request reconcile.Request) (r
 	}
 
 	//Check if Service for the app exist, if not create one
-	if _, err := r.fetchService(reqLogger, instance); err != nil {
-		return r.create(instance, reqLogger, SERVICE)
+	if _, err := r.fetchService(reqLogger, instance, utils.PROXY_SERVICE_INSTANCE_NAME); err != nil {
+		return r.create(instance, reqLogger, PROXY_SERVICE)
 	}
 
 	//Check if Service for the app exist, if not create one
-	if _, err := r.fetchServerService(reqLogger, instance); err != nil {
-		return r.create(instance, reqLogger, SERVERSERVICE)
+	if _, err := r.fetchService(reqLogger, instance, utils.APPLICATION_SERVICE_INSTANCE_NAME); err != nil {
+		return r.create(instance, reqLogger, APPLICATION_SERVICE)
 	}
 
 	//Check if Route for the Service exist, if not create one
@@ -216,13 +216,13 @@ func (r *ReconcileMobileSecurityService) Reconcile(request reconcile.Request) (r
 		return reconcile.Result{}, err
 	}
 
-	//Update status for Service
-	serviceStatus, err := r.updateServiceStatus(reqLogger, request)
+	//Update status for Proxy Service
+	proxyServiceStatus, err := r.updateServiceStatus(reqLogger, request, utils.PROXY_SERVICE_INSTANCE_NAME)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
 
-	serverServiceStatus, err := r.updateServiceStatus(reqLogger, request)
+	applicationServiceStatus, err := r.updateServiceStatus(reqLogger, request, utils.APPLICATION_SERVICE_INSTANCE_NAME)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
@@ -234,7 +234,7 @@ func (r *ReconcileMobileSecurityService) Reconcile(request reconcile.Request) (r
 	}
 
 	//Update status for App
-	if err := r.updateStatus(reqLogger, configMapStatus, deploymentStatus, serviceStatus, serverServiceStatus, routeStatus, request); err != nil {
+	if err := r.updateStatus(reqLogger, configMapStatus, deploymentStatus, proxyServiceStatus, applicationServiceStatus, routeStatus, request); err != nil {
 
 		return reconcile.Result{}, err
 	}

pkg/controller/mobilesecurityservice/fetches.go

@@ -30,19 +30,11 @@ func (r *ReconcileMobileSecurityService) fetchRoute(reqLogger logr.Logger, insta
 	return route, err
 }
 
-//fetchService returns the service resource created for this instance
-func (r *ReconcileMobileSecurityService) fetchService(reqLogger logr.Logger, instance *mobilesecurityservicev1alpha1.MobileSecurityService) (*corev1.Service, error) {
+//fetchServerService returns the application service resource created for this instance
+func (r *ReconcileMobileSecurityService) fetchService(reqLogger logr.Logger, instance *mobilesecurityservicev1alpha1.MobileSecurityService, name string) (*corev1.Service, error) {
 	reqLogger.Info("Checking if the service already exists")
 	service := &corev1.Service{}
-	err := r.client.Get(context.TODO(), types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}, service)
-	return service, err
-}
-
-//fetchServerService returns the service resource created for this instance
-func (r *ReconcileMobileSecurityService) fetchServerService(reqLogger logr.Logger, instance *mobilesecurityservicev1alpha1.MobileSecurityService) (*corev1.Service, error) {
-	reqLogger.Info("Checking if the service already exists")
-	service := &corev1.Service{}
-	err := r.client.Get(context.TODO(), types.NamespacedName{Name: utils.SERVER_SERVICE_INSTANCE_NAME, Namespace: instance.Namespace}, service)
+	err := r.client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: instance.Namespace}, service)
 	return service, err
 }
 

pkg/controller/mobilesecurityservice/route.go

@@ -22,7 +22,7 @@ func (r *ReconcileMobileSecurityService) buildRoute(m *mobilesecurityservicev1al
 		Spec: routev1.RouteSpec{
 			To: routev1.RouteTargetReference{
 				Kind: "Service",
-				Name: m.Name,
+				Name: utils.PROXY_SERVICE_INSTANCE_NAME,
 			},
 			Port: &routev1.RoutePort{
 				TargetPort: intstr.FromString("web"),

pkg/controller/mobilesecurityservice/services.go

@@ -11,12 +11,12 @@ import (
 )
 
 //buildService returns the service resource
-func (r *ReconcileMobileSecurityService) buildService(m *mobilesecurityservicev1alpha1.MobileSecurityService) *corev1.Service {
+func (r *ReconcileMobileSecurityService) buildProxyService(m *mobilesecurityservicev1alpha1.MobileSecurityService) *corev1.Service {
 	ls := getAppLabels(m.Name)
 	targetPort := intstr.FromInt(int(m.Spec.OAuthPort))
 	ser := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      m.Name,
+			Name:      utils.PROXY_SERVICE_INSTANCE_NAME,
 			Namespace: m.Namespace,
 			Labels:    ls,
 		},
@@ -27,41 +27,32 @@ func (r *ReconcileMobileSecurityService) buildService(m *mobilesecurityservicev1
 				{
 					TargetPort: targetPort,
 					Port:       80,
-					Protocol:   "TCP",
 					Name:       "web",
 				},
 			},
-			SessionAffinity: "None",
 		},
 	}
 	// Set MobileSecurityService instance as the owner and controller
 	controllerutil.SetControllerReference(m, ser, r.scheme)
 	return ser
 }
 
-func (r *ReconcileMobileSecurityService) buildServerService(m *mobilesecurityservicev1alpha1.MobileSecurityService) *corev1.Service {
+func (r *ReconcileMobileSecurityService) buildApplicationService(m *mobilesecurityservicev1alpha1.MobileSecurityService) *corev1.Service {
 	ls := getAppLabels(m.Name)
 	ser := &corev1.Service{
-		TypeMeta: metav1.TypeMeta{
-			APIVersion: "v1",
-			Kind:       "Service",
-		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      utils.SERVER_SERVICE_INSTANCE_NAME,
+			Name:      utils.APPLICATION_SERVICE_INSTANCE_NAME,
 			Namespace: m.Namespace,
 			Labels:    ls,
 		},
 		Spec: corev1.ServiceSpec{
 			Selector: ls,
-			Type:     corev1.ServiceTypeClusterIP,
 			Ports: []corev1.ServicePort{
 				{
-					Port:     m.Spec.Port,
-					Protocol: "TCP",
-					Name:     "server",
+					Port: m.Spec.Port,
+					Name: "server",
 				},
 			},
-			SessionAffinity: "None",
 		},
 	}
 	// Set MobileSecurityService instance as the owner and controller

pkg/controller/mobilesecurityservice/status.go

@@ -14,7 +14,7 @@ import (
 )
 
 //updateStatus returns error when status regards the all required resources could not be updated
-func (r *ReconcileMobileSecurityService) updateStatus(reqLogger logr.Logger, configMapStatus *corev1.ConfigMap, deploymentStatus *v1beta1.Deployment, serviceStatus *corev1.Service, serverServiceStatus *corev1.Service, routeStatus *routev1.Route, request reconcile.Request) error {
+func (r *ReconcileMobileSecurityService) updateStatus(reqLogger logr.Logger, configMapStatus *corev1.ConfigMap, deploymentStatus *v1beta1.Deployment, proxyServiceStatus *corev1.Service, applicationServiceStatus *corev1.Service, routeStatus *routev1.Route, request reconcile.Request) error {
 	reqLogger.Info("Updating App Status for the MobileSecurityService")
 	// Get the latest version of the CR
 	instance, err := r.fetchInstance(reqLogger, request)
@@ -23,7 +23,7 @@ func (r *ReconcileMobileSecurityService) updateStatus(reqLogger logr.Logger, con
 	}
 
 	//Check if all required objects are created
-	if len(configMapStatus.UID) < 1 && len(deploymentStatus.UID) < 1 && len(serviceStatus.UID) < 1 && len(serverServiceStatus.UID) < 1 && len(routeStatus.Name) < 1 {
+	if len(configMapStatus.UID) < 1 && len(deploymentStatus.UID) < 1 && len(proxyServiceStatus.UID) < 1 && len(applicationServiceStatus.UID) < 1 && len(routeStatus.Name) < 1 {
 		err := fmt.Errorf("Failed to get OK Status for MobileSecurityService")
 		reqLogger.Error(err, "One of the resources are not created", "MobileSecurityService.Namespace", instance.Namespace, "MobileSecurityService.Name", instance.Name)
 		return err
@@ -32,11 +32,6 @@ func (r *ReconcileMobileSecurityService) updateStatus(reqLogger logr.Logger, con
 
 	// Update CR with the AppStatus == OK
 	if !reflect.DeepEqual(status, instance.Status.AppStatus) {
-		// Get the latest version of the CR
-		instance, err := r.fetchInstance(reqLogger, request)
-		if err != nil {
-			return err
-		}
 
 		// Set the data
 		instance.Status.AppStatus = status
@@ -69,12 +64,6 @@ func (r *ReconcileMobileSecurityService) updateConfigMapStatus(reqLogger logr.Lo
 
 	// Update ConfigMap Name
 	if configMapStatus.Name != instance.Status.ConfigMapName {
-		// Get the latest version of the CR
-		instance, err := r.fetchInstance(reqLogger, request)
-		if err != nil {
-			return nil, err
-		}
-
 		// Set the data
 		instance.Status.ConfigMapName = configMapStatus.Name
 
@@ -128,27 +117,22 @@ func (r *ReconcileMobileSecurityService) updateDeploymentStatus(reqLogger logr.L
 }
 
 //updateServiceStatus returns error when status regards the Service resource could not be updated
-func (r *ReconcileMobileSecurityService) updateServiceStatus(reqLogger logr.Logger, request reconcile.Request) (*corev1.Service, error) {
-	reqLogger.Info("Updating Service Status for the MobileSecurityService")
+func (r *ReconcileMobileSecurityService) updateServiceStatus(reqLogger logr.Logger, request reconcile.Request, name string) (*corev1.Service, error) {
+	reqLogger.Info("Updating ", name, " Service Status for the MobileSecurityService")
 	// Get the latest version of the CR
 	instance, err := r.fetchInstance(reqLogger, request)
 	if err != nil {
 		return nil, err
 	}
 	// Get the Service Object
-	serviceStatus, err := r.fetchService(reqLogger, instance)
+	serviceStatus, err := r.fetchService(reqLogger, instance, name)
 	if err != nil {
-		reqLogger.Error(err, "Failed to get Service for Status", "MobileSecurityService.Namespace", instance.Namespace, "MobileSecurityService.Name", instance.Name)
+		reqLogger.Error(err, "Failed to get ", name, " Service for Status", "MobileSecurityService.Namespace", instance.Namespace, "MobileSecurityService.Name", instance.Name)
 		return serviceStatus, err
 	}
 
 	// Update the Service Status and Name
-	if serviceStatus.Name != instance.Status.ServiceName || !reflect.DeepEqual(serviceStatus.Status, instance.Status.ServiceStatus) {
-		// Get the latest version of the CR
-		instance, err := r.fetchInstance(reqLogger, request)
-		if err != nil {
-			return nil, err
-		}
+	if serviceStatus.Name != name {
 
 		// Set the data
 		instance.Status.ServiceName = serviceStatus.Name
@@ -157,7 +141,7 @@ func (r *ReconcileMobileSecurityService) updateServiceStatus(reqLogger logr.Logg
 		// Update the CR
 		err = r.client.Status().Update(context.TODO(), instance)
 		if err != nil {
-			reqLogger.Error(err, "Failed to update Service Name and Status for the MobileSecurityService")
+			reqLogger.Error(err, "Failed to update ", name, " Service Name and Status for the MobileSecurityService")
 			return serviceStatus, err
 		}
 	}
@@ -187,12 +171,6 @@ func (r *ReconcileMobileSecurityService) updateRouteStatus(reqLogger logr.Logger
 	// Update the Route Status and Name
 	if routeName != instance.Status.RouteName || !reflect.DeepEqual(route.Status, instance.Status.RouteStatus) {
 
-		// Get the latest version of the CR
-		instance, err := r.fetchInstance(reqLogger, request)
-		if err != nil {
-			return nil, err
-		}
-
 		// Set the data
 		instance.Status.RouteName = routeName
 		instance.Status.RouteStatus = route.Status

pkg/controller/mobilesecurityserviceapp/controller.go

@@ -135,16 +135,17 @@ func (r *ReconcileMobileSecurityServiceApp) Reconcile(request reconcile.Request)
 	reqLogger.Info("Valid namespace for MobileSecurityServiceApp", "Namespace", request.Namespace)
 
 	reqLogger.Info("Checking for service instance ...")
-	serviceInstance := &mobilesecurityservicev1alpha1.MobileSecurityService{}
+	mssInstance := &mobilesecurityservicev1alpha1.MobileSecurityService{}
 	operatorNamespace, _ := k8sutil.GetOperatorNamespace()
-	if err := r.client.Get(context.TODO(), types.NamespacedName{Name: utils.SERVICE_INSTANCE_NAME, Namespace: operatorNamespace}, serviceInstance); err != nil {
+
+	if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "mobile-security-service", Namespace: operatorNamespace}, mssInstance); err != nil {
 		// Return and don't create
-		reqLogger.Info("Mobile Security Service instance resource not found. Ignoring since object must be deleted")
+		reqLogger.Info("Mobile Security Service instance resource not found. Mobile Security Service Application is required to create the application")
 		return reconcile.Result{}, nil
 	}
 
 	//Get the REST Service Endpoint
-	serviceAPI := service.GetServiceAPIURL(serviceInstance)
+	serviceAPI := service.GetServiceAPIURL(mssInstance)
 
 	//Check if the APP CR was marked to be deleted
 	isAppMarkedToBeDeleted := instance.GetDeletionTimestamp() != nil
@@ -174,7 +175,7 @@ func (r *ReconcileMobileSecurityServiceApp) Reconcile(request reconcile.Request)
 	}
 
 	//Check specs
-	if !hasMandatorySpecs(instance, serviceInstance, reqLogger) {
+	if !hasMandatorySpecs(instance, mssInstance, reqLogger) {
 		return reconcile.Result{Requeue: true}, nil
 	}
 
@@ -188,28 +189,30 @@ func (r *ReconcileMobileSecurityServiceApp) Reconcile(request reconcile.Request)
 		return r.create(instance, CONFIGMAP, serviceAPI, reqLogger, request)
 	}
 
-	//Check if App is Bind in the REST Service, if not then bind it
-	if app, err := fetchBindAppRestServiceByAppID(serviceAPI, instance, reqLogger); err == nil {
-		//Update the name by the REST API
-		if app.AppName != instance.Spec.AppName {
-			app.AppName = instance.Spec.AppName
-			//Check if App was update with success
+	app, err := fetchBindAppRestServiceByAppID(serviceAPI, instance, reqLogger)
 
-			if err := service.UpdateAppNameByRestAPI(serviceAPI, app, reqLogger); err != nil {
-				return reconcile.Result{}, err
-			}
-			return reconcile.Result{Requeue: true}, nil
-		}
-		// Bind App in the Service by the REST API
-		if app.ID == "" {
-			newApp := models.NewApp(instance.Spec.AppName, instance.Spec.AppId)
-			if err := service.CreateAppByRestAPI(serviceAPI, newApp, reqLogger); err != nil {
-				return reconcile.Result{}, err
-			}
-			return reconcile.Result{Requeue: true}, nil
+	if err != nil {
+		return reconcile.Result{Requeue: true}, err
+	}
+	//if no error found, i.e. request completed successfully
+
+	//Check if the app exists, if yes update
+	if app.AppName != "" && app.AppName != instance.Spec.AppName {
+		app.AppName = instance.Spec.AppName
+		//Check if App was update with success
+		//TODO if the app is found but it is soft deleted, we need to remove the soft delete
+		err := service.UpdateAppNameByRestAPI(serviceAPI, app, reqLogger)
+		if err != nil {
+			return reconcile.Result{}, err
 		}
-		return reconcile.Result{Requeue: true}, nil
+
+	}
+	//else create the app
+	newApp := models.NewApp(instance.Spec.AppName, instance.Spec.AppId)
+	if err := service.CreateAppByRestAPI(serviceAPI, newApp, reqLogger); err != nil {
+		return reconcile.Result{}, err
 	}
+	return reconcile.Result{Requeue: false}, nil
 
 	//Update status for SDKConfigMap
 	SDKConfigMapStatus, err := r.updateSDKConfigMapStatus(reqLogger, request)

pkg/controller/mobilesecurityserviceapp/fetches.go

@@ -2,6 +2,7 @@ package mobilesecurityserviceapp
 
 import (
 	"context"
+
 	mobilesecurityservicev1alpha1 "github.com/aerogear/mobile-security-service-operator/pkg/apis/mobilesecurityservice/v1alpha1"
 	"github.com/aerogear/mobile-security-service-operator/pkg/models"
 	"github.com/aerogear/mobile-security-service-operator/pkg/service"
@@ -13,14 +14,13 @@ import (
 
 // Request object not found, could have been deleted after reconcile request.
 // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
-func (r *ReconcileMobileSecurityServiceApp) fetchInstance( reqLogger logr.Logger, request reconcile.Request) (*mobilesecurityservicev1alpha1.MobileSecurityServiceApp, error) {
+func (r *ReconcileMobileSecurityServiceApp) fetchInstance(reqLogger logr.Logger, request reconcile.Request) (*mobilesecurityservicev1alpha1.MobileSecurityServiceApp, error) {
 	instance := &mobilesecurityservicev1alpha1.MobileSecurityServiceApp{}
 	//Fetch the MobileSecurityServiceApp instance
 	err := r.client.Get(context.TODO(), request.NamespacedName, instance)
 	return instance, err
 }
 
-
 //fetchSDKConfigMap returns the config map resource created for this instance
 func (r *ReconcileMobileSecurityServiceApp) fetchSDKConfigMap(reqLogger logr.Logger, instance *mobilesecurityservicev1alpha1.MobileSecurityServiceApp) (*corev1.ConfigMap, error) {
 	reqLogger.Info("Checking if the ConfigMap already exists")
@@ -30,6 +30,6 @@ func (r *ReconcileMobileSecurityServiceApp) fetchSDKConfigMap(reqLogger logr.Log
 }
 
 //fetchBindAppRestServiceByAppID return app struct from Mobile Security Service Project/REST API or error
-func fetchBindAppRestServiceByAppID(serviceURL string, instance *mobilesecurityservicev1alpha1.MobileSecurityServiceApp, reqLogger logr.Logger) (models.App, error){
+func fetchBindAppRestServiceByAppID(serviceURL string, instance *mobilesecurityservicev1alpha1.MobileSecurityServiceApp, reqLogger logr.Logger) (*models.App, error) {
 	return service.GetAppFromServiceByRestApi(serviceURL, instance.Spec.AppId, reqLogger)
-}
+}