Update to Go 1.22 and minor refactors (#48)

* fix: log for user existence check

* fix: tests OK upto secret deletion

* Add re-fetch specifically for a OverrideExistingSecret edge case

* Cleanup debug/useless logs

* Removed some TODO/commented lines

* logs: remove/replace WithName

* TODO removal

Author: phlg

Dockerfile

@@ -1,5 +1,5 @@
 # Build the manager binary
-FROM golang:1.21 as builder
+FROM golang:1.22 as builder
 ARG TARGETOS
 ARG TARGETARCH
 

controllers/path_controller.go

@@ -184,7 +184,7 @@ func (r *PathReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func (r *PathReconciler) finalizePath(pathResource *s3v1alpha1.Path) error {
-	logger := log.Log.WithName("finalize")
+	logger := log.Log.WithValues("controller", "path")
 	if r.PathDeletion {
 		var failedPaths []string = make([]string, 0)
 		for _, path := range pathResource.Spec.Paths {

controllers/s3/factory/interface.go

@@ -9,7 +9,7 @@ import (
 )
 
 var (
-	s3Logger = ctrl.Log.WithName("s3Client")
+	s3Logger = ctrl.Log.WithValues("logger", "s3client")
 )
 
 type S3Client interface {
@@ -30,11 +30,11 @@ type S3Client interface {
 	UserExist(name string) (bool, error)
 	CheckUserCredentialsValid(name string, accessKey string, secretKey string) (bool, error)
 	AddServiceAccountForUser(name string, accessKey string, secretKey string) error
-	CreateUser(name string, password string) error
-	DeleteUser(name string) error
+	CreateUser(accessKey string, secretKey string) error
+	DeleteUser(accessKey string) error
 	GetUserPolicies(name string) ([]string, error)
-	AddPoliciesToUser(username string, policies []string) error
-	RemovePoliciesFromUser(username string, policies []string) error
+	AddPoliciesToUser(accessKey string, policies []string) error
+	RemovePoliciesFromUser(accessKey string, policies []string) error
 }
 
 type S3Config struct {

controllers/s3/factory/minioS3Client.go

@@ -254,18 +254,18 @@ func (minioS3Client *MinioS3Client) DeletePolicy(name string) error {
 // USER   methods //
 ////////////////////
 
-func (minioS3Client *MinioS3Client) CreateUser(name string, password string) error {
-	s3Logger.Info("Creating user", "user", name)
-	err := minioS3Client.adminClient.AddUser(context.Background(), name, password)
+func (minioS3Client *MinioS3Client) CreateUser(accessKey string, secretKey string) error {
+	s3Logger.Info("Creating user", "accessKey", accessKey)
+	err := minioS3Client.adminClient.AddUser(context.Background(), accessKey, secretKey)
 	if err != nil {
-		s3Logger.Error(err, "Error while creating user", "user", name)
+		s3Logger.Error(err, "Error while creating user", "user", accessKey)
 		return err
 	}
 	return nil
 }
 
 func (minioS3Client *MinioS3Client) AddServiceAccountForUser(name string, accessKey string, secretKey string) error {
-	s3Logger.Info("Adding service account for user", "user", name)
+	s3Logger.Info("Adding service account for user", "user", name, "accessKey", accessKey)
 
 	opts := madmin.AddServiceAccountReq{
 		AccessKey:   accessKey,
@@ -285,27 +285,37 @@ func (minioS3Client *MinioS3Client) AddServiceAccountForUser(name string, access
 
 }
 
-func (minioS3Client *MinioS3Client) UserExist(name string) (bool, error) {
-	s3Logger.Info("checking user existence", "user", name)
-	_, _err := minioS3Client.adminClient.GetUserInfo(context.Background(), name)
+func (minioS3Client *MinioS3Client) UserExist(accessKey string) (bool, error) {
+	s3Logger.Info("checking user existence", "accessKey", accessKey)
+	_, _err := minioS3Client.adminClient.GetUserInfo(context.Background(), accessKey)
 	if _err != nil {
-		s3Logger.Info("received code", "user", minio.ToErrorResponse(_err))
-		if minio.ToErrorResponse(_err).StatusCode == 0 {
+		if madmin.ToErrorResponse(_err).Code == "XMinioAdminNoSuchUser" {
 			return false, nil
 		}
+		s3Logger.Error(_err, "an error occurred when checking user's existence")
 		return false, _err
 	}
+
 	return true, nil
 }
 
-func (minioS3Client *MinioS3Client) DeleteUser(name string) error {
-	s3Logger.Info("delete user", "user", name)
-	return minioS3Client.adminClient.RemoveUser(context.Background(), name)
+func (minioS3Client *MinioS3Client) DeleteUser(accessKey string) error {
+	s3Logger.Info("delete user with accessKey", "accessKey", accessKey)
+	err := minioS3Client.adminClient.RemoveUser(context.Background(), accessKey)
+	if err != nil {
+		if madmin.ToErrorResponse(err).Code == "XMinioAdminNoSuchUser" {
+			s3Logger.Info("the user was already deleted from s3 backend")
+			return nil
+		}
+		s3Logger.Error(err, "an error occurred when attempting to delete the user")
+		return err
+	}
+	return nil
 }
 
-func (minioS3Client *MinioS3Client) GetUserPolicies(name string) ([]string, error) {
-	s3Logger.Info("Get user policies", "user", name)
-	userInfo, err := minioS3Client.adminClient.GetUserInfo(context.Background(), name)
+func (minioS3Client *MinioS3Client) GetUserPolicies(accessKey string) ([]string, error) {
+	s3Logger.Info("Get user policies", "accessKey", accessKey)
+	userInfo, err := minioS3Client.adminClient.GetUserInfo(context.Background(), accessKey)
 	if err != nil {
 		s3Logger.Error(err, "Error when getting userInfo")
 
@@ -315,7 +325,7 @@ func (minioS3Client *MinioS3Client) GetUserPolicies(name string) ([]string, erro
 }
 
 func (minioS3Client *MinioS3Client) CheckUserCredentialsValid(name string, accessKey string, secretKey string) (bool, error) {
-	s3Logger.Info("Check credential for user", "user", name)
+	s3Logger.Info("Check credentials for user", "user", name, "accessKey", accessKey)
 	minioTestClientOptions := &minio.Options{
 		Creds:  credentials.NewStaticV4(accessKey, secretKey, ""),
 		Region: minioS3Client.s3Config.Region,
@@ -324,43 +334,64 @@ func (minioS3Client *MinioS3Client) CheckUserCredentialsValid(name string, acces
 	addTransportOptions(&minioS3Client.s3Config, minioTestClientOptions)
 	minioTestClient, err := minio.New(minioS3Client.s3Config.S3UrlEndpoint, minioTestClientOptions)
 	if err != nil {
-		s3Logger.Error(err, "An error occurred while creating a new minio test client")
+		s3Logger.Error(err, "An error occurred while creating a new Minio test client")
 	}
 
 	_, err = minioTestClient.ListBuckets(context.Background())
 	if err != nil {
-		s3Logger.Error(err, "An error occurred while listing bucket")
-		return false, err
+		errAsResponse := minio.ToErrorResponse(err)
+		if errAsResponse.Code == "SignatureDoesNotMatch" {
+			s3Logger.Info("the user credentials appear to be invalid", "accessKey", accessKey, "s3BackendError", errAsResponse)
+			return false, nil
+		} else if errAsResponse.Code == "InvalidAccessKeyId" {
+			s3Logger.Info("this accessKey does not exist on the s3 backend", "accessKey", accessKey, "s3BackendError", errAsResponse)
+			return false, nil
+		} else {
+			s3Logger.Error(err, "an error occurred while checking if the S3 user's credentials were valid", "accessKey", accessKey, "code", errAsResponse.Code)
+			return false, err
+		}
 	}
 	return true, nil
 }
 
-func (minioS3Client *MinioS3Client) RemovePoliciesFromUser(username string, policies []string) error {
-	s3Logger.Info(fmt.Sprintf("Remove policy [%s] from user [%s]", policies, username))
+func (minioS3Client *MinioS3Client) RemovePoliciesFromUser(accessKey string, policies []string) error {
+	s3Logger.Info(fmt.Sprintf("Remove policy [%s] from user [%s]", policies, accessKey))
 
 	opts := madmin.PolicyAssociationReq{
 		Policies: policies,
-		User:     username,
+		User:     accessKey,
 	}
 
 	_, err := minioS3Client.adminClient.DetachPolicy(context.Background(), opts)
 
 	if err != nil {
+		errAsResp := madmin.ToErrorResponse(err)
+		if errAsResp.Code == "XMinioAdminPolicyChangeAlreadyApplied" {
+			s3Logger.Info("The policy change has no net effect")
+			return nil
+		}
+		s3Logger.Error(err, "an error occurred when attaching a policy to the user", "code", errAsResp.Code)
 		return err
 	}
 
 	return nil
 }
 
-func (minioS3Client *MinioS3Client) AddPoliciesToUser(username string, policies []string) error {
-	s3Logger.Info("Adding policies to user", "user", username, "policies", policies)
+func (minioS3Client *MinioS3Client) AddPoliciesToUser(accessKey string, policies []string) error {
+	s3Logger.Info("Adding policies to user", "user", accessKey, "policies", policies)
 	opts := madmin.PolicyAssociationReq{
-		User:     username,
+		User:     accessKey,
 		Policies: policies,
 	}
 	_, err := minioS3Client.adminClient.AttachPolicy(context.Background(), opts)
 	if err != nil {
+		errAsResp := madmin.ToErrorResponse(err)
+		if errAsResp.Code == "XMinioAdminPolicyChangeAlreadyApplied" {
+			s3Logger.Info("The policy change has no net effect")
+			return nil
+		}
+		s3Logger.Error(err, "an error occurred when attaching a policy to the user", "code", errAsResp.Code)
 		return err
 	}
 	return nil
-}
+}