Skip to content

Commit

Permalink
Some refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
lubronzhan committed Feb 5, 2025
1 parent 95f744a commit 6365671
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 17 deletions.
2 changes: 2 additions & 0 deletions api/v1alpha1/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,6 @@ const (
HAAVIInfraSettingAnnotationsKey = "aviinfrasetting.ako.vmware.com/name"

AKODeploymentConfigControllerName = "akodeploymentconfig-controller"

AVIControllerEnterpriseOnlyVersion = "v30.0.0"
)
4 changes: 0 additions & 4 deletions controllers/akodeploymentconfig/user/ako_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 10 additions & 2 deletions controllers/akodeploymentconfig/user/user_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand Down Expand Up @@ -325,7 +333,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) {
Expand Down
27 changes: 22 additions & 5 deletions controllers/akodeploymentconfig/user/user_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
})
}
6 changes: 0 additions & 6 deletions hack/e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -408,4 +403,3 @@ EOF
[[ -n "${E2E_DOWN}" ]] && e2e_down

exit 0

0 comments on commit 6365671

Please sign in to comment.