chore: add iam logger (#257)

* chore: add iam logger Signed-off-by: Kasper J. Hermansen <[email protected]> * chore: add logger to interface Signed-off-by: Kasper J. Hermansen <[email protected]> * chore: add logger to interface Signed-off-by: Kasper J. Hermansen <[email protected]> --------- Signed-off-by: Kasper J. Hermansen <[email protected]>
lunarway · Aug 14, 2023 · 167b952 · 167b952
1 parent 50fb816
commit 167b952
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 12 deletions.
diff --git a/controllers/postgresqluser_controller.go b/controllers/postgresqluser_controller.go
@@ -47,7 +47,7 @@ type PostgreSQLUserReconciler struct {
 	Scheme *runtime.Scheme
 
 	Granter       grants.Granter
-	EnsureIAMUser func(client *iam.Client, config iam.EnsureUserConfig, username, rolename string) error
+	EnsureIAMUser func(client *iam.Client, logger logr.Logger, config iam.EnsureUserConfig, username, rolename string) error
 	RemoveIAMUser func(client *iam.Client, awsLoginRoles []string, username string) error
 
 	RolePrefix         string
@@ -176,7 +176,7 @@ func (r *PostgreSQLUserReconciler) reconcile(ctx context.Context, reqLogger logr
 	// Error check in the bottom because we want aws policy to be set no matter what.
 	granterErr := r.Granter.SyncUser(reqLogger, request.Namespace, r.RolePrefix, *sanitizedUser)
 
-	awsPolicyErr := r.EnsureIAMUser(client, iam.EnsureUserConfig{
+	awsPolicyErr := r.EnsureIAMUser(client, reqLogger, iam.EnsureUserConfig{
 		PolicyBaseName:    r.AWSPolicyName,
 		Region:            r.AWSRegion,
 		AccountID:         r.AWSAccountID,

diff --git a/controllers/postgresqluser_controller_test.go b/controllers/postgresqluser_controller_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
 	lunarwayv1alpha1 "go.lunarway.com/postgresql-controller/api/v1alpha1"
 	"go.lunarway.com/postgresql-controller/pkg/grants"
@@ -145,7 +146,7 @@ func TestReconcile_badConfigmapReference(t *testing.T) {
 				return kube.ResourceValue(cl, resource, namespace)
 			},
 		},
-		EnsureIAMUser: func(client *iam.Client, config iam.EnsureUserConfig, username, rolename string) error {
+		EnsureIAMUser: func(client *iam.Client, logger logr.Logger, config iam.EnsureUserConfig, username, rolename string) error {
 			return nil
 		},
 	}
@@ -262,7 +263,7 @@ func TestReconcile_rolePrefix(t *testing.T) {
 				return kube.ResourceValue(cl, resource, namespace)
 			},
 		},
-		EnsureIAMUser: func(client *iam.Client, config iam.EnsureUserConfig, username, rolename string) error {
+		EnsureIAMUser: func(client *iam.Client, logger logr.Logger, config iam.EnsureUserConfig, username, rolename string) error {
 			return nil
 		},
 	}
@@ -381,7 +382,7 @@ func TestReconcile_dotInName(t *testing.T) {
 				return kube.ResourceValue(cl, resource, namespace)
 			},
 		},
-		EnsureIAMUser: func(client *iam.Client, config iam.EnsureUserConfig, username, rolename string) error {
+		EnsureIAMUser: func(client *iam.Client, logger logr.Logger, config iam.EnsureUserConfig, username, rolename string) error {
 			assert.Equal(t, userName, username, "iam username must be the original")
 			assert.Equal(t, rolename, userNameSanitized, "iam rolename must be the sanitized")
 			return nil
@@ -529,7 +530,7 @@ func TestReconcile_multipleDatabaseResources(t *testing.T) {
 				return kube.ResourceValue(cl, resource, namespace)
 			},
 		},
-		EnsureIAMUser: func(client *iam.Client, config iam.EnsureUserConfig, username, rolename string) error {
+		EnsureIAMUser: func(client *iam.Client, logger logr.Logger, config iam.EnsureUserConfig, username, rolename string) error {
 			return nil
 		},
 	}

diff --git a/pkg/iam/iam.go b/pkg/iam/iam.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/aws/aws-sdk-go/service/iam"
+	"github.com/go-logr/logr"
 	"go.uber.org/multierr"
 )
 
@@ -16,8 +17,9 @@ type EnsureUserConfig struct {
 	AWSLoginRoles     []string
 }
 
-func EnsureUser(client *Client, config EnsureUserConfig, userName, rolename string) error {
+func EnsureUser(client *Client, log logr.Logger, config EnsureUserConfig, userName, rolename string) error {
 	users := make(map[string]struct{})
+	log.Info("listing iam policies")
 	policies, err := client.ListPolicies()
 	if err != nil {
 		return err
@@ -40,6 +42,7 @@ func EnsureUser(client *Client, config EnsureUserConfig, userName, rolename stri
 			// Try to update the document where the user is present to ensure correct roleName.
 			updated := policy.Document.Update(config.Region, config.AccountID, config.RolePrefix, userName, rolename)
 			if updated {
+				log.Info("updating policies for user", "userName", userName, "roleName", rolename)
 				err = updatePolicies(client, policies)
 				if err != nil {
 					return err
@@ -52,6 +55,7 @@ func EnsureUser(client *Client, config EnsureUserConfig, userName, rolename stri
 	} else {
 		for _, policy := range policies {
 			if policy.Document.Count() < config.MaxUsersPerPolicy {
+				log.Info("adding user to policy document", "userName", userName, "roleName", rolename)
 				policy.Document.Add(config.Region, config.AccountID, config.RolePrefix, userName, rolename)
 				err = updatePolicies(client, policies)
 				if err != nil {
@@ -65,6 +69,7 @@ func EnsureUser(client *Client, config EnsureUserConfig, userName, rolename stri
 
 	// User could not be handled in an existing policy so we create a new one instead.
 	if !userHandled {
+		log.Info("creating a new policy document because of user", "userName", userName, "roleName", rolename)
 		// TODO : There is a bug where where the new name might exist. This could for instance be the case where a policy i is deleted but i+1 exists. Then len(policies) = i+1 and there is a clash.
 		newPolicy := &Policy{
 			Name:     fmt.Sprintf("%s_%d", config.PolicyBaseName, len(policies)),

diff --git a/pkg/iam/iam_test.go b/pkg/iam/iam_test.go
@@ -75,11 +75,11 @@ func TestEnsureUser_roleChange(t *testing.T) {
 	}
 
 	// add a user
-	err := EnsureUser(client, addUserConfig, "user1", "role1")
+	err := EnsureUser(client, logger, addUserConfig, "user1", "role1")
 	require.NoError(t, err, "unexpected error when adding the first user")
 
 	// update with a new role
-	err = EnsureUser(client, addUserConfig, "user1", "role2")
+	err = EnsureUser(client, logger, addUserConfig, "user1", "role2")
 	require.NoError(t, err, "unexpected error when updating the first user")
 
 	expectedPolicies := []*Policy{
@@ -137,7 +137,7 @@ func TestEnsureUser_AWSLoginRole_Added(t *testing.T) {
 	}
 
 	// add a user
-	err := EnsureUser(client, existingUserConfig, "user1", "role1")
+	err := EnsureUser(client, logger, existingUserConfig, "user1", "role1")
 	require.NoError(t, err, "unexpected error when adding the first user")
 
 	assertPolicyOnAWSLoginRole(t, client, existingRole)
@@ -157,7 +157,7 @@ func TestEnsureUser_AWSLoginRole_Added(t *testing.T) {
 		},
 	}
 
-	err = EnsureUser(client, newUserConfig, "user1", "role1")
+	err = EnsureUser(client, logger, newUserConfig, "user1", "role1")
 	require.NoError(t, err, "unexpected error when adding the user to the new AWSLoginRole")
 
 	assertPolicyOnAWSLoginRole(t, client, newRole)
@@ -362,7 +362,7 @@ func Test_AddRemoveUser(t *testing.T) {
 			}
 
 			if tt.operation == EnsureUserOperation {
-				err = EnsureUser(client, config, tt.user, tt.user)
+				err = EnsureUser(client, logger, config, tt.user, tt.user)
 				assert.NoError(err)
 			} else if tt.operation == RemoveUserOperation {
 				err = RemoveUser(client, []string{awsLoginRole}, tt.user)