Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iam/policy: Allow IAM policy leading whitespace #36597

Merged
merged 9 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .changelog/36597.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
```release-note:enhancement
resource/aws_glacier_vault_lock: Allow `policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_iam_group_policy: Allow `policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_iam_policy: Allow `policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_iam_role_policy: Allow `policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_iam_role: Allow `assume_role_policy` and `inline_policy.*.policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_iam_user_policy: Allow `policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_media_store_container_policy: Allow `policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_ssoadmin_permission_set_inline_policy: Allow `inline_policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_transfer_access: Allow `policy` to have leading whitespace
```

```release-note:enhancement
resource/aws_transfer_user: Allow `policy` to have leading whitespace
```
61 changes: 61 additions & 0 deletions internal/service/iam/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,44 @@ func TestAccIAMPolicy_description(t *testing.T) {
})
}

func TestAccIAMPolicy_whitespace(t *testing.T) {
ctx := acctest.Context(t)
var out iam.Policy
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_iam_policy.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.IAMServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckPolicyDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccPolicyConfig_whitespace(rName, "", "", ""),
Check: resource.ComposeTestCheckFunc(
testAccCheckPolicyExists(ctx, resourceName, &out),
),
},
{
Config: testAccPolicyConfig_whitespace(rName, " ", "", ""),
PlanOnly: true,
},
{
Config: testAccPolicyConfig_whitespace(rName, " ", "\n", ""),
PlanOnly: true,
},
{
Config: testAccPolicyConfig_whitespace(rName, " ", "\n", " "),
PlanOnly: true,
},
{
Config: testAccPolicyConfig_whitespace(rName, " \n", "\n", "\t "),
PlanOnly: true,
},
},
})
}

