[1.12][TKG-29306] Filter user permission (#396)

Cherry pick #369

Filter permissions, remove the one that's been deprecated in corresponding AVI Controller version
Add username to user object if it's enterprise license
More logging
Refactor makefile and dockerfile

Author: lubronzhan

Dockerfile

@@ -2,15 +2,19 @@
 # SPDX-License-Identifier: Apache-2.0
 
 # Build the manager binary
-FROM golang:1.22-bullseye AS builder
+ARG GOLANG_IMAGE=golang:1.23-bullseye
+ARG BASE_IMAGE=gcr.io/distroless/static:nonroot
+FROM $GOLANG_IMAGE AS builder
 
 WORKDIR /workspace
 # Copy the Go Modules manifests
 COPY go.mod go.mod
 COPY go.sum go.sum
 # cache deps before building and copying source so that we don't need to re-download as much
 # and so that source changes don't invalidate our downloaded layer
-ENV GOPROXY=https://goproxy.io,direct
+ARG GOPROXY=https://goproxy.io,direct
+
+ENV GOPROXY=$GOPROXY
 RUN go mod download
 
 # Copy the go source
@@ -24,7 +28,7 @@ RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o manager
 
 # Use distroless as minimal base image to package the manager binary
 # Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM gcr.io/distroless/static:nonroot
+FROM $BASE_IMAGE
 WORKDIR /
 COPY --from=builder /workspace/manager .
 USER nonroot:nonroot

Makefile

@@ -7,6 +7,8 @@ IMG ?= ako-operator:$(IMAGE_TAG)
 # Produce CRDs that work back to Kubernetes 1.11 (no version conversion)
 CRD_OPTIONS ?= "crd"
 
+GOPROXY ?= https://goproxy.io,direct
+
 # downstream cache to avoid docker pull limitation
 CACHE_IMAGE_REGISTRY ?= harbor-repo.vmware.com/dockerhub-proxy-cache
 
@@ -78,7 +80,11 @@ generate: $(CONTROLLER_GEN)
 
 # Build the docker image
 docker-build:
-	docker build . -t ${IMG} -f Dockerfile
+	docker build . -t ${IMG} \
+		--build-arg GOPROXY=$(GOPROXY) \
+		--build-arg GOLANG_IMAGE=$(GOLANG_IMAGE) \
+		--build-arg BASE_IMAGE=$(BASE_IMAGE) \
+		-f Dockerfile
 
 # Push the docker image
 docker-push:

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

@@ -4,7 +4,9 @@
 package user
 
 import (
+	"github.com/go-logr/logr"
 	"github.com/vmware/alb-sdk/go/models"
+	"golang.org/x/mod/semver"
 	"k8s.io/utils/ptr"
 )
 
@@ -320,3 +322,22 @@ var AkoRolePermission = []*models.Permission{
 		Resource: ptr.To("PERMISSION_L4POLICYSET"),
 	},
 }
+
+var deprecatePermissionMap = map[string]string{
+	"PERMISSION_PINGACCESSAGENT": "v30.2.1",
+}
+
+func filterAkoRolePermissionByVersion(log logr.Logger, permissions []*models.Permission, version string) []*models.Permission {
+	filtered := []*models.Permission{}
+	for _, permission := range permissions {
+		if v, ok := deprecatePermissionMap[*permission.Resource]; ok && semver.Compare(version, v) >= 0 {
+			log.Info("Skip deprecated permission", "permission", *permission.Resource)
+			// Skip deprecated permission
+			continue
+		}
+
+		filtered = append(filtered, permission)
+
+	}
+	return filtered
+}

controllers/akodeploymentconfig/user/user_controller.go

