feat(operator): allow users to configure virtual-host-style for S3 secrets (#17363)

Signed-off-by: Joao Marcal <jmarcal@redhat.com>
Co-authored-by: Robert Jacob <rojacob@redhat.com>

Author: JoaoBraveCoding

operator/docs/lokistack/object_storage.md

@@ -13,7 +13,7 @@ weight: 100
 toc: true
 ---
 
-Loki Operator supports [AWS S3](https://aws.amazon.com/), [Azure](https://azure.microsoft.com), [GCS](https://cloud.google.com/), [Minio](https://min.io/), [OpenShift Data Foundation](https://www.redhat.com/en/technologies/cloud-computing/openshift-data-foundation) and  [Swift](https://docs.openstack.org/swift/latest/) for LokiStack object storage.  
+Loki Operator supports [AWS S3](https://aws.amazon.com/), [Azure](https://azure.microsoft.com), [GCS](https://cloud.google.com/), [Minio](https://min.io/), [OpenShift Data Foundation](https://www.redhat.com/en/technologies/cloud-computing/openshift-data-foundation) and  [Swift](https://docs.openstack.org/swift/latest/) for LokiStack object storage.
 
 _Note_: Upon setting up LokiStack for any object storage provider, you should configure a logging collector that references the LokiStack in order to view the logs.
 
@@ -46,11 +46,12 @@ _Note_: Upon setting up LokiStack for any object storage provider, you should co
       --from-literal=endpoint="<AWS_BUCKET_ENDPOINT>" \
       --from-literal=access_key_id="<AWS_ACCESS_KEY_ID>" \
       --from-literal=access_key_secret="<AWS_ACCESS_KEY_SECRET>" \
+      --from-literal=region="<AWS_REGION_YOUR_BUCKET_LIVES_IN>" \
       --from-literal=sse_type="SSE-KMS" \
       --from-literal=sse_kms_key_id="<AWS_SSE_KMS_KEY_ID>" \
       --from-literal=sse_kms_encryption_context="<OPTIONAL_AWS_SSE_KMS_ENCRYPTION_CONTEXT_JSON>"
     ```
-    
+
     See also official docs on [AWS KMS Key ID](https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#key-id) and [AWS KMS Encryption Context](https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#encrypt_context) (**Note:** Only content without newlines allowed, because it is exposed via environment variable to the containers).
 
     or with `SSE-S3` encryption
@@ -61,9 +62,28 @@ _Note_: Upon setting up LokiStack for any object storage provider, you should co
       --from-literal=endpoint="<AWS_BUCKET_ENDPOINT>" \
       --from-literal=access_key_id="<AWS_ACCESS_KEY_ID>" \
       --from-literal=access_key_secret="<AWS_ACCESS_KEY_SECRET>" \
+      --from-literal=region="<AWS_REGION_YOUR_BUCKET_LIVES_IN>" \
       --from-literal=sse_type="SSE-S3"
     ```
 
+    Additionally, you can control the S3 URL style access behavior with `force_path_style` parameter:
+
+    ```console
+    kubectl create secret generic lokistack-dev-s3 \
+      --from-literal=bucketnames="<BUCKET_NAME>" \
+      --from-literal=endpoint="<AWS_BUCKET_ENDPOINT>" \
+      --from-literal=access_key_id="<AWS_ACCESS_KEY_ID>" \
+      --from-literal=access_key_secret="<AWS_ACCESS_KEY_SECRET>" \
+      --from-literal=region="<AWS_REGION_YOUR_BUCKET_LIVES_IN>" \
+      --from-literal=force_path_style="true"
+    ```
+
+    By default:
+    * AWS endpoints (ending with `.amazonaws.com`) use virtual hosted style (`force_path_style=false`)
+    * Non-AWS endpoints use path style (`force_path_style=true`)
+
+    Set `force_path_style` to `false` if you need to use virtual-hosted style with non-AWS S3 compatible services.
+
     where `lokistack-dev-s3` is the secret name.
 
 * Create an instance of [LokiStack](../hack/lokistack_dev.yaml) by referencing the secret name and type as `s3`:
@@ -130,7 +150,7 @@ _Note_: Upon setting up LokiStack for any object storage provider, you should co
       --from-literal=bucketname="<BUCKET_NAME>" \
       --from-file=key.json="<PATH/TO/KEY.JSON>"
     ```
-  
+
     where `lokistack-dev-gcs` is the secret name, `<BUCKET_NAME>` is the name of bucket created in requirements step and `<PATH/TO/KEY.JSON>` is the file path where the `key.json` was copied to.
 
 * Create an instance of [LokiStack](../hack/lokistack_dev.yaml) by referencing the secret name and type as `gcs`:

operator/internal/handlers/internal/storage/secrets.go

@@ -43,6 +43,7 @@ var (
 	errS3EndpointNoURL             = errors.New("endpoint for S3 must be an HTTP or HTTPS URL")
 	errS3EndpointUnsupportedScheme = errors.New("scheme of S3 endpoint URL is unsupported")
 	errS3EndpointAWSInvalid        = errors.New("endpoint for AWS S3 must include correct region")
+	errS3ForcePathStyleInvalid     = errors.New(`forcepathstyle must be "true" or "false"`)
 
 	errGCPParseCredentialsFile      = errors.New("gcp storage secret cannot be parsed from JSON content")
 	errGCPWrongCredentialSourceFile = errors.New("credential source in secret needs to point to token file")
@@ -410,10 +411,23 @@ func extractS3ConfigSecret(s *corev1.Secret, credentialMode lokiv1.CredentialMod
 		roleArn  = s.Data[storage.KeyAWSRoleArn]
 		audience = s.Data[storage.KeyAWSAudience]
 		// Optional fields
-		region         = s.Data[storage.KeyAWSRegion]
-		forcePathStyle = !strings.HasSuffix(string(endpoint), awsEndpointSuffix)
+		region = s.Data[storage.KeyAWSRegion]
 	)
 
+	// Determine if we should use path style URLs for S3
+	// default to false for non-AWS endpoints
+	forcePathStyle := !strings.HasSuffix(string(endpoint), awsEndpointSuffix)
+	// Check if the user has specified force_path_style
+	if configForcePathStyle, ok := s.Data[storage.KeyAWSForcePathStyle]; ok {
+		strForcePathStyle := string(configForcePathStyle)
+		switch strForcePathStyle {
+		case "true", "false":
+			forcePathStyle = strForcePathStyle == "true"
+		default:
+			return nil, fmt.Errorf("%w: %s", errS3ForcePathStyleInvalid, strForcePathStyle)
+		}
+	}
+
 	sseCfg, err := extractS3SSEConfig(s.Data)
 	if err != nil {
 		return nil, err

operator/internal/handlers/internal/storage/secrets_test.go

@@ -654,14 +654,15 @@ func TestS3Extract(t *testing.T) {
 	}
 }
 
-func TestS3Extract_S3ForcePathStyle(t *testing.T) {
+func TestS3Extract_ForcePathStyle(t *testing.T) {
 	tt := []struct {
 		desc        string
 		secret      *corev1.Secret
 		wantOptions *storage.S3StorageConfig
+		wantError   string
 	}{
 		{
-			desc: "aws s3 endpoint",
+			desc: "aws endpoint without forcepathstyle specified (default behavior)",
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
@@ -673,38 +674,81 @@ func TestS3Extract_S3ForcePathStyle(t *testing.T) {
 				},
 			},
 			wantOptions: &storage.S3StorageConfig{
-				Endpoint: "https://s3.region.amazonaws.com",
-				Region:   "region",
-				Buckets:  "this,that",
+				Endpoint:       "https://s3.region.amazonaws.com",
+				Region:         "region",
+				Buckets:        "this,that",
+				ForcePathStyle: false, // defaults to virtual style for AWS endpoints
 			},
 		},
 		{
-			desc: "non-aws s3 endpoint",
+			desc: "non-aws s3 endpoint without forcepathstyle specified (default behavior)",
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"endpoint":          []byte("https://test.default.svc.cluster.local:9000"),
+					"endpoint":          []byte("http://minio:9000"),
+					"region":            []byte(""),
+					"bucketnames":       []byte("this,that"),
+					"access_key_id":     []byte("id"),
+					"access_key_secret": []byte("secret"),
+				},
+			},
+			wantOptions: &storage.S3StorageConfig{
+				Endpoint:       "http://minio:9000",
+				Region:         "",
+				Buckets:        "this,that",
+				ForcePathStyle: true, // defaults to path style for non-AWS endpoints
+			},
+		},
+		{
+			desc: "aws s3 endpoint with forcepathstyle=true",
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "test"},
+				Data: map[string][]byte{
+					"endpoint":          []byte("https://s3.region.amazonaws.com"),
 					"region":            []byte("region"),
 					"bucketnames":       []byte("this,that"),
 					"access_key_id":     []byte("id"),
 					"access_key_secret": []byte("secret"),
+					"forcepathstyle":    []byte("true"),
 				},
 			},
 			wantOptions: &storage.S3StorageConfig{
-				Endpoint:       "https://test.default.svc.cluster.local:9000",
+				Endpoint:       "https://s3.region.amazonaws.com",
 				Region:         "region",
 				Buckets:        "this,that",
 				ForcePathStyle: true,
 			},
 		},
+		{
+			desc: "invalid forcepathstyle value",
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "test"},
+				Data: map[string][]byte{
+					"endpoint":          []byte("https://s3.region.amazonaws.com"),
+					"region":            []byte("region"),
+					"bucketnames":       []byte("this,that"),
+					"access_key_id":     []byte("id"),
+					"access_key_secret": []byte("secret"),
+					"forcepathstyle":    []byte("yes"),
+				},
+			},
+			wantError: `forcepathstyle must be "true" or "false": yes`,
+		},
 	}
 
 	for _, tc := range tt {
 		t.Run(tc.desc, func(t *testing.T) {
-			t.Parallel()
-			options, err := extractS3ConfigSecret(tc.secret, lokiv1.CredentialModeStatic)
-			require.NoError(t, err)
-			require.Equal(t, tc.wantOptions, options)
+			got, err := extractS3ConfigSecret(tc.secret, lokiv1.CredentialModeStatic)
+			if tc.wantError == "" {
+				require.NoError(t, err)
+
+				require.Equal(t, tc.wantOptions.ForcePathStyle, got.ForcePathStyle)
+				require.Equal(t, tc.wantOptions.Endpoint, got.Endpoint)
+				require.Equal(t, tc.wantOptions.Region, got.Region)
+				require.Equal(t, tc.wantOptions.Buckets, got.Buckets)
+			} else {
+				require.EqualError(t, err, tc.wantError)
+			}
 		})
 	}
 }

operator/internal/manifests/storage/var.go

@@ -59,6 +59,8 @@ const (
 	KeyAWSEndpoint = "endpoint"
 	// KeyAWSRegion is the secret data key for the AWS region.
 	KeyAWSRegion = "region"
+	// KeyAWSForcePathStyle is the secret data key for controlling S3 force the requests to use path-style addressing
+	KeyAWSForcePathStyle = "forcepathstyle"
 	// KeyAWSSSEType is the secret data key for the AWS server-side encryption type.
 	KeyAWSSSEType = "sse_type"
 	// KeyAWSSseKmsEncryptionContext is the secret data key for the AWS SSE KMS encryption context.