From 79557bca807d4c3b3124239d5cd59a12b7881336 Mon Sep 17 00:00:00 2001 From: stack72 Date: Tue, 25 Oct 2016 13:18:41 +0100 Subject: [PATCH] provider/aws: Add validation to IAM User and Group Name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow us to catch errors at plan time rather than waiting for the API to tell us... Documentation for IAM User NAme Validation - http://docs.aws.amazon.com/cli/latest/reference/iam/create-user.html Documentation for IAM Group Name validation - http://docs.aws.amazon.com/cli/latest/reference/iam/create-group.html ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSIAMGroup_' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2016/10/25 13:18:41 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSIAMGroup_ -timeout 120m === RUN TestAccAWSIAMGroup_importBasic --- PASS: TestAccAWSIAMGroup_importBasic (13.80s) === RUN TestAccAWSIAMGroup_basic --- PASS: TestAccAWSIAMGroup_basic (23.30s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws37.121s ``` ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSUser_' ✚ ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2016/10/25 13:22:23 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSUser_ -timeout 120m === RUN TestAccAWSUser_importBasic --- PASS: TestAccAWSUser_importBasic (14.33s) === RUN TestAccAWSUser_basic --- PASS: TestAccAWSUser_basic (25.36s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 39.710s ``` --- .../providers/aws/resource_aws_iam_group.go | 16 +++++++-- .../aws/resource_aws_iam_group_test.go | 36 +++++++++++++++++++ .../providers/aws/resource_aws_iam_user.go | 16 +++++++-- .../aws/resource_aws_iam_user_test.go | 36 +++++++++++++++++++ .../providers/aws/r/iam_group.html.markdown | 2 +- .../providers/aws/r/iam_user.html.markdown | 2 +- 6 files changed, 102 insertions(+), 6 deletions(-) diff --git a/builtin/providers/aws/resource_aws_iam_group.go b/builtin/providers/aws/resource_aws_iam_group.go index ecc5cae8e..9e294989e 100644 --- a/builtin/providers/aws/resource_aws_iam_group.go +++ b/builtin/providers/aws/resource_aws_iam_group.go @@ -2,6 +2,7 @@ package aws import ( "fmt" + "regexp" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -30,8 +31,9 @@ func resourceAwsIamGroup() *schema.Resource { Computed: true, }, "name": &schema.Schema{ - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + ValidateFunc: validateAwsIamGroupName, }, "path": &schema.Schema{ Type: schema.TypeString, @@ -127,3 +129,13 @@ func resourceAwsIamGroupDelete(d *schema.ResourceData, meta interface{}) error { } return nil } + +func validateAwsIamGroupName(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + if !regexp.MustCompile(`^[0-9A-Za-z=,.@-]+$`).MatchString(value) { + errors = append(errors, fmt.Errorf( + "only alphanumeric characters, hyphens, commas, periods, @ symbols and equals signs allowed in %q: %q", + k, value)) + } + return +} diff --git a/builtin/providers/aws/resource_aws_iam_group_test.go b/builtin/providers/aws/resource_aws_iam_group_test.go index ae895b0a0..3585571cc 100644 --- a/builtin/providers/aws/resource_aws_iam_group_test.go +++ b/builtin/providers/aws/resource_aws_iam_group_test.go @@ -11,6 +11,42 @@ import ( "github.com/hashicorp/terraform/terraform" ) +func TestValidateIamGroupName(t *testing.T) { + validNames := []string{ + "test-group", + "testgroup123", + "TestGroup", + "Test-Group", + "test.group", + "test.123,group", + "testgroup@hashicorp", + } + for _, v := range validNames { + _, errors := validateAwsIamGroupName(v, "name") + if len(errors) != 0 { + t.Fatalf("%q should be a valid IAM Group name: %q", v, errors) + } + } + + invalidNames := []string{ + "!", + "/", + " ", + ":", + ";", + "testgroup_123", + "test name", + "/slash-at-the-beginning", + "slash-at-the-end/", + } + for _, v := range invalidNames { + _, errors := validateAwsIamGroupName(v, "name") + if len(errors) == 0 { + t.Fatalf("%q should be an invalid IAM Group name", v) + } + } +} + func TestAccAWSIAMGroup_basic(t *testing.T) { var conf iam.GetGroupOutput diff --git a/builtin/providers/aws/resource_aws_iam_user.go b/builtin/providers/aws/resource_aws_iam_user.go index d37646360..106113077 100644 --- a/builtin/providers/aws/resource_aws_iam_user.go +++ b/builtin/providers/aws/resource_aws_iam_user.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "regexp" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -39,8 +40,9 @@ func resourceAwsIamUser() *schema.Resource { Computed: true, }, "name": &schema.Schema{ - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + ValidateFunc: validateAwsIamUserName, }, "path": &schema.Schema{ Type: schema.TypeString, @@ -202,3 +204,13 @@ func resourceAwsIamUserDelete(d *schema.ResourceData, meta interface{}) error { } return nil } + +func validateAwsIamUserName(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + if !regexp.MustCompile(`^[0-9A-Za-z=,.@-]+$`).MatchString(value) { + errors = append(errors, fmt.Errorf( + "only alphanumeric characters, hyphens, commas, periods, @ symbols and equals signs allowed in %q: %q", + k, value)) + } + return +} diff --git a/builtin/providers/aws/resource_aws_iam_user_test.go b/builtin/providers/aws/resource_aws_iam_user_test.go index a89e1769e..927b9be77 100644 --- a/builtin/providers/aws/resource_aws_iam_user_test.go +++ b/builtin/providers/aws/resource_aws_iam_user_test.go @@ -12,6 +12,42 @@ import ( "github.com/hashicorp/terraform/terraform" ) +func TestValidateIamUserName(t *testing.T) { + validNames := []string{ + "test-user", + "testuser123", + "TestUser", + "Test-User", + "test.user", + "test.123,user", + "testuser@hashicorp", + } + for _, v := range validNames { + _, errors := validateAwsIamUserName(v, "name") + if len(errors) != 0 { + t.Fatalf("%q should be a valid IAM User name: %q", v, errors) + } + } + + invalidNames := []string{ + "!", + "/", + " ", + ":", + ";", + "testuser_123", + "test name", + "/slash-at-the-beginning", + "slash-at-the-end/", + } + for _, v := range invalidNames { + _, errors := validateAwsIamUserName(v, "name") + if len(errors) == 0 { + t.Fatalf("%q should be an invalid IAM User name", v) + } + } +} + func TestAccAWSUser_basic(t *testing.T) { var conf iam.GetUserOutput diff --git a/website/source/docs/providers/aws/r/iam_group.html.markdown b/website/source/docs/providers/aws/r/iam_group.html.markdown index 3440a25dd..fc4b0d141 100644 --- a/website/source/docs/providers/aws/r/iam_group.html.markdown +++ b/website/source/docs/providers/aws/r/iam_group.html.markdown @@ -23,7 +23,7 @@ resource "aws_iam_group" "developers" { The following arguments are supported: -* `name` - (Required) The group's name. +* `name` - (Required) The group's name. The name must consist of upper and lowercase alphanumeric characters with no spaces. You can also include any of the following characters: `=,.@-.`. * `path` - (Optional, default "/") Path in which to create the group. ## Attributes Reference diff --git a/website/source/docs/providers/aws/r/iam_user.html.markdown b/website/source/docs/providers/aws/r/iam_user.html.markdown index 3ec681cd6..783dfe5ec 100644 --- a/website/source/docs/providers/aws/r/iam_user.html.markdown +++ b/website/source/docs/providers/aws/r/iam_user.html.markdown @@ -46,7 +46,7 @@ EOF The following arguments are supported: -* `name` - (Required) The user's name. +* `name` - (Required) The user's name. The name must consist of upper and lowercase alphanumeric characters with no spaces. You can also include any of the following characters: `=,.@-.`. * `path` - (Optional, default "/") Path in which to create the user. * `force_destroy` - (Optional, default false) When destroying this user, destroy even if it has non-Terraform-managed IAM access keys. Without `force_destroy`