diff --git a/builtin/providers/aws/resource_aws_elb.go b/builtin/providers/aws/resource_aws_elb.go index 6f2fb610c6cd..91d523d3f363 100644 --- a/builtin/providers/aws/resource_aws_elb.go +++ b/builtin/providers/aws/resource_aws_elb.go @@ -115,6 +115,7 @@ func resourceAwsElb() *schema.Resource { "access_logs": &schema.Schema{ Type: schema.TypeList, Optional: true, + MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "interval": &schema.Schema{ @@ -392,7 +393,26 @@ func resourceAwsElbRead(d *schema.ResourceData, meta interface{}) error { d.Set("connection_draining_timeout", lbAttrs.ConnectionDraining.Timeout) d.Set("cross_zone_load_balancing", lbAttrs.CrossZoneLoadBalancing.Enabled) if lbAttrs.AccessLog != nil { - if err := d.Set("access_logs", flattenAccessLog(lbAttrs.AccessLog)); err != nil { + // The AWS API does not allow users to remove access_logs, only disable them. + // During creation of the ELB, Terraform sets the access_logs to disabled, + // so there should not be a case where lbAttrs.AccessLog above is nil. + + // Here we do not record the remove value of access_log if: + // - there is no access_log block in the configuration + // - the remote access_logs are disabled + // + // This indicates there is no access_log in the configuration. + // - externally added access_logs will be enabled, so we'll detect the drift + // - locally added access_logs will be in the config, so we'll add to the + // API/state + // See https://github.com/hashicorp/terraform/issues/10138 + _, n := d.GetChange("access_logs") + elbal := lbAttrs.AccessLog + nl := n.([]interface{}) + if len(nl) == 0 && !*elbal.Enabled { + elbal = nil + } + if err := d.Set("access_logs", flattenAccessLog(elbal)); err != nil { return err } } @@ -533,18 +553,16 @@ func resourceAwsElbUpdate(d *schema.ResourceData, meta interface{}) error { } logs := d.Get("access_logs").([]interface{}) - if len(logs) > 1 { - return fmt.Errorf("Only one access logs config per ELB is supported") - } else if len(logs) == 1 { - log := logs[0].(map[string]interface{}) + if len(logs) == 1 { + l := logs[0].(map[string]interface{}) accessLog := &elb.AccessLog{ - Enabled: aws.Bool(log["enabled"].(bool)), - EmitInterval: aws.Int64(int64(log["interval"].(int))), - S3BucketName: aws.String(log["bucket"].(string)), + Enabled: aws.Bool(l["enabled"].(bool)), + EmitInterval: aws.Int64(int64(l["interval"].(int))), + S3BucketName: aws.String(l["bucket"].(string)), } - if log["bucket_prefix"] != "" { - accessLog.S3BucketPrefix = aws.String(log["bucket_prefix"].(string)) + if l["bucket_prefix"] != "" { + accessLog.S3BucketPrefix = aws.String(l["bucket_prefix"].(string)) } attrs.LoadBalancerAttributes.AccessLog = accessLog diff --git a/builtin/providers/aws/resource_aws_elb_test.go b/builtin/providers/aws/resource_aws_elb_test.go index 0115705fd255..763bd6a2ca4d 100644 --- a/builtin/providers/aws/resource_aws_elb_test.go +++ b/builtin/providers/aws/resource_aws_elb_test.go @@ -82,9 +82,56 @@ func TestAccAWSELB_fullCharacterRange(t *testing.T) { }) } -func TestAccAWSELB_AccessLogs(t *testing.T) { +func TestAccAWSELB_AccessLogs_enabled(t *testing.T) { var conf elb.LoadBalancerDescription + rName := fmt.Sprintf("terraform-access-logs-bucket-%d", acctest.RandInt()) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_elb.foo", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSELBDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSELBAccessLogs, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSELBExists("aws_elb.foo", &conf), + ), + }, + + resource.TestStep{ + Config: testAccAWSELBAccessLogsOn(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSELBExists("aws_elb.foo", &conf), + resource.TestCheckResourceAttr( + "aws_elb.foo", "access_logs.#", "1"), + resource.TestCheckResourceAttr( + "aws_elb.foo", "access_logs.0.bucket", rName), + resource.TestCheckResourceAttr( + "aws_elb.foo", "access_logs.0.interval", "5"), + resource.TestCheckResourceAttr( + "aws_elb.foo", "access_logs.0.enabled", "true"), + ), + }, + + resource.TestStep{ + Config: testAccAWSELBAccessLogs, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSELBExists("aws_elb.foo", &conf), + resource.TestCheckResourceAttr( + "aws_elb.foo", "access_logs.#", "0"), + ), + }, + }, + }) +} + +func TestAccAWSELB_AccessLogs_disabled(t *testing.T) { + var conf elb.LoadBalancerDescription + + rName := fmt.Sprintf("terraform-access-logs-bucket-%d", acctest.RandInt()) + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, IDRefreshName: "aws_elb.foo", @@ -99,15 +146,17 @@ func TestAccAWSELB_AccessLogs(t *testing.T) { }, resource.TestStep{ - Config: testAccAWSELBAccessLogsOn, + Config: testAccAWSELBAccessLogsDisabled(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSELBExists("aws_elb.foo", &conf), resource.TestCheckResourceAttr( "aws_elb.foo", "access_logs.#", "1"), resource.TestCheckResourceAttr( - "aws_elb.foo", "access_logs.0.bucket", "terraform-access-logs-bucket"), + "aws_elb.foo", "access_logs.0.bucket", rName), resource.TestCheckResourceAttr( "aws_elb.foo", "access_logs.0.interval", "5"), + resource.TestCheckResourceAttr( + "aws_elb.foo", "access_logs.0.enabled", "false"), ), }, @@ -995,12 +1044,14 @@ resource "aws_elb" "foo" { } } ` -const testAccAWSELBAccessLogsOn = ` + +func testAccAWSELBAccessLogsOn(r string) string { + return fmt.Sprintf(` # an S3 bucket configured for Access logs # The 797873946194 is the AWS ID for us-west-2, so this test # must be ran in us-west-2 resource "aws_s3_bucket" "acceslogs_bucket" { - bucket = "terraform-access-logs-bucket" + bucket = "%s" acl = "private" force_destroy = true policy = <