More general IAM policy normalization

Some earlier work in #6956 implemented IAM policy normalization within
the aws_iam_policy_document data source. Some other (unmerged) work in
#7785 implemented normalization across many different IAM policy
attributes in the provider.

The two are in conflict due to a difference in approach. This is an
attempt to reconcile the two by generalizing the normalization already
implemented in #6956 and then applying it to the various places that
were addressed by #7785.

builtin/providers/aws/iam_policy_model.go

@@ -2,9 +2,16 @@ package aws
 
 import (
 	"encoding/json"
+	"fmt"
 	"sort"
 )
 
+// IAMPolicyDoc represents an AWS IAM Policy Document.
+//
+// This struct can represent various different ways of writing the same
+// rules, with things specified in different orders and single-item sets
+// expressed as either strings or lists. Use function NormalizeIAMPolicyDoc
+// to normalize into a predictable, consistent shape.
 type IAMPolicyDoc struct {
 	Version    string                `json:",omitempty"`
 	Id         string                `json:",omitempty"`
@@ -93,3 +100,126 @@ func iamPolicyDecodeConfigStringList(lI []interface{}) interface{} {
 	sort.Sort(sort.Reverse(sort.StringSlice(ret)))
 	return ret
 }
+
+// Does in-place normalizations on a given IAMPolicyDoc and its child objects.
+//
+// If an error is returned, the document may have been partially normalized.
+func NormalizeIAMPolicyDoc(in *IAMPolicyDoc) error {
+
+	if in.Statements == nil {
+		in.Statements = []*IAMPolicyStatement{}
+		return nil
+	}
+
+	for i, stmt := range in.Statements {
+		var stmtDesc string
+		if stmt.Sid != "" {
+			stmtDesc = fmt.Sprintf("statement %q", stmt.Sid)
+		} else {
+			stmtDesc = fmt.Sprintf("statement #%d", i+1)
+		}
+
+		if norm, err := normalizeIAMPolicyStringSet(stmt.Actions); err == nil {
+			stmt.Actions = norm
+		} else {
+			return fmt.Errorf("in %s: Action: %s", stmtDesc, err)
+		}
+
+		if norm, err := normalizeIAMPolicyStringSet(stmt.NotActions); err == nil {
+			stmt.NotActions = norm
+		} else {
+			return fmt.Errorf("in %s: NotAction: %s", stmtDesc, err)
+		}
+
+		if norm, err := normalizeIAMPolicyStringSet(stmt.Resources); err == nil {
+			stmt.Resources = norm
+		} else {
+			return fmt.Errorf("in %s: Resource: %s", stmtDesc, err)
+		}
+
+		if norm, err := normalizeIAMPolicyStringSet(stmt.NotResources); err == nil {
+			stmt.NotResources = norm
+		} else {
+			return fmt.Errorf("in %s: NotResource: %s", stmtDesc, err)
+		}
+
+	}
+
+	return nil
+}
+
+// In the event of an error, returns the input string unmodified along with the error
+func NormalizeIAMPolicyJSON(in string) (string, error) {
+	doc := &IAMPolicyDoc{}
+	err := json.Unmarshal([]byte(in), doc)
+	if err != nil {
+		return in, err
+	}
+
+	err = NormalizeIAMPolicyDoc(doc)
+	if err != nil {
+		return in, err
+	}
+
+	resultBytes, err := json.Marshal(doc)
+	if err != nil {
+		return in, err
+	}
+
+	return string(resultBytes), nil
+}
+
+// Can be used as a StateFunc for attributes that take IAM policies in JSON format.
+// Should usually be used in conjunction with iamPolicyJSONValidateFunc
+func iamPolicyJSONStateFunc(jsonSrcI interface{}) string {
+	// Safe to ignore the error because normalizeIAMPolicyJSON will pass through
+	// the given string verbatim if it's not valid.
+	result, _ := NormalizeIAMPolicyJSON(jsonSrcI.(string))
+	return result
+}
+
+// Can be used as a ValidateFunc for attributes that take IAM policies in JSON format.
+// Does simple syntactic validation.
+func iamPolicyJSONValidateFunc(jsonSrcI interface{}, _ string) ([]string, []error) {
+	_, err := NormalizeIAMPolicyJSON(jsonSrcI.(string))
+	if err != nil {
+		return nil, []error{err}
+	}
+
+	return nil, nil
+}
+
+// Several policy attributes can either be a single string or a list of strings.
+// If given a single string, it will be returned as a []string with a single element
+// If given a slice of strings, it will be sorted in-place and returned.
+// If given a slice of interface{}, it will be converted to []string and then treated as such
+// Passing a nil interface is acceptable, and acts as a no-op
+func normalizeIAMPolicyStringSet(in interface{}) (interface{}, error) {
+	if in == nil {
+		return nil, nil
+	}
+
+	switch in.(type) {
+	case []interface{}:
+		// convert to []string for processing below
+		stringIn := make([]string, len(in.([]interface{})))
+		for i, valI := range in.([]interface{}) {
+			if s, ok := valI.(string); ok {
+				stringIn[i] = s
+			} else {
+				return nil, fmt.Errorf("must be string or array of strings")
+			}
+		}
+		in = stringIn
+	}
+
+	switch in.(type) {
+	case []string:
+		sort.Sort(sort.StringSlice(in.([]string)))
+		return in, nil
+	case string:
+		return []string{in.(string)}, nil
+	default:
+		return nil, fmt.Errorf("must be string or array of strings")
+	}
+}

builtin/providers/aws/iam_policy_model_test.go

@@ -0,0 +1,84 @@
+package aws
+
+import (
+	"testing"
+)
+
+func TestNormalizeIAMPolicyJSON(t *testing.T) {
+	type testCase struct {
+		Input       string
+		Expected    string
+		ExpectError bool
+	}
+
+	tests := []testCase{
+		{
+			`{}`,
+			`{"Statement":[]}`,
+			false,
+		},
+		{
+			`{"Statement":[]}`,
+			`{"Statement":[]}`,
+			false,
+		},
+		{
+			// Single action string becomes single-item list
+			`{"Statement":[{"Action":"foo:Baz"}]}`,
+			`{"Statement":[{"Sid":"","Action":["foo:Baz"]}]}`,
+			false,
+		},
+		{
+			// Multiple actions are sorted
+			`{"Statement":[{"Sid":"","Action":["foo:Zeek","foo:Baz"]}]}`,
+			`{"Statement":[{"Sid":"","Action":["foo:Baz","foo:Zeek"]}]}`,
+			false,
+		},
+		{
+			`{"Statement":[{"Sid":"","NotAction":"foo:Zeek"}]}`,
+			`{"Statement":[{"Sid":"","NotAction":["foo:Zeek"]}]}`,
+			false,
+		},
+		{
+			`{"Statement":[{"Sid":"","Resource":"foo:Zeek"}]}`,
+			`{"Statement":[{"Sid":"","Resource":["foo:Zeek"]}]}`,
+			false,
+		},
+		{
+			`{"Statement":[{"Sid":"","NotResource":"foo:Zeek"}]}`,
+			`{"Statement":[{"Sid":"","NotResource":["foo:Zeek"]}]}`,
+			false,
+		},
+		{
+			// Statement attribute order is normalized
+			`{"Statement":[{"Sid":"","NotAction":["foo:Zeek"],"Action":["foo:Baz"]}]}`,
+			`{"Statement":[{"Sid":"","Action":["foo:Baz"],"NotAction":["foo:Zeek"]}]}`,
+			false,
+		},
+	}
+
+	for _, test := range tests {
+		result, err := NormalizeIAMPolicyJSON(test.Input)
+
+		if test.ExpectError {
+			if err == nil {
+				t.Errorf("%s normalized successfully; want error", test.Input)
+				continue
+			}
+
+			if result != test.Input {
+				t.Errorf("%s\nproduced %s\nshould match input", test.Input, result)
+				continue
+			}
+		} else {
+			if err != nil {
+				t.Errorf("%s returned error; want success\n%s", test.Input, err)
+				continue
+			}
+
+			if result != test.Expected {
+				t.Errorf("%s\nproduced %s\n    want %s", test.Input, result, test.Expected)
+			}
+		}
+	}
+}