fix: ignore statement action order when comparing policy documents (#12)

Implements #2

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, ignore statement action order because Minio returns actions in inconsistent order.
+
 ## [0.1.0]
 
 ### Added

GNUmakefile

@@ -28,7 +28,7 @@ test:
 	go test -v -cover -timeout=120s -parallel=10 ./...
 
 testacc:
-	TF_ACC=1 go test -v -cover -timeout 120m ./...
+	TF_ACC=1 go test -v -count 1 -cover -timeout 120m ./...
 
 release-notes: CHANGELOG_HEADER = ^\#\# \[
 release-notes: CHANGELOG_VERSION = $(subst v,,$(VERSION))

go.mod

@@ -12,6 +12,7 @@ require (
 	github.com/hashicorp/terraform-plugin-go v0.25.0
 	github.com/hashicorp/terraform-plugin-log v0.9.0
 	github.com/hashicorp/terraform-plugin-testing v1.10.0
+	github.com/stretchr/testify v1.10.0
 )
 
 require (
@@ -27,6 +28,7 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.5 // indirect
 	github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.3 // indirect
 	github.com/cloudflare/circl v1.3.7 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/fatih/color v1.16.0 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect
@@ -58,8 +60,8 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/oklog/run v1.0.0 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/rogpeppe/go-internal v1.12.0 // indirect
-	github.com/stretchr/testify v1.9.0 // indirect
 	github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
 	github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
 	github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
@@ -75,4 +77,5 @@ require (
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142 // indirect
 	google.golang.org/grpc v1.67.1 // indirect
 	google.golang.org/protobuf v1.35.1 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -165,8 +165,8 @@ github.com/skeema/knownhosts v1.2.2 h1:Iug2P4fLmDw9f41PB6thxUkNUkJzB5i+1/exaj40L
 github.com/skeema/knownhosts v1.2.2/go.mod h1:xYbVRSPxqBZFrdmDyMmsOs+uX1UZC3nTN3ThzgDxUwo=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals=
-github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
-github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
+github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
+github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
 github.com/vmihailenco/msgpack v4.0.4+incompatible h1:dSLoQfGFAo3F6OoNhwUmLwVgaUXK79GlxNBwueZn0xI=
 github.com/vmihailenco/msgpack v4.0.4+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=

internal/provider/bucket_policy_resource.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"sort"
 
 	awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
 	"github.com/aws/aws-sdk-go-v2/service/s3"
@@ -196,6 +197,24 @@ 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 {
+				if actions, ok := statement["Action"].([]interface{}); ok {
+					sort.Slice(actions, func(i, j int) bool {
+						a, aOk := actions[i].(string)
+						b, bOk := actions[j].(string)
+						if !aOk || !bOk {
+							return false
+						}
+						return a < b
+					})
+				}
+			}
+		}
+	}
+
 	// Remove "null" Id (UpCloud Managed Object Storage)
 	if id := unmarshaled["Id"]; id == "null" {
 		delete(unmarshaled, "Id")

internal/provider/bucket_policy_resource_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/config"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
+	"github.com/stretchr/testify/assert"
 )
 
 func checkGetUrl(name, key string, expectedStatus int) resource.TestCheckFunc {
@@ -31,6 +32,37 @@ func checkGetUrl(name, key string, expectedStatus int) resource.TestCheckFunc {
 	}
 }
 
+func TestNormalizePolicyDocument(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "Change ID to Id",
+			input:    `{"ID":"PublicRead"}`,
+			expected: `{"Id":"PublicRead"}`,
+		},
+		{
+			name:     "Removes null Id",
+			input:    `{"Id":"null"}`,
+			expected: `{}`,
+		},
+		{
+			name:     "Sorts statement actions",
+			input:    `{"Statement":[{"Action":["s3:ListBucket","s3:GetBucketLocation"]}]}`,
+			expected: `{"Statement":[{"Action":["s3:GetBucketLocation","s3:ListBucket"]}]}`,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual, diags := normalizePolicyDocument(test.input)
+			assert.False(t, diags.HasError())
+			assert.Equal(t, test.expected, actual)
+		})
+	}
+}
+
 func TestAccBucketPolicyResource(t *testing.T) {
 	bucket_name := withSuffix("bucket-policy")
 	variables := func(public_read_access bool) map[string]config.Variable {