From c0e4240c9b5663931f88159a16a284a44e35b26b Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Tue, 17 Dec 2024 10:33:27 +0100 Subject: [PATCH 01/10] COSI-35: move-helper-methods-below-driver-apis Align with GO standards, move helper methods below driver APIs. --- pkg/driver/provisioner_server_impl.go | 184 +++++++++++++------------- 1 file changed, 92 insertions(+), 92 deletions(-) diff --git a/pkg/driver/provisioner_server_impl.go b/pkg/driver/provisioner_server_impl.go index e10ef0f7..90394674 100644 --- a/pkg/driver/provisioner_server_impl.go +++ b/pkg/driver/provisioner_server_impl.go @@ -153,98 +153,6 @@ func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context, }, nil } -func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Interface, parameters map[string]string, service string) (interface{}, *util.StorageClientParameters, error) { - klog.V(3).InfoS("Initializing object storage provider clients", "parameters", parameters) - - ospSecretName, namespace, err := FetchSecretInformation(parameters) - if err != nil { - klog.ErrorS(err, "Failed to fetch object storage provider secret info") - return nil, nil, err - } - - klog.V(4).InfoS("Fetching secret", "secretName", ospSecretName, "namespace", namespace) - ospSecret, err := clientset.CoreV1().Secrets(namespace).Get(ctx, ospSecretName, metav1.GetOptions{}) - if err != nil { - klog.ErrorS(err, "Failed to get object store user secret", "secretName", ospSecretName) - return nil, nil, status.Error(codes.Internal, "failed to get object store user secret") - } - - storageClientParameters, err := FetchParameters(ospSecret.Data) - if err != nil { - klog.ErrorS(err, "Failed to fetch S3 parameters from secret", "secretName", ospSecretName) - return nil, nil, err - } - - var client interface{} - switch service { - case "S3": - client, err = s3client.InitS3Client(*storageClientParameters) - if err != nil { - klog.ErrorS(err, "Failed to initialize S3 client", "endpoint", storageClientParameters.Endpoint) - return nil, nil, status.Error(codes.Internal, "failed to initialize S3 client") - } - klog.V(3).InfoS("Successfully initialized S3 client", "endpoint", storageClientParameters.Endpoint) - case "IAM": - client, err = iamclient.InitIAMClient(*storageClientParameters) - if err != nil { - klog.ErrorS(err, "Failed to initialize IAM client", "endpoint", storageClientParameters.Endpoint) - return nil, nil, status.Error(codes.Internal, "failed to initialize IAM client") - } - klog.V(3).InfoS("Successfully initialized IAM client", "endpoint", storageClientParameters.Endpoint) - default: - klog.ErrorS(nil, "Unsupported object storage provider service", "service", service) - return nil, nil, status.Error(codes.Internal, "unsupported object storage provider service") - } - return client, storageClientParameters, nil -} - -func fetchObjectStorageProviderSecretInfo(parameters map[string]string) (string, string, error) { - klog.V(4).InfoS("Fetching object storage provider secret info", "parameters", parameters) - - secretName := parameters["objectStorageSecretName"] - namespace := os.Getenv("POD_NAMESPACE") - if parameters["objectStorageSecretNamespace"] != "" { - namespace = parameters["objectStorageSecretNamespace"] - } - if secretName == "" || namespace == "" { - klog.ErrorS(nil, "Missing object storage provider secret name or namespace", "secretName", secretName, "namespace", namespace) - return "", "", status.Error(codes.InvalidArgument, "Object storage provider secret name and namespace are required") - } - - klog.V(4).InfoS("Object storage provider secret info fetched", "secretName", secretName, "namespace", namespace) - return secretName, namespace, nil -} - -func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParameters, error) { - klog.V(5).InfoS("Fetching S3 parameters from secret") - - params := util.NewStorageClientParameters() - - params.AccessKeyID = string(secretData["accessKeyId"]) - params.SecretAccessKey = string(secretData["secretAccessKey"]) - params.Endpoint = string(secretData["endpoint"]) - params.Region = string(secretData["region"]) - - if cert, exists := secretData["tlsCert"]; exists { - params.TLSCert = cert - } else { - klog.V(5).InfoS("TLS certificate is not provided, proceeding without it") - } - - if err := params.Validate(); err != nil { - klog.ErrorS(err, "invalid object storage parameters") - return nil, err - } - - params.IAMEndpoint = params.Endpoint - if value, exists := secretData["iamEndpoint"]; exists && len(value) > 0 { - params.IAMEndpoint = string(value) - klog.V(5).InfoS("IAM endpoint specified", "iamEndpoint", params.IAMEndpoint) - } - - return params, nil -} - // DriverDeleteBucket is an idempotent method for deleting buckets // It is expected to delete the same bucket given a bucketId // If the bucket does not exist, then it MUST return no error @@ -382,3 +290,95 @@ func (s *ProvisionerServer) DriverRevokeBucketAccess(ctx context.Context, klog.V(3).InfoS("Successfully revoked bucket access", "bucketName", bucketName, "userName", userName) return &cosiapi.DriverRevokeBucketAccessResponse{}, nil } + +func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Interface, parameters map[string]string, service string) (interface{}, *util.StorageClientParameters, error) { + klog.V(3).InfoS("Initializing object storage provider clients", "parameters", parameters) + + ospSecretName, namespace, err := FetchSecretInformation(parameters) + if err != nil { + klog.ErrorS(err, "Failed to fetch object storage provider secret info") + return nil, nil, err + } + + klog.V(4).InfoS("Fetching secret", "secretName", ospSecretName, "namespace", namespace) + ospSecret, err := clientset.CoreV1().Secrets(namespace).Get(ctx, ospSecretName, metav1.GetOptions{}) + if err != nil { + klog.ErrorS(err, "Failed to get object store user secret", "secretName", ospSecretName) + return nil, nil, status.Error(codes.Internal, "failed to get object store user secret") + } + + storageClientParameters, err := FetchParameters(ospSecret.Data) + if err != nil { + klog.ErrorS(err, "Failed to fetch S3 parameters from secret", "secretName", ospSecretName) + return nil, nil, err + } + + var client interface{} + switch service { + case "S3": + client, err = s3client.InitS3Client(*storageClientParameters) + if err != nil { + klog.ErrorS(err, "Failed to initialize S3 client", "endpoint", storageClientParameters.Endpoint) + return nil, nil, status.Error(codes.Internal, "failed to initialize S3 client") + } + klog.V(3).InfoS("Successfully initialized S3 client", "endpoint", storageClientParameters.Endpoint) + case "IAM": + client, err = iamclient.InitIAMClient(*storageClientParameters) + if err != nil { + klog.ErrorS(err, "Failed to initialize IAM client", "endpoint", storageClientParameters.Endpoint) + return nil, nil, status.Error(codes.Internal, "failed to initialize IAM client") + } + klog.V(3).InfoS("Successfully initialized IAM client", "endpoint", storageClientParameters.Endpoint) + default: + klog.ErrorS(nil, "Unsupported object storage provider service", "service", service) + return nil, nil, status.Error(codes.Internal, "unsupported object storage provider service") + } + return client, storageClientParameters, nil +} + +func fetchObjectStorageProviderSecretInfo(parameters map[string]string) (string, string, error) { + klog.V(4).InfoS("Fetching object storage provider secret info", "parameters", parameters) + + secretName := parameters["objectStorageSecretName"] + namespace := os.Getenv("POD_NAMESPACE") + if parameters["objectStorageSecretNamespace"] != "" { + namespace = parameters["objectStorageSecretNamespace"] + } + if secretName == "" || namespace == "" { + klog.ErrorS(nil, "Missing object storage provider secret name or namespace", "secretName", secretName, "namespace", namespace) + return "", "", status.Error(codes.InvalidArgument, "Object storage provider secret name and namespace are required") + } + + klog.V(4).InfoS("Object storage provider secret info fetched", "secretName", secretName, "namespace", namespace) + return secretName, namespace, nil +} + +func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParameters, error) { + klog.V(5).InfoS("Fetching S3 parameters from secret") + + params := util.NewStorageClientParameters() + + params.AccessKeyID = string(secretData["accessKeyId"]) + params.SecretAccessKey = string(secretData["secretAccessKey"]) + params.Endpoint = string(secretData["endpoint"]) + params.Region = string(secretData["region"]) + + if cert, exists := secretData["tlsCert"]; exists { + params.TLSCert = cert + } else { + klog.V(5).InfoS("TLS certificate is not provided, proceeding without it") + } + + if err := params.Validate(); err != nil { + klog.ErrorS(err, "invalid object storage parameters") + return nil, err + } + + params.IAMEndpoint = params.Endpoint + if value, exists := secretData["iamEndpoint"]; exists && len(value) > 0 { + params.IAMEndpoint = string(value) + klog.V(5).InfoS("IAM endpoint specified", "iamEndpoint", params.IAMEndpoint) + } + + return params, nil +} From 7416cd58b4390fd85fb3f73e365bc5cc9829a387 Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Tue, 17 Dec 2024 11:19:47 +0100 Subject: [PATCH 02/10] COSI-35: update-provisioner-with-higher-level-logs --- pkg/driver/provisioner_server_impl.go | 71 +++++++++++++-------------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/pkg/driver/provisioner_server_impl.go b/pkg/driver/provisioner_server_impl.go index 90394674..6e13b995 100644 --- a/pkg/driver/provisioner_server_impl.go +++ b/pkg/driver/provisioner_server_impl.go @@ -21,7 +21,6 @@ import ( "os" s3types "github.com/aws/aws-sdk-go-v2/service/s3/types" - "github.com/aws/smithy-go" iamclient "github.com/scality/cosi-driver/pkg/clients/iam" s3client "github.com/scality/cosi-driver/pkg/clients/s3" "github.com/scality/cosi-driver/pkg/util" @@ -110,12 +109,11 @@ func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context, parameters := req.GetParameters() service := "S3" - klog.V(3).InfoS("Received DriverCreateBucket request", "bucketName", bucketName) - klog.V(5).InfoS("Processing DriverCreateBucket", "bucketName", bucketName, "parameters", parameters) + klog.V(2).InfoS("Processing DriverCreateBucket request", "bucketName", bucketName) client, s3Params, err := InitializeClient(ctx, s.Clientset, parameters, service) if err != nil { - klog.ErrorS(err, "Failed to initialize object storage provider S3 client", "bucketName", bucketName) + klog.ErrorS(err, "Failed to initialize S3 client", "bucketName", bucketName) return nil, status.Error(codes.Internal, "failed to initialize object storage provider S3 client") } @@ -125,29 +123,26 @@ func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context, return nil, status.Error(codes.InvalidArgument, "unsupported client type for bucket creation") } + klog.V(3).InfoS("Creating bucket", "bucketName", bucketName) err = s3Client.CreateBucket(ctx, bucketName, *s3Params) if err != nil { var bucketAlreadyExists *s3types.BucketAlreadyExists var bucketOwnedByYou *s3types.BucketAlreadyOwnedByYou if errors.As(err, &bucketAlreadyExists) { - klog.V(3).InfoS("Bucket already exists", "bucketName", bucketName) + klog.V(2).InfoS("Bucket already exists", "bucketName", bucketName) return nil, status.Errorf(codes.AlreadyExists, "Bucket already exists: %s", bucketName) } else if errors.As(err, &bucketOwnedByYou) { - klog.V(3).InfoS("A bucket with this name exists and is already owned by you: success", "bucketName", bucketName) + klog.V(2).InfoS("Bucket already exists and is owned by you", "bucketName", bucketName) return &cosiapi.DriverCreateBucketResponse{ BucketId: bucketName, }, nil } else { - var opErr *smithy.OperationError - if errors.As(err, &opErr) { - klog.V(4).InfoS("AWS operation error", "operation", opErr.OperationName, "message", opErr.Err.Error(), "bucketName", bucketName) - } klog.ErrorS(err, "Failed to create bucket", "bucketName", bucketName) return nil, status.Error(codes.Internal, "Failed to create bucket") } } - klog.V(3).InfoS("Successfully created bucket", "bucketName", bucketName) + klog.V(2).InfoS("Successfully created bucket", "bucketName", bucketName) return &cosiapi.DriverCreateBucketResponse{ BucketId: bucketName, }, nil @@ -166,15 +161,17 @@ func (s *ProvisionerServer) DriverDeleteBucket(ctx context.Context, bucketName := req.GetBucketId() - klog.V(3).InfoS("Received DriverDeleteBucket request", "bucketName", bucketName) + klog.V(2).InfoS("Processing DriverDeleteBucket request", "bucketName", bucketName) bucket, err := s.BucketClientset.ObjectstorageV1alpha1().Buckets().Get(ctx, bucketName, metav1.GetOptions{}) if err != nil { - klog.ErrorS(err, "Failed to get bucket object from kubernetes", "bucketName", bucketName) + klog.ErrorS(err, "Failed to fetch bucket object", "bucketName", bucketName) return nil, status.Error(codes.Internal, "failed to get bucket object from kubernetes") } + klog.V(5).InfoS("Successfully fetched Bucket object", "bucketName", bucket.Name, "parameters", bucket.Spec.Parameters) + client, _, err := InitializeClient(ctx, s.Clientset, bucket.Spec.Parameters, "S3") if err != nil { - klog.ErrorS(err, "Failed to initialize object storage provider S3 client", "bucketName", bucketName) + klog.ErrorS(err, "Failed to initialize S3 client for bucket deletion", "bucketName", bucketName) return nil, status.Error(codes.Internal, "failed to initialize object storage provider S3 client") } @@ -190,7 +187,7 @@ func (s *ProvisionerServer) DriverDeleteBucket(ctx context.Context, return nil, status.Error(codes.Internal, "failed to delete bucket") } - klog.V(3).InfoS("Successfully deleted bucket", "bucketName", bucketName) + klog.V(2).InfoS("Successfully deleted bucket", "bucketName", bucketName) return &cosiapi.DriverDeleteBucketResponse{}, nil } @@ -207,14 +204,12 @@ func (s *ProvisionerServer) DriverGrantBucketAccess(ctx context.Context, userName := req.GetName() parameters := req.GetParameters() - klog.V(3).InfoS("Received DriverGrantBucketAccess request", "bucketName", bucketName, "userName", userName) - klog.V(4).InfoS("Processing DriverGrantBucketAccess", "parameters", parameters) - klog.V(5).InfoS("Request DriverGrantBucketAccess", "req", req) + klog.V(2).InfoS("Processing DriverGrantBucketAccess request", "bucketName", bucketName, "userName", userName) client, iamParams, err := InitializeClient(ctx, s.Clientset, parameters, "IAM") if err != nil { - klog.ErrorS(err, "Failed to initialize object storage provider IAM client", "bucketName", bucketName, "userName", userName) + klog.ErrorS(err, "Failed to initialize IAM client", "bucketName", bucketName, "userName", userName) return nil, status.Error(codes.Internal, "failed to initialize object storage provider IAM client") } @@ -224,13 +219,14 @@ func (s *ProvisionerServer) DriverGrantBucketAccess(ctx context.Context, return nil, status.Error(codes.Internal, "failed to initialize object storage provider IAM client") } + klog.V(2).InfoS("Granting bucket access", "bucketName", bucketName, "userName", userName) userInfo, err := iamClient.CreateBucketAccess(ctx, userName, bucketName) if err != nil { klog.ErrorS(err, "Failed to create bucket access", "bucketName", bucketName, "userName", userName) return nil, status.Error(codes.Internal, "failed to create bucket access") } - klog.V(3).InfoS("Successfully created bucket access", "bucketName", bucketName, "userName", userName) + klog.V(2).InfoS("Successfully granted bucket access", "bucketName", bucketName, "userName", userName) return &cosiapi.DriverGrantBucketAccessResponse{ AccountId: userName, Credentials: map[string]*cosiapi.CredentialDetails{ @@ -260,14 +256,15 @@ func (s *ProvisionerServer) DriverRevokeBucketAccess(ctx context.Context, bucketName := req.GetBucketId() userName := req.GetAccountId() - klog.V(3).InfoS("Received DriverRevokeBucketAccess request", "bucketName", bucketName, "userName", userName) + klog.V(2).InfoS("Processing DriverRevokeBucketAccess request", "bucketName", bucketName, "userName", userName) // Fetch the bucket to retrieve parameters bucket, err := s.BucketClientset.ObjectstorageV1alpha1().Buckets().Get(ctx, bucketName, metav1.GetOptions{}) if err != nil { - klog.ErrorS(err, "Failed to get bucket object from kubernetes", "bucketName", bucketName) + klog.ErrorS(err, "Failed to fetch bucket object", "bucketName", bucketName) return nil, status.Error(codes.Internal, "failed to get bucket object from kubernetes") } + klog.V(5).InfoS("Successfully fetched Bucket object", "bucketName", bucket.Name, "parameters", bucket.Spec.Parameters) client, _, err := InitializeClient(ctx, s.Clientset, bucket.Spec.Parameters, "IAM") if err != nil { @@ -277,22 +274,23 @@ func (s *ProvisionerServer) DriverRevokeBucketAccess(ctx context.Context, iamClient, ok := client.(*iamclient.IAMClient) if !ok { - klog.ErrorS(nil, "Unsupported client type for revoking bucket access", "bucketName", bucketName, "userName", userName) + klog.ErrorS(nil, "Unsupported client type for revoking bucket access", "bucketName", bucketName) return nil, status.Error(codes.Internal, "unsupported client type for IAM operations") } + klog.V(2).InfoS("Revoking bucket access", "bucketName", bucketName, "userName", userName) err = iamClient.RevokeBucketAccess(ctx, userName, bucketName) if err != nil { klog.ErrorS(err, "Failed to revoke bucket access", "bucketName", bucketName, "userName", userName) return nil, status.Error(codes.Internal, "failed to revoke bucket access") } - klog.V(3).InfoS("Successfully revoked bucket access", "bucketName", bucketName, "userName", userName) + klog.V(2).InfoS("Successfully revoked bucket access", "bucketName", bucketName, "userName", userName) return &cosiapi.DriverRevokeBucketAccessResponse{}, nil } func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Interface, parameters map[string]string, service string) (interface{}, *util.StorageClientParameters, error) { - klog.V(3).InfoS("Initializing object storage provider clients", "parameters", parameters) + klog.V(4).InfoS("Initializing object storage provider client", "service", service) ospSecretName, namespace, err := FetchSecretInformation(parameters) if err != nil { @@ -300,16 +298,17 @@ func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Int return nil, nil, err } - klog.V(4).InfoS("Fetching secret", "secretName", ospSecretName, "namespace", namespace) + klog.V(4).InfoS("Fetching secret data", "secretName", ospSecretName, "namespace", namespace) ospSecret, err := clientset.CoreV1().Secrets(namespace).Get(ctx, ospSecretName, metav1.GetOptions{}) if err != nil { - klog.ErrorS(err, "Failed to get object store user secret", "secretName", ospSecretName) + klog.ErrorS(err, "Failed to get object store user secret", "secretName", ospSecretName, "namespace", namespace) return nil, nil, status.Error(codes.Internal, "failed to get object store user secret") } + klog.V(4).InfoS("Successfully fetched object storage provider secret", "secretName", ospSecretName, "namespace", namespace) storageClientParameters, err := FetchParameters(ospSecret.Data) if err != nil { - klog.ErrorS(err, "Failed to fetch S3 parameters from secret", "secretName", ospSecretName) + klog.ErrorS(err, "Failed to fetch object storage provider parameters from secret", "secretName", ospSecretName) return nil, nil, err } @@ -321,14 +320,14 @@ func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Int klog.ErrorS(err, "Failed to initialize S3 client", "endpoint", storageClientParameters.Endpoint) return nil, nil, status.Error(codes.Internal, "failed to initialize S3 client") } - klog.V(3).InfoS("Successfully initialized S3 client", "endpoint", storageClientParameters.Endpoint) + klog.V(4).InfoS("Successfully initialized S3 client", "endpoint", storageClientParameters.Endpoint) case "IAM": client, err = iamclient.InitIAMClient(*storageClientParameters) if err != nil { klog.ErrorS(err, "Failed to initialize IAM client", "endpoint", storageClientParameters.Endpoint) return nil, nil, status.Error(codes.Internal, "failed to initialize IAM client") } - klog.V(3).InfoS("Successfully initialized IAM client", "endpoint", storageClientParameters.Endpoint) + klog.V(4).InfoS("Successfully initialized IAM client", "endpoint", storageClientParameters.Endpoint) default: klog.ErrorS(nil, "Unsupported object storage provider service", "service", service) return nil, nil, status.Error(codes.Internal, "unsupported object storage provider service") @@ -337,7 +336,7 @@ func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Int } func fetchObjectStorageProviderSecretInfo(parameters map[string]string) (string, string, error) { - klog.V(4).InfoS("Fetching object storage provider secret info", "parameters", parameters) + klog.V(4).InfoS("Validating object storage provider secret parameters", "parameters", parameters) secretName := parameters["objectStorageSecretName"] namespace := os.Getenv("POD_NAMESPACE") @@ -349,12 +348,12 @@ func fetchObjectStorageProviderSecretInfo(parameters map[string]string) (string, return "", "", status.Error(codes.InvalidArgument, "Object storage provider secret name and namespace are required") } - klog.V(4).InfoS("Object storage provider secret info fetched", "secretName", secretName, "namespace", namespace) + klog.V(4).InfoS("Successfully validated object storage provider secret parameters", "secretName", secretName, "namespace", namespace) return secretName, namespace, nil } func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParameters, error) { - klog.V(5).InfoS("Fetching S3 parameters from secret") + klog.V(5).InfoS("Extracting object storage parameters from secret") params := util.NewStorageClientParameters() @@ -366,11 +365,11 @@ func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParamet if cert, exists := secretData["tlsCert"]; exists { params.TLSCert = cert } else { - klog.V(5).InfoS("TLS certificate is not provided, proceeding without it") + klog.V(5).InfoS("TLS certificate not provided, proceeding without it") } if err := params.Validate(); err != nil { - klog.ErrorS(err, "invalid object storage parameters") + klog.ErrorS(err, "Invalid object storage parameters") return nil, err } @@ -379,6 +378,6 @@ func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParamet params.IAMEndpoint = string(value) klog.V(5).InfoS("IAM endpoint specified", "iamEndpoint", params.IAMEndpoint) } - + klog.V(5).InfoS("Successfully validated object storage parameters", "endpoint", params.Endpoint, "region", params.Region) return params, nil } From c8f8040e27c1cd2952e1c2aaee0a057b1cd49295 Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Tue, 17 Dec 2024 11:21:43 +0100 Subject: [PATCH 03/10] COSI-35: update-logging-for-s3-client --- pkg/clients/s3/s3_client.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/pkg/clients/s3/s3_client.go b/pkg/clients/s3/s3_client.go index 662b8ce8..692a77c4 100644 --- a/pkg/clients/s3/s3_client.go +++ b/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 { From 3c86f495cb6e14f09cd352b9dc769fba2bd98a78 Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Tue, 17 Dec 2024 12:12:04 +0100 Subject: [PATCH 04/10] COSI-35 update-logging-for-iam-client --- pkg/clients/iam/iam_client.go | 58 ++++++++++++++--------------------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/pkg/clients/iam/iam_client.go b/pkg/clients/iam/iam_client.go index ae1c7959..78e7f7da 100644 --- a/pkg/clients/iam/iam_client.go +++ b/pkg/clients/iam/iam_client.go @@ -48,6 +48,7 @@ var InitIAMClient = func(params util.StorageClientParameters) (*IAMClient, error } if strings.HasPrefix(params.IAMEndpoint, "https://") { + klog.V(4).InfoS("Configuring TLS transport for IAM client", "IAMEndpoint", params.IAMEndpoint) httpClient.Transport = util.ConfigureTLSTransport(params.TLSCert) } @@ -60,7 +61,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 +80,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 +106,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 +116,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 +125,19 @@ func (client *IAMClient) CreateBucketAccess(ctx context.Context, userName, bucke if err != nil { return nil, err } + klog.V(2).InfoS("Successfully created IAM user", "userName", userName) err = client.AttachS3WildcardInlinePolicy(ctx, userName, bucketName) if err != nil { return nil, err } + klog.V(2).InfoS("Successfully attached inline policy", "userName", userName, "policyName", bucketName) accessKeyOutput, err := client.CreateAccessKey(ctx, userName) if err != nil { return nil, err } + klog.V(2).InfoS("Successfully created access key", "userName", userName) return accessKeyOutput, nil } @@ -159,32 +148,31 @@ func (client *IAMClient) RevokeBucketAccess(ctx context.Context, userName, bucke if err != nil { return err } + klog.V(2).InfoS("Verified IAM user exists", "userName", userName) err = client.DeleteInlinePolicy(ctx, userName, bucketName) if err != nil { return err } + klog.V(2).InfoS("Deleted inline policy if it existed", "userName", userName, "policyName", bucketName) err = client.DeleteAllAccessKeys(ctx, userName) if err != nil { return err } + klog.V(2).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(2).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 +183,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(4).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(4).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(5).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(5).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(5).InfoS("Successfully deleted access key", "userName", userName, "accessKeyId", *key.AccessKeyId) } - klog.InfoS("Successfully deleted all access keys", "user", userName) + klog.V(4).InfoS("Successfully deleted all access keys", "userName", userName) return nil } @@ -236,8 +225,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 } From fdc75118cebefe53cd7f70b422bb1ce2d7553b9b Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Tue, 17 Dec 2024 12:21:58 +0100 Subject: [PATCH 05/10] COSI-35: remove-log-checks-from-unit-tests --- pkg/clients/iam/iam_client_test.go | 5 ----- pkg/clients/s3/s3_client_test.go | 1 - 2 files changed, 6 deletions(-) diff --git a/pkg/clients/iam/iam_client_test.go b/pkg/clients/iam/iam_client_test.go index bcd35c5e..a12786ab 100644 --- a/pkg/clients/iam/iam_client_test.go +++ b/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()) }) diff --git a/pkg/clients/s3/s3_client_test.go b/pkg/clients/s3/s3_client_test.go index 68e2301c..01c5fe04 100644 --- a/pkg/clients/s3/s3_client_test.go +++ b/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()) }) From d6f1719ee670fa9670b6eb24eb87cbc414db7c94 Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Tue, 17 Dec 2024 13:01:28 +0100 Subject: [PATCH 06/10] COSI-35: changed-default-production-logging-level --- helm/scality-cosi-driver/values.yaml | 4 ++-- kustomize/overlays/production/scality-cosi-driver.properties | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/helm/scality-cosi-driver/values.yaml b/helm/scality-cosi-driver/values.yaml index 787e703d..a9b96751 100644 --- a/helm/scality-cosi-driver/values.yaml +++ b/helm/scality-cosi-driver/values.yaml @@ -8,8 +8,8 @@ replicaCount: 1 logLevels: - driver: "5" - sidecar: "5" + driver: "2" + sidecar: "2" namespace: scality-object-storage diff --git a/kustomize/overlays/production/scality-cosi-driver.properties b/kustomize/overlays/production/scality-cosi-driver.properties index 58af9b72..8b0d5889 100644 --- a/kustomize/overlays/production/scality-cosi-driver.properties +++ b/kustomize/overlays/production/scality-cosi-driver.properties @@ -1,2 +1,2 @@ -COSI_DRIVER_LOG_LEVEL=5 -OBJECTSTORAGE_PROVISIONER_SIDECAR_LOG_LEVEL=5 +COSI_DRIVER_LOG_LEVEL=2 +OBJECTSTORAGE_PROVISIONER_SIDECAR_LOG_LEVEL=2 From 7ac6369b2cb6cca50475ab069c0918232e078e2f Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Tue, 17 Dec 2024 14:25:29 +0100 Subject: [PATCH 07/10] COSI-35: switch-log-level-to-contants Adds a constants package which will be expanded, hence a test suite has been also added --- pkg/clients/iam/iam_client.go | 29 +++++++------- pkg/constants/constants.go | 12 ++++++ pkg/constants/constants_test.go | 26 ++++++++++++ pkg/driver/provisioner_server_impl.go | 57 ++++++++++++++------------- pkg/util/storage_client.go | 3 +- 5 files changed, 84 insertions(+), 43 deletions(-) create mode 100644 pkg/constants/constants.go create mode 100644 pkg/constants/constants_test.go diff --git a/pkg/clients/iam/iam_client.go b/pkg/clients/iam/iam_client.go index 78e7f7da..7de57348 100644 --- a/pkg/clients/iam/iam_client.go +++ b/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,7 +49,7 @@ var InitIAMClient = func(params util.StorageClientParameters) (*IAMClient, error } if strings.HasPrefix(params.IAMEndpoint, "https://") { - klog.V(4).InfoS("Configuring TLS transport for IAM client", "IAMEndpoint", params.IAMEndpoint) + klog.V(c.LvlDebug).InfoS("Configuring TLS transport for IAM client", "IAMEndpoint", params.IAMEndpoint) httpClient.Transport = util.ConfigureTLSTransport(params.TLSCert) } @@ -125,19 +126,19 @@ func (client *IAMClient) CreateBucketAccess(ctx context.Context, userName, bucke if err != nil { return nil, err } - klog.V(2).InfoS("Successfully created IAM user", "userName", userName) + 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(2).InfoS("Successfully attached inline policy", "userName", userName, "policyName", bucketName) + 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(2).InfoS("Successfully created access key", "userName", userName) + klog.V(c.LvlInfo).InfoS("Successfully created access key", "userName", userName) return accessKeyOutput, nil } @@ -148,25 +149,25 @@ func (client *IAMClient) RevokeBucketAccess(ctx context.Context, userName, bucke if err != nil { return err } - klog.V(2).InfoS("Verified IAM user exists", "userName", userName) + klog.V(c.LvlInfo).InfoS("Verified IAM user exists", "userName", userName) err = client.DeleteInlinePolicy(ctx, userName, bucketName) if err != nil { return err } - klog.V(2).InfoS("Deleted inline policy if it existed", "userName", userName, "policyName", bucketName) + 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(2).InfoS("Deleted all access keys if any existed", "userName", userName) + 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.V(2).InfoS("Deleted IAM user", "userName", userName) + klog.V(c.LvlInfo).InfoS("Deleted IAM user", "userName", userName) return nil } @@ -183,12 +184,12 @@ func (client *IAMClient) DeleteInlinePolicy(ctx context.Context, userName, bucke if err != nil { var noSuchEntityErr *types.NoSuchEntityException if errors.As(err, &noSuchEntityErr) { - klog.V(4).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 err } - klog.V(4).InfoS("Successfully deleted inline policy", "userName", userName, "policyName", bucketName) + klog.V(c.LvlDebug).InfoS("Successfully deleted inline policy", "userName", userName, "policyName", bucketName) return nil } @@ -199,21 +200,21 @@ func (client *IAMClient) DeleteAllAccessKeys(ctx context.Context, userName strin } var noSuchEntityErr *types.NoSuchEntityException for _, key := range listKeysOutput.AccessKeyMetadata { - klog.V(5).InfoS("Deleting access key", "userName", userName, "accessKeyId", *key.AccessKeyId) + 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", "userName", userName, "accessKeyId", *key.AccessKeyId) + klog.V(c.LvlTrace).InfoS("Access key does not exist, skipping deletion", "userName", userName, "accessKeyId", *key.AccessKeyId) continue } return err } - klog.V(5).InfoS("Successfully deleted access key", "userName", userName, "accessKeyId", *key.AccessKeyId) + klog.V(c.LvlTrace).InfoS("Successfully deleted access key", "userName", userName, "accessKeyId", *key.AccessKeyId) } - klog.V(4).InfoS("Successfully deleted all access keys", "userName", userName) + klog.V(c.LvlDebug).InfoS("Successfully deleted all access keys", "userName", userName) return nil } diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go new file mode 100644 index 00000000..f0356062 --- /dev/null +++ b/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 +) diff --git a/pkg/constants/constants_test.go b/pkg/constants/constants_test.go new file mode 100644 index 00000000..926641bf --- /dev/null +++ b/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") + }) + }) +}) diff --git a/pkg/driver/provisioner_server_impl.go b/pkg/driver/provisioner_server_impl.go index 6e13b995..8ab28bb6 100644 --- a/pkg/driver/provisioner_server_impl.go +++ b/pkg/driver/provisioner_server_impl.go @@ -23,6 +23,7 @@ import ( s3types "github.com/aws/aws-sdk-go-v2/service/s3/types" iamclient "github.com/scality/cosi-driver/pkg/clients/iam" s3client "github.com/scality/cosi-driver/pkg/clients/s3" + c "github.com/scality/cosi-driver/pkg/constants" "github.com/scality/cosi-driver/pkg/util" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -63,7 +64,7 @@ func InitProvisionerServer(provisioner string) (cosiapi.ProvisionerServer, error klog.ErrorS(err, "Failed to initialize ProvisionerServer: empty provisioner name") return nil, err } - klog.V(3).InfoS("Initializing ProvisionerServer", "provisioner", provisioner) + klog.V(c.LvlEvent).InfoS("Initializing ProvisionerServer", "provisioner", provisioner) kubeConfig, err := InClusterConfig() if err != nil { @@ -83,7 +84,7 @@ func InitProvisionerServer(provisioner string) (cosiapi.ProvisionerServer, error return nil, err } - klog.V(3).InfoS("Successfully initialized ProvisionerServer", "provisioner", provisioner) + klog.V(c.LvlEvent).InfoS("Successfully initialized ProvisionerServer", "provisioner", provisioner) return &ProvisionerServer{ Provisioner: provisioner, Clientset: clientset, @@ -109,7 +110,7 @@ func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context, parameters := req.GetParameters() service := "S3" - klog.V(2).InfoS("Processing DriverCreateBucket request", "bucketName", bucketName) + klog.V(c.LvlInfo).InfoS("Processing DriverCreateBucket request", "bucketName", bucketName) client, s3Params, err := InitializeClient(ctx, s.Clientset, parameters, service) if err != nil { @@ -123,17 +124,17 @@ func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context, return nil, status.Error(codes.InvalidArgument, "unsupported client type for bucket creation") } - klog.V(3).InfoS("Creating bucket", "bucketName", bucketName) + klog.V(c.LvlDebug).InfoS("Creating bucket", "bucketName", bucketName) err = s3Client.CreateBucket(ctx, bucketName, *s3Params) if err != nil { var bucketAlreadyExists *s3types.BucketAlreadyExists var bucketOwnedByYou *s3types.BucketAlreadyOwnedByYou if errors.As(err, &bucketAlreadyExists) { - klog.V(2).InfoS("Bucket already exists", "bucketName", bucketName) + klog.V(c.LvlInfo).InfoS("Bucket already exists", "bucketName", bucketName) return nil, status.Errorf(codes.AlreadyExists, "Bucket already exists: %s", bucketName) } else if errors.As(err, &bucketOwnedByYou) { - klog.V(2).InfoS("Bucket already exists and is owned by you", "bucketName", bucketName) + klog.V(c.LvlInfo).InfoS("Bucket already exists and is owned by you", "bucketName", bucketName) return &cosiapi.DriverCreateBucketResponse{ BucketId: bucketName, }, nil @@ -142,7 +143,7 @@ func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context, return nil, status.Error(codes.Internal, "Failed to create bucket") } } - klog.V(2).InfoS("Successfully created bucket", "bucketName", bucketName) + klog.V(c.LvlInfo).InfoS("Successfully created bucket", "bucketName", bucketName) return &cosiapi.DriverCreateBucketResponse{ BucketId: bucketName, }, nil @@ -161,13 +162,13 @@ func (s *ProvisionerServer) DriverDeleteBucket(ctx context.Context, bucketName := req.GetBucketId() - klog.V(2).InfoS("Processing DriverDeleteBucket request", "bucketName", bucketName) + klog.V(c.LvlInfo).InfoS("Processing DriverDeleteBucket request", "bucketName", bucketName) bucket, err := s.BucketClientset.ObjectstorageV1alpha1().Buckets().Get(ctx, bucketName, metav1.GetOptions{}) if err != nil { klog.ErrorS(err, "Failed to fetch bucket object", "bucketName", bucketName) return nil, status.Error(codes.Internal, "failed to get bucket object from kubernetes") } - klog.V(5).InfoS("Successfully fetched Bucket object", "bucketName", bucket.Name, "parameters", bucket.Spec.Parameters) + klog.V(c.LvlTrace).InfoS("Successfully fetched Bucket object", "bucketName", bucket.Name, "parameters", bucket.Spec.Parameters) client, _, err := InitializeClient(ctx, s.Clientset, bucket.Spec.Parameters, "S3") if err != nil { @@ -187,7 +188,7 @@ func (s *ProvisionerServer) DriverDeleteBucket(ctx context.Context, return nil, status.Error(codes.Internal, "failed to delete bucket") } - klog.V(2).InfoS("Successfully deleted bucket", "bucketName", bucketName) + klog.V(c.LvlInfo).InfoS("Successfully deleted bucket", "bucketName", bucketName) return &cosiapi.DriverDeleteBucketResponse{}, nil } @@ -204,7 +205,7 @@ func (s *ProvisionerServer) DriverGrantBucketAccess(ctx context.Context, userName := req.GetName() parameters := req.GetParameters() - klog.V(2).InfoS("Processing DriverGrantBucketAccess request", "bucketName", bucketName, "userName", userName) + klog.V(c.LvlInfo).InfoS("Processing DriverGrantBucketAccess request", "bucketName", bucketName, "userName", userName) client, iamParams, err := InitializeClient(ctx, s.Clientset, parameters, "IAM") @@ -219,14 +220,14 @@ func (s *ProvisionerServer) DriverGrantBucketAccess(ctx context.Context, return nil, status.Error(codes.Internal, "failed to initialize object storage provider IAM client") } - klog.V(2).InfoS("Granting bucket access", "bucketName", bucketName, "userName", userName) + klog.V(c.LvlInfo).InfoS("Granting bucket access", "bucketName", bucketName, "userName", userName) userInfo, err := iamClient.CreateBucketAccess(ctx, userName, bucketName) if err != nil { klog.ErrorS(err, "Failed to create bucket access", "bucketName", bucketName, "userName", userName) return nil, status.Error(codes.Internal, "failed to create bucket access") } - klog.V(2).InfoS("Successfully granted bucket access", "bucketName", bucketName, "userName", userName) + klog.V(c.LvlInfo).InfoS("Successfully granted bucket access", "bucketName", bucketName, "userName", userName) return &cosiapi.DriverGrantBucketAccessResponse{ AccountId: userName, Credentials: map[string]*cosiapi.CredentialDetails{ @@ -256,7 +257,7 @@ func (s *ProvisionerServer) DriverRevokeBucketAccess(ctx context.Context, bucketName := req.GetBucketId() userName := req.GetAccountId() - klog.V(2).InfoS("Processing DriverRevokeBucketAccess request", "bucketName", bucketName, "userName", userName) + klog.V(c.LvlInfo).InfoS("Processing DriverRevokeBucketAccess request", "bucketName", bucketName, "userName", userName) // Fetch the bucket to retrieve parameters bucket, err := s.BucketClientset.ObjectstorageV1alpha1().Buckets().Get(ctx, bucketName, metav1.GetOptions{}) @@ -264,7 +265,7 @@ func (s *ProvisionerServer) DriverRevokeBucketAccess(ctx context.Context, klog.ErrorS(err, "Failed to fetch bucket object", "bucketName", bucketName) return nil, status.Error(codes.Internal, "failed to get bucket object from kubernetes") } - klog.V(5).InfoS("Successfully fetched Bucket object", "bucketName", bucket.Name, "parameters", bucket.Spec.Parameters) + klog.V(c.LvlTrace).InfoS("Successfully fetched Bucket object", "bucketName", bucket.Name, "parameters", bucket.Spec.Parameters) client, _, err := InitializeClient(ctx, s.Clientset, bucket.Spec.Parameters, "IAM") if err != nil { @@ -278,19 +279,19 @@ func (s *ProvisionerServer) DriverRevokeBucketAccess(ctx context.Context, return nil, status.Error(codes.Internal, "unsupported client type for IAM operations") } - klog.V(2).InfoS("Revoking bucket access", "bucketName", bucketName, "userName", userName) + klog.V(c.LvlInfo).InfoS("Revoking bucket access", "bucketName", bucketName, "userName", userName) err = iamClient.RevokeBucketAccess(ctx, userName, bucketName) if err != nil { klog.ErrorS(err, "Failed to revoke bucket access", "bucketName", bucketName, "userName", userName) return nil, status.Error(codes.Internal, "failed to revoke bucket access") } - klog.V(2).InfoS("Successfully revoked bucket access", "bucketName", bucketName, "userName", userName) + klog.V(c.LvlInfo).InfoS("Successfully revoked bucket access", "bucketName", bucketName, "userName", userName) return &cosiapi.DriverRevokeBucketAccessResponse{}, nil } func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Interface, parameters map[string]string, service string) (interface{}, *util.StorageClientParameters, error) { - klog.V(4).InfoS("Initializing object storage provider client", "service", service) + klog.V(c.LvlDebug).InfoS("Initializing object storage provider client", "service", service) ospSecretName, namespace, err := FetchSecretInformation(parameters) if err != nil { @@ -298,13 +299,13 @@ func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Int return nil, nil, err } - klog.V(4).InfoS("Fetching secret data", "secretName", ospSecretName, "namespace", namespace) + klog.V(c.LvlDebug).InfoS("Fetching secret data", "secretName", ospSecretName, "namespace", namespace) ospSecret, err := clientset.CoreV1().Secrets(namespace).Get(ctx, ospSecretName, metav1.GetOptions{}) if err != nil { klog.ErrorS(err, "Failed to get object store user secret", "secretName", ospSecretName, "namespace", namespace) return nil, nil, status.Error(codes.Internal, "failed to get object store user secret") } - klog.V(4).InfoS("Successfully fetched object storage provider secret", "secretName", ospSecretName, "namespace", namespace) + klog.V(c.LvlDebug).InfoS("Successfully fetched object storage provider secret", "secretName", ospSecretName, "namespace", namespace) storageClientParameters, err := FetchParameters(ospSecret.Data) if err != nil { @@ -320,14 +321,14 @@ func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Int klog.ErrorS(err, "Failed to initialize S3 client", "endpoint", storageClientParameters.Endpoint) return nil, nil, status.Error(codes.Internal, "failed to initialize S3 client") } - klog.V(4).InfoS("Successfully initialized S3 client", "endpoint", storageClientParameters.Endpoint) + klog.V(c.LvlDebug).InfoS("Successfully initialized S3 client", "endpoint", storageClientParameters.Endpoint) case "IAM": client, err = iamclient.InitIAMClient(*storageClientParameters) if err != nil { klog.ErrorS(err, "Failed to initialize IAM client", "endpoint", storageClientParameters.Endpoint) return nil, nil, status.Error(codes.Internal, "failed to initialize IAM client") } - klog.V(4).InfoS("Successfully initialized IAM client", "endpoint", storageClientParameters.Endpoint) + klog.V(c.LvlDebug).InfoS("Successfully initialized IAM client", "endpoint", storageClientParameters.Endpoint) default: klog.ErrorS(nil, "Unsupported object storage provider service", "service", service) return nil, nil, status.Error(codes.Internal, "unsupported object storage provider service") @@ -336,7 +337,7 @@ func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Int } func fetchObjectStorageProviderSecretInfo(parameters map[string]string) (string, string, error) { - klog.V(4).InfoS("Validating object storage provider secret parameters", "parameters", parameters) + klog.V(c.LvlDebug).InfoS("Validating object storage provider secret parameters", "parameters", parameters) secretName := parameters["objectStorageSecretName"] namespace := os.Getenv("POD_NAMESPACE") @@ -348,12 +349,12 @@ func fetchObjectStorageProviderSecretInfo(parameters map[string]string) (string, return "", "", status.Error(codes.InvalidArgument, "Object storage provider secret name and namespace are required") } - klog.V(4).InfoS("Successfully validated object storage provider secret parameters", "secretName", secretName, "namespace", namespace) + klog.V(c.LvlDebug).InfoS("Successfully validated object storage provider secret parameters", "secretName", secretName, "namespace", namespace) return secretName, namespace, nil } func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParameters, error) { - klog.V(5).InfoS("Extracting object storage parameters from secret") + klog.V(c.LvlTrace).InfoS("Extracting object storage parameters from secret") params := util.NewStorageClientParameters() @@ -365,7 +366,7 @@ func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParamet if cert, exists := secretData["tlsCert"]; exists { params.TLSCert = cert } else { - klog.V(5).InfoS("TLS certificate not provided, proceeding without it") + klog.V(c.LvlTrace).InfoS("TLS certificate not provided, proceeding without it") } if err := params.Validate(); err != nil { @@ -376,8 +377,8 @@ func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParamet params.IAMEndpoint = params.Endpoint if value, exists := secretData["iamEndpoint"]; exists && len(value) > 0 { params.IAMEndpoint = string(value) - klog.V(5).InfoS("IAM endpoint specified", "iamEndpoint", params.IAMEndpoint) + klog.V(c.LvlTrace).InfoS("IAM endpoint specified", "iamEndpoint", params.IAMEndpoint) } - klog.V(5).InfoS("Successfully validated object storage parameters", "endpoint", params.Endpoint, "region", params.Region) + klog.V(c.LvlTrace).InfoS("Successfully validated object storage parameters", "endpoint", params.Endpoint, "region", params.Region) return params, nil } diff --git a/pkg/util/storage_client.go b/pkg/util/storage_client.go index 831c7a0e..28c05820 100644 --- a/pkg/util/storage_client.go +++ b/pkg/util/storage_client.go @@ -6,6 +6,7 @@ import ( "net/http" "time" + c "github.com/scality/cosi-driver/pkg/constants" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "k8s.io/klog/v2" @@ -62,7 +63,7 @@ func ConfigureTLSTransport(certData []byte) *http.Transport { } tlsSettings.RootCAs = caCertPool } else { - klog.V(4).Info("No certificate data provided; skipping TLS verification") + klog.V(c.LvlDebug).Info("No certificate data provided; skipping TLS verification") tlsSettings.InsecureSkipVerify = true } From 02ab5ac14446c4f4ec488c8d9ddf44ed75aff02d Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Tue, 17 Dec 2024 14:46:04 +0100 Subject: [PATCH 08/10] COSI-35: added-constants-package-to-codecov --- codecov.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/codecov.yml b/codecov.yml index e3997375..3a364536 100644 --- a/codecov.yml +++ b/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 From d8a7b3fd1d823c423eed003e49ae314f672f78b0 Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Wed, 18 Dec 2024 12:31:16 +0100 Subject: [PATCH 09/10] COSI-35: add-comments-for-log-levels --- helm/scality-cosi-driver/values.yaml | 8 +++++++- kustomize/overlays/debug/scality-cosi-driver.properties | 7 +++++++ kustomize/overlays/dev/scality-cosi-driver.properties | 7 +++++++ .../overlays/production/scality-cosi-driver.properties | 7 +++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/helm/scality-cosi-driver/values.yaml b/helm/scality-cosi-driver/values.yaml index a9b96751..492ad115 100644 --- a/helm/scality-cosi-driver/values.yaml +++ b/helm/scality-cosi-driver/values.yaml @@ -6,7 +6,13 @@ 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: "2" sidecar: "2" diff --git a/kustomize/overlays/debug/scality-cosi-driver.properties b/kustomize/overlays/debug/scality-cosi-driver.properties index 58af9b72..17c1ec47 100644 --- a/kustomize/overlays/debug/scality-cosi-driver.properties +++ b/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 diff --git a/kustomize/overlays/dev/scality-cosi-driver.properties b/kustomize/overlays/dev/scality-cosi-driver.properties index 58af9b72..17c1ec47 100644 --- a/kustomize/overlays/dev/scality-cosi-driver.properties +++ b/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 diff --git a/kustomize/overlays/production/scality-cosi-driver.properties b/kustomize/overlays/production/scality-cosi-driver.properties index 8b0d5889..28aa2ff3 100644 --- a/kustomize/overlays/production/scality-cosi-driver.properties +++ b/kustomize/overlays/production/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=2 OBJECTSTORAGE_PROVISIONER_SIDECAR_LOG_LEVEL=2 From 535757d049e725d0232e4652efa735c4fa0d34c2 Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Wed, 18 Dec 2024 12:34:00 +0100 Subject: [PATCH 10/10] COSI-35: add-request-to-trace-logs --- pkg/driver/provisioner_server_impl.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/driver/provisioner_server_impl.go b/pkg/driver/provisioner_server_impl.go index 8ab28bb6..1b8f0ed9 100644 --- a/pkg/driver/provisioner_server_impl.go +++ b/pkg/driver/provisioner_server_impl.go @@ -106,6 +106,7 @@ func InitProvisionerServer(provisioner string) (cosiapi.ProvisionerServer, error // non-nil err - Internal error [requeue'd with exponential backoff] func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context, req *cosiapi.DriverCreateBucketRequest) (*cosiapi.DriverCreateBucketResponse, error) { + klog.V(c.LvlTrace).InfoS("DriverCreateBucket request received", "request", req) bucketName := req.GetName() parameters := req.GetParameters() service := "S3" @@ -159,6 +160,7 @@ func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context, // non-nil err - Internal error [requeue'd with exponential backoff] func (s *ProvisionerServer) DriverDeleteBucket(ctx context.Context, req *cosiapi.DriverDeleteBucketRequest) (*cosiapi.DriverDeleteBucketResponse, error) { + klog.V(c.LvlTrace).InfoS("DriverDeleteBucket request received", "request", req) bucketName := req.GetBucketId() @@ -201,6 +203,8 @@ func (s *ProvisionerServer) DriverDeleteBucket(ctx context.Context, // non-nil err - Internal error [requeue'd with exponential backoff] func (s *ProvisionerServer) DriverGrantBucketAccess(ctx context.Context, req *cosiapi.DriverGrantBucketAccessRequest) (*cosiapi.DriverGrantBucketAccessResponse, error) { + klog.V(c.LvlTrace).InfoS("DriverGrantBucketAccess request received", "request", req) + bucketName := req.GetBucketId() userName := req.GetName() parameters := req.GetParameters() @@ -253,6 +257,7 @@ func (s *ProvisionerServer) DriverGrantBucketAccess(ctx context.Context, // non-nil err - Internal error [requeue'd with exponential backoff] func (s *ProvisionerServer) DriverRevokeBucketAccess(ctx context.Context, req *cosiapi.DriverRevokeBucketAccessRequest) (*cosiapi.DriverRevokeBucketAccessResponse, error) { + klog.V(c.LvlTrace).InfoS("DriverRevokeBucketAccess request received", "request", req) bucketName := req.GetBucketId() userName := req.GetAccountId()