Merge pull request #188 from scality/bugfix/COSI-86-resolve-infonite-retry-loops

COSI-86: resolve infinite retry loops

Author: anurag4DSB

.gitignore

@@ -8,3 +8,4 @@ bin
 .idea
 vendor
 coverage.txt
+coverage.out

codecov.yml

@@ -52,6 +52,10 @@ component_management:
       name: 🔖 Constants Package
       paths:
         - pkg/constants/**
+    - component_id: osperrors-package
+      name: 🚨 Object Storage Provider Errors Package
+      paths:
+        - pkg/osperrors/**
 
 flag_management:
   default_rules: # the rules that will be followed for any flag added, generally

pkg/constants/constants.go

@@ -10,3 +10,12 @@ const (
 	LvlDebug              // 4 - Debug-level logs, tricky logic areas
 	LvlTrace              // 5 - Trace-level logs, detailed troubleshooting context
 )
+
+// Action constants for error translation context
+// These align with the driver operations and underlying API calls
+const (
+	ActionCreateBucket       = "CreateBucket"
+	ActionDeleteBucket       = "DeleteBucket"
+	ActionGrantBucketAccess  = "GrantBucketAccess"
+	ActionRevokeBucketAccess = "RevokeBucketAccess"
+)

pkg/driver/provisioner_server_impl.go

@@ -20,10 +20,10 @@ import (
 	"errors"
 	"os"
 
-	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"
+	constants "github.com/scality/cosi-driver/pkg/constants"
+	"github.com/scality/cosi-driver/pkg/osperrors"
 	"github.com/scality/cosi-driver/pkg/util"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -64,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(c.LvlEvent).InfoS("Initializing ProvisionerServer", "provisioner", provisioner)
+	klog.V(constants.LvlEvent).InfoS("Initializing ProvisionerServer", "provisioner", provisioner)
 
 	kubeConfig, err := InClusterConfig()
 	if err != nil {
@@ -84,7 +84,7 @@ func InitProvisionerServer(provisioner string) (cosiapi.ProvisionerServer, error
 		return nil, err
 	}
 
-	klog.V(c.LvlEvent).InfoS("Successfully initialized ProvisionerServer", "provisioner", provisioner)
+	klog.V(constants.LvlEvent).InfoS("Successfully initialized ProvisionerServer", "provisioner", provisioner)
 	return &ProvisionerServer{
 		Provisioner:     provisioner,
 		Clientset:       clientset,
@@ -106,12 +106,12 @@ 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)
+	klog.V(constants.LvlTrace).InfoS("DriverCreateBucket request received", "request", req)
 	bucketName := req.GetName()
 	parameters := req.GetParameters()
 	service := "S3"
 
-	klog.V(c.LvlInfo).InfoS("Processing DriverCreateBucket request", "bucketName", bucketName)
+	klog.V(constants.LvlInfo).InfoS("Processing DriverCreateBucket request", "bucketName", bucketName)
 
 	client, s3Params, err := InitializeClient(ctx, s.Clientset, parameters, service)
 	if err != nil {
@@ -125,20 +125,14 @@ func (s *ProvisionerServer) DriverCreateBucket(ctx context.Context,
 		return nil, status.Error(codes.InvalidArgument, "unsupported client type for bucket creation")
 	}
 
-	klog.V(c.LvlDebug).InfoS("Creating bucket", "bucketName", bucketName)
+	klog.V(constants.LvlDebug).InfoS("Creating bucket", "bucketName", bucketName)
 	err = s3Client.CreateBucket(ctx, bucketName, *s3Params)
 	if err != nil {
-		var bucketAlreadyExists *s3types.BucketAlreadyExists
-
-		if errors.As(err, &bucketAlreadyExists) {
-			klog.V(c.LvlInfo).InfoS("Bucket already exists", "bucketName", bucketName)
-			return nil, status.Errorf(codes.AlreadyExists, "Bucket already exists: %s", bucketName)
-		} else {
-			klog.ErrorS(err, "Failed to create bucket", "bucketName", bucketName)
-			return nil, status.Error(codes.Internal, "Failed to create bucket")
+		if translatedErr := osperrors.TranslateS3Error(constants.ActionCreateBucket, bucketName, err); translatedErr != nil {
+			return nil, translatedErr
 		}
 	}
-	klog.V(c.LvlInfo).InfoS("Successfully created bucket", "bucketName", bucketName)
+	klog.V(constants.LvlInfo).InfoS("Successfully created bucket", "bucketName", bucketName)
 	return &cosiapi.DriverCreateBucketResponse{
 		BucketId: bucketName,
 	}, nil
@@ -154,17 +148,17 @@ 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)
+	klog.V(constants.LvlTrace).InfoS("DriverDeleteBucket request received", "request", req)
 
 	bucketName := req.GetBucketId()
 
-	klog.V(c.LvlInfo).InfoS("Processing DriverDeleteBucket request", "bucketName", bucketName)
+	klog.V(constants.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(c.LvlTrace).InfoS("Successfully fetched Bucket object", "bucketName", bucket.Name, "parameters", bucket.Spec.Parameters)
+	klog.V(constants.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 {
@@ -180,11 +174,12 @@ func (s *ProvisionerServer) DriverDeleteBucket(ctx context.Context,
 
 	err = s3Client.DeleteBucket(ctx, bucketName)
 	if err != nil {
-		klog.ErrorS(err, "Failed to delete bucket", "bucketName", bucketName)
-		return nil, status.Error(codes.Internal, "failed to delete bucket")
+		if translatedErr := osperrors.TranslateS3Error(constants.ActionDeleteBucket, bucketName, err); translatedErr != nil {
+			return nil, translatedErr
+		}
 	}
 
-	klog.V(c.LvlInfo).InfoS("Successfully deleted bucket", "bucketName", bucketName)
+	klog.V(constants.LvlInfo).InfoS("Successfully deleted bucket", "bucketName", bucketName)
 	return &cosiapi.DriverDeleteBucketResponse{}, nil
 }
 
@@ -197,13 +192,13 @@ 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)
+	klog.V(constants.LvlTrace).InfoS("DriverGrantBucketAccess request received", "request", req)
 
 	bucketName := req.GetBucketId()
 	userName := req.GetName()
 	parameters := req.GetParameters()
 
-	klog.V(c.LvlInfo).InfoS("Processing DriverGrantBucketAccess request", "bucketName", bucketName, "userName", userName)
+	klog.V(constants.LvlInfo).InfoS("Processing DriverGrantBucketAccess request", "bucketName", bucketName, "userName", userName)
 
 	client, iamParams, err := InitializeClient(ctx, s.Clientset, parameters, "IAM")
 
@@ -218,14 +213,15 @@ func (s *ProvisionerServer) DriverGrantBucketAccess(ctx context.Context,
 		return nil, status.Error(codes.Internal, "failed to initialize object storage provider IAM client")
 	}
 
-	klog.V(c.LvlInfo).InfoS("Granting bucket access", "bucketName", bucketName, "userName", userName)
+	klog.V(constants.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")
+		if translatedErr := osperrors.TranslateIAMError(constants.ActionGrantBucketAccess, userName, err); translatedErr != nil {
+			return nil, translatedErr
+		}
 	}
 
-	klog.V(c.LvlInfo).InfoS("Successfully granted bucket access", "bucketName", bucketName, "userName", userName)
+	klog.V(constants.LvlInfo).InfoS("Successfully granted bucket access", "bucketName", bucketName, "userName", userName)
 	return &cosiapi.DriverGrantBucketAccessResponse{
 		AccountId: userName,
 		Credentials: map[string]*cosiapi.CredentialDetails{
@@ -251,20 +247,20 @@ 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)
+	klog.V(constants.LvlTrace).InfoS("DriverRevokeBucketAccess request received", "request", req)
 
 	bucketName := req.GetBucketId()
 	userName := req.GetAccountId()
 
-	klog.V(c.LvlInfo).InfoS("Processing DriverRevokeBucketAccess request", "bucketName", bucketName, "userName", userName)
+	klog.V(constants.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{})
 	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(c.LvlTrace).InfoS("Successfully fetched Bucket object", "bucketName", bucket.Name, "parameters", bucket.Spec.Parameters)
+	klog.V(constants.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,33 +274,34 @@ func (s *ProvisionerServer) DriverRevokeBucketAccess(ctx context.Context,
 		return nil, status.Error(codes.Internal, "unsupported client type for IAM operations")
 	}
 
-	klog.V(c.LvlInfo).InfoS("Revoking bucket access", "bucketName", bucketName, "userName", userName)
+	klog.V(constants.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")
+		if translatedErr := osperrors.TranslateIAMError(constants.ActionRevokeBucketAccess, userName, err); translatedErr != nil {
+			return nil, translatedErr
+		}
 	}
 
-	klog.V(c.LvlInfo).InfoS("Successfully revoked bucket access", "bucketName", bucketName, "userName", userName)
+	klog.V(constants.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(c.LvlDebug).InfoS("Initializing object storage provider client", "service", service)
+	klog.V(constants.LvlDebug).InfoS("Initializing object storage provider client", "service", service)
 
 	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(c.LvlDebug).InfoS("Fetching secret data", "secretName", ospSecretName, "namespace", namespace)
+	klog.V(constants.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(c.LvlDebug).InfoS("Successfully fetched object storage provider secret", "secretName", ospSecretName, "namespace", namespace)
+	klog.V(constants.LvlDebug).InfoS("Successfully fetched object storage provider secret", "secretName", ospSecretName, "namespace", namespace)
 
 	storageClientParameters, err := FetchParameters(ospSecret.Data)
 	if err != nil {
@@ -320,14 +317,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(c.LvlDebug).InfoS("Successfully initialized S3 client", "endpoint", storageClientParameters.Endpoint)
+		klog.V(constants.LvlDebug).InfoS("Successfully initialized S3 client", "endpoint", storageClientParameters.Endpoint)
 	case "IAM":
 		client, err = iamclient.InitIAMClient(ctx, *storageClientParameters)
 		if err != nil {
 			klog.ErrorS(err, "Failed to initialize IAM client", "endpoint", storageClientParameters.IAMEndpoint)
 			return nil, nil, status.Error(codes.Internal, "failed to initialize IAM client")
 		}
-		klog.V(c.LvlDebug).InfoS("Successfully initialized IAM client", "endpoint", storageClientParameters.IAMEndpoint)
+		klog.V(constants.LvlDebug).InfoS("Successfully initialized IAM client", "endpoint", storageClientParameters.IAMEndpoint)
 	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 +333,7 @@ func initializeObjectStorageClient(ctx context.Context, clientset kubernetes.Int
 }
 
 func fetchObjectStorageProviderSecretInfo(parameters map[string]string) (string, string, error) {
-	klog.V(c.LvlDebug).InfoS("Validating object storage provider secret parameters", "parameters", parameters)
+	klog.V(constants.LvlDebug).InfoS("Validating object storage provider secret parameters", "parameters", parameters)
 
 	secretName := parameters["objectStorageSecretName"]
 	namespace := os.Getenv("POD_NAMESPACE")
@@ -348,12 +345,12 @@ func fetchObjectStorageProviderSecretInfo(parameters map[string]string) (string,
 		return "", "", status.Error(codes.InvalidArgument, "Object storage provider secret name and namespace are required")
 	}
 
-	klog.V(c.LvlDebug).InfoS("Successfully validated object storage provider secret parameters", "secretName", secretName, "namespace", namespace)
+	klog.V(constants.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(c.LvlTrace).InfoS("Extracting object storage parameters from secret")
+	klog.V(constants.LvlTrace).InfoS("Extracting object storage parameters from secret")
 
 	params := util.NewStorageClientParameters()
 
@@ -365,7 +362,7 @@ func fetchS3Parameters(secretData map[string][]byte) (*util.StorageClientParamet
 	if cert, exists := secretData["tlsCert"]; exists {
 		params.TLSCert = cert
 	} else {
-		klog.V(c.LvlTrace).InfoS("TLS certificate not provided, proceeding without it")
+		klog.V(constants.LvlTrace).InfoS("TLS certificate not provided, proceeding without it")
 	}
 
 	if err := params.Validate(); err != nil {
@@ -376,8 +373,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(c.LvlTrace).InfoS("IAM endpoint specified", "iamEndpoint", params.IAMEndpoint)
+		klog.V(constants.LvlTrace).InfoS("IAM endpoint specified", "iamEndpoint", params.IAMEndpoint)
 	}
-	klog.V(c.LvlTrace).InfoS("Successfully validated object storage parameters", "endpoint", params.Endpoint, "region", params.Region)
+	klog.V(constants.LvlTrace).InfoS("Successfully validated object storage parameters", "endpoint", params.Endpoint, "region", params.Region)
 	return params, nil
 }