ResiLien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

provider/aws: validation: Add validation function for IAM Policies 

The previous JSON validator that we were using for IAM policy documents wouldn't catch AWS IAM Policy errors.
The supplied policy document would pass our validator, then fail with the following API error:

```
 * aws_iam_role_policy.foo: Error putting IAM role policy tf_test_policy_ymw7hbil9w: MalformedPolicyDocument: The policy failed legacy parsing
                        status code: 400, request id: e7615d90-3c99-11e7-babc-c14e741605bf
```

This happens if the Policy Document doesn't start with the opening JSON bracket, and often happens in the following case:

```
policy = <<EOF
  {
      "Version": "2012-10-17",
      "Statement": [
          {
            ...
          }
      ]
  }
  EOF
```

Where, when using a HEREDOC, the policy document is indented incorrectly.

The new validation function for the IAM policies verifies that the first character of the supplied policy document is the leading JSON bracket, prior to validating the JSON string.

Test Output:

```
$ make test TEST=./builtin/providers/aws/ TESTARGS="-v -run=TestValidateIAMPolicyJsonString"
==> Checking that code complies with gofmt requirements...
==> Checking AWS provider for unchecked errors...
==> NOTE: at this time we only look for uncheck errors in the AWS package
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/19 10:56:32 Generated command/internal_plugin_list.go
go test -i ./builtin/providers/aws/ || exit 1
echo ./builtin/providers/aws/ | \
        xargs -t -n4 go test -v -run=TestValidateIAMPolicyJsonString -timeout=60s -parallel=4
go test -v -run=TestValidateIAMPolicyJsonString -timeout=60s -parallel=4 ./builtin/providers/aws/
=== RUN   TestValidateIAMPolicyJsonString
--- PASS: TestValidateIAMPolicyJsonString (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    0.009s
```

```
$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAWSPolicy_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/19 10:38:43 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAWSPolicy_ -timeout 120m
=== RUN   TestAWSPolicy_namePrefix
--- PASS: TestAWSPolicy_namePrefix (20.01s)
=== RUN   TestAWSPolicy_invalidJson
--- PASS: TestAWSPolicy_invalidJson (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    20.027s
```

```
$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSIAMRolePolicy_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/19 11:02:56 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSIAMRolePolicy_ -timeout 120m
=== RUN   TestAccAWSIAMRolePolicy_importBasic
--- PASS: TestAccAWSIAMRolePolicy_importBasic (18.45s)
=== RUN   TestAccAWSIAMRolePolicy_basic
--- PASS: TestAccAWSIAMRolePolicy_basic (35.92s)
=== RUN   TestAccAWSIAMRolePolicy_namePrefix
--- PASS: TestAccAWSIAMRolePolicy_namePrefix (14.78s)
=== RUN   TestAccAWSIAMRolePolicy_generatedName
--- PASS: TestAccAWSIAMRolePolicy_generatedName (20.20s)
=== RUN   TestAccAWSIAMRolePolicy_invalidJSON
--- PASS: TestAccAWSIAMRolePolicy_invalidJSON (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    89.363s
```
This commit is contained in:
Jake Champlin 2017-05-19 11:11:44 -04:00
parent 7c6e19689f
commit 96e83817ef
No known key found for this signature in database
GPG Key ID: DC31F41958EF4AC2
7 changed files with 191 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
builtin/providers/aws/resource_aws_iam_policy.go
View File