func TestAccIAMPolicy_disappears(t *testing.T) {
ctx := acctest.Context(t)
var out iam.Policy
Expand Down Expand Up @@ -408,6 +446,29 @@ EOF
`, rName)
}

func testAccPolicyConfig_whitespace(rName, ws1, ws2, ws3 string) string {
return fmt.Sprintf(`
resource "aws_iam_policy" "test" {
name = %[1]q

policy = <<EOF
%[2]s{
"Version": "2012-10-17",%[3]s
"Statement": [
{
"Action": [
"ec2:Describe*"
],
"Effect": %[3]s"Allow",
"Resource": "*"
}
]
}
EOF
}
`, rName, ws1, ws2, ws3)
}

func testAccPolicyConfig_namePrefix(namePrefix string) string {
return fmt.Sprintf(`
resource "aws_iam_policy" "test" {
Expand Down
17 changes: 14 additions & 3 deletions internal/verify/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,13 @@ func ValidCIDRNetworkAddress(v interface{}, k string) (ws []string, errors []err
func ValidIAMPolicyJSON(v interface{}, k string) (ws []string, errors []error) {
// IAM Policy documents need to be valid JSON, and pass legacy parsing
value := v.(string)
value = strings.TrimSpace(value)
if len(value) < 1 {
errors = append(errors, fmt.Errorf("%q is an empty string, which is not a valid JSON value", k))
} else if first := value[:1]; first != "{" {
return //nolint:nakedret // Naked return due to legacy, non-idiomatic Go function, error handling
}

if first := value[:1]; first != "{" {
switch value[:1] {
YakDriver marked this conversation as resolved.
Show resolved Hide resolved
case " ", "\t", "\r", "\n":
errors = append(errors, fmt.Errorf("%q contains an invalid JSON policy: leading space characters are not allowed", k))
Expand All @@ -193,14 +197,21 @@ func ValidIAMPolicyJSON(v interface{}, k string) (ws []string, errors []error) {
// Generic error for if we didn't find something more specific to say.
errors = append(errors, fmt.Errorf("%q contains an invalid JSON policy: not a JSON object", k))
}
} else if _, err := structure.NormalizeJsonString(v); err != nil {
return //nolint:nakedret // Naked return due to legacy, non-idiomatic Go function, error handling
}

if _, err := structure.NormalizeJsonString(v); err != nil {
errStr := err.Error()
if err, ok := errs.As[*json.SyntaxError](err); ok {
errStr = fmt.Sprintf("%s, at byte offset %d", errStr, err.Offset)
}
errors = append(errors, fmt.Errorf("%q contains an invalid JSON policy: %s", k, errStr))
} else if err := basevalidation.JSONNoDuplicateKeys(value); err != nil {
return //nolint:nakedret // Naked return due to legacy, non-idiomatic Go function, error handling
}

if err := basevalidation.JSONNoDuplicateKeys(value); err != nil {
errors = append(errors, fmt.Errorf("%q contains duplicate JSON keys: %s", k, err))
return //nolint:nakedret // Naked return due to legacy, non-idiomatic Go function, error handling
}

return //nolint:nakedret // Just a long function.
Expand Down
4 changes: 0 additions & 4 deletions internal/verify/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,10 +391,6 @@ func TestValidIAMPolicyJSONString(t *testing.T) {
Value: ``,
WantError: `"json" is an empty string, which is not a valid JSON value`,
},
{
Value: ` {"xyz": "foo"}`,
WantError: `"json" contains an invalid JSON policy: leading space characters are not allowed`,
},
{
Value: `"blub"`,
WantError: `"json" contains an invalid JSON policy: contains a JSON-encoded string, not a JSON-encoded object`,
Expand Down
19 changes: 8 additions & 11 deletions website/docs/r/iam_policy.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ description: |-

Provides an IAM policy.

~> **NOTE:** We suggest using [`jsonencode()`](https://developer.hashicorp.com/terraform/language/functions/jsonencode) when assigning a value to `policy`. This function seamlessly translates Terraform language into JSON, enabling you to maintain consistency within your configuration without the need for context switches. By employing `jsonencode()`, you can sidestep potential complications arising from formatting discrepancies, whitespace inconsistencies, and other nuances inherent to JSON.
ewbankkit marked this conversation as resolved.
Show resolved Hide resolved

## Example Usage

```terraform
Expand Down Expand Up @@ -40,24 +42,19 @@ resource "aws_iam_policy" "policy" {
This resource supports the following arguments:

* `description` - (Optional, Forces new resource) Description of the IAM policy.
* `name` - (Optional, Forces new resource) The name of the policy. If omitted, Terraform will assign a random, unique name.
* `name_prefix` - (Optional, Forces new resource) Creates a unique name beginning with the specified prefix. Conflicts with `name`.
* `path` - (Optional, default "/") Path in which to create the policy.
See [IAM Identifiers](https://docs.aws.amazon.com/IAM/latest/UserGuide/Using_Identifiers.html) for more information.
* `policy` - (Required) The policy document. This is a JSON formatted string. For more information about building AWS IAM policy documents with Terraform, see the [AWS IAM Policy Document Guide](https://learn.hashicorp.com/terraform/aws/iam-policy)
* `name` - (Optional, Forces new resource) Name of the policy. If omitted, Terraform will assign a random, unique name.
* `path` - (Optional, default "/") Path in which to create the policy. See [IAM Identifiers](https://docs.aws.amazon.com/IAM/latest/UserGuide/Using_Identifiers.html) for more information.
* `policy` - (Required) Policy document. This is a JSON formatted string. For more information about building AWS IAM policy documents with Terraform, see the [AWS IAM Policy Document Guide](https://learn.hashicorp.com/terraform/aws/iam-policy)
* `tags` - (Optional) Map of resource tags for the IAM Policy. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level.

## Attribute Reference

This resource exports the following attributes in addition to the arguments above:

* `id` - The ARN assigned by AWS to this policy.
* `arn` - The ARN assigned by AWS to this policy.
* `description` - The description of the policy.
* `name` - The name of the policy.
* `path` - The path of the policy in IAM.
* `policy` - The policy document.
* `policy_id` - The policy's ID.
* `arn` - ARN assigned by AWS to this policy.
* `id` - ARN assigned by AWS to this policy.
* `policy_id` - Policy's ID.
* `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block).

## Import
Expand Down
Loading