@@ -9,6 +9,7 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	"github.com/vmware/alb-sdk/go/models"
+	"golang.org/x/mod/semver"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -241,7 +242,7 @@ func (r *AkoUserReconciler) reconcileAviUserNormal(
 		aviPassword := string(mcSecret.Data["password"][:])
 
 		// ensures the AVI User exists and matches the mc secret
-		if _, err = r.createOrUpdateAviUser(aviUsername, aviPassword, obj.Spec.Tenant.Name); err != nil {
+		if err = r.createOrUpdateAviUser(log, aviUsername, aviPassword, obj.Spec.Tenant.Name); err != nil {
 			log.Error(err, "Failed to create/update cluster avi user")
 			return res, err
 		} else {
@@ -263,21 +264,31 @@ func (r *AkoUserReconciler) getAVIControllerCA(ctx context.Context, obj *akoov1a
 }
 
 // createOrUpdateAviUser create an avi user in avi controller
-func (r *AkoUserReconciler) createOrUpdateAviUser(aviUsername, aviPassword, tenantName string) (*models.User, error) {
+func (r *AkoUserReconciler) createOrUpdateAviUser(log logr.Logger, aviUsername, aviPassword, tenantName string) error {
+	version, err := r.aviClient.GetControllerVersion()
+	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 user", "user", aviUsername)
 		// for avi essential version the default tenant is admin
 		if tenantName == "" {
 			tenantName = "admin"
 		}
 		tenant, err := r.aviClient.TenantGet(tenantName)
 		if err != nil {
-			return nil, err
+			return err
 		}
-		role, err := r.getOrCreateAkoUserRole(tenant.URL)
+		role, err := r.getOrCreateAkoUserRole(log, tenant.URL, version)
 		if err != nil {
-			return nil, err
+			return err
 		}
 		aviUser = &models.User{
 			Name:             &aviUsername,
@@ -291,50 +302,67 @@ func (r *AkoUserReconciler) createOrUpdateAviUser(aviUsername, aviPassword, tena
 				},
 			},
 		}
-		return r.aviClient.UserCreate(aviUser)
+		// 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
+		}
+		return nil
+	} else if err != nil {
+		log.Info("Failed to get AVI User", "user", aviUsername, "error", err)
+		return err
 	}
 
-	if err == nil {
-		// ensure user's role align with latest essential permission when user found
-		if _, err := r.ensureAkoUserRole(); err != nil {
-			return nil, err
-		}
-		// 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
+	// ensure user's role align with latest essential permission when user found
+	if _, err := r.ensureAkoUserRole(log, version); err != nil {
+		return err
+	}
+	// 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 == nil || *aviUser.Password != aviPassword {
+		log.Info("AVI User found, updating the password")
 		aviUser.Password = &aviPassword
-		return r.aviClient.UserUpdate(aviUser)
+		if _, err := r.aviClient.UserUpdate(aviUser); err != nil {
+			return err
+		}
 	}
-	return nil, err
+	return nil
 }
 
 // getOrCreateAkoUserRole get ako user's role, create one if not exist
-func (r *AkoUserReconciler) getOrCreateAkoUserRole(roleTenantRef *string) (*models.Role, error) {
+func (r *AkoUserReconciler) getOrCreateAkoUserRole(log logr.Logger, roleTenantRef *string, version string) (*models.Role, error) {
+	log.Info("Ensure AKO User Role")
 	role, err := r.aviClient.RoleGetByName(akoov1alpha1.AkoUserRoleName)
 	// not found ako user role, create one
 	if aviclient.IsAviRoleNonExistentError(err) {
+		log.V(3).Info("Creating AKO User Role since it's not found", "role", akoov1alpha1.AkoUserRoleName)
+		log.Info("current avi version", "version", version)
 		role = &models.Role{
 			Name:       ptr.To(akoov1alpha1.AkoUserRoleName),
-			Privileges: AkoRolePermission,
+			Privileges: filterAkoRolePermissionByVersion(log, AkoRolePermission, version),
 			TenantRef:  roleTenantRef,
 		}
 		return r.aviClient.RoleCreate(role)
 	}
 	if err == nil {
-		return r.ensureAkoUserRole()
+		return r.ensureAkoUserRole(log, version)
 	}
 	return role, err
 }
 
 // ensureAkoUserRole ensure ako-essential-role has the latest permission
-func (r *AkoUserReconciler) ensureAkoUserRole() (*models.Role, error) {
+func (r *AkoUserReconciler) ensureAkoUserRole(log logr.Logger, version string) (*models.Role, error) {
 	role, err := r.aviClient.RoleGetByName(akoov1alpha1.AkoUserRoleName)
 	if err != nil {
 		return role, err
 	}
 
 	// check if role needs to be synced
-	if syncAkoUserRole(role) {
+	if syncAkoUserRole(role, version) {
+		log.Info("Syncing AKO User Role with expected permissions")
 		return r.aviClient.RoleUpdate(role)
 	}
 
@@ -346,15 +374,18 @@ func (r *AkoUserReconciler) ensureAkoUserRole() (*models.Role, error) {
 // Any additional permissions on the Role that are not part of
 // the desired AKO role are left as-is. It returns a bool
 // indicating whether the Role was changed.
-func syncAkoUserRole(role *models.Role) bool {
+// It also filters out permissions that are deprecated in the current AVI version.
+func syncAkoUserRole(role *models.Role, version string) bool {
 	existingResources := sets.New[string]()
 	updated := false
 
 	for i, permission := range role.Privileges {
 		desiredType, ok := AkoRolePermissionMap[*permission.Resource]
 		if !ok {
-			// Existing AVI role has a permission that's not part of
-			// the desired AKO role: leave it as-is.
+			// Existing AVI role in AVI Controller has a permission that's not part of
+			// the desired AKO role defined in AkoRolePermissionMap: leave it as-is.
+			// Since those could come from a new AVI Controller version that AKO-Operator
+			// Might not be aware of.
 			continue
 		}
 
@@ -370,6 +401,11 @@ func syncAkoUserRole(role *models.Role) bool {
 
 	for resource, desiredType := range AkoRolePermissionMap {
 		if !existingResources.Has(resource) {
+			// Filter out permissions that are deprecated in the current AVI version.
+			if deprecateVersion, ok := deprecatePermissionMap[resource]; ok && semver.Compare(version, deprecateVersion) >= 0 {
+				// Skip adding deprecated permissions to the role
+				continue
+			}
 			// Existing AVI role is missing a permission that's
 			// part of the desired AKO role: add it.
 			role.Privileges = append(role.Privileges, &models.Permission{

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)
+		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)
+		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)
+		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)
+		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)
+		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))
+	})
 }

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/vmware-tanzu/tanzu-framework/apis/run v0.0.0-20221104044415-a462bbe793b9
 	github.com/vmware/alb-sdk v0.0.0-20240502042605-947bfcf176dd
 	github.com/vmware/load-balancer-and-ingress-services-for-kubernetes v0.0.0-20231012053946-537d99c1eba2
+	golang.org/x/mod v0.14.0
 	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.29.6
 	k8s.io/apiextensions-apiserver v0.29.6

go.sum

@@ -205,6 +205,8 @@ golang.org/x/exp v0.0.0-20230905200255-921286631fa9 h1:GoHiUyI/Tp2nVkLI2mCxVkOjs
 golang.org/x/exp v0.0.0-20230905200255-921286631fa9/go.mod h1:S2oDrQGGwySpoQPVqRShND87VCbxmc6bL1Yd2oYrm6k=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0=
+golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=