From 5481e9ecb32b58b1154bcc4db44835664bbc58c3 Mon Sep 17 00:00:00 2001 From: Ninir Date: Wed, 30 Nov 2016 12:50:26 +0100 Subject: [PATCH] provider/aws: Enforced kms_key_* attributes to be ARNs (#10356) * Added kms_key_* validation to force ARNs * Added Redshift Cluster KMS key test * Added cloudtrail kms key test * Added EBS volume kms key * Added Elastic Transcoder Pipeline kms key arn test --- .../data_source_aws_s3_bucket_object_test.go | 2 +- .../providers/aws/resource_aws_cloudtrail.go | 31 ++--- .../aws/resource_aws_cloudtrail_test.go | 116 +++++++++++++++--- .../providers/aws/resource_aws_db_instance.go | 93 +++++++------- .../providers/aws/resource_aws_ebs_volume.go | 23 ++-- .../aws/resource_aws_ebs_volume_test.go | 87 ++++++++++--- ...esource_aws_elastic_transcoder_pipeline.go | 55 +++++---- ...ce_aws_elastic_transcoder_pipeline_test.go | 90 +++++++++++++- ...ce_aws_kinesis_firehose_delivery_stream.go | 5 +- .../providers/aws/resource_aws_rds_cluster.go | 55 +++++---- .../aws/resource_aws_rds_cluster_test.go | 12 +- .../aws/resource_aws_redshift_cluster.go | 69 ++++++----- .../aws/resource_aws_redshift_cluster_test.go | 89 ++++++++++++-- .../aws/resource_aws_s3_bucket_object.go | 33 ++--- .../aws/resource_aws_ses_receipt_rule.go | 89 +++++++------- .../providers/aws/r/db_instance.html.markdown | 4 +- .../docs/providers/aws/r/ebs_volume.html.md | 2 +- ...sis_firehose_delivery_stream.html.markdown | 2 +- .../providers/aws/r/rds_cluster.html.markdown | 2 +- .../aws/r/redshift_cluster.html.markdown | 2 +- .../aws/r/s3_bucket_object.html.markdown | 4 +- 21 files changed, 579 insertions(+), 286 deletions(-) diff --git a/builtin/providers/aws/data_source_aws_s3_bucket_object_test.go b/builtin/providers/aws/data_source_aws_s3_bucket_object_test.go index 7c7c7e922..ecfae9098 100644 --- a/builtin/providers/aws/data_source_aws_s3_bucket_object_test.go +++ b/builtin/providers/aws/data_source_aws_s3_bucket_object_test.go @@ -107,7 +107,7 @@ func TestAccDataSourceAWSS3BucketObject_kmsEncrypted(t *testing.T) { resource.TestMatchResourceAttr("data.aws_s3_bucket_object.obj", "etag", regexp.MustCompile("^[a-f0-9]{32}$")), resource.TestCheckResourceAttr("data.aws_s3_bucket_object.obj", "server_side_encryption", "aws:kms"), resource.TestMatchResourceAttr("data.aws_s3_bucket_object.obj", "sse_kms_key_id", - regexp.MustCompile("^arn:aws:kms:us-west-2:[0-9]{12}:key/[a-z0-9-]{36}$")), + regexp.MustCompile("^arn:aws:kms:[a-z]{2}-[a-z]+-\\d{1}:[0-9]{12}:key/[a-z0-9-]{36}$")), resource.TestMatchResourceAttr("data.aws_s3_bucket_object.obj", "last_modified", regexp.MustCompile("^[a-zA-Z]{3}, [0-9]+ [a-zA-Z]+ [0-9]{4} [0-9:]+ [A-Z]+$")), resource.TestCheckResourceAttr("data.aws_s3_bucket_object.obj", "body", "Keep Calm and Carry On"), diff --git a/builtin/providers/aws/resource_aws_cloudtrail.go b/builtin/providers/aws/resource_aws_cloudtrail.go index 00a3557ec..8c59a5d94 100644 --- a/builtin/providers/aws/resource_aws_cloudtrail.go +++ b/builtin/providers/aws/resource_aws_cloudtrail.go @@ -20,60 +20,61 @@ func resourceAwsCloudTrail() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "enable_logging": &schema.Schema{ + "enable_logging": { Type: schema.TypeBool, Optional: true, Default: true, }, - "s3_bucket_name": &schema.Schema{ + "s3_bucket_name": { Type: schema.TypeString, Required: true, }, - "s3_key_prefix": &schema.Schema{ + "s3_key_prefix": { Type: schema.TypeString, Optional: true, }, - "cloud_watch_logs_role_arn": &schema.Schema{ + "cloud_watch_logs_role_arn": { Type: schema.TypeString, Optional: true, }, - "cloud_watch_logs_group_arn": &schema.Schema{ + "cloud_watch_logs_group_arn": { Type: schema.TypeString, Optional: true, }, - "include_global_service_events": &schema.Schema{ + "include_global_service_events": { Type: schema.TypeBool, Optional: true, Default: true, }, - "is_multi_region_trail": &schema.Schema{ + "is_multi_region_trail": { Type: schema.TypeBool, Optional: true, Default: false, }, - "sns_topic_name": &schema.Schema{ + "sns_topic_name": { Type: schema.TypeString, Optional: true, }, - "enable_log_file_validation": &schema.Schema{ + "enable_log_file_validation": { Type: schema.TypeBool, Optional: true, Default: false, }, - "kms_key_id": &schema.Schema{ - Type: schema.TypeString, - Optional: true, + "kms_key_id": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validateArn, }, - "home_region": &schema.Schema{ + "home_region": { Type: schema.TypeString, Computed: true, }, - "arn": &schema.Schema{ + "arn": { Type: schema.TypeString, Computed: true, }, diff --git a/builtin/providers/aws/resource_aws_cloudtrail_test.go b/builtin/providers/aws/resource_aws_cloudtrail_test.go index 79a89cb2d..bf99e8614 100644 --- a/builtin/providers/aws/resource_aws_cloudtrail_test.go +++ b/builtin/providers/aws/resource_aws_cloudtrail_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + "regexp" ) func TestAccAWSCloudTrail_basic(t *testing.T) { @@ -21,7 +22,7 @@ func TestAccAWSCloudTrail_basic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSCloudTrailDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -30,7 +31,7 @@ func TestAccAWSCloudTrail_basic(t *testing.T) { testAccCheckCloudTrailKmsKeyIdEquals("aws_cloudtrail.foobar", "", &trail), ), }, - resource.TestStep{ + { Config: testAccAWSCloudTrailConfigModified(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -53,7 +54,7 @@ func TestAccAWSCloudTrail_enable_logging(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSCloudTrailDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -64,7 +65,7 @@ func TestAccAWSCloudTrail_enable_logging(t *testing.T) { testAccCheckCloudTrailKmsKeyIdEquals("aws_cloudtrail.foobar", "", &trail), ), }, - resource.TestStep{ + { Config: testAccAWSCloudTrailConfigModified(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -73,7 +74,7 @@ func TestAccAWSCloudTrail_enable_logging(t *testing.T) { testAccCheckCloudTrailKmsKeyIdEquals("aws_cloudtrail.foobar", "", &trail), ), }, - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -95,7 +96,7 @@ func TestAccAWSCloudTrail_is_multi_region(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSCloudTrailDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -104,7 +105,7 @@ func TestAccAWSCloudTrail_is_multi_region(t *testing.T) { testAccCheckCloudTrailKmsKeyIdEquals("aws_cloudtrail.foobar", "", &trail), ), }, - resource.TestStep{ + { Config: testAccAWSCloudTrailConfigMultiRegion(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -113,7 +114,7 @@ func TestAccAWSCloudTrail_is_multi_region(t *testing.T) { testAccCheckCloudTrailKmsKeyIdEquals("aws_cloudtrail.foobar", "", &trail), ), }, - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -130,14 +131,12 @@ func TestAccAWSCloudTrail_logValidation(t *testing.T) { var trail cloudtrail.Trail cloudTrailRandInt := acctest.RandInt() - // TODO: Add test for KMS Key ID - // once https://github.com/hashicorp/terraform/pull/3928 is merged resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSCloudTrailDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig_logValidation(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -147,7 +146,7 @@ func TestAccAWSCloudTrail_logValidation(t *testing.T) { testAccCheckCloudTrailKmsKeyIdEquals("aws_cloudtrail.foobar", "", &trail), ), }, - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig_logValidationModified(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -161,6 +160,30 @@ func TestAccAWSCloudTrail_logValidation(t *testing.T) { }) } +func TestAccAWSCloudTrail_kmsKey(t *testing.T) { + var trail cloudtrail.Trail + cloudTrailRandInt := acctest.RandInt() + keyRegex := regexp.MustCompile("^arn:aws:([a-zA-Z0-9\\-])+:([a-z]{2}-[a-z]+-\\d{1})?:(\\d{12})?:(.*)$") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSCloudTrailDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSCloudTrailConfig_kmsKey(cloudTrailRandInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), + resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "s3_key_prefix", ""), + resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "include_global_service_events", "true"), + testAccCheckCloudTrailLogValidationEnabled("aws_cloudtrail.foobar", false, &trail), + resource.TestMatchResourceAttr("aws_cloudtrail.foobar", "kms_key_id", keyRegex), + ), + }, + }, + }) +} + func TestAccAWSCloudTrail_tags(t *testing.T) { var trail cloudtrail.Trail var trailTags []*cloudtrail.Tag @@ -172,7 +195,7 @@ func TestAccAWSCloudTrail_tags(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSCloudTrailDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig_tags(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -183,7 +206,7 @@ func TestAccAWSCloudTrail_tags(t *testing.T) { testAccCheckCloudTrailKmsKeyIdEquals("aws_cloudtrail.foobar", "", &trail), ), }, - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig_tagsModified(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -194,7 +217,7 @@ func TestAccAWSCloudTrail_tags(t *testing.T) { testAccCheckCloudTrailKmsKeyIdEquals("aws_cloudtrail.foobar", "", &trail), ), }, - resource.TestStep{ + { Config: testAccAWSCloudTrailConfig_tagsModifiedAgain(cloudTrailRandInt), Check: resource.ComposeTestCheckFunc( testAccCheckCloudTrailExists("aws_cloudtrail.foobar", &trail), @@ -577,6 +600,69 @@ POLICY `, cloudTrailRandInt, cloudTrailRandInt, cloudTrailRandInt, cloudTrailRandInt) } +func testAccAWSCloudTrailConfig_kmsKey(cloudTrailRandInt int) string { + return fmt.Sprintf(` +resource "aws_kms_key" "foo" { + description = "Terraform acc test %d" + policy = <