From 027955ad99b9ed6c10bd854a458d72b781605edf Mon Sep 17 00:00:00 2001 From: Lubron Zhan Date: Tue, 4 Feb 2025 18:02:54 -0800 Subject: [PATCH] Some refactor --- api/v1alpha1/constants.go | 2 ++ .../akodeploymentconfig/user/ako_role.go | 4 --- .../user/user_controller.go | 15 ++++++++--- .../user/user_controller_test.go | 27 +++++++++++++++---- hack/e2e.sh | 6 ----- 5 files changed, 36 insertions(+), 18 deletions(-) diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go index 0bc8f2af..0fda923e 100644 --- a/api/v1alpha1/constants.go +++ b/api/v1alpha1/constants.go @@ -55,4 +55,6 @@ const ( HAAVIInfraSettingAnnotationsKey = "aviinfrasetting.ako.vmware.com/name" AKODeploymentConfigControllerName = "akodeploymentconfig-controller" + + AVIControllerEnterpriseOnlyVersion = "v30.0.0" ) diff --git a/controllers/akodeploymentconfig/user/ako_role.go b/controllers/akodeploymentconfig/user/ako_role.go index 6ef0de5c..b765a49f 100644 --- a/controllers/akodeploymentconfig/user/ako_role.go +++ b/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 { diff --git a/controllers/akodeploymentconfig/user/user_controller.go b/controllers/akodeploymentconfig/user/user_controller.go index 98bf0475..c4a33b73 100644 --- a/controllers/akodeploymentconfig/user/user_controller.go +++ b/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) { diff --git a/controllers/akodeploymentconfig/user/user_controller_test.go b/controllers/akodeploymentconfig/user/user_controller_test.go index fa236461..143aa848 100644 --- a/controllers/akodeploymentconfig/user/user_controller_test.go +++ b/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)) + }) } diff --git a/hack/e2e.sh b/hack/e2e.sh index 94e7f088..f94bfb09 100755 --- a/hack/e2e.sh +++ b/hack/e2e.sh @@ -128,11 +128,6 @@ function fatal() { error "${@}" || exit "${?}"; } # safe for linux and darwin function base64d() { base64 -D 2>/dev/null || base64 -d; } -# safe for linux and darwin -function base64e() { base64 -w0 2>/dev/null || base64; } - -function md5safe() { md5sum 2>/dev/null || md5; } - # base64e encodes STDIN to base64 # kubectl_mgc executes kubectl against management cluster. @@ -408,4 +403,3 @@ EOF [[ -n "${E2E_DOWN}" ]] && e2e_down exit 0 -