Sfoglia il codice sorgente

Log protocol used when interfacing with bucket storage (#3516)

Signed-off-by: thomasvn <thomasvn.dev@gmail.com>
Thomas Nguyen 4 mesi fa
parent
commit
e4a0915e0d

+ 10 - 5
core/pkg/storage/azurestorage.go

@@ -231,7 +231,7 @@ func (b *AzureStorage) Read(name string) ([]byte, error) {
 	name = trimLeading(name)
 	ctx := context.Background()
 
-	log.Debugf("AzureStorage::Read(%s)", name)
+	log.Debugf("AzureStorage::Read::HTTPS(%s)", name)
 
 	downloadResponse, err := b.containerClient.NewBlobClient(name).DownloadStream(ctx, nil)
 	if err != nil {
@@ -260,7 +260,7 @@ func (b *AzureStorage) Write(name string, data []byte) error {
 	name = trimLeading(name)
 	ctx := context.Background()
 
-	log.Debugf("AzureStorage::Write(%s)", name)
+	log.Debugf("AzureStorage::Write::HTTPS(%s)", name)
 
 	r := bytes.NewReader(data)
 	blobClient := b.containerClient.NewBlockBlobClient(name)
@@ -279,7 +279,7 @@ func (b *AzureStorage) Write(name string, data []byte) error {
 func (b *AzureStorage) Remove(name string) error {
 	name = trimLeading(name)
 
-	log.Debugf("AzureStorage::Remove(%s)", name)
+	log.Debugf("AzureStorage::Remove::HTTPS(%s)", name)
 	ctx := context.Background()
 
 	blobClient := b.containerClient.NewBlobClient(name)
@@ -312,7 +312,7 @@ func (b *AzureStorage) Exists(name string) (bool, error) {
 func (b *AzureStorage) List(path string) ([]*StorageInfo, error) {
 	path = trimLeading(path)
 
-	log.Debugf("AzureStorage::List(%s)", path)
+	log.Debugf("AzureStorage::List::HTTPS(%s)", path)
 	ctx := context.Background()
 
 	// Ensure the object name actually ends with a dir suffix. Otherwise we'll just iterate the
@@ -355,7 +355,7 @@ func (b *AzureStorage) List(path string) ([]*StorageInfo, error) {
 func (b *AzureStorage) ListDirectories(path string) ([]*StorageInfo, error) {
 	path = trimLeading(path)
 
-	log.Debugf("AzureStorage::ListDirectories(%s)", path)
+	log.Debugf("AzureStorage::ListDirectories::HTTPS(%s)", path)
 	ctx := context.Background()
 
 	// Ensure the object name actually ends with a dir suffix. Otherwise we'll just iterate the
@@ -433,6 +433,7 @@ func getContainerClient(conf AzureConfig) (*container.Client, error) {
 		if err != nil {
 			return nil, fmt.Errorf("error creating client from connection string: %w", err)
 		}
+		log.Debugf("AzureStorage: New Azure client initialized for container '%s' using connection string", conf.ContainerName)
 		return containerClient, nil
 	}
 
@@ -440,7 +441,11 @@ func getContainerClient(conf AzureConfig) (*container.Client, error) {
 		conf.Endpoint = "blob.core.windows.net"
 	}
 
+	// HTTPS Protocol Configuration: Azure Storage always uses HTTPS protocol.
+	// The containerURL is explicitly constructed with "https://" scheme.
+	// All Azure blob operations (read, write, delete, list) use this HTTPS URL.
 	containerURL := fmt.Sprintf("https://%s.%s/%s", conf.StorageAccountName, conf.Endpoint, conf.ContainerName)
+	log.Debugf("AzureStorage: New Azure client initialized with '%s'", containerURL)
 
 	// Use shared keys if set
 	if conf.StorageAccountKey != "" {

+ 17 - 0
core/pkg/storage/clusterstorage.go

@@ -95,6 +95,8 @@ func NewClusterStorageWith(config ClusterConfig) (*ClusterStorage, error) {
 		retry++
 	}
 
+	log.Debugf("ClusterStorage: New cluster storage client initialized with '%s://%s:%d'", cs.scheme(), config.Host, config.Port)
+
 	return cs, nil
 }
 
@@ -173,6 +175,7 @@ func (c *ClusterStorage) StorageType() StorageType {
 	return StorageTypeCluster
 }
 
+// scheme returns the protocol scheme (http or https) based on TLS configuration
 func (c *ClusterStorage) scheme() string {
 	if c.client.Transport != nil {
 		if transport, ok := c.client.Transport.(*http.Transport); ok {
@@ -218,6 +221,8 @@ type Response[T any] struct {
 }
 
 func (c *ClusterStorage) Stat(path string) (*StorageInfo, error) {
+	log.Debugf("ClusterStorage::Stat::%s(%s)", strings.ToUpper(c.scheme()), path)
+
 	var jsonResp Response[*StorageInfo]
 	fn := func(resp *http.Response) error {
 		err := json.NewDecoder(resp.Body).Decode(&jsonResp)
@@ -245,6 +250,8 @@ func (c *ClusterStorage) Stat(path string) (*StorageInfo, error) {
 }
 
 func (c *ClusterStorage) Read(path string) ([]byte, error) {
+	log.Debugf("ClusterStorage::Read::%s(%s)", strings.ToUpper(c.scheme()), path)
+
 	var jsonResp Response[[]byte]
 	fn := func(resp *http.Response) error {
 		err := json.NewDecoder(resp.Body).Decode(&jsonResp)
@@ -272,6 +279,8 @@ func (c *ClusterStorage) Read(path string) ([]byte, error) {
 }
 
 func (c *ClusterStorage) Write(path string, data []byte) error {
+	log.Debugf("ClusterStorage::Write::%s(%s)", strings.ToUpper(c.scheme()), path)
+
 	fn := func(resp *http.Response) error {
 		return nil
 	}
@@ -294,6 +303,8 @@ func (c *ClusterStorage) Write(path string, data []byte) error {
 }
 
 func (c *ClusterStorage) Remove(path string) error {
+	log.Debugf("ClusterStorage::Remove::%s(%s)", strings.ToUpper(c.scheme()), path)
+
 	fn := func(resp *http.Response) error {
 		return nil
 	}
@@ -316,6 +327,8 @@ func (c *ClusterStorage) Remove(path string) error {
 }
 
 func (c *ClusterStorage) Exists(path string) (bool, error) {
+	log.Debugf("ClusterStorage::Exists::%s(%s)", strings.ToUpper(c.scheme()), path)
+
 	var jsonResp Response[bool]
 	fn := func(resp *http.Response) error {
 		err := json.NewDecoder(resp.Body).Decode(&jsonResp)
@@ -343,6 +356,8 @@ func (c *ClusterStorage) Exists(path string) (bool, error) {
 }
 
 func (c *ClusterStorage) List(path string) ([]*StorageInfo, error) {
+	log.Debugf("ClusterStorage::List::%s(%s)", strings.ToUpper(c.scheme()), path)
+
 	var jsonResp Response[[]*StorageInfo]
 	fn := func(resp *http.Response) error {
 		err := json.NewDecoder(resp.Body).Decode(&jsonResp)
@@ -370,6 +385,8 @@ func (c *ClusterStorage) List(path string) ([]*StorageInfo, error) {
 }
 
 func (c *ClusterStorage) ListDirectories(path string) ([]*StorageInfo, error) {
+	log.Debugf("ClusterStorage::ListDirectories::%s(%s)", strings.ToUpper(c.scheme()), path)
+
 	var jsonResp Response[[]*StorageInfo]
 	fn := func(resp *http.Response) error {
 		err := json.NewDecoder(resp.Body).Decode(&jsonResp)

+ 65 - 0
core/pkg/storage/clusterstorage_test.go

@@ -0,0 +1,65 @@
+package storage
+
+import (
+	"crypto/tls"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+// TestClusterStorage_scheme tests the scheme() method returns correct values based on TLS configuration
+func TestClusterStorage_scheme(t *testing.T) {
+	tests := []struct {
+		name      string
+		transport http.RoundTripper
+		want      string
+	}{
+		{
+			name:      "nil transport returns http",
+			transport: nil,
+			want:      "http",
+		},
+		{
+			name:      "transport without TLS config returns http",
+			transport: &http.Transport{},
+			want:      "http",
+		},
+		{
+			name: "transport with TLS config returns https",
+			transport: &http.Transport{
+				TLSClientConfig: &tls.Config{},
+			},
+			want: "https",
+		},
+		{
+			name: "transport with InsecureSkipVerify returns http",
+			transport: &http.Transport{
+				TLSClientConfig: &tls.Config{
+					InsecureSkipVerify: true,
+				},
+			},
+			want: "http",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cs := &ClusterStorage{
+				client: &http.Client{
+					Transport: tt.transport,
+				},
+			}
+			got := cs.scheme()
+			if got != tt.want {
+				t.Errorf("ClusterStorage.scheme() = %v, want %v", got, tt.want)
+			}
+
+			// Also test that strings.ToUpper(scheme()) works as expected in log statements
+			gotUpper := strings.ToUpper(cs.scheme())
+			wantUpper := strings.ToUpper(tt.want)
+			if gotUpper != wantUpper {
+				t.Errorf("strings.ToUpper(ClusterStorage.scheme()) = %v, want %v", gotUpper, wantUpper)
+			}
+		})
+	}
+}

+ 9 - 5
core/pkg/storage/gcsstorage.go

@@ -59,11 +59,15 @@ func NewGCSStorageWith(gc GCSConfig) (*GCSStorage, error) {
 		opts = append(opts, option.WithCredentials(credentials))
 	}
 
+	// HTTPS Protocol Configuration: Google Cloud Storage client uses HTTPS by default.
+	// The GCS client library (cloud.google.com/go/storage) automatically uses HTTPS
+	// for all API calls. All GCS operations (read, write, delete, list) use HTTPS protocol.
 	gcsClient, err := gcs.NewClient(ctx, opts...)
 	if err != nil {
 		return nil, err
 	}
 
+	log.Debugf("GCSStorage: New GCS client initialized with 'https://storage.googleapis.com/%s'", gc.Bucket)
 	return &GCSStorage{
 		name:   gc.Bucket,
 		bucket: gcsClient.Bucket(gc.Bucket),
@@ -118,7 +122,7 @@ func (gs *GCSStorage) isDoesNotExist(err error) bool {
 // read the contents.
 func (gs *GCSStorage) Read(name string) ([]byte, error) {
 	name = trimLeading(name)
-	log.Debugf("GCSStorage::Read(%s)", name)
+	log.Debugf("GCSStorage::Read::HTTPS(%s)", name)
 
 	ctx := context.Background()
 	reader, err := gs.bucket.Object(name).NewReader(ctx)
@@ -138,7 +142,7 @@ func (gs *GCSStorage) Read(name string) ([]byte, error) {
 // to write a new file or overwrite an existing file.
 func (gs *GCSStorage) Write(name string, data []byte) error {
 	name = trimLeading(name)
-	log.Debugf("GCSStorage::Write(%s)", name)
+	log.Debugf("GCSStorage::Write::HTTPS(%s)", name)
 
 	ctx := context.Background()
 
@@ -167,7 +171,7 @@ func (gs *GCSStorage) Write(name string, data []byte) error {
 func (gs *GCSStorage) Remove(name string) error {
 	name = trimLeading(name)
 
-	log.Debugf("GCSStorage::Remove(%s)", name)
+	log.Debugf("GCSStorage::Remove::HTTPS(%s)", name)
 	ctx := context.Background()
 
 	return gs.bucket.Object(name).Delete(ctx)
@@ -196,7 +200,7 @@ func (gs *GCSStorage) Exists(name string) (bool, error) {
 func (gs *GCSStorage) List(path string) ([]*StorageInfo, error) {
 	path = trimLeading(path)
 
-	log.Debugf("GCSStorage::List(%s)", path)
+	log.Debugf("GCSStorage::List::HTTPS(%s)", path)
 	ctx := context.Background()
 
 	// Ensure the object name actually ends with a dir suffix. Otherwise we'll just iterate the
@@ -239,7 +243,7 @@ func (gs *GCSStorage) List(path string) ([]*StorageInfo, error) {
 func (gs *GCSStorage) ListDirectories(path string) ([]*StorageInfo, error) {
 	path = trimLeading(path)
 
-	log.Debugf("GCSStorage::ListDirectories(%s)", path)
+	log.Debugf("GCSStorage::ListDirectories::HTTPS(%s)", path)
 	ctx := context.Background()
 
 	// Ensure the object name actually ends with a dir suffix. Otherwise we'll just iterate the

+ 20 - 7
core/pkg/storage/s3storage.go

@@ -103,6 +103,7 @@ type S3Storage struct {
 	putUserMetadata map[string]string
 	partSize        uint64
 	listObjectsV1   bool
+	insecure        bool
 }
 
 // parseConfig unmarshals a buffer into a Config with default HTTPConfig values.
@@ -170,6 +171,8 @@ func NewS3StorageWith(config S3Config) (*S3Storage, error) {
 		return nil, err
 	}
 
+	// HTTPS Protocol Configuration: The 'Secure' option controls whether HTTPS is used.
+	// By default, config.Insecure is false if not set, so Secure=true.
 	client, err := minio.New(config.Endpoint, &minio.Options{
 		Creds:     credentials.NewChainCredentials(chain),
 		Secure:    !config.Insecure,
@@ -226,7 +229,9 @@ func NewS3StorageWith(config S3Config) (*S3Storage, error) {
 		putUserMetadata: config.PutUserMetadata,
 		partSize:        config.PartSize,
 		listObjectsV1:   config.ListObjectsVersion == "v1",
+		insecure:        config.Insecure,
 	}
+	log.Debugf("S3Storage: New S3 client initialized with '%s://%s/%s'", bkt.protocol(), config.Endpoint, config.Bucket)
 	return bkt, nil
 }
 
@@ -240,6 +245,14 @@ func (s3 *S3Storage) StorageType() StorageType {
 	return StorageTypeBucketS3
 }
 
+// protocol returns the protocol string (HTTP or HTTPS) based on configuration
+func (s3 *S3Storage) protocol() string {
+	if s3.insecure {
+		return "HTTP"
+	}
+	return "HTTPS"
+}
+
 // validate checks to see the config options are set.
 func validate(conf S3Config) error {
 	if conf.Endpoint == "" {
@@ -278,7 +291,7 @@ func (s3 *S3Storage) FullPath(name string) string {
 func (s3 *S3Storage) Read(name string) ([]byte, error) {
 	name = trimLeading(name)
 
-	log.Tracef("S3Storage::Read(%s)", name)
+	log.Debugf("S3Storage::Read::%s(%s)", s3.protocol(), name)
 	ctx := context.Background()
 
 	return s3.getRange(ctx, name, 0, -1)
@@ -288,7 +301,7 @@ func (s3 *S3Storage) Read(name string) ([]byte, error) {
 // Exists checks if the given object exists.
 func (s3 *S3Storage) Exists(name string) (bool, error) {
 	name = trimLeading(name)
-	log.Tracef("S3Storage::Exists(%s)", name)
+	log.Debugf("S3Storage::Exists::%s(%s)", s3.protocol(), name)
 
 	ctx := context.Background()
 
@@ -307,7 +320,7 @@ func (s3 *S3Storage) Exists(name string) (bool, error) {
 func (s3 *S3Storage) Write(name string, data []byte) error {
 	name = trimLeading(name)
 
-	log.Tracef("S3Storage::Write(%s)", name)
+	log.Debugf("S3Storage::Write::%s(%s)", s3.protocol(), name)
 
 	ctx := context.Background()
 	sse, err := s3.getServerSideEncryption(ctx)
@@ -341,7 +354,7 @@ func (s3 *S3Storage) Write(name string, data []byte) error {
 func (s3 *S3Storage) Stat(name string) (*StorageInfo, error) {
 	name = trimLeading(name)
 
-	log.Tracef("S3Storage::Stat(%s)", name)
+	log.Debugf("S3Storage::Stat::%s(%s)", s3.protocol(), name)
 	ctx := context.Background()
 
 	objInfo, err := s3.client.StatObject(ctx, s3.name, name, minio.StatObjectOptions{})
@@ -363,7 +376,7 @@ func (s3 *S3Storage) Stat(name string) (*StorageInfo, error) {
 func (s3 *S3Storage) Remove(name string) error {
 	name = trimLeading(name)
 
-	log.Tracef("S3Storage::Remove(%s)", name)
+	log.Debugf("S3Storage::Remove::%s(%s)", s3.protocol(), name)
 	ctx := context.Background()
 
 	return s3.client.RemoveObject(ctx, s3.name, name, minio.RemoveObjectOptions{})
@@ -372,7 +385,7 @@ func (s3 *S3Storage) Remove(name string) error {
 func (s3 *S3Storage) List(path string) ([]*StorageInfo, error) {
 	path = trimLeading(path)
 
-	log.Tracef("S3Storage::List(%s)", path)
+	log.Debugf("S3Storage::List::%s(%s)", s3.protocol(), path)
 	ctx := context.Background()
 
 	// Ensure the object name actually ends with a dir suffix. Otherwise we'll just iterate the
@@ -419,7 +432,7 @@ func (s3 *S3Storage) List(path string) ([]*StorageInfo, error) {
 func (s3 *S3Storage) ListDirectories(path string) ([]*StorageInfo, error) {
 	path = trimLeading(path)
 
-	log.Tracef("S3Storage::ListDirectories(%s)", path)
+	log.Debugf("S3Storage::ListDirectories::%s(%s)", s3.protocol(), path)
 	ctx := context.Background()
 
 	if path != "" {

+ 37 - 0
core/pkg/storage/s3storage_test.go

@@ -0,0 +1,37 @@
+package storage
+
+import (
+	"testing"
+)
+
+// TestS3Storage_protocol tests the protocol() method returns correct values based on insecure flag
+func TestS3Storage_protocol(t *testing.T) {
+	tests := []struct {
+		name     string
+		insecure bool
+		want     string
+	}{
+		{
+			name:     "secure connection returns HTTPS",
+			insecure: false,
+			want:     "HTTPS",
+		},
+		{
+			name:     "insecure connection returns HTTP",
+			insecure: true,
+			want:     "HTTP",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s3 := &S3Storage{
+				insecure: tt.insecure,
+			}
+			got := s3.protocol()
+			if got != tt.want {
+				t.Errorf("S3Storage.protocol() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}