Skip to content

Commit 19853d7

Browse files
jar-bjar-b
committed
d/aws_iam_policy_document: handle duplicated conditions consistently
1 parent 45e00a4 commit 19853d7

File tree

3 files changed

+141
-10
lines changed

3 files changed

+141
-10
lines changed

‎internal/service/iam/policy_document_data_source_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,12 +1409,8 @@ func testAccPolicyDocumentConditionWithBoolValueExpectedJSON() string {
14091409
"aws:ResourceTag/SpecialTag": "false"
14101410
},
14111411
"StringLike": {
1412-
"aws:ResourceAccount": [
1413-
"123456"
1414-
],
1415-
"aws:PrincipalArn": [
1416-
"arn:%[1]s:iam::*:role/AWSAFTExecution"
1417-
]
1412+
"aws:ResourceAccount": "123456",
1413+
"aws:PrincipalArn": "arn:%[1]s:iam::*:role/AWSAFTExecution"
14181414
}
14191415
}
14201416
}

‎internal/service/iam/policy_model.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,20 +177,30 @@ func (cs IAMPolicyStatementConditionSet) MarshalJSON() ([]byte, error) {
177177
if _, ok := raw[c.Test]; !ok {
178178
raw[c.Test] = map[string]interface{}{}
179179
}
180+
if _, ok := raw[c.Test][c.Variable]; !ok {
181+
raw[c.Test][c.Variable] = []string{}
182+
}
180183
switch i := c.Values.(type) {
181184
case []string:
182-
if _, ok := raw[c.Test][c.Variable]; !ok {
183-
raw[c.Test][c.Variable] = make([]string, 0, len(i))
184-
}
185185
// order matters with values so not sorting here
186186
raw[c.Test][c.Variable] = append(raw[c.Test][c.Variable].([]string), i...)
187187
case string:
188-
raw[c.Test][c.Variable] = i
188+
raw[c.Test][c.Variable] = append(raw[c.Test][c.Variable].([]string), i)
189189
default:
190190
return nil, fmt.Errorf("Unsupported data type for IAMPolicyStatementConditionSet: %s", i)
191191
}
192192
}
193193

194+
// flatten entries with a single item to match AWS IAM syntax
195+
for k1 := range raw {
196+
for k2 := range raw[k1] {
197+
items := raw[k1][k2].([]string)
198+
if len(items) == 1 {
199+
raw[k1][k2] = items[0]
200+
}
201+
}
202+
}
203+
194204
return json.Marshal(&raw)
195205
}
196206

‎internal/service/iam/policy_model_test.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package iam
55

