Add support for Authorization upgrade (#549)

* Add support for authorization upgrade

* Add upgrade-path.yaml for authorization proxy server

* Add min upgrade path support for authorization proxy server

* Fix status loggers

* Fix linting issues

* Fix linting issues

* Fix authorization pre-checks

* Update checkUpgrade for authorization

* Update checkUpgrade for authorization

* Fix controller UT

* Add test scenarios for authorization sidecar upgrade to e2e

* Fix module UT failure

* Increase code coverage of controller pkg

* Increase code coverage of controller pkg

* Address review comments

---------

Co-authored-by: Jooseppi Luna <jooseppi_luna@dell.com>

api/v1/csm_types.go

@@ -104,6 +104,26 @@ func (cr *ContainerStorageModule) GetDriverType() DriverType {
 	return cr.Spec.Driver.CSIDriverType
 }
 
+// GetModule - Returns the module of type moduleType
+func (cr *ContainerStorageModule) GetModule(moduleType ModuleType) Module {
+	for _, m := range cr.Spec.Modules {
+		if m.Name == moduleType {
+			return m
+		}
+	}
+	return Module{}
+}
+
+// HasModule - Returns true if the cr has a module of type moduleType
+func (cr *ContainerStorageModule) HasModule(moduleType ModuleType) bool {
+	for _, m := range cr.Spec.Modules {
+		if m.Name == moduleType {
+			return true
+		}
+	}
+	return false
+}
+
 // IsBeingDeleted  - Returns  true if a deletion timestamp is set
 func (cr *ContainerStorageModule) IsBeingDeleted() bool {
 	return !cr.ObjectMeta.DeletionTimestamp.IsZero()

api/v1/types.go

@@ -31,6 +31,11 @@ type DriverType string
 // ModuleType - type representing the type of the modules. e.g. - authorization, podmon
 type ModuleType string
 
+// CSMComponentType - type constraint for DriverType and ModuleType
+type CSMComponentType interface {
+	ModuleType | DriverType
+}
+
 // ObservabilityComponentType - type representing the type of components inside observability module. e.g. - topology
 type ObservabilityComponentType string
 

controllers/csm_controller.go

@@ -328,10 +328,8 @@ func (r *ContainerStorageModuleReconciler) Reconcile(ctx context.Context, req ct
 	}
 
 	newStatus := csm.GetCSMStatus()
-	requeue, err := utils.HandleSuccess(ctx, csm, r, newStatus, oldStatus)
-	if err != nil {
-		log.Error(err, "Failed to update CR status")
-	}
+	requeue := utils.HandleSuccess(ctx, csm, r, newStatus, oldStatus)
+
 	// Update the driver
 	syncErr := r.SyncCSM(ctx, *csm, *operatorConfig, r.Client)
 	if syncErr == nil && !requeue.Requeue {
@@ -1248,7 +1246,6 @@ func (r *ContainerStorageModuleReconciler) removeModule(ctx context.Context, ins
 // PreChecks - validate input values
 func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *csmv1.ContainerStorageModule, operatorConfig utils.OperatorConfig) error {
 	log := logger.GetLogger(ctx)
-	var am bool
 	// Check drivers
 	switch cr.Spec.Driver.CSIDriverType {
 	case csmv1.PowerScale:
@@ -1278,17 +1275,11 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs
 			return fmt.Errorf("failed powermax validation: %v", err)
 		}
 	default:
-		for _, m := range cr.Spec.Modules {
-			if m.Name == csmv1.AuthorizationServer {
-				return nil
-			}
-			if m.Name == csmv1.ApplicationMobility {
-				am = true
-			}
-		}
-		if am {
+		// Go to checkUpgrade if it is standalone module i.e. app mobility or authorizatio proxy server
+		if cr.HasModule(csmv1.ApplicationMobility) || cr.HasModule(csmv1.AuthorizationServer) {
 			break
 		}
+
 		return fmt.Errorf("unsupported driver type %s", cr.Spec.Driver.CSIDriverType)
 	}
 
@@ -1368,40 +1359,25 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs
 // Check for upgrade/if upgrade is appropriate
 func checkUpgrade(ctx context.Context, cr *csmv1.ContainerStorageModule, operatorConfig utils.OperatorConfig) (bool, error) {
 	log := logger.GetLogger(ctx)
-	driverType := cr.Spec.Driver.CSIDriverType
-	if driverType == csmv1.PowerScale {
-		// use powerscale instead of isilon as the folder name is powerscale
-		driverType = csmv1.PowerScaleName
-	}
 
 	// If it is an upgrade/downgrade, check to see if we meet the minimum version using GetUpgradeInfo, which returns the minimum version required
 	// for the desired upgrade. If the upgrade path is not valid fail
 	// Existing version
 	annotations := cr.GetAnnotations()
 	oldVersion, configVersionExists := annotations[configVersionKey]
-
 	// If annotation exists, we are doing an upgrade or modify
 	if configVersionExists {
-		// if versions are equal, it is a modification
-		if oldVersion == cr.Spec.Driver.ConfigVersion {
-			log.Infow("proceeding with modification of driver install")
-			return true, nil
-		}
-		// if not equal, it is an upgrade/downgrade
-		// get minimum required version for upgrade
-		minUpgradePath, err := drivers.GetUpgradeInfo(ctx, operatorConfig, driverType, oldVersion)
-		if err != nil {
-			log.Infow("GetUpgradeInfo not successful")
-			return false, err
+		if cr.HasModule(csmv1.AuthorizationServer) {
+			newVersion := cr.GetModule(csmv1.AuthorizationServer).ConfigVersion
+			return utils.IsValidUpgrade(ctx, oldVersion, newVersion, csmv1.Authorization, operatorConfig)
 		}
-		//
-		installValid, _ := utils.MinVersionCheck(minUpgradePath, cr.Spec.Driver.ConfigVersion)
-		if installValid {
-			log.Infow("proceeding with valid driver upgrade from version %s to version %s", oldVersion, cr.Spec.Driver.ConfigVersion)
-			return installValid, nil
+		driverType := cr.Spec.Driver.CSIDriverType
+		if driverType == csmv1.PowerScale {
+			// use powerscale instead of isilon as the folder name is powerscale
+			driverType = csmv1.PowerScaleName
 		}
-		log.Infow("not proceeding with invalid driver upgrade")
-		return installValid, fmt.Errorf("failed upgrade check: upgrade from version %s to %s not valid", oldVersion, cr.Spec.Driver.ConfigVersion)
+		newVersion := cr.Spec.Driver.ConfigVersion
+		return utils.IsValidUpgrade(ctx, oldVersion, newVersion, driverType, operatorConfig)
 	}
 	log.Infow("proceeding with fresh driver install")
 	return true, nil
@@ -1411,23 +1387,28 @@ func checkUpgrade(ctx context.Context, cr *csmv1.ContainerStorageModule, operato
 func applyConfigVersionAnnotations(ctx context.Context, instance *csmv1.ContainerStorageModule) bool {
 	log := logger.GetLogger(ctx)
 
-	// If driver has not been initialized yet, we first annotate the driver with the config version annotation
+	// If driver/module has not been initialized yet, we first annotate the component with the config version annotation
 
 	annotations := instance.GetAnnotations()
 	isUpdated := false
 	if annotations == nil {
 		annotations = make(map[string]string)
 	}
 	annotations[CSMVersionKey] = CSMVersion
-	instance.SetAnnotations(annotations)
 
 	if _, ok := annotations[configVersionKey]; !ok {
-		annotations[configVersionKey] = instance.Spec.Driver.ConfigVersion
+		configVersion := ""
+		if instance.HasModule(csmv1.AuthorizationServer) {
+			configVersion = instance.GetModule(csmv1.AuthorizationServer).ConfigVersion
+		} else {
+			configVersion = instance.Spec.Driver.ConfigVersion
+		}
+		annotations[configVersionKey] = configVersion
 		isUpdated = true
-		instance.SetAnnotations(annotations)
-		log.Infof("Installing storage component %s with config Version %s. Updating Annotations with Config Version",
-			instance.GetName(), instance.Spec.Driver.ConfigVersion)
+		log.Infof("Installing csm component %s with config Version %s. Updating Annotations with Config Version",
+			instance.GetName(), configVersion)
 	}
+	instance.SetAnnotations(annotations)
 	return isUpdated
 }
 

controllers/csm_controller_test.go

@@ -192,6 +192,13 @@ func (suite *CSMControllerTestSuite) TestAuthorizationServerReconcile() {
 	suite.runFakeAuthCSMManager("", true)
 }
 
+func (suite *CSMControllerTestSuite) TestAuthorizationServerPreCheck() {
+	suite.makeFakeAuthServerCSMWithoutPreRequisite(csmName, suite.namespace)
+	suite.runFakeAuthCSMManager("failed authorization proxy server validation", false)
+	suite.deleteCSM(csmName)
+	suite.runFakeAuthCSMManager("", true)
+}
+
 func (suite *CSMControllerTestSuite) TestAppMobReconcile() {
 	suite.makeFakeAppMobCSM(csmName, suite.namespace, getAppMob())
 	suite.runFakeAuthCSMManager("", false)
@@ -416,10 +423,12 @@ func (suite *CSMControllerTestSuite) TestCsmUpgradeSkipVersion() {
 		annotations = make(map[string]string)
 	}
 	if _, ok := annotations[configVersionKey]; !ok {
-		annotations[configVersionKey] = jumpUpgradeConfigVersion
+		annotations[configVersionKey] = configVersion
 		csm.SetAnnotations(annotations)
 	}
 
+	csm.Spec.Driver.ConfigVersion = jumpUpgradeConfigVersion
+
 	reconciler := suite.createReconciler()
 	_, err := reconciler.Reconcile(ctx, req)
 	assert.Error(suite.T(), err)
@@ -1693,30 +1702,40 @@ func (suite *CSMControllerTestSuite) makeFakeAppMobCSM(name, ns string, _ []csmv
 
 func (suite *CSMControllerTestSuite) makeFakeAuthServerCSM(name, ns string, _ []csmv1.Module) {
 	// this secret required by authorization module
-	sec := shared.MakeSecret("karavi-config-secret", ns, configVersion)
+	sec := shared.MakeSecret("karavi-config-secret", ns, shared.AuthServerConfigVersion)
 	err := suite.fakeClient.Create(ctx, sec)
 	assert.Nil(suite.T(), err)
 
 	// this secret required by authorization module
-	sec = shared.MakeSecret("proxy-storage-secret", ns, configVersion)
+	sec = shared.MakeSecret("karavi-storage-secret", ns, shared.AuthServerConfigVersion)
 	err = suite.fakeClient.Create(ctx, sec)
 	assert.Nil(suite.T(), err)
 
 	// this secret required by authorization module
-	sec = shared.MakeSecret("karavi-auth-tls", ns, configVersion)
+	sec = shared.MakeSecret("karavi-auth-tls", ns, shared.AuthServerConfigVersion)
 	err = suite.fakeClient.Create(ctx, sec)
 	assert.Nil(suite.T(), err)
 
-	csm := shared.MakeModuleCSM(name, ns, configVersion)
+	csm := shared.MakeModuleCSM(name, ns, shared.AuthServerConfigVersion)
 
 	csm.Spec.Modules = getAuthProxyServer()
 	csm.Spec.Modules[0].ForceRemoveModule = true
-	csm.Annotations[configVersionKey] = configVersion
 
 	err = suite.fakeClient.Create(ctx, &csm)
 	assert.Nil(suite.T(), err)
 }
 
+func (suite *CSMControllerTestSuite) makeFakeAuthServerCSMWithoutPreRequisite(name, ns string) {
+	csm := shared.MakeModuleCSM(name, ns, shared.AuthServerConfigVersion)
+
+	csm.Spec.Modules = getAuthProxyServer()
+	csm.Spec.Modules[0].ForceRemoveModule = true
+	csm.Annotations[configVersionKey] = shared.AuthServerConfigVersion
+
+	err := suite.fakeClient.Create(ctx, &csm)
+	assert.Nil(suite.T(), err)
+}
+
 func (suite *CSMControllerTestSuite) makeFakePod(name, ns string) {
 	pod := shared.MakePod(name, ns)
 	pod.Labels["csm"] = csmName

operatorconfig/moduleconfig/authorization/v1.10.0/upgrade-path.yaml

@@ -0,0 +1,2 @@
+
+minUpgradePath: v1.9.0

operatorconfig/moduleconfig/authorization/v1.8.0/upgrade-path.yaml

@@ -0,0 +1 @@
+minUpgradePath: v1.7.0

operatorconfig/moduleconfig/authorization/v1.9.0/upgrade-path.yaml

@@ -0,0 +1,2 @@
+
+minUpgradePath: v1.8.0

operatorconfig/moduleconfig/authorization/v1.9.1/upgrade-path.yaml

@@ -0,0 +1,2 @@
+
+minUpgradePath: v1.8.0

pkg/modules/authorization.go

@@ -447,7 +447,7 @@ func AuthorizationServerPrecheck(ctx context.Context, op utils.OperatorConfig, a
 	}
 
 	// Check for secrets
-	proxyServerSecrets := []string{"karavi-config-secret", "karavi-storage-secret", "karavi-auth-tls"}
+	proxyServerSecrets := []string{"karavi-config-secret", "karavi-storage-secret"}
 	for _, name := range proxyServerSecrets {
 		found := &corev1.Secret{}
 		err := r.GetClient().Get(ctx, types.NamespacedName{Name: name, Namespace: cr.GetNamespace()}, found)