From 96e83817efa91328e7771f58ad4cdca52d945faa Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Fri, 19 May 2017 11:11:44 -0400 Subject: [PATCH] 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 = < 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 ``` --- .../providers/aws/resource_aws_iam_policy.go | 12 ++-- .../aws/resource_aws_iam_policy_test.go | 37 ++++++++++- .../aws/resource_aws_iam_role_policy.go | 14 ++-- .../aws/resource_aws_iam_role_policy_test.go | 64 +++++++++++++++++-- builtin/providers/aws/structure.go | 6 +- builtin/providers/aws/validators.go | 17 +++++ builtin/providers/aws/validators_test.go | 59 +++++++++++++++++ 7 files changed, 191 insertions(+), 18 deletions(-) diff --git a/builtin/providers/aws/resource_aws_iam_policy.go b/builtin/providers/aws/resource_aws_iam_policy.go index e983dc7262..b3fdf1c5fd 100644 --- a/builtin/providers/aws/resource_aws_iam_policy.go +++ b/builtin/providers/aws/resource_aws_iam_policy.go @@ -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, }, diff --git a/builtin/providers/aws/resource_aws_iam_policy_test.go b/builtin/providers/aws/resource_aws_iam_policy_test.go index ce52d50677..a63a75eb69 100644 --- a/builtin/providers/aws/resource_aws_iam_policy_test.go +++ b/builtin/providers/aws/resource_aws_iam_policy_test.go @@ -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 = <