Some refactor

Author: lubronzhan

api/v1alpha1/constants.go

@@ -55,4 +55,6 @@ const (
 	HAAVIInfraSettingAnnotationsKey    = "aviinfrasetting.ako.vmware.com/name"
 
 	AKODeploymentConfigControllerName = "akodeploymentconfig-controller"
+
+	AVIControllerEnterpriseOnlyVersion = "v30.0.0"
 )

controllers/akodeploymentconfig/user/ako_role.go

@@ -328,10 +328,6 @@ var deprecatePermissionMap = map[string]string{
 }
 
 func filterAkoRolePermissionByVersion(log logr.Logger, permissions []*models.Permission, version string) []*models.Permission {
-	// Add v prefix if not present so semver can parse it
-	if len(version) > 0 && version[0] != 'v' {
-		version = "v" + version
-	}
 	filtered := []*models.Permission{}
 	for _, permission := range permissions {
 		if v, ok := deprecatePermissionMap[*permission.Resource]; ok && semver.Compare(version, v) >= 0 {

controllers/akodeploymentconfig/user/user_controller.go

@@ -269,11 +269,15 @@ func (r *AkoUserReconciler) createOrUpdateAviUser(log logr.Logger, aviUsername,
 	if err != nil {
 		return err
 	}
+	// Add v prefix if not present so semver can parse it
+	if len(version) > 0 && version[0] != 'v' {
+		version = "v" + version
+	}
 
 	aviUser, err := r.aviClient.UserGetByName(aviUsername)
 	// user not found, create one
 	if aviclient.IsAviUserNonExistentError(err) {
-		log.Info("AVI User not found, creating a new one")
+		log.Info("AVI User not found, creating a new user", "user", aviUsername)
 		// for avi essential version the default tenant is admin
 		if tenantName == "" {
 			tenantName = "admin"
@@ -298,6 +302,10 @@ func (r *AkoUserReconciler) createOrUpdateAviUser(log logr.Logger, aviUsername,
 				},
 			},
 		}
+		// since v30.0.0, there is only enterprise edition
+		if semver.Compare(version, akoov1alpha1.AVIControllerEnterpriseOnlyVersion) >= 0 {
+			aviUser.Username = &aviUsername
+		}
 		if _, err := r.aviClient.UserCreate(aviUser); err != nil {
 			return err
 		}
@@ -314,8 +322,9 @@ func (r *AkoUserReconciler) createOrUpdateAviUser(log logr.Logger, aviUsername,
 	// Update the password when user found, this is needed when the AVI user was
 	// created before the mc Secret. And this operation will sync
 	// the User's password to be the same as mc Secret's
-	if *aviUser.Password != aviPassword {
+	if aviUser.Password == nil || *aviUser.Password != aviPassword {
 		log.Info("AVI User found, updating the password")
+		aviUser.Password = &aviPassword
 		if _, err := r.aviClient.UserUpdate(aviUser); err != nil {
 			return err
 		}
@@ -325,7 +334,7 @@ func (r *AkoUserReconciler) createOrUpdateAviUser(log logr.Logger, aviUsername,
 
 // getOrCreateAkoUserRole get ako user's role, create one if not exist
 func (r *AkoUserReconciler) getOrCreateAkoUserRole(log logr.Logger, roleTenantRef *string, version string) (*models.Role, error) {
-	log.V(3).Info("Ensure AKO User Role")
+	log.Info("Ensure AKO User Role")
 	role, err := r.aviClient.RoleGetByName(akoov1alpha1.AkoUserRoleName)
 	// not found ako user role, create one
 	if aviclient.IsAviRoleNonExistentError(err) {

controllers/akodeploymentconfig/user/user_controller_test.go

@@ -139,7 +139,7 @@ func SyncAkoUserRoleTest() {
 	Specify("role has no permissions", func() {
 		role := &models.Role{}
 
-		updated := syncAkoUserRole(role, "v30.2.1")
+		updated := syncAkoUserRole(role, "v20.0.0")
 		Expect(updated).To(BeTrue())
 		Expect(role.Privileges).To(HaveLen(len(AkoRolePermission)))
 		Expect(role.Privileges).To(ContainElements(AkoRolePermission))
@@ -159,7 +159,7 @@ func SyncAkoUserRoleTest() {
 			}
 		}
 
-		updated := syncAkoUserRole(role, "v30.2.1")
+		updated := syncAkoUserRole(role, "v20.0.0")
 		Expect(updated).To(BeTrue())
 		Expect(role.Privileges).To(HaveLen(len(AkoRolePermission)))
 		Expect(role.Privileges).To(ContainElements(AkoRolePermission))
@@ -179,7 +179,7 @@ func SyncAkoUserRoleTest() {
 			})
 		}
 
-		updated := syncAkoUserRole(role, "v30.2.1")
+		updated := syncAkoUserRole(role, "v20.0.0")
 		Expect(updated).To(BeTrue())
 		Expect(role.Privileges).To(HaveLen(len(AkoRolePermission)))
 		Expect(role.Privileges).To(ContainElements(AkoRolePermission))
@@ -208,7 +208,7 @@ func SyncAkoUserRoleTest() {
 			role.Privileges[i], role.Privileges[j] = role.Privileges[j], role.Privileges[i]
 		})
 
-		updated := syncAkoUserRole(role, "v30.2.1")
+		updated := syncAkoUserRole(role, "v20.0.0")
 		Expect(updated).To(BeFalse())
 		Expect(role.Privileges).To(HaveLen(len(AkoRolePermission) + len(additionalPrivileges)))
 		Expect(role.Privileges).To(ContainElements(AkoRolePermission))
@@ -246,10 +246,27 @@ func SyncAkoUserRoleTest() {
 			role.Privileges[i], role.Privileges[j] = role.Privileges[j], role.Privileges[i]
 		})
 
-		updated := syncAkoUserRole(role, "v30.2.1")
+		updated := syncAkoUserRole(role, "v20.0.0")
 		Expect(updated).To(BeTrue())
 		Expect(role.Privileges).To(HaveLen(len(AkoRolePermission) + len(additionalPrivileges)))
 		Expect(role.Privileges).To(ContainElements(AkoRolePermission))
 		Expect(role.Privileges).To(ContainElements(additionalPrivileges))
 	})
+
+	Specify("AVI Controller is higher than 30.2.1", func() {
+		role := &models.Role{}
+
+		updated := syncAkoUserRole(role, "v30.2.1")
+		newPermissions := []*models.Permission{}
+		for _, permission := range AkoRolePermission {
+			if *permission.Resource == "PERMISSION_PINGACCESSAGENT" {
+				continue
+			}
+			newPermissions = append(newPermissions, permission)
+
+		}
+		Expect(updated).To(BeTrue())
+		Expect(role.Privileges).To(HaveLen(len(AkoRolePermission) - 1))
+		Expect(role.Privileges).To(ContainElements(newPermissions))
+	})
 }