66
import (
77
"encoding/json"
8+
"reflect"
89
"testing"
910

1011
"github.com/hashicorp/terraform-provider-aws/internal/errs"
@@ -265,3 +266,127 @@ func TestIsValidAWSPrincipal(t *testing.T) { // nosemgrep:ci.aws-in-func-name
265266
})
266267
}
267268
}
269+
270+
func TestIAMPolicyStatementConditionSet_MarshalJSON(t *testing.T) { // nosemgrep:ci.iam-in-func-name
271+
t.Parallel()
272+
273+
testcases := map[string]struct {
274+
cs IAMPolicyStatementConditionSet
275+
want []byte
276+
wantErr bool
277+
}{
278+
"invalid value type": {
279+
cs: IAMPolicyStatementConditionSet{
280+
{Test: "StringLike", Variable: "s3:prefix", Values: 1},
281+
},
282+
wantErr: true,
283+
},
284+
"single condition single value": {
285+
cs: IAMPolicyStatementConditionSet{
286+
{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
287+
},
288+
want: []byte(`{"StringLike":{"s3:prefix":"one/"}}`),
289+
},
290+
"single condition multiple values": {
291+
cs: IAMPolicyStatementConditionSet{
292+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
293+
},
294+
want: []byte(`{"StringLike":{"s3:prefix":["one/","two/"]}}`),
295+
},
296+
// Multiple distinct conditions
297+
"multiple condition single value": {
298+
cs: IAMPolicyStatementConditionSet{
299+
{Test: "ArnNotLike", Variable: "aws:PrincipalArn", Values: "1"},
300+
{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
301+
},
302+
want: []byte(`{"ArnNotLike":{"aws:PrincipalArn":"1"},"StringLike":{"s3:prefix":"one/"}}`),
303+
},
304+
"multiple condition multiple values": {
305+
cs: IAMPolicyStatementConditionSet{
306+
{Test: "ArnNotLike", Variable: "aws:PrincipalArn", Values: []string{"1", "2"}},
307+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
308+
},
309+
want: []byte(`{"ArnNotLike":{"aws:PrincipalArn":["1","2"]},"StringLike":{"s3:prefix":["one/","two/"]}}`),
310+
},
311+
"multiple condition mixed value lengths": {
312+
cs: IAMPolicyStatementConditionSet{
313+
{Test: "ArnNotLike", Variable: "aws:PrincipalArn", Values: "1"},
314+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
315+
},
316+
want: []byte(`{"ArnNotLike":{"aws:PrincipalArn":"1"},"StringLike":{"s3:prefix":["one/","two/"]}}`),
317+
},
318+
// Multiple conditions with duplicated `test` arguments
319+
"duplicate condition test single value": {
320+
cs: IAMPolicyStatementConditionSet{
321+
{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
322+
{Test: "StringLike", Variable: "s3:versionid", Values: "abc123"},
323+
},
324+
want: []byte(`{"StringLike":{"s3:prefix":"one/","s3:versionid":"abc123"}}`),
325+
},
326+
"duplicate condition test multiple values": {
327+
cs: IAMPolicyStatementConditionSet{
328+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
329+
{Test: "StringLike", Variable: "s3:versionid", Values: []string{"abc123", "def456"}},
330+
},
331+
want: []byte(`{"StringLike":{"s3:prefix":["one/","two/"],"s3:versionid":["abc123","def456"]}}`),
332+
},
333+
"duplicate condition test mixed value lengths": {
334+
cs: IAMPolicyStatementConditionSet{
335+
{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
336+
{Test: "StringLike", Variable: "s3:versionid", Values: []string{"abc123", "def456"}},
337+
},
338+
want: []byte(`{"StringLike":{"s3:prefix":"one/","s3:versionid":["abc123","def456"]}}`),
339+
},
340+
"duplicate condition test mixed value lengths reversed": {
341+
cs: IAMPolicyStatementConditionSet{
342+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
343+
{Test: "StringLike", Variable: "s3:versionid", Values: "abc123"},
344+
},
345+
want: []byte(`{"StringLike":{"s3:prefix":["one/","two/"],"s3:versionid":"abc123"}}`),
346+
},
347+
// Multiple conditions with duplicated `test` and `variable` arguments
348+
"duplicate condition test and variable single value": {
349+
cs: IAMPolicyStatementConditionSet{
350+
{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
351+
{Test: "StringLike", Variable: "s3:prefix", Values: "two/"},
352+
},
353+
want: []byte(`{"StringLike":{"s3:prefix":["one/","two/"]}}`),
354+
},
355+
"duplicate condition test and variable multiple values": {
356+
cs: IAMPolicyStatementConditionSet{
357+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
358+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"three/", "four/"}},
359+
},
360+
want: []byte(`{"StringLike":{"s3:prefix":["one/","two/","three/","four/"]}}`),
361+
},
362+
"duplicate condition test and variable mixed value lengths": {
363+
cs: IAMPolicyStatementConditionSet{
364+
{Test: "StringLike", Variable: "s3:prefix", Values: "one/"},
365+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"three/", "four/"}},
366+
},
367+
want: []byte(`{"StringLike":{"s3:prefix":["one/","three/","four/"]}}`),
368+
},
369+
"duplicate condition test and variable mixed value lengths reversed": {
370+
cs: IAMPolicyStatementConditionSet{
371+
{Test: "StringLike", Variable: "s3:prefix", Values: []string{"one/", "two/"}},
372+
{Test: "StringLike", Variable: "s3:prefix", Values: "three/"},
373+
},
374+
want: []byte(`{"StringLike":{"s3:prefix":["one/","two/","three/"]}}`),
375+
},
376+
}
377+
for name, tc := range testcases {
378+
tc := tc
379+
t.Run(name, func(t *testing.T) {
380+
t.Parallel()
381+
382+
got, err := tc.cs.MarshalJSON()
383+
if (err != nil) != tc.wantErr {
384+
t.Errorf("IAMPolicyStatementConditionSet.MarshalJSON() error = %v, wantErr %v", err, tc.wantErr)
385+
return
386+
}
387+
if !reflect.DeepEqual(got, tc.want) {
388+
t.Errorf("IAMPolicyStatementConditionSet.MarshalJSON() = %v, want %v", string(got), string(tc.want))
389+
}
390+
})
391+
}
392+
}

0 commit comments

Comments
 (0)