Merge pull request #67 from scality/improvement/COSI-35-update-loggin…

…g-mechanism

COSI-35: Improve Logging Clarity and Accuracy for Object Storage and IAM Operations

codecov.yml

@@ -48,6 +48,10 @@ component_management:
       name: 🔧 Util Package
       paths:
         - pkg/util/**
+    - component_id: constants-package
+      name: 🔖 Constants Package
+      paths:
+        - pkg/constants/**
 
 flag_management:
   default_rules: # the rules that will be followed for any flag added, generally

helm/scality-cosi-driver/values.yaml

@@ -6,10 +6,16 @@ image:
 
 replicaCount: 1
 
-
+# Log levels define the verbosity of logs for various parts of the system.
+# Use these levels to control the detail included in the logs:
+# 1 - General configuration, routine logs
+# 2 - Steady-state operations, HTTP requests, system state changes (default)
+# 3 - Extended changes, additional system details
+# 4 - Debug-level logs, tricky logic areas
+# 5 - Trace-level logs, context for troubleshooting
 logLevels:
-  driver: "5"
-  sidecar: "5"
+  driver: "2"
+  sidecar: "2"
 
 
 namespace: scality-object-storage

kustomize/overlays/debug/scality-cosi-driver.properties

@@ -1,2 +1,9 @@
+# Log levels define the verbosity of logs for various parts of the system.
+# Use these levels to control the detail included in the logs:
+# 1 - General configuration, routine logs
+# 2 - Steady-state operations, HTTP requests, system state changes (default)
+# 3 - Extended changes, additional system details
+# 4 - Debug-level logs, tricky logic areas
+# 5 - Trace-level logs, context for troubleshooting
 COSI_DRIVER_LOG_LEVEL=5
 OBJECTSTORAGE_PROVISIONER_SIDECAR_LOG_LEVEL=5

kustomize/overlays/dev/scality-cosi-driver.properties

@@ -1,2 +1,9 @@
+# Log levels define the verbosity of logs for various parts of the system.
+# Use these levels to control the detail included in the logs:
+# 1 - General configuration, routine logs
+# 2 - Steady-state operations, HTTP requests, system state changes (default)
+# 3 - Extended changes, additional system details
+# 4 - Debug-level logs, tricky logic areas
+# 5 - Trace-level logs, context for troubleshooting
 COSI_DRIVER_LOG_LEVEL=5
 OBJECTSTORAGE_PROVISIONER_SIDECAR_LOG_LEVEL=5

kustomize/overlays/production/scality-cosi-driver.properties

@@ -1,2 +1,9 @@
-COSI_DRIVER_LOG_LEVEL=5
-OBJECTSTORAGE_PROVISIONER_SIDECAR_LOG_LEVEL=5
+# Log levels define the verbosity of logs for various parts of the system.
+# Use these levels to control the detail included in the logs:
+# 1 - General configuration, routine logs
+# 2 - Steady-state operations, HTTP requests, system state changes (default)
+# 3 - Extended changes, additional system details
+# 4 - Debug-level logs, tricky logic areas
+# 5 - Trace-level logs, context for troubleshooting
+COSI_DRIVER_LOG_LEVEL=2
+OBJECTSTORAGE_PROVISIONER_SIDECAR_LOG_LEVEL=2

pkg/clients/iam/iam_client.go

@@ -13,6 +13,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/iam"
 	"github.com/aws/aws-sdk-go-v2/service/iam/types"
 	"github.com/aws/smithy-go/logging"
+	c "github.com/scality/cosi-driver/pkg/constants"
 	"github.com/scality/cosi-driver/pkg/util"
 	"k8s.io/klog/v2"
 )
@@ -48,6 +49,7 @@ var InitIAMClient = func(params util.StorageClientParameters) (*IAMClient, error
 	}
 
 	if strings.HasPrefix(params.IAMEndpoint, "https://") {
+		klog.V(c.LvlDebug).InfoS("Configuring TLS transport for IAM client", "IAMEndpoint", params.IAMEndpoint)
 		httpClient.Transport = util.ConfigureTLSTransport(params.TLSCert)
 	}
 
@@ -60,7 +62,7 @@ var InitIAMClient = func(params util.StorageClientParameters) (*IAMClient, error
 		config.WithLogger(logger),
 	)
 	if err != nil {
-		return nil, fmt.Errorf("failed to load AWS config: %w", err)
+		return nil, err
 	}
 
 	iamClient := iam.NewFromConfig(awsCfg, func(o *iam.Options) {
@@ -79,12 +81,7 @@ func (client *IAMClient) CreateUser(ctx context.Context, userName string) error
 	}
 
 	_, err := client.IAMService.CreateUser(ctx, input)
-	if err != nil {
-		return fmt.Errorf("failed to create IAM user %s: %w", userName, err)
-	}
-
-	klog.InfoS("IAM user creation succeeded", "user", userName)
-	return nil
+	return err
 }
 
 // AttachS3WildcardInlinePolicy attaches an inline policy to an IAM user for a specific bucket.
@@ -110,12 +107,7 @@ func (client *IAMClient) AttachS3WildcardInlinePolicy(ctx context.Context, userN
 	}
 
 	_, err := client.IAMService.PutUserPolicy(ctx, input)
-	if err != nil {
-		return fmt.Errorf("failed to attach inline policy to IAM user %s: %w", userName, err)
-	}
-
-	klog.InfoS("Inline policy attachment succeeded", "user", userName, "policyName", bucketName)
-	return nil
+	return err
 }
 
 // CreateAccessKey generates access keys for an IAM user.
@@ -125,12 +117,7 @@ func (client *IAMClient) CreateAccessKey(ctx context.Context, userName string) (
 	}
 
 	output, err := client.IAMService.CreateAccessKey(ctx, input)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create access key for IAM user %s: %w", userName, err)
-	}
-
-	klog.InfoS("Access key creation succeeded", "user", userName)
-	return output, nil
+	return output, err
 }
 
 // CreateBucketAccess is a helper that combines user creation, policy attachment, and access key generation.
@@ -139,16 +126,19 @@ func (client *IAMClient) CreateBucketAccess(ctx context.Context, userName, bucke
 	if err != nil {
 		return nil, err
 	}
+	klog.V(c.LvlInfo).InfoS("Successfully created IAM user", "userName", userName)
 
 	err = client.AttachS3WildcardInlinePolicy(ctx, userName, bucketName)
 	if err != nil {
 		return nil, err
 	}
+	klog.V(c.LvlInfo).InfoS("Successfully attached inline policy", "userName", userName, "policyName", bucketName)
 
 	accessKeyOutput, err := client.CreateAccessKey(ctx, userName)
 	if err != nil {
 		return nil, err
 	}
+	klog.V(c.LvlInfo).InfoS("Successfully created access key", "userName", userName)
 
 	return accessKeyOutput, nil
 }
@@ -159,32 +149,31 @@ func (client *IAMClient) RevokeBucketAccess(ctx context.Context, userName, bucke
 	if err != nil {
 		return err
 	}
+	klog.V(c.LvlInfo).InfoS("Verified IAM user exists", "userName", userName)
 
 	err = client.DeleteInlinePolicy(ctx, userName, bucketName)
 	if err != nil {
 		return err
 	}
+	klog.V(c.LvlInfo).InfoS("Deleted inline policy if it existed", "userName", userName, "policyName", bucketName)
 
 	err = client.DeleteAllAccessKeys(ctx, userName)
 	if err != nil {
 		return err
 	}
+	klog.V(c.LvlInfo).InfoS("Deleted all access keys if any existed", "userName", userName)
 
 	err = client.DeleteUser(ctx, userName)
 	if err != nil {
 		return err
 	}
-
-	klog.InfoS("Successfully revoked bucket access", "user", userName, "bucket", bucketName)
+	klog.V(c.LvlInfo).InfoS("Deleted IAM user", "userName", userName)
 	return nil
 }
 
 func (client *IAMClient) EnsureUserExists(ctx context.Context, userName string) error {
 	_, err := client.IAMService.GetUser(ctx, &iam.GetUserInput{UserName: &userName})
-	if err != nil {
-		return fmt.Errorf("failed to get IAM user %s: %w", userName, err)
-	}
-	return nil
+	return err
 }
 
 func (client *IAMClient) DeleteInlinePolicy(ctx context.Context, userName, bucketName string) error {
@@ -195,36 +184,37 @@ func (client *IAMClient) DeleteInlinePolicy(ctx context.Context, userName, bucke
 	if err != nil {
 		var noSuchEntityErr *types.NoSuchEntityException
 		if errors.As(err, &noSuchEntityErr) {
-			klog.V(3).InfoS("Inline policy does not exist, skipping deletion", "user", userName, "policyName", bucketName)
+			klog.V(c.LvlDebug).InfoS("Inline policy does not exist, skipping deletion", "user", userName, "policyName", bucketName)
 			return nil
 		}
-		return fmt.Errorf("failed to delete inline policy %s for user %s: %w", bucketName, userName, err)
+		return err
 	}
-	klog.InfoS("Successfully deleted inline policy", "user", userName, "policyName", bucketName)
+	klog.V(c.LvlDebug).InfoS("Successfully deleted inline policy", "userName", userName, "policyName", bucketName)
 	return nil
 }
 
 func (client *IAMClient) DeleteAllAccessKeys(ctx context.Context, userName string) error {
 	listKeysOutput, err := client.IAMService.ListAccessKeys(ctx, &iam.ListAccessKeysInput{UserName: &userName})
 	if err != nil {
-		return fmt.Errorf("failed to list access keys for IAM user %s: %w", userName, err)
+		return err
 	}
 	var noSuchEntityErr *types.NoSuchEntityException
 	for _, key := range listKeysOutput.AccessKeyMetadata {
+		klog.V(c.LvlTrace).InfoS("Deleting access key", "userName", userName, "accessKeyId", *key.AccessKeyId)
 		_, err := client.IAMService.DeleteAccessKey(ctx, &iam.DeleteAccessKeyInput{
 			UserName:    &userName,
 			AccessKeyId: key.AccessKeyId,
 		})
 		if err != nil {
 			if errors.As(err, &noSuchEntityErr) {
-				klog.V(5).InfoS("Access key does not exist, skipping deletion", "user", userName, "accessKeyId", *key.AccessKeyId)
+				klog.V(c.LvlTrace).InfoS("Access key does not exist, skipping deletion", "userName", userName, "accessKeyId", *key.AccessKeyId)
 				continue
 			}
-			return fmt.Errorf("failed to delete access key %s for IAM user %s: %w", *key.AccessKeyId, userName, err)
+			return err
 		}
-		klog.V(5).InfoS("Successfully deleted access key", "user", userName, "accessKeyId", *key.AccessKeyId)
+		klog.V(c.LvlTrace).InfoS("Successfully deleted access key", "userName", userName, "accessKeyId", *key.AccessKeyId)
 	}
-	klog.InfoS("Successfully deleted all access keys", "user", userName)
+	klog.V(c.LvlDebug).InfoS("Successfully deleted all access keys", "userName", userName)
 	return nil
 }
 
@@ -236,8 +226,7 @@ func (client *IAMClient) DeleteUser(ctx context.Context, userName string) error
 			klog.InfoS("IAM user does not exist, skipping deletion", "user", userName)
 			return nil // User doesn't exist, nothing to delete
 		}
-		return fmt.Errorf("failed to delete IAM user %s: %w", userName, err)
+		return err
 	}
-	klog.InfoS("Successfully deleted IAM user", "user", userName)
 	return nil
 }

pkg/clients/iam/iam_client_test.go

@@ -69,7 +69,6 @@ var _ = Describe("IAMClient", func() {
 
 			err := client.CreateUser(ctx, "test-user")
 			Expect(err).NotTo(BeNil())
-			Expect(err.Error()).To(ContainSubstring("failed to create IAM user test-user"))
 			Expect(err.Error()).To(ContainSubstring("simulated CreateUser failure"))
 		})
 
@@ -101,7 +100,6 @@ var _ = Describe("IAMClient", func() {
 
 			err := client.AttachS3WildcardInlinePolicy(ctx, "test-user", "test-bucket")
 			Expect(err).NotTo(BeNil())
-			Expect(err.Error()).To(ContainSubstring("failed to attach inline policy to IAM user test-user"))
 			Expect(err.Error()).To(ContainSubstring("simulated PutUserPolicy failure"))
 		})
 
@@ -136,7 +134,6 @@ var _ = Describe("IAMClient", func() {
 			output, err := client.CreateAccessKey(ctx, "test-user")
 			Expect(err).NotTo(BeNil())
 			Expect(output).To(BeNil())
-			Expect(err.Error()).To(ContainSubstring("failed to create access key for IAM user test-user"))
 			Expect(err.Error()).To(ContainSubstring("simulated CreateAccessKey failure"))
 		})
 	})
@@ -246,7 +243,6 @@ var _ = Describe("IAMClient", func() {
 
 			client, err := iamclient.InitIAMClient(params)
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("failed to load AWS config: mock LoadAWSConfig failure"))
 			Expect(client).To(BeNil())
 		})
 
@@ -299,7 +295,6 @@ var _ = Describe("IAMClient", func() {
 
 			err := client.RevokeBucketAccess(ctx, "non-existent-user", "test-bucket")
 			Expect(err).NotTo(BeNil())
-			Expect(err.Error()).To(ContainSubstring("failed to get IAM user non-existent-user"))
 			Expect(errors.As(err, &noSuchEntityError)).To(BeTrue())
 		})
 

pkg/clients/s3/s3_client.go

@@ -2,7 +2,6 @@ package s3client
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"os"
 	"strings"
@@ -14,7 +13,6 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/aws/smithy-go/logging"
 	"github.com/scality/cosi-driver/pkg/util"
-	"k8s.io/klog/v2"
 )
 
 type S3API interface {
@@ -53,7 +51,7 @@ var InitS3Client = func(params util.StorageClientParameters) (*S3Client, error)
 		config.WithLogger(logger),
 	)
 	if err != nil {
-		return nil, fmt.Errorf("failed to load AWS config: %w", err)
+		return nil, err
 	}
 
 	s3Client := s3.NewFromConfig(awsCfg, func(o *s3.Options) {
@@ -79,12 +77,7 @@ func (client *S3Client) CreateBucket(ctx context.Context, bucketName string, par
 	}
 
 	_, err := client.S3Service.CreateBucket(ctx, input)
-	if err != nil {
-		return err
-	}
-
-	klog.InfoS("Bucket creation operation succeeded", "name", bucketName, "region", params.Region)
-	return nil
+	return err
 }
 
 func (client *S3Client) DeleteBucket(ctx context.Context, bucketName string) error {

pkg/clients/s3/s3_client_test.go

@@ -62,7 +62,6 @@ var _ = Describe("S3Client", func() {
 
 			client, err := s3client.InitS3Client(params)
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("failed to load AWS config: mock config loading error"))
 			Expect(client).To(BeNil())
 		})
 

pkg/constants/constants.go

@@ -0,0 +1,12 @@
+package constants
+
+// Log level constants for structured logging, starting from 1
+// 0 is default if no level is provided
+// Guidelines: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md#what-method-to-use
+const (
+	LvlDefault = iota + 1 // 1 - General configuration, routine logs
+	LvlInfo               // 2 - Steady-state operations, HTTP requests, system state changes
+	LvlEvent              // 3 - Extended changes, additional system details
+	LvlDebug              // 4 - Debug-level logs, tricky logic areas
+	LvlTrace              // 5 - Trace-level logs, detailed troubleshooting context
+)

pkg/constants/constants_test.go

@@ -0,0 +1,26 @@
+package constants_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"                  // Ginkgo for test descriptions
+	. "github.com/onsi/gomega"                     // Gomega for assertions
+	"github.com/scality/cosi-driver/pkg/constants" // Import the constants package
+)
+
+func TestConstants(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Constants Suite")
+}
+
+var _ = Describe("Constants", func() {
+	Context("Log level constants", func() {
+		It("should have the correct values for log levels", func() {
+			Expect(constants.LvlDefault).To(Equal(1), "LvlDefault should start at 1")
+			Expect(constants.LvlInfo).To(Equal(2), "LvlInfo should have the value 2")
+			Expect(constants.LvlEvent).To(Equal(3), "LvlEvent should have the value 3")
+			Expect(constants.LvlDebug).To(Equal(4), "LvlDebug should have the value 4")
+			Expect(constants.LvlTrace).To(Equal(5), "LvlTrace should have the value 5")
+		})
+	})
+})