fix: normalize policy action data-type

Author: kangasta

CHANGELOG.md

@@ -5,6 +5,10 @@ See updating [Changelog example here](https://keepachangelog.com/en/1.0.0/)
 
 ## [Unreleased]
 
+### Fixed
+
+- objsto_bucket_policy: when comparing policy documents, treat string value in `Action` field as equal to list containing that string as its only item, e.g. `"s3:PutObject"` vs. `["s3:PutObject"]`.
+
 ## [0.2.0]
 
 ### Added

internal/provider/bucket_policy_resource.go

@@ -197,10 +197,15 @@ func normalizePolicyDocument(document string) (string, diag.Diagnostics) {
 		delete(unmarshaled, "ID")
 	}
 
-	// Sort statement actions (Minio)
 	if statements, ok := unmarshaled["Statement"].([]interface{}); ok {
 		for _, statement := range statements {
 			if statement, ok := statement.(map[string]interface{}); ok {
+				// Always use array type for Action (Minio)
+				if action, ok := statement["Action"].(string); ok {
+					statement["Action"] = []string{action}
+				}
+
+				// Sort statement actions (Minio)
 				if actions, ok := statement["Action"].([]interface{}); ok {
 					sort.Slice(actions, func(i, j int) bool {
 						a, aOk := actions[i].(string)

internal/provider/bucket_policy_resource_test.go

@@ -103,38 +103,63 @@ func TestAccBucketPolicyResource(t *testing.T) {
 	})
 }
 
-func TestAccBucketPolicyResource_NoId(t *testing.T) {
-	bucket_name := withSuffix("bucket-policy-no-id")
-	variables := func(allow_get_object bool) map[string]config.Variable {
-		return map[string]config.Variable{
-			"bucket_name":      config.StringVariable(bucket_name),
-			"allow_get_object": config.BoolVariable(allow_get_object),
-		}
+func TestAccBucketPolicyResource_Normalization(t *testing.T) {
+	tests := []struct {
+		configName string
+		testImport bool
+	}{
+		{
+			configName: "bucket_policy_str_action",
+			testImport: false,
+		},
+		{
+			configName: "bucket_policy_no_id",
+			testImport: true,
+		},
 	}
 
-	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { testAccPreCheck(t) },
-		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
-		Steps: []resource.TestStep{
-			{
-				ConfigFile:      config.StaticFile("testdata/bucket_policy_no_id.tf"),
-				ConfigVariables: variables(false),
-				Check:           resource.ComposeAggregateTestCheckFunc(),
-			},
-			{
-				ConfigFile:      config.StaticFile("testdata/bucket_policy_no_id.tf"),
-				ConfigVariables: variables(true),
-				Check:           resource.ComposeAggregateTestCheckFunc(),
-			},
-			{
-				ConfigFile:                           config.StaticFile("testdata/bucket_policy_no_id.tf"),
-				ConfigVariables:                      variables(true),
-				ResourceName:                         "objsto_bucket_policy.this",
-				ImportState:                          true,
-				ImportStateId:                        bucket_name,
-				ImportStateVerifyIdentifierAttribute: "bucket",
-				ImportStateVerify:                    true,
-			},
-		},
-	})
+	for _, test := range tests {
+		t.Run(test.configName, func(t *testing.T) {
+			configPath := fmt.Sprintf("testdata/%s.tf", test.configName)
+
+			bucket_name := withSuffix("bucket-policy-no-id")
+			variables := func(allow_get_object bool) map[string]config.Variable {
+				return map[string]config.Variable{
+					"bucket_name":      config.StringVariable(bucket_name),
+					"allow_get_object": config.BoolVariable(allow_get_object),
+				}
+			}
+
+			steps := []resource.TestStep{
+				{
+					ConfigFile:      config.StaticFile(configPath),
+					ConfigVariables: variables(false),
+					Check:           resource.ComposeAggregateTestCheckFunc(),
+				},
+				{
+					ConfigFile:      config.StaticFile(configPath),
+					ConfigVariables: variables(true),
+					Check:           resource.ComposeAggregateTestCheckFunc(),
+				},
+			}
+
+			if test.testImport {
+				steps = append(steps, resource.TestStep{
+					ConfigFile:                           config.StaticFile(configPath),
+					ConfigVariables:                      variables(true),
+					ResourceName:                         "objsto_bucket_policy.this",
+					ImportState:                          true,
+					ImportStateId:                        bucket_name,
+					ImportStateVerifyIdentifierAttribute: "bucket",
+					ImportStateVerify:                    true,
+				})
+			}
+
+			resource.Test(t, resource.TestCase{
+				PreCheck:                 func() { testAccPreCheck(t) },
+				ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+				Steps:                    steps,
+			})
+		})
+	}
 }

internal/provider/testdata/bucket_policy_str_action.tf

@@ -0,0 +1,31 @@
+variable "bucket_name" {
+  type    = string
+  default = "objsto-acc-test"
+}
+
+variable "allow_get_object" {
+  type    = bool
+  default = false
+}
+
+resource "objsto_bucket" "this" {
+  bucket = var.bucket_name
+}
+
+resource "objsto_bucket_policy" "this" {
+  bucket = objsto_bucket.this.bucket
+  policy = jsonencode({
+    Version = "2012-10-17",
+    Statement = [
+      {
+        Principal = {
+          "AWS" = ["*"]
+        }
+        Effect = var.allow_get_object ? "Allow" : "Deny"
+        Action = "s3:GetObject"
+        Resource = [
+          "${objsto_bucket.this.arn}/*",
+        ]
+      },
+  ] })
+}