@ -24,24 +24,24 @@ func resourceAwsIamPolicy() *schema.Resource {
},
Schema: map[string]*schema.Schema{
"description": &schema.Schema{
"description": {
Type: schema.TypeString,
ForceNew: true,
Optional: true,
},
"path": &schema.Schema{
"path": {
Type: schema.TypeString,
Optional: true,
Default: "/",
ForceNew: true,
},
"policy": &schema.Schema{
"policy": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validateJsonString,
ValidateFunc: validateIAMPolicyJson,
DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs,
},
"name": &schema.Schema{
"name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
@ -79,7 +79,7 @@ func resourceAwsIamPolicy() *schema.Resource {
return
},
},
"arn": &schema.Schema{
"arn": {
Type: schema.TypeString,
Computed: true,
},

37
builtin/providers/aws/resource_aws_iam_policy_test.go
View File

@ -2,6 +2,7 @@ package aws
import (
"fmt"
"regexp"
"strings"
"testing"
@ -19,7 +20,7 @@ func TestAWSPolicy_namePrefix(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSPolicyDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSPolicyPrefixNameConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSPolicyExists("aws_iam_policy.policy", &out),
@ -31,6 +32,20 @@ func TestAWSPolicy_namePrefix(t *testing.T) {
})
}
func TestAWSPolicy_invalidJson(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSPolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSPolicyInvalidJsonConfig,
ExpectError: regexp.MustCompile("invalid JSON"),
},
},
})
}
func testAccCheckAWSPolicyExists(resource string, res *iam.GetPolicyOutput) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[resource]
@ -94,3 +109,23 @@ resource "aws_iam_policy" "policy" {
EOF
}
`
const testAccAWSPolicyInvalidJsonConfig = `
resource "aws_iam_policy" "policy" {
name_prefix = "test-policy-"
path = "/"
policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Action": [
"ec2:Describe*"
],
"Effect": "Allow",
"Resource": "*"
}
]
}
EOF
}
`

14
builtin/providers/aws/resource_aws_iam_role_policy.go
View File

@ -26,11 +26,13 @@ func resourceAwsIamRolePolicy() *schema.Resource {
},
Schema: map[string]*schema.Schema{
"policy": &schema.Schema{
Type: schema.TypeString,
Required: true,
"policy": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validateIAMPolicyJson,
DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs,
},
"name": &schema.Schema{
"name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
@ -38,13 +40,13 @@ func resourceAwsIamRolePolicy() *schema.Resource {
ConflictsWith: []string{"name_prefix"},
ValidateFunc: validateIamRolePolicyName,
},
"name_prefix": &schema.Schema{
"name_prefix": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: validateIamRolePolicyNamePrefix,
},
"role": &schema.Schema{
"role": {
Type: schema.TypeString,
Required: true,
ForceNew: true,

64
builtin/providers/aws/resource_aws_iam_role_policy_test.go
View File

@ -2,6 +2,7 @@ package aws
import (
"fmt"
"regexp"
"testing"
"github.com/aws/aws-sdk-go/aws"
@ -22,7 +23,7 @@ func TestAccAWSIAMRolePolicy_basic(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckIAMRolePolicyDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccIAMRolePolicyConfig(role, policy1),
Check: resource.ComposeTestCheckFunc(
testAccCheckIAMRolePolicy(
@ -31,7 +32,7 @@ func TestAccAWSIAMRolePolicy_basic(t *testing.T) {
),
),
},
resource.TestStep{
{
Config: testAccIAMRolePolicyConfigUpdate(role, policy1, policy2),
Check: resource.ComposeTestCheckFunc(
testAccCheckIAMRolePolicy(
@ -53,7 +54,7 @@ func TestAccAWSIAMRolePolicy_namePrefix(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckIAMRolePolicyDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccIAMRolePolicyConfig_namePrefix(role),
Check: resource.ComposeTestCheckFunc(
testAccCheckIAMRolePolicy(
@ -75,7 +76,7 @@ func TestAccAWSIAMRolePolicy_generatedName(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckIAMRolePolicyDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccIAMRolePolicyConfig_generatedName(role),
Check: resource.ComposeTestCheckFunc(
testAccCheckIAMRolePolicy(
@ -88,6 +89,22 @@ func TestAccAWSIAMRolePolicy_generatedName(t *testing.T) {
})
}
func TestAccAWSIAMRolePolicy_invalidJSON(t *testing.T) {
role := acctest.RandString(10)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckIAMRolePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccIAMRolePolicyConfig_invalidJSON(role),
ExpectError: regexp.MustCompile("invalid JSON"),
},
},
})
}
func testAccCheckIAMRolePolicyDestroy(s *terraform.State) error {
iamconn := testAccProvider.Meta().(*AWSClient).iamconn
@ -328,3 +345,42 @@ EOF
}
`, role, policy1, policy2)
}
func testAccIAMRolePolicyConfig_invalidJSON(role string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "role" {
name = "tf_test_role_%s"
path = "/"
assume_role_policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Action": "sts:AssumeRole",
"Principal": {
"Service": "ec2.amazonaws.com"
},
"Effect": "Allow",
"Sid": ""
}
]
}
EOF
}
resource "aws_iam_role_policy" "foo" {
name = "tf_test_policy_%s"
role = "${aws_iam_role.role.name}"
policy = <<EOF
{
"Version": "2012-10-17",
"Statement": {
"Effect": "Allow",
"Action": "*",
"Resource": "*"
}
}
EOF
}
`, role, role)
}

6
builtin/providers/aws/structure.go
View File

@ -1886,7 +1886,11 @@ func normalizeJsonString(jsonString interface{}) (string, error) {
return s, err
}
bytes, _ := json.Marshal(j)
bytes, err := json.Marshal(j)
if err != nil {
return s, err
}
return string(bytes[:]), nil
}

17
builtin/providers/aws/validators.go
View File

@ -605,6 +605,23 @@ func validateJsonString(v interface{}, k string) (ws []string, errors []error) {
return
}
func validateIAMPolicyJson(v interface{}, k string) (ws []string, errors []error) {
// IAM Policy documents need to be valid JSON, and pass legacy parsing
value := v.(string)
if len(value) < 1 {
errors = append(errors, fmt.Errorf("%q contains an invalid JSON policy", k))
return
}
if value[:1] != "{" {
errors = append(errors, fmt.Errorf("%q conatains an invalid JSON policy", k))
return
}
if _, err := normalizeJsonString(v); err != nil {
errors = append(errors, fmt.Errorf("%q contains an invalid JSON: %s", k, err))
}
return
}
func validateCloudFormationTemplate(v interface{}, k string) (ws []string, errors []error) {
if looksLikeJsonString(v) {
if _, err := normalizeJsonString(v); err != nil {

59
builtin/providers/aws/validators_test.go
View File

@ -799,6 +799,65 @@ func TestValidateJsonString(t *testing.T) {
}
}
func TestValidateIAMPolicyJsonString(t *testing.T) {
type testCases struct {
Value string
ErrCount int
}
invalidCases := []testCases{
{
Value: `{0:"1"}`,
ErrCount: 1,
},
{
Value: `{'abc':1}`,
ErrCount: 1,
},
{
Value: `{"def":}`,
ErrCount: 1,
},
{
Value: `{"xyz":[}}`,
ErrCount: 1,
},
{
Value: ``,
ErrCount: 1,
},
{
Value: ` {"xyz": "foo"}`,
ErrCount: 1,
},
}
for _, tc := range invalidCases {
_, errors := validateIAMPolicyJson(tc.Value, "json")
if len(errors) != tc.ErrCount {
t.Fatalf("Expected %q to trigger a validation error.", tc.Value)
}
}
validCases := []testCases{
{
Value: `{}`,
ErrCount: 0,
},
{
Value: `{"abc":["1","2"]}`,
ErrCount: 0,
},
}
for _, tc := range validCases {
_, errors := validateIAMPolicyJson(tc.Value, "json")
if len(errors) != tc.ErrCount {
t.Fatalf("Expected %q not to trigger a validation error.", tc.Value)
}
}
}
func TestValidateCloudFormationTemplate(t *testing.T) {
type testCases struct {
Value string