fix(iam): the order of service principals should not matter

.changelog/25967.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+data-source/aws_iam_policy_document: When using multiple principals, sort them to avoid differences based only on order
+```

internal/service/iam/policy_model.go

@@ -157,6 +157,7 @@ func (ps *IAMPolicyStatementPrincipalSet) UnmarshalJSON(b []byte) error {
 				for _, v := range value.([]interface{}) {
 					values = append(values, v.(string))
 				}
+				sort.Strings(values)
 				out = append(out, IAMPolicyStatementPrincipal{Type: key, Identifiers: values})
 			default:
 				return fmt.Errorf("Unsupported data type %T for IAMPolicyStatementPrincipalSet.Identifiers", vt)
@@ -269,12 +270,12 @@ func PolicyHasValidAWSPrincipals(policy string) (bool, error) { // nosemgrep:ci.
 	for _, principal := range principals {
 		switch x := principal.(type) {
 		case string:
-			if !isValidPolicyAWSPrincipal(x) {
+			if !IsValidPolicyAWSPrincipal(x) {
 				return false, nil
 			}
 		case []string:
 			for _, s := range x {
-				if !isValidPolicyAWSPrincipal(s) {
+				if !IsValidPolicyAWSPrincipal(s) {
 					return false, nil
 				}
 			}
@@ -284,9 +285,9 @@ func PolicyHasValidAWSPrincipals(policy string) (bool, error) { // nosemgrep:ci.
 	return true, nil
 }
 
-// isValidPolicyAWSPrincipal returns true if a string is a valid AWS Princial for an IAM Policy document
+// IsValidPolicyAWSPrincipal returns true if a string is a valid AWS Princial for an IAM Policy document
 // That is: either an ARN, an AWS account ID, or `*`
-func isValidPolicyAWSPrincipal(principal string) bool { // nosemgrep:ci.aws-in-func-name
+func IsValidPolicyAWSPrincipal(principal string) bool { // nosemgrep:ci.aws-in-func-name
 	if principal == "*" {
 		return true
 	}

internal/service/iam/policy_model_test.go

@@ -1,14 +1,15 @@
 // Copyright (c) HashiCorp, Inc.
 // SPDX-License-Identifier: MPL-2.0
 
-package iam
+package iam_test
 
 import (
 	"encoding/json"
 	"reflect"
 	"testing"
 
 	"github.com/hashicorp/terraform-provider-aws/internal/errs"
+	tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam"
 )
 
 func TestPolicyHasValidAWSPrincipals(t *testing.T) { // nosemgrep:ci.aws-in-func-name
@@ -208,7 +209,7 @@ func TestPolicyHasValidAWSPrincipals(t *testing.T) { // nosemgrep:ci.aws-in-func
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			valid, err := PolicyHasValidAWSPrincipals(testcase.json)
+			valid, err := tfiam.PolicyHasValidAWSPrincipals(testcase.json)
 
 			if testcase.err == nil {
 				if err != nil {
@@ -258,7 +259,7 @@ func TestIsValidAWSPrincipal(t *testing.T) { // nosemgrep:ci.aws-in-func-name
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			a := isValidPolicyAWSPrincipal(testcase.value)
+			a := tfiam.IsValidPolicyAWSPrincipal(testcase.value)
 
 			if e := testcase.valid; a != e {
 				t.Fatalf("expected %t, got %t", e, a)
@@ -271,103 +272,103 @@ func TestIAMPolicyStatementConditionSet_MarshalJSON(t *testing.T) { // nosemgrep
 	t.Parallel()
 
 	testcases := map[string]struct {
-		cs      IAMPolicyStatementConditionSet
+		cs      tfiam.IAMPolicyStatementConditionSet
 		want    []byte
 		wantErr bool
 	}{
 		"invalid value type": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: 1},
 			},
 			wantErr: true,
 		},
 		"single condition single value": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":"one/"}}`),
 		},
 		"single condition multiple values": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":["one/","two/"]}}`),
 		},
 		// Multiple distinct conditions
 		"multiple condition single value": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "ArnNotLike", Variable: "aws:PrincipalArn", Values: "1"},
 				{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
 			},
 			want: []byte(`{"ArnNotLike":{"aws:PrincipalArn":"1"},"StringLike":{"s3:prefix":"one/"}}`),
 		},
 		"multiple condition multiple values": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "ArnNotLike", Variable: "aws:PrincipalArn", Values: []string{"1", "2"}},
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
 			},
 			want: []byte(`{"ArnNotLike":{"aws:PrincipalArn":["1","2"]},"StringLike":{"s3:prefix":["one/","two/"]}}`),
 		},
 		"multiple condition mixed value lengths": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "ArnNotLike", Variable: "aws:PrincipalArn", Values: "1"},
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
 			},
 			want: []byte(`{"ArnNotLike":{"aws:PrincipalArn":"1"},"StringLike":{"s3:prefix":["one/","two/"]}}`),
 		},
 		// Multiple conditions with duplicated `test` arguments
 		"duplicate condition test single value": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
 				{Test: "StringLike", Variable: "s3:versionid", Values: "abc123"},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":"one/","s3:versionid":"abc123"}}`),
 		},
 		"duplicate condition test multiple values": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
 				{Test: "StringLike", Variable: "s3:versionid", Values: []string{"abc123", "def456"}},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":["one/","two/"],"s3:versionid":["abc123","def456"]}}`),
 		},
 		"duplicate condition test mixed value lengths": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
 				{Test: "StringLike", Variable: "s3:versionid", Values: []string{"abc123", "def456"}},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":"one/","s3:versionid":["abc123","def456"]}}`),
 		},
 		"duplicate condition test mixed value lengths reversed": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
 				{Test: "StringLike", Variable: "s3:versionid", Values: "abc123"},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":["one/","two/"],"s3:versionid":"abc123"}}`),
 		},
 		// Multiple conditions with duplicated `test` and `variable` arguments
 		"duplicate condition test and variable single value": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
 				{Test: "StringLike", Variable: "s3:prefix", Values: "two/"},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":["one/","two/"]}}`),
 		},
 		"duplicate condition test and variable multiple values": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"three/", "four/"}},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":["one/","two/","three/","four/"]}}`),
 		},
 		"duplicate condition test and variable mixed value lengths": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"three/", "four/"}},
 			},
 			want: []byte(`{"StringLike":{"s3:prefix":["one/","three/","four/"]}}`),
 		},
 		"duplicate condition test and variable mixed value lengths reversed": {
-			cs: IAMPolicyStatementConditionSet{
+			cs: tfiam.IAMPolicyStatementConditionSet{
 				{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
 				{Test: "StringLike", Variable: "s3:prefix", Values: "three/"},
 			},
@@ -390,3 +391,41 @@ func TestIAMPolicyStatementConditionSet_MarshalJSON(t *testing.T) { // nosemgrep
 		})
 	}
 }
+
+func TestPolicyUnmarshalServicePrincipalOrder(t *testing.T) {
+	t.Parallel()
+
+	policy1 := `
+		  {
+			"Action": "sts:AssumeRole",
+			"Principal": {
+			  "Service": ["lambda.amazonaws.com", "service2.amazonaws.com"]
+			},
+			"Effect": "Allow",
+			"Sid": ""
+		  }`
+	// Service order is different, but should be the same object for terraform
+	policy2 := `
+		  {
+			"Action": "sts:AssumeRole",
+			"Principal": {
+			  "Service": ["service2.amazonaws.com", "lambda.amazonaws.com"]
+			},
+			"Effect": "Allow",
+			"Sid": ""
+		  }`
+
+	var data1 tfiam.IAMPolicyStatement
+	var data2 tfiam.IAMPolicyStatement
+	err := json.Unmarshal([]byte(policy1), &data1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = json.Unmarshal([]byte(policy2), &data2)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(data1, data2) {
+		t.Fatalf("should be equal, but was:\n%#v\nVS\n%#v\n", data1, data2)
+	}
+}