From e000d8c7d110b8b9b9d32fba0eb08fe8c4f61607 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Wed, 3 Apr 2024 11:24:12 +0530 Subject: [PATCH 1/4] Initial changes for refactoring acl code in lib --- api/v1/access_control_validate.go | 48 +-- controllers/access_control.go | 630 +++--------------------------- go.mod | 4 +- go.sum | 9 + test/access_control_test.go | 11 +- 5 files changed, 89 insertions(+), 613 deletions(-) diff --git a/api/v1/access_control_validate.go b/api/v1/access_control_validate.go index b68183496..d2f0b108f 100644 --- a/api/v1/access_control_validate.go +++ b/api/v1/access_control_validate.go @@ -8,17 +8,7 @@ import ( "strings" lib "github.com/aerospike/aerospike-management-lib" -) - -// PrivilegeScope enumerates valid scopes for privileges. -type PrivilegeScope int - -const ( - // Global scoped privileges. - Global PrivilegeScope = iota - - // NamespaceSet is namespace and optional set scoped privilege. - NamespaceSet + acl "github.com/aerospike/aerospike-management-lib/accesscontrol" ) const ( @@ -33,6 +23,9 @@ const ( // DefaultAdminPassword si default admin user password. DefaultAdminPassword = "admin" + + // Version6 server version 6 tag + Version6 = "6.0.0.0" ) // roleNameForbiddenChars are characters forbidden in role name. @@ -68,27 +61,6 @@ var requiredRoles = []string{ "user-admin", } -// Privileges are all privilege string allowed in the spec and associated scopes. -var Privileges = map[string][]PrivilegeScope{ - "read": {Global, NamespaceSet}, - "write": {Global, NamespaceSet}, - "read-write": {Global, NamespaceSet}, - "read-write-udf": {Global, NamespaceSet}, - "data-admin": {Global}, - "sys-admin": {Global}, - "user-admin": {Global}, - "truncate": {Global, NamespaceSet}, - "sindex-admin": {Global}, - "udf-admin": {Global}, -} - -// Post6Privileges are post version 6.0 privilege strings allowed in the spec and associated scopes. -var Post6Privileges = map[string][]PrivilegeScope{ - "truncate": {Global, NamespaceSet}, - "sindex-admin": {Global}, - "udf-admin": {Global}, -} - // IsAerospikeAccessControlValid validates the accessControl specification in the clusterSpec. // // Asserts that the AerospikeAccessControlSpec @@ -313,19 +285,19 @@ func isPrivilegeValid( ) (bool, error) { parts := strings.Split(privilege, ".") - _, ok := Privileges[parts[0]] + _, ok := acl.Privileges[parts[0]] if !ok { return false, fmt.Errorf("invalid privilege %s", privilege) } // Check if new privileges are used in an older version. - cmp, err := lib.CompareVersions(version, "6.0.0.0") + cmp, err := lib.CompareVersions(version, Version6) if err != nil { return false, err } if cmp < 0 { - if _, ok := Post6Privileges[parts[0]]; ok { + if _, ok := acl.Post6Privileges[parts[0]]; ok { // Version < 6.0 using post 6.0 privilege. return false, fmt.Errorf("invalid privilege %s", privilege) } @@ -339,9 +311,9 @@ func isPrivilegeValid( if nParts > 1 { // This privilege should necessarily have NamespaceSet scope. - scopes := Privileges[parts[0]] + scopes := acl.Privileges[parts[0]] - if !scopeContains(scopes, NamespaceSet) { + if !scopeContains(scopes, acl.NamespaceSet) { return false, fmt.Errorf( "privilege %s cannot have namespace or set scope", privilege, ) @@ -396,7 +368,7 @@ func isNetAddressValid(address string) (bool, error) { } // scopeContains indicates if scopes contains the queryScope. -func scopeContains(scopes []PrivilegeScope, queryScope PrivilegeScope) bool { +func scopeContains(scopes []acl.PrivilegeScope, queryScope acl.PrivilegeScope) bool { for _, scope := range scopes { if scope == queryScope { return true diff --git a/controllers/access_control.go b/controllers/access_control.go index 33378ed3c..5beff8209 100644 --- a/controllers/access_control.go +++ b/controllers/access_control.go @@ -3,30 +3,14 @@ package controllers // Aerospike access control reconciliation of access control. import ( - "bytes" "fmt" - "reflect" - "strings" "time" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/record" as "github.com/aerospike/aerospike-client-go/v7" asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" -) - -// logger type alias. -type logger = logr.Logger - -const ( - - // Error marker for user not found errors. - userNotFoundErr = "Invalid user" - - // Error marker for role not found errors. - roleNotFoundErr = "Invalid role" + acl "github.com/aerospike/aerospike-management-lib/accesscontrol" ) // AerospikeAdminCredentials to use for aerospike clients. @@ -158,33 +142,36 @@ func (r *SingleClusterReconciler) reconcileRoles( } // Create a list of role commands to drop. - rolesToDrop := SliceSubtract(currentRoleNames, requiredRoleNames) - roleReconcileCmds := make([]aerospikeAccessControlReconcileCmd, 0, len(rolesToDrop)+len(desired)) + rolesToDrop := acl.SliceSubtract(currentRoleNames, requiredRoleNames) + roleReconcileCmds := make([]acl.AerospikeAccessControlReconcileCmd, 0, len(rolesToDrop)+len(desired)) for _, roleToDrop := range rolesToDrop { if _, ok := asdbv1.PredefinedRoles[roleToDrop]; !ok { // Not a predefined role and can be dropped. roleReconcileCmds = append( - roleReconcileCmds, aerospikeRoleDrop{name: roleToDrop}, + roleReconcileCmds, acl.AerospikeRoleDrop{Name: roleToDrop}, ) } } for roleName, roleSpec := range desired { roleReconcileCmds = append( - roleReconcileCmds, aerospikeRoleCreateUpdate{ - name: roleName, privileges: roleSpec.Privileges, - whitelist: roleSpec.Whitelist, readQuota: roleSpec.ReadQuota, - writeQuota: roleSpec.WriteQuota, + roleReconcileCmds, acl.AerospikeRoleCreateUpdate{ + Name: roleName, Privileges: roleSpec.Privileges, + Whitelist: roleSpec.Whitelist, ReadQuota: roleSpec.ReadQuota, + WriteQuota: roleSpec.WriteQuota, }, ) } // execute all commands. for _, cmd := range roleReconcileCmds { - if err := cmd.execute(client, &adminPolicy, r.Log, r.Recorder, r.aeroCluster); err != nil { + if err := cmd.Execute(client, &adminPolicy, r.Log); err != nil { + r.recordACLEvent(cmd, true) return err } + + r.recordACLEvent(cmd, false) } return nil @@ -210,18 +197,18 @@ func (r *SingleClusterReconciler) reconcileUsers( } // Create a list of user commands to drop. - usersToDrop := SliceSubtract(currentUserNames, requiredUserNames) - userReconcileCmds := make([]aerospikeAccessControlReconcileCmd, 0, len(usersToDrop)+len(desired)) + usersToDrop := acl.SliceSubtract(currentUserNames, requiredUserNames) + userReconcileCmds := make([]acl.AerospikeAccessControlReconcileCmd, 0, len(usersToDrop)+len(desired)) for _, userToDrop := range usersToDrop { userReconcileCmds = append( - userReconcileCmds, aerospikeUserDrop{name: userToDrop}, + userReconcileCmds, acl.AerospikeUserDrop{Name: userToDrop}, ) } // Admin user update command should be executed last to ensure admin password // update does not disrupt reconciliation. - var adminUpdateCmd *aerospikeUserCreateUpdate + var adminUpdateCmd *acl.AerospikeUserCreateUpdate for userName := range desired { userSpec := desired[userName] @@ -231,8 +218,8 @@ func (r *SingleClusterReconciler) reconcileUsers( return err } - cmd := aerospikeUserCreateUpdate{ - name: userName, password: &password, roles: userSpec.Roles, + cmd := acl.AerospikeUserCreateUpdate{ + Name: userName, Password: &password, Roles: userSpec.Roles, } if userName == asdbv1.AdminUsername { adminUpdateCmd = &cmd @@ -247,147 +234,15 @@ func (r *SingleClusterReconciler) reconcileUsers( } for _, cmd := range userReconcileCmds { - if err := cmd.execute(client, &adminPolicy, r.Log, r.Recorder, r.aeroCluster); err != nil { + if err := cmd.Execute(client, &adminPolicy, r.Log); err != nil { + r.recordACLEvent(cmd, true) return err } - } - - return nil -} - -// privilegeStringToAerospikePrivilege converts privilegeString to an Aerospike privilege. -func privilegeStringToAerospikePrivilege(privilegeStrings []string) ( - []as.Privilege, error, -) { - aerospikePrivileges := make([]as.Privilege, 0, len(privilegeStrings)) - - for _, privilege := range privilegeStrings { - parts := strings.Split(privilege, ".") - if _, ok := asdbv1.Privileges[parts[0]]; !ok { - // First part of the privilege is not part of defined privileges. - return nil, fmt.Errorf("invalid privilege %s", privilege) - } - - privilegeCode := parts[0] - namespaceName := "" - setName := "" - nParts := len(parts) - - switch nParts { - case 2: - namespaceName = parts[1] - - case 3: - namespaceName = parts[1] - setName = parts[2] - } - - var code = as.Read //nolint:ineffassign // type is a private type in the pkg - - switch privilegeCode { - case "read": - code = as.Read - - case "write": - code = as.Write - - case "read-write": - code = as.ReadWrite - - case "read-write-udf": - code = as.ReadWriteUDF - - case "data-admin": - code = as.DataAdmin - - case "sys-admin": - code = as.SysAdmin - - case "user-admin": - code = as.UserAdmin - - case "truncate": - code = as.Truncate - - case "sindex-admin": - code = as.SIndexAdmin - - case "udf-admin": - code = as.UDFAdmin - - default: - return nil, fmt.Errorf("unknown privilege %s", privilegeCode) - } - aerospikePrivilege := as.Privilege{ - Code: code, Namespace: namespaceName, SetName: setName, - } - aerospikePrivileges = append(aerospikePrivileges, aerospikePrivilege) + r.recordACLEvent(cmd, false) } - return aerospikePrivileges, nil -} - -// AerospikePrivilegeToPrivilegeString converts aerospikePrivilege to controller spec privilege string. -func AerospikePrivilegeToPrivilegeString(aerospikePrivileges []as.Privilege) ( - []string, error, -) { - privileges := make([]string, 0, len(aerospikePrivileges)) - - for _, aerospikePrivilege := range aerospikePrivileges { - var buffer bytes.Buffer - - switch aerospikePrivilege.Code { - case as.Read: - buffer.WriteString("read") - - case as.Write: - buffer.WriteString("write") - - case as.ReadWrite: - buffer.WriteString("read-write") - - case as.ReadWriteUDF: - buffer.WriteString("read-write-udf") - - case as.DataAdmin: - buffer.WriteString("data-admin") - - case as.SysAdmin: - buffer.WriteString("sys-admin") - - case as.UserAdmin: - buffer.WriteString("user-admin") - - case as.Truncate: - buffer.WriteString("truncate") - - case as.SIndexAdmin: - buffer.WriteString("sindex-admin") - - case as.UDFAdmin: - buffer.WriteString("udf-admin") - - default: - return nil, fmt.Errorf( - "unknown privilege code %v", aerospikePrivilege.Code, - ) - } - - if aerospikePrivilege.Namespace != "" { - buffer.WriteString(".") - buffer.WriteString(aerospikePrivilege.Namespace) - - if aerospikePrivilege.SetName != "" { - buffer.WriteString(".") - buffer.WriteString(aerospikePrivilege.SetName) - } - } - - privileges = append(privileges, buffer.String()) - } - - return privileges, nil + return nil } // AerospikeUserPasswordProvider provides password for a give user.. @@ -398,418 +253,57 @@ type AerospikeUserPasswordProvider interface { ) } -// aerospikeAccessControlReconcileCmd commands needed to Reconcile a single access control entry, -// for example a role or a user. -type aerospikeAccessControlReconcileCmd interface { - // Execute executes the command. The implementation should be idempotent. - execute( - client *as.Client, adminPolicy *as.AdminPolicy, logger logger, recorder record.EventRecorder, - aeroCluster *asdbv1.AerospikeCluster, - ) error -} - -// aerospikeRoleCreateUpdate creates or updates an Aerospike role. -type aerospikeRoleCreateUpdate struct { - // The role's name. - name string - - // The privileges to set for the role. These privileges and only these privileges will be granted to the role - // after this operation. - privileges []string - - // The whitelist to set for the role. These whitelist addresses and only these whitelist addresses will be - // granted to the role after this operation. - whitelist []string - - // The readQuota specifies the read query rate that is permitted for the current role. - readQuota uint32 - - // The writeQuota specifies the write rate that is permitted for the current role. - writeQuota uint32 -} - -// Execute creates a new Aerospike role or updates an existing one. -func (roleCreate aerospikeRoleCreateUpdate) execute( - client *as.Client, adminPolicy *as.AdminPolicy, logger logger, - recorder record.EventRecorder, aeroCluster *asdbv1.AerospikeCluster, -) error { - role, err := client.QueryRole(adminPolicy, roleCreate.name) - isCreate := false +func (r *SingleClusterReconciler) recordACLEvent(cmd acl.AerospikeAccessControlReconcileCmd, failed bool) { + var eventType, message, reason string - if err != nil { - if strings.Contains(err.Error(), roleNotFoundErr) { - isCreate = true + switch v := cmd.(type) { + case acl.AerospikeUserCreateUpdate: + if failed { + eventType = corev1.EventTypeWarning + reason = "UserCreateOrUpdateFailed" + message = fmt.Sprintf("Failed to Create or Update User %s", v.Name) } else { - // Failure to query for the role. - return fmt.Errorf( - "error querying role %s: %v", roleCreate.name, err, - ) - } - } - - if isCreate { - if err := roleCreate.createRole(client, adminPolicy, logger, recorder, aeroCluster); err != nil { - recorder.Eventf( - aeroCluster, corev1.EventTypeWarning, "RoleCreateFailed", - "Failed to Create Role %s", roleCreate.name, - ) - } - - return err - } - - if errorUpdate := roleCreate.updateRole( - client, adminPolicy, role, logger, recorder, aeroCluster, - ); errorUpdate != nil { - recorder.Eventf( - aeroCluster, corev1.EventTypeWarning, "RoleUpdateFailed", - "Failed to Update Role %s", roleCreate.name, - ) - - return errorUpdate - } - - return nil -} - -// createRole creates a new Aerospike role. -func (roleCreate aerospikeRoleCreateUpdate) createRole( - client *as.Client, adminPolicy *as.AdminPolicy, logger logger, - recorder record.EventRecorder, aeroCluster *asdbv1.AerospikeCluster, -) error { - logger.Info("Creating role", "role name", roleCreate.name) - - aerospikePrivileges, err := privilegeStringToAerospikePrivilege(roleCreate.privileges) - if err != nil { - return fmt.Errorf("could not create role %s: %v", roleCreate.name, err) - } - - if err = client.CreateRole( - adminPolicy, roleCreate.name, aerospikePrivileges, roleCreate.whitelist, - roleCreate.readQuota, roleCreate.writeQuota, - ); err != nil { - return fmt.Errorf("could not create role %s: %v", roleCreate.name, err) - } - - logger.Info("Created role", "role name", roleCreate.name) - recorder.Eventf( - aeroCluster, corev1.EventTypeNormal, "RoleCreated", - "Created Role %s", roleCreate.name, - ) - - return nil -} - -// updateRole updates an existing Aerospike role. -func (roleCreate aerospikeRoleCreateUpdate) updateRole( - client *as.Client, adminPolicy *as.AdminPolicy, role *as.Role, - logger logger, recorder record.EventRecorder, - aeroCluster *asdbv1.AerospikeCluster, -) error { - // Update the role. - logger.Info("Updating role", "role name", roleCreate.name) - - // Find the privileges to drop. - currentPrivileges, err := AerospikePrivilegeToPrivilegeString(role.Privileges) - if err != nil { - return fmt.Errorf("could not update role %s: %v", roleCreate.name, err) - } - - desiredPrivileges := roleCreate.privileges - privilegesToRevoke := SliceSubtract(currentPrivileges, desiredPrivileges) - privilegesToGrant := SliceSubtract(desiredPrivileges, currentPrivileges) - - if len(privilegesToRevoke) > 0 { - aerospikePrivileges, err := privilegeStringToAerospikePrivilege(privilegesToRevoke) - if err != nil { - return fmt.Errorf( - "could not update role %s: %v", roleCreate.name, err, - ) + eventType = corev1.EventTypeNormal + reason = "UserCreatedOrUpdated" + message = fmt.Sprintf("Created or Updated User %s", v.Name) } - if err := client.RevokePrivileges( - adminPolicy, roleCreate.name, aerospikePrivileges, - ); err != nil { - return fmt.Errorf( - "error revoking privileges for role %s: %v", roleCreate.name, - err, - ) - } - - logger.Info( - "Revoked privileges for role", "role name", roleCreate.name, - "privileges", privilegesToRevoke, - ) - } - - if len(privilegesToGrant) > 0 { - aerospikePrivileges, err := privilegeStringToAerospikePrivilege(privilegesToGrant) - if err != nil { - return fmt.Errorf( - "could not update role %s: %v", roleCreate.name, err, - ) - } - - if err := client.GrantPrivileges( - adminPolicy, roleCreate.name, aerospikePrivileges, - ); err != nil { - return fmt.Errorf( - "error granting privileges for role %s: %v", roleCreate.name, - err, - ) - } - - logger.Info( - "Granted privileges to role", "role name", roleCreate.name, - "privileges", privilegesToGrant, - ) - } - - if !reflect.DeepEqual(role.Whitelist, roleCreate.whitelist) { - // Set whitelist. - if err := client.SetWhitelist( - adminPolicy, roleCreate.name, roleCreate.whitelist, - ); err != nil { - return fmt.Errorf( - "error setting whitelist for role %s: %v", roleCreate.name, err, - ) - } - } - - logger.Info("Updated role", "role name", roleCreate.name) - recorder.Eventf( - aeroCluster, corev1.EventTypeNormal, "RoleUpdated", - "Updated Role %s", roleCreate.name, - ) - - return nil -} - -// aerospikeUserCreateUpdate creates or updates an Aerospike user. -type aerospikeUserCreateUpdate struct { - // The user's name. - name string - - // The password to set. Required for create. Optional for update. - password *string - - // The roles to set for the user. These roles and only these roles will be granted to the user after this operation. - roles []string -} - -// Execute creates a new Aerospike user or updates an existing one. -func (userCreate aerospikeUserCreateUpdate) execute( - client *as.Client, adminPolicy *as.AdminPolicy, logger logger, - recorder record.EventRecorder, aeroCluster *asdbv1.AerospikeCluster, -) error { - user, err := client.QueryUser(adminPolicy, userCreate.name) - isCreate := false - - if err != nil { - if strings.Contains(err.Error(), userNotFoundErr) { - isCreate = true + case acl.AerospikeUserDrop: + if failed { + eventType = corev1.EventTypeWarning + reason = "UserDeleteFailed" + message = fmt.Sprintf("Failed to Drop User %s", v.Name) } else { - // Failure to query for the user. - return fmt.Errorf( - "error querying user %s: %v", userCreate.name, err, - ) - } - } - - if isCreate { - err := userCreate.createUser(client, adminPolicy, logger, recorder, aeroCluster) - if err != nil { - recorder.Eventf( - aeroCluster, corev1.EventTypeWarning, "UserCreateFailed", - "Failed to Create User %s", userCreate.name, - ) + eventType = corev1.EventTypeNormal + reason = "UserDeleted" + message = fmt.Sprintf("Dropped User %s", v.Name) } - return err - } - - if errorUpdate := userCreate.updateUser( - client, adminPolicy, user, logger, recorder, aeroCluster, - ); errorUpdate != nil { - recorder.Eventf( - aeroCluster, corev1.EventTypeWarning, "UserUpdateFailed", - "Failed to Update User %s", userCreate.name, - ) - - return errorUpdate - } - - return nil -} - -// createUser creates a new Aerospike user. -func (userCreate aerospikeUserCreateUpdate) createUser( - client *as.Client, adminPolicy *as.AdminPolicy, logger logger, - recorder record.EventRecorder, aeroCluster *asdbv1.AerospikeCluster, -) error { - logger.Info("Creating user", "username", userCreate.name) - - if userCreate.password == nil { - return fmt.Errorf( - "error creating user %s. Password not specified", userCreate.name, - ) - } - - if err := client.CreateUser( - adminPolicy, userCreate.name, *userCreate.password, userCreate.roles, - ); err != nil { - return fmt.Errorf("could not create user %s: %v", userCreate.name, err) - } - - logger.Info("Created user", "username", userCreate.name) - recorder.Eventf( - aeroCluster, corev1.EventTypeNormal, "UserCreated", - "Created User %s", userCreate.name, - ) - - return nil -} - -// updateUser updates an existing Aerospike user. -func (userCreate aerospikeUserCreateUpdate) updateUser( - client *as.Client, adminPolicy *as.AdminPolicy, user *as.UserRoles, - logger logger, recorder record.EventRecorder, aeroCluster *asdbv1.AerospikeCluster, -) error { - // Update the user. - logger.Info("Updating user", "username", userCreate.name) - - if userCreate.password != nil { - logger.Info("Updating password for user", "username", userCreate.name) - - if err := client.ChangePassword( - adminPolicy, userCreate.name, *userCreate.password, - ); err != nil { - return fmt.Errorf( - "error updating password for user %s: %v", userCreate.name, err, - ) - } - - logger.Info("Updated password for user", "username", userCreate.name) - } - - // Find the roles to grant and revoke. - currentRoles := user.Roles - desiredRoles := userCreate.roles - rolesToRevoke := SliceSubtract(currentRoles, desiredRoles) - rolesToGrant := SliceSubtract(desiredRoles, currentRoles) - - if len(rolesToRevoke) > 0 { - if err := client.RevokeRoles(adminPolicy, userCreate.name, rolesToRevoke); err != nil { - return fmt.Errorf( - "error revoking roles for user %s: %v", userCreate.name, err, - ) - } - - logger.Info( - "Revoked roles for user", "username", userCreate.name, "roles", - rolesToRevoke, - ) - } - - if len(rolesToGrant) > 0 { - if err := client.GrantRoles(adminPolicy, userCreate.name, rolesToGrant); err != nil { - return fmt.Errorf( - "error granting roles for user %s: %v", userCreate.name, err, - ) - } - - logger.Info( - "Granted roles to user", "username", userCreate.name, "roles", - rolesToGrant, - ) - } - - logger.Info("Updated user", "username", userCreate.name) - recorder.Eventf( - aeroCluster, corev1.EventTypeNormal, "UserUpdated", - "Updated User %s", userCreate.name, - ) - - return nil -} - -// aerospikeUserDrop drops an Aerospike user. -type aerospikeUserDrop struct { - // The user's name. - name string -} - -// Execute implements dropping the user. -func (userDrop aerospikeUserDrop) execute( - client *as.Client, adminPolicy *as.AdminPolicy, logger logger, - recorder record.EventRecorder, aeroCluster *asdbv1.AerospikeCluster, -) error { - logger.Info("Dropping user", "username", userDrop.name) - - if err := client.DropUser(adminPolicy, userDrop.name); err != nil { - if !strings.Contains(err.Error(), userNotFoundErr) { - // Failure to drop for the user. - return fmt.Errorf("error dropping user %s: %v", userDrop.name, err) + case acl.AerospikeRoleCreateUpdate: + if failed { + eventType = corev1.EventTypeWarning + reason = "RoleCreateOrUpdateFailed" + message = fmt.Sprintf("Failed to Create or Update Role %s", v.Name) + } else { + eventType = corev1.EventTypeNormal + reason = "RoleCreatedOrUpdated" + message = fmt.Sprintf("Created or Updated Role %s", v.Name) } - } - - logger.Info("Dropped user", "username", userDrop.name) - recorder.Eventf( - aeroCluster, corev1.EventTypeNormal, "UserDeleted", - "Dropped User %s", userDrop.name, - ) - - return nil -} - -// aerospikeRoleDrop drops an Aerospike role. -type aerospikeRoleDrop struct { - // The role's name. - name string -} - -// Execute implements dropping the role. -func (roleDrop aerospikeRoleDrop) execute( - client *as.Client, adminPolicy *as.AdminPolicy, logger logger, - recorder record.EventRecorder, aeroCluster *asdbv1.AerospikeCluster, -) error { - logger.Info("Dropping role", "role", roleDrop.name) - if err := client.DropRole(adminPolicy, roleDrop.name); err != nil { - if !strings.Contains(err.Error(), roleNotFoundErr) { - // Failure to drop for the role. - return fmt.Errorf("error dropping role %s: %v", roleDrop.name, err) + case acl.AerospikeRoleDrop: + if failed { + eventType = corev1.EventTypeWarning + reason = "RoleDeleteFailed" + message = fmt.Sprintf("Failed to Drop Role %s", v.Name) + } else { + eventType = corev1.EventTypeNormal + reason = "RoleDeleted" + message = fmt.Sprintf("Dropped Role %s", v.Name) } } - logger.Info("Dropped role", "role", roleDrop.name) - recorder.Eventf( - aeroCluster, corev1.EventTypeNormal, "RoleDeleted", - "Dropped Role %s", roleDrop.name, + r.Recorder.Eventf( + r.aeroCluster, eventType, reason, + message, ) - - return nil -} - -// SliceSubtract removes elements of slice2 from slice1 and returns the result. -func SliceSubtract(slice1, slice2 []string) []string { - var result []string - - for _, s1 := range slice1 { - found := false - - for _, toSubtract := range slice2 { - if s1 == toSubtract { - found = true - break - } - } - - if !found { - // s1 not found. Should be retained. - result = append(result, s1) - } - } - - return result } diff --git a/go.mod b/go.mod index 204c7cf87..25fd9a763 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.21 toolchain go1.21.8 require ( - github.com/aerospike/aerospike-management-lib v1.2.1-0.20240319095728-1600222cec4c + github.com/aerospike/aerospike-management-lib v1.3.1-0.20240402145824-a9f0013313d0 github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d github.com/evanphx/json-patch v4.12.0+incompatible github.com/go-logr/logr v1.3.0 @@ -68,7 +68,7 @@ require ( github.com/prometheus/common v0.44.0 // indirect github.com/prometheus/procfs v0.10.1 // indirect github.com/qdm12/reprint v0.0.0-20200326205758-722754a53494 // indirect - github.com/rogpeppe/go-internal v1.11.0 // indirect + github.com/rogpeppe/go-internal v1.12.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect diff --git a/go.sum b/go.sum index 26033d0ed..bdce734e9 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,14 @@ github.com/aerospike/aerospike-client-go/v7 v7.1.0 h1:yvCTKdbpqZxHvv7sWsFHV1j49j github.com/aerospike/aerospike-client-go/v7 v7.1.0/go.mod h1:AkHiKvCbqa1c16gCNGju3c5X/yzwLVvblNczqjxNwNk= github.com/aerospike/aerospike-management-lib v1.2.1-0.20240319095728-1600222cec4c h1:wCscajyxCdQ9NeDxJdMbBascFym9MQV0aALTJ2dANOc= github.com/aerospike/aerospike-management-lib v1.2.1-0.20240319095728-1600222cec4c/go.mod h1:o1TV3BTsAiuZ5HtZi9E4FgXqWRwjDzlkS4bfvfaAHLU= +github.com/aerospike/aerospike-management-lib v1.3.1-0.20240401162054-2ce9f8971e6f h1:2Ds/Ce8W1BkZtwdb6ehiCkgpgg4ONb8a5O7b9CdXOw0= +github.com/aerospike/aerospike-management-lib v1.3.1-0.20240401162054-2ce9f8971e6f/go.mod h1:E4dk798IikCp9a8fugpYoeQVIXuvdxogHvt6sKhaORQ= +github.com/aerospike/aerospike-management-lib v1.3.1-0.20240401165331-8f1e1d9ec1f8 h1:TKyRLzQRPpzFGsHZl1HpMNlX5QaPe6eYfzLoRMbpnYo= +github.com/aerospike/aerospike-management-lib v1.3.1-0.20240401165331-8f1e1d9ec1f8/go.mod h1:E4dk798IikCp9a8fugpYoeQVIXuvdxogHvt6sKhaORQ= +github.com/aerospike/aerospike-management-lib v1.3.1-0.20240402140618-0adf93a70361 h1:czISeybCy2sbfaoCTrMzLLLVXgwHD6+w/1LvC4ZAxMk= +github.com/aerospike/aerospike-management-lib v1.3.1-0.20240402140618-0adf93a70361/go.mod h1:o1TV3BTsAiuZ5HtZi9E4FgXqWRwjDzlkS4bfvfaAHLU= +github.com/aerospike/aerospike-management-lib v1.3.1-0.20240402145824-a9f0013313d0 h1:RbRkHrSVrX5T3Eq/pngjyMLS9O5Bf7Kb3ZQ5loKv7uo= +github.com/aerospike/aerospike-management-lib v1.3.1-0.20240402145824-a9f0013313d0/go.mod h1:o1TV3BTsAiuZ5HtZi9E4FgXqWRwjDzlkS4bfvfaAHLU= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ= @@ -121,6 +129,7 @@ github.com/qdm12/reprint v0.0.0-20200326205758-722754a53494 h1:wSmWgpuccqS2IOfmY github.com/qdm12/reprint v0.0.0-20200326205758-722754a53494/go.mod h1:yipyliwI08eQ6XwDm1fEwKPdF/xdbkiHtrU+1Hg+vc4= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= +github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0= github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= diff --git a/test/access_control_test.go b/test/access_control_test.go index 1b784f456..1c2041af8 100644 --- a/test/access_control_test.go +++ b/test/access_control_test.go @@ -19,6 +19,7 @@ import ( as "github.com/aerospike/aerospike-client-go/v7" asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" aerospikecluster "github.com/aerospike/aerospike-kubernetes-operator/controllers" + acl "github.com/aerospike/aerospike-management-lib/accesscontrol" ) const ( @@ -2224,7 +2225,7 @@ func validateRoles( // Check values. if len( - aerospikecluster.SliceSubtract( + acl.SliceSubtract( expectedRoleNames, currentRoleNames, ), ) != 0 { @@ -2250,7 +2251,7 @@ func validateRoles( var currentPrivilegeNames []string for _, privilege := range asRole.Privileges { - temp, _ := aerospikecluster.AerospikePrivilegeToPrivilegeString([]as.Privilege{privilege}) + temp, _ := acl.AerospikePrivilegeToPrivilegeString([]as.Privilege{privilege}) currentPrivilegeNames = append(currentPrivilegeNames, temp[0]) } @@ -2264,7 +2265,7 @@ func validateRoles( // Check values. if len( - aerospikecluster.SliceSubtract( + acl.SliceSubtract( expectedPrivilegeNames, currentPrivilegeNames, ), ) != 0 { @@ -2340,7 +2341,7 @@ func validateUsers( // Check values. if len( - aerospikecluster.SliceSubtract( + acl.SliceSubtract( expectedUserNames, currentUserNames, ), ) != 0 { @@ -2381,7 +2382,7 @@ func validateUsers( // Check values. if len( - aerospikecluster.SliceSubtract( + acl.SliceSubtract( expectedRoleNames, currentRoleNames, ), ) != 0 { From 1c46fc3455b9caaf701bc73c77e78f4a165d4521 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Mon, 26 Aug 2024 12:02:40 +0530 Subject: [PATCH 2/4] updating lib --- go.mod | 4 +--- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index e36a424b2..9d29b4ec7 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.21 toolchain go1.21.8 require ( - github.com/aerospike/aerospike-management-lib v1.4.1-0.20240612085328-0a60cdefd5e3 + github.com/aerospike/aerospike-management-lib v1.4.1-0.20240826062810-10b385feb3f6 github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d github.com/evanphx/json-patch v4.12.0+incompatible github.com/go-logr/logr v1.4.1 @@ -68,8 +68,6 @@ require ( github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/common v0.44.0 // indirect github.com/prometheus/procfs v0.10.1 // indirect - github.com/qdm12/reprint v0.0.0-20200326205758-722754a53494 // indirect - github.com/rogpeppe/go-internal v1.12.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect diff --git a/go.sum b/go.sum index 92cede2c9..a391c2139 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ github.com/aerospike/aerospike-client-go/v7 v7.4.0 h1:g8/7v8RHhQhTArhW3C7Au7o+u8j8x5eySZL6MXfpHKU= github.com/aerospike/aerospike-client-go/v7 v7.4.0/go.mod h1:pPKnWiS8VDJcH4IeB1b8SA2TWnkjcVLHwAAJ+BHfGK8= -github.com/aerospike/aerospike-management-lib v1.4.1-0.20240612085328-0a60cdefd5e3 h1:hO34kkRP8hCOYlx6SEjWxmLVgIWJaKNECWQOVQv13pA= -github.com/aerospike/aerospike-management-lib v1.4.1-0.20240612085328-0a60cdefd5e3/go.mod h1:Gf/jFdURQZ3UXg9LfdYQ1/TYlGFbB/U/TxCa+LN+rcw= +github.com/aerospike/aerospike-management-lib v1.4.1-0.20240826062810-10b385feb3f6 h1:kv8m3r+abLnVy8qUIJzBSR9FlxBIfxtQhNWib3Jlrfg= +github.com/aerospike/aerospike-management-lib v1.4.1-0.20240826062810-10b385feb3f6/go.mod h1:Gf/jFdURQZ3UXg9LfdYQ1/TYlGFbB/U/TxCa+LN+rcw= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ= From dfcedc3fe409732bbb07d4040765a2bd1a6d5ec8 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Wed, 23 Oct 2024 02:45:23 +0530 Subject: [PATCH 3/4] Getting namespaces stats using management-lib. --- go.mod | 2 +- go.sum | 6 ++-- internal/controller/cluster/access_control.go | 4 +++ .../cluster/aerospikecluster_controller.go | 3 +- internal/controller/cluster/reconciler.go | 3 +- test/cluster/access_control_test.go | 4 +-- test/cluster/cluster_helper.go | 10 ++---- test/cluster/dynamic_config_test.go | 32 ++++--------------- test/cluster/rack_utils.go | 5 +-- test/cluster/utils.go | 28 ++++++++-------- 10 files changed, 34 insertions(+), 63 deletions(-) diff --git a/go.mod b/go.mod index d669a58f4..863cbfbe6 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/aerospike/aerospike-kubernetes-operator go 1.22 require ( - github.com/aerospike/aerospike-management-lib v1.5.1-0.20241021185647-7857f87c24d5 + github.com/aerospike/aerospike-management-lib v1.5.1-0.20241022182350-ec5a19ae3dc9 github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d github.com/evanphx/json-patch v4.12.0+incompatible github.com/go-logr/logr v1.4.2 diff --git a/go.sum b/go.sum index 0fefb6358..408e765d3 100644 --- a/go.sum +++ b/go.sum @@ -2,10 +2,8 @@ github.com/aerospike/aerospike-backup-service v0.0.0-20240822110128-dc2b4811b9d3 github.com/aerospike/aerospike-backup-service v0.0.0-20240822110128-dc2b4811b9d3/go.mod h1:PFWhqxcMsEEyoOZtQ70b+X8xWbbemDYuitT24EPBizk= github.com/aerospike/aerospike-client-go/v7 v7.6.1 h1:VZK6S9YKq2w6ptTk3kXXjTxG2U9M9Y7Oi3YQ+3T7wQQ= github.com/aerospike/aerospike-client-go/v7 v7.6.1/go.mod h1:uCbSYMpjlRcH/9f26VSF/luzDDXrcDaV8c6/WIcKtT4= -github.com/aerospike/aerospike-management-lib v1.4.1-0.20240826062810-10b385feb3f6 h1:kv8m3r+abLnVy8qUIJzBSR9FlxBIfxtQhNWib3Jlrfg= -github.com/aerospike/aerospike-management-lib v1.4.1-0.20240826062810-10b385feb3f6/go.mod h1:Gf/jFdURQZ3UXg9LfdYQ1/TYlGFbB/U/TxCa+LN+rcw= -github.com/aerospike/aerospike-management-lib v1.5.1-0.20241021185647-7857f87c24d5 h1:BXeMw5wcHAtE6ne2PpgZl5fFOgeGr0hkVFaQ8vk0bqA= -github.com/aerospike/aerospike-management-lib v1.5.1-0.20241021185647-7857f87c24d5/go.mod h1:hsEptY/AmTmHoJnItJNmfJ4yCMG8LIB8YPnIpIyvGXI= +github.com/aerospike/aerospike-management-lib v1.5.1-0.20241022182350-ec5a19ae3dc9 h1:GGYS+xSZyzxeZQ42e+8fcfGWH32B7gJszs3T6ZD19Lk= +github.com/aerospike/aerospike-management-lib v1.5.1-0.20241022182350-ec5a19ae3dc9/go.mod h1:Pbejci/QaooN6XW3FZwfv7qTidHOs+r8XgJ0Onw2xi4= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ= diff --git a/internal/controller/cluster/access_control.go b/internal/controller/cluster/access_control.go index 239a246a7..41a1ff05f 100644 --- a/internal/controller/cluster/access_control.go +++ b/internal/controller/cluster/access_control.go @@ -6,6 +6,7 @@ import ( "fmt" "time" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" as "github.com/aerospike/aerospike-client-go/v7" @@ -13,6 +14,9 @@ import ( acl "github.com/aerospike/aerospike-management-lib/accesscontrol" ) +// logger type alias. +type logger = logr.Logger + // AerospikeAdminCredentials to use for aerospike clients. // // Returns a tuple of admin username and password to use. If the cluster is not security diff --git a/internal/controller/cluster/aerospikecluster_controller.go b/internal/controller/cluster/aerospikecluster_controller.go index 07563e3a4..56418e7b0 100644 --- a/internal/controller/cluster/aerospikecluster_controller.go +++ b/internal/controller/cluster/aerospikecluster_controller.go @@ -3,7 +3,6 @@ package cluster import ( "context" - "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/errors" k8sRuntime "k8s.io/apimachinery/pkg/runtime" @@ -32,7 +31,7 @@ type AerospikeClusterReconciler struct { KubeClient *kubernetes.Clientset KubeConfig *rest.Config Scheme *k8sRuntime.Scheme - Log logr.Logger + Log logger } // SetupWithManager sets up the controller with the Manager diff --git a/internal/controller/cluster/reconciler.go b/internal/controller/cluster/reconciler.go index 15542b638..de8caeab7 100644 --- a/internal/controller/cluster/reconciler.go +++ b/internal/controller/cluster/reconciler.go @@ -7,7 +7,6 @@ import ( "reflect" "strings" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" k8sRuntime "k8s.io/apimachinery/pkg/runtime" @@ -38,7 +37,7 @@ type SingleClusterReconciler struct { KubeClient *kubernetes.Clientset KubeConfig *rest.Config Scheme *k8sRuntime.Scheme - Log logr.Logger + Log logger } func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error) { diff --git a/test/cluster/access_control_test.go b/test/cluster/access_control_test.go index e4b668745..05751c3e7 100644 --- a/test/cluster/access_control_test.go +++ b/test/cluster/access_control_test.go @@ -1,5 +1,3 @@ -//go:build !noac - package cluster import ( @@ -11,6 +9,8 @@ import ( "time" "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" diff --git a/test/cluster/cluster_helper.go b/test/cluster/cluster_helper.go index 13d13c4de..774875691 100644 --- a/test/cluster/cluster_helper.go +++ b/test/cluster/cluster_helper.go @@ -28,7 +28,6 @@ import ( "github.com/aerospike/aerospike-kubernetes-operator/pkg/utils" "github.com/aerospike/aerospike-kubernetes-operator/test" lib "github.com/aerospike/aerospike-management-lib" - "github.com/aerospike/aerospike-management-lib/info" ) const ( @@ -501,15 +500,11 @@ func validateAerospikeConfigServiceClusterUpdate( // TODO: // We may need to check for all keys in aerospikeConfig in rack // but we know that we are changing for service only for now - host, err := createHost(&pod) + asinfo, err := getASInfo(log, &pod, getClientPolicy(aeroCluster, k8sClient)) if err != nil { return err } - asinfo := info.NewAsInfo( - log, host, getClientPolicy(aeroCluster, k8sClient), - ) - confs, err := getAsConfig(asinfo, "service") if err != nil { return err @@ -566,12 +561,11 @@ func validateMigrateFillDelay( return fmt.Errorf("pod %s missing from the status", firstPodName) } - host, err := createHost(&firstPod) + asinfo, err := getASInfo(log, &firstPod, getClientPolicy(aeroCluster, k8sClient)) if err != nil { return err } - asinfo := info.NewAsInfo(log, host, getClientPolicy(aeroCluster, k8sClient)) err = wait.PollUntilContextTimeout(ctx, retryInterval, getTimeout(1), true, func(goctx.Context) (done bool, err error) { confs, err := getAsConfig(asinfo, "service") diff --git a/test/cluster/dynamic_config_test.go b/test/cluster/dynamic_config_test.go index a6f7d9785..c037eb8c3 100644 --- a/test/cluster/dynamic_config_test.go +++ b/test/cluster/dynamic_config_test.go @@ -23,7 +23,6 @@ import ( "github.com/aerospike/aerospike-kubernetes-operator/test" lib "github.com/aerospike/aerospike-management-lib" "github.com/aerospike/aerospike-management-lib/asconfig" - "github.com/aerospike/aerospike-management-lib/info" ) type podID struct { @@ -684,29 +683,16 @@ var _ = Describe( By("Fetch and verify dynamic configs") - pod := aeroCluster.Status.Pods["dynamic-config-test-1-0"] - - info, err := requestInfoFromNode(logger, k8sClient, ctx, clusterNamespacedName, "namespace/test", &pod) + podList, err := getPodList(aeroCluster, k8sClient) Expect(err).ToNot(HaveOccurred()) - confs := strings.Split(info["namespace/test"], ";") - for _, conf := range confs { - if strings.Contains(conf, "effective_active_rack") { - keyValue := strings.Split(conf, "=") - Expect(keyValue[1]).To(Equal("1")) - } - } - - info, err = requestInfoFromNode(logger, k8sClient, ctx, clusterNamespacedName, "namespace/test1", &pod) + stats, err := getNamespaceStats(logger, k8sClient, aeroCluster, "test", &podList.Items[0]) Expect(err).ToNot(HaveOccurred()) + Expect(stats["effective_active_rack"]).To(Equal("1")) - confs = strings.Split(info["namespace/test1"], ";") - for _, conf := range confs { - if strings.Contains(conf, "effective_active_rack") { - keyValue := strings.Split(conf, "=") - Expect(keyValue[1]).To(Equal("2")) - } - } + stats, err = getNamespaceStats(logger, k8sClient, aeroCluster, "test1", &podList.Items[0]) + Expect(err).ToNot(HaveOccurred()) + Expect(stats["effective_active_rack"]).To(Equal("2")) By("Verify no warm/cold restarts in Pods") @@ -1075,15 +1061,11 @@ func getAerospikeConfigFromNodeAndSpec(aeroCluster *asdbv1.AerospikeCluster) (fl pod := aeroCluster.Status.Pods[pods.Items[0].Name] - host, err := createHost(&pod) + asinfo, err := getASInfo(logger, &pod, getClientPolicy(aeroCluster, k8sClient)) if err != nil { return nil, nil, err } - asinfo := info.NewAsInfo( - logger, host, getClientPolicy(aeroCluster, k8sClient), - ) - serverConf, err := asconfig.GenerateConf(logger, asinfo, false) if err != nil { return nil, nil, err diff --git a/test/cluster/rack_utils.go b/test/cluster/rack_utils.go index 21f3bce9c..6756c7214 100644 --- a/test/cluster/rack_utils.go +++ b/test/cluster/rack_utils.go @@ -18,7 +18,6 @@ import ( asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" "github.com/aerospike/aerospike-kubernetes-operator/pkg/utils" lib "github.com/aerospike/aerospike-management-lib" - "github.com/aerospike/aerospike-management-lib/info" ) type RackState struct { @@ -146,13 +145,11 @@ func isNamespaceRackEnabled( pod = aeroCluster.Status.Pods[podName] } - host, err := createHost(&pod) + asinfo, err := getASInfo(log, &pod, getClientPolicy(aeroCluster, k8sClient)) if err != nil { return false, err } - asinfo := info.NewAsInfo(log, host, getClientPolicy(aeroCluster, k8sClient)) - confs, err := getAsConfig(asinfo, "racks") if err != nil { return false, err diff --git a/test/cluster/utils.go b/test/cluster/utils.go index a37b6cf6a..8454e13e4 100644 --- a/test/cluster/utils.go +++ b/test/cluster/utils.go @@ -28,6 +28,7 @@ import ( asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" operatorUtils "github.com/aerospike/aerospike-kubernetes-operator/pkg/utils" lib "github.com/aerospike/aerospike-management-lib" + "github.com/aerospike/aerospike-management-lib/deployment" "github.com/aerospike/aerospike-management-lib/info" ) @@ -587,15 +588,11 @@ func getAerospikeConfigFromNode(log logr.Logger, k8sClient client.Client, ctx go return nil, err } - host, err := createHost(pod) + asinfo, err := getASInfo(log, pod, getClientPolicy(aeroCluster, k8sClient)) if err != nil { return nil, err } - asinfo := info.NewAsInfo( - log, host, getClientPolicy(aeroCluster, k8sClient), - ) - confs, err := getAsConfig(asinfo, configContext) if err != nil { return nil, err @@ -604,28 +601,29 @@ func getAerospikeConfigFromNode(log logr.Logger, k8sClient client.Client, ctx go return confs[configContext].(lib.Stats), nil } -func requestInfoFromNode(log logr.Logger, k8sClient client.Client, ctx goctx.Context, - clusterNamespacedName types.NamespacedName, cmd string, pod *asdbv1.AerospikePodStatus) (map[string]string, error) { - aeroCluster, err := getCluster(k8sClient, ctx, clusterNamespacedName) +func getASInfo(log logr.Logger, pod *asdbv1.AerospikePodStatus, policy *as.ClientPolicy) (*info.AsInfo, error) { + host, err := createHost(pod) if err != nil { return nil, err } - host, err := createHost(pod) + return info.NewAsInfo(log, host, policy), nil +} + +func getNamespaceStats(log logr.Logger, k8sClient client.Client, aeroCluster *asdbv1.AerospikeCluster, ns string, + pod *corev1.Pod) (map[string]string, error) { + hostConn, err := newHostConn(log, aeroCluster, pod, k8sClient) if err != nil { return nil, err } - asinfo := info.NewAsInfo( - log, host, getClientPolicy(aeroCluster, k8sClient), - ) - - confs, err := asinfo.RequestInfo(cmd) + stats, err := deployment.GetNamespaceStats([]*deployment.HostConn{hostConn}, getClientPolicy(aeroCluster, + k8sClient), ns) if err != nil { return nil, err } - return confs, nil + return stats[hostConn.ID], nil } func getPasswordFromSecret(k8sClient client.Client, From 73390a4cd82080e9867952eb93e405dbe46d7146 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 12 Nov 2024 12:03:55 +0530 Subject: [PATCH 4/4] Addressing comments --- internal/controller/cluster/access_control.go | 18 ++++++------------ .../cluster/aerospikecluster_controller.go | 3 ++- internal/controller/cluster/pod.go | 3 ++- internal/controller/cluster/reconciler.go | 3 ++- test/cluster/dynamic_config_test.go | 4 ++-- test/cluster/utils.go | 2 +- 6 files changed, 15 insertions(+), 18 deletions(-) diff --git a/internal/controller/cluster/access_control.go b/internal/controller/cluster/access_control.go index 41a1ff05f..86ad688f7 100644 --- a/internal/controller/cluster/access_control.go +++ b/internal/controller/cluster/access_control.go @@ -6,7 +6,6 @@ import ( "fmt" "time" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" as "github.com/aerospike/aerospike-client-go/v7" @@ -14,9 +13,6 @@ import ( acl "github.com/aerospike/aerospike-management-lib/accesscontrol" ) -// logger type alias. -type logger = logr.Logger - // AerospikeAdminCredentials to use for aerospike clients. // // Returns a tuple of admin username and password to use. If the cluster is not security @@ -263,47 +259,45 @@ type AerospikeUserPasswordProvider interface { func (r *SingleClusterReconciler) recordACLEvent(cmd acl.AerospikeAccessControlReconcileCmd, failed bool) { var eventType, message, reason string + if failed { + eventType = corev1.EventTypeWarning + } else { + eventType = corev1.EventTypeNormal + } + switch v := cmd.(type) { case acl.AerospikeUserCreateUpdate: if failed { - eventType = corev1.EventTypeWarning reason = "UserCreateOrUpdateFailed" message = fmt.Sprintf("Failed to Create or Update User %s", v.Name) } else { - eventType = corev1.EventTypeNormal reason = "UserCreatedOrUpdated" message = fmt.Sprintf("Created or Updated User %s", v.Name) } case acl.AerospikeUserDrop: if failed { - eventType = corev1.EventTypeWarning reason = "UserDeleteFailed" message = fmt.Sprintf("Failed to Drop User %s", v.Name) } else { - eventType = corev1.EventTypeNormal reason = "UserDeleted" message = fmt.Sprintf("Dropped User %s", v.Name) } case acl.AerospikeRoleCreateUpdate: if failed { - eventType = corev1.EventTypeWarning reason = "RoleCreateOrUpdateFailed" message = fmt.Sprintf("Failed to Create or Update Role %s", v.Name) } else { - eventType = corev1.EventTypeNormal reason = "RoleCreatedOrUpdated" message = fmt.Sprintf("Created or Updated Role %s", v.Name) } case acl.AerospikeRoleDrop: if failed { - eventType = corev1.EventTypeWarning reason = "RoleDeleteFailed" message = fmt.Sprintf("Failed to Drop Role %s", v.Name) } else { - eventType = corev1.EventTypeNormal reason = "RoleDeleted" message = fmt.Sprintf("Dropped Role %s", v.Name) } diff --git a/internal/controller/cluster/aerospikecluster_controller.go b/internal/controller/cluster/aerospikecluster_controller.go index 56418e7b0..07563e3a4 100644 --- a/internal/controller/cluster/aerospikecluster_controller.go +++ b/internal/controller/cluster/aerospikecluster_controller.go @@ -3,6 +3,7 @@ package cluster import ( "context" + "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/errors" k8sRuntime "k8s.io/apimachinery/pkg/runtime" @@ -31,7 +32,7 @@ type AerospikeClusterReconciler struct { KubeClient *kubernetes.Clientset KubeConfig *rest.Config Scheme *k8sRuntime.Scheme - Log logger + Log logr.Logger } // SetupWithManager sets up the controller with the Manager diff --git a/internal/controller/cluster/pod.go b/internal/controller/cluster/pod.go index 5c2deb308..606008bb4 100644 --- a/internal/controller/cluster/pod.go +++ b/internal/controller/cluster/pod.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" @@ -1439,7 +1440,7 @@ func mutateMap(in asconfig.Conf, funcs []mapping) { } } -func getFlatConfig(log logger, confStr string) (*asconfig.Conf, error) { +func getFlatConfig(log logr.Logger, confStr string) (*asconfig.Conf, error) { asConf, err := asconfig.FromConfFile(log, strings.NewReader(confStr)) if err != nil { return nil, fmt.Errorf("failed to load config map by lib: %v", err) diff --git a/internal/controller/cluster/reconciler.go b/internal/controller/cluster/reconciler.go index de8caeab7..15542b638 100644 --- a/internal/controller/cluster/reconciler.go +++ b/internal/controller/cluster/reconciler.go @@ -7,6 +7,7 @@ import ( "reflect" "strings" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" k8sRuntime "k8s.io/apimachinery/pkg/runtime" @@ -37,7 +38,7 @@ type SingleClusterReconciler struct { KubeClient *kubernetes.Clientset KubeConfig *rest.Config Scheme *k8sRuntime.Scheme - Log logger + Log logr.Logger } func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error) { diff --git a/test/cluster/dynamic_config_test.go b/test/cluster/dynamic_config_test.go index c037eb8c3..316ab6e8c 100644 --- a/test/cluster/dynamic_config_test.go +++ b/test/cluster/dynamic_config_test.go @@ -686,11 +686,11 @@ var _ = Describe( podList, err := getPodList(aeroCluster, k8sClient) Expect(err).ToNot(HaveOccurred()) - stats, err := getNamespaceStats(logger, k8sClient, aeroCluster, "test", &podList.Items[0]) + stats, err := getNamespaceStatsPerHost(logger, k8sClient, aeroCluster, "test", &podList.Items[0]) Expect(err).ToNot(HaveOccurred()) Expect(stats["effective_active_rack"]).To(Equal("1")) - stats, err = getNamespaceStats(logger, k8sClient, aeroCluster, "test1", &podList.Items[0]) + stats, err = getNamespaceStatsPerHost(logger, k8sClient, aeroCluster, "test1", &podList.Items[0]) Expect(err).ToNot(HaveOccurred()) Expect(stats["effective_active_rack"]).To(Equal("2")) diff --git a/test/cluster/utils.go b/test/cluster/utils.go index 8454e13e4..c61d29370 100644 --- a/test/cluster/utils.go +++ b/test/cluster/utils.go @@ -610,7 +610,7 @@ func getASInfo(log logr.Logger, pod *asdbv1.AerospikePodStatus, policy *as.Clien return info.NewAsInfo(log, host, policy), nil } -func getNamespaceStats(log logr.Logger, k8sClient client.Client, aeroCluster *asdbv1.AerospikeCluster, ns string, +func getNamespaceStatsPerHost(log logr.Logger, k8sClient client.Client, aeroCluster *asdbv1.AerospikeCluster, ns string, pod *corev1.Pod) (map[string]string, error) { hostConn, err := newHostConn(log, aeroCluster, pod, k8sClient) if err != nil {