providers/aws: Add ELB Access Logs (continues #3708)

- continues #3708
- adds some tests
- other fixes I found along the way

builtin/providers/aws/resource_aws_elb.go

@@ -106,11 +106,6 @@ func resourceAwsElb() *schema.Resource {
 				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
-						"enabled": &schema.Schema{
-							Type:     schema.TypeBool,
-							Optional: true,
-							Default:  true,
-						},
 						"interval": &schema.Schema{
 							Type:     schema.TypeInt,
 							Optional: true,
@@ -333,7 +328,11 @@ func resourceAwsElbRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("idle_timeout", lbAttrs.ConnectionSettings.IdleTimeout)
 	d.Set("connection_draining", lbAttrs.ConnectionDraining.Enabled)
 	d.Set("connection_draining_timeout", lbAttrs.ConnectionDraining.Timeout)
-	d.Set("access_logs", flattenAccessLog(lbAttrs.AccessLog))
+	if lbAttrs.AccessLog != nil {
+		if err := d.Set("access_logs", flattenAccessLog(lbAttrs.AccessLog)); err != nil {
+			log.Printf("[WARN] Error setting ELB Access Logs for (%s): %s", d.Id(), err)
+		}
+	}
 
 	resp, err := elbconn.DescribeTags(&elb.DescribeTagsInput{
 		LoadBalancerNames: []*string{lb.LoadBalancerName},
@@ -453,7 +452,7 @@ func resourceAwsElbUpdate(d *schema.ResourceData, meta interface{}) error {
 		} else if len(logs) == 1 {
 			log := logs[0].(map[string]interface{})
 			accessLogs := &elb.AccessLog{
-				Enabled:      aws.Bool(log["enabled"].(bool)),
+				Enabled:      aws.Bool(true),
 				EmitInterval: aws.Int64(int64(log["interval"].(int))),
 				S3BucketName: aws.String(log["bucket"].(string)),
 			}
@@ -463,8 +462,14 @@ func resourceAwsElbUpdate(d *schema.ResourceData, meta interface{}) error {
 			}
 
 			attrs.LoadBalancerAttributes.AccessLog = accessLogs
+		} else if len(logs) == 0 {
+			// disable access logs
+			attrs.LoadBalancerAttributes.AccessLog = &elb.AccessLog{
+				Enabled: aws.Bool(false),
+			}
 		}
 
+		log.Printf("[DEBUG] ELB Modify Load Balancer Attributes Request: %#v", attrs)
 		_, err := elbconn.ModifyLoadBalancerAttributes(&attrs)
 		if err != nil {
 			return fmt.Errorf("Failure configuring ELB attributes: %s", err)
@@ -600,7 +605,6 @@ func resourceAwsElbHealthCheckHash(v interface{}) int {
 func resourceAwsElbAccessLogsHash(v interface{}) int {
 	var buf bytes.Buffer
 	m := v.(map[string]interface{})
-	buf.WriteString(fmt.Sprintf("%t-", m["enabled"].(bool)))
 	buf.WriteString(fmt.Sprintf("%d-", m["interval"].(int)))
 	buf.WriteString(fmt.Sprintf("%s-",
 		strings.ToLower(m["bucket"].(string))))

builtin/providers/aws/resource_aws_elb_test.go

@@ -75,6 +75,52 @@ func TestAccAWSELB_fullCharacterRange(t *testing.T) {
 	})
 }
 
+func TestAccAWSELB_AccessLogs(t *testing.T) {
+	var conf elb.LoadBalancerDescription
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSELBDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSELBAccessLogs,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSELBExists("aws_elb.foo", &conf),
+					resource.TestCheckResourceAttr(
+						"aws_elb.foo", "name", "FoobarTerraform-test123"),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccAWSELBAccessLogsOn,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSELBExists("aws_elb.foo", &conf),
+					resource.TestCheckResourceAttr(
+						"aws_elb.foo", "name", "FoobarTerraform-test123"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.foo", "access_logs.#", "1"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.foo", "access_logs.1713209538.bucket", "terraform-access-logs-bucket"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.foo", "access_logs.1713209538.interval", "5"),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccAWSELBAccessLogs,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSELBExists("aws_elb.foo", &conf),
+					resource.TestCheckResourceAttr(
+						"aws_elb.foo", "name", "FoobarTerraform-test123"),
+					resource.TestCheckResourceAttr(
+						"aws_elb.foo", "access_logs.#", "0"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAWSELB_generatedName(t *testing.T) {
 	var conf elb.LoadBalancerDescription
 	generatedNameRegexp := regexp.MustCompile("^tf-lb-")
@@ -650,6 +696,64 @@ resource "aws_elb" "foo" {
 }
 `
 
+const testAccAWSELBAccessLogs = `
+resource "aws_elb" "foo" {
+  name = "FoobarTerraform-test123"
+  availability_zones = ["us-west-2a", "us-west-2b", "us-west-2c"]
+
+  listener {
+    instance_port = 8000
+    instance_protocol = "http"
+    lb_port = 80
+    lb_protocol = "http"
+  }
+}
+`
+const testAccAWSELBAccessLogsOn = `
+# 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"
+  acl = "private"
+  force_destroy = true
+  policy = <<EOF
+{
+  "Id": "Policy1446577137248",
+  "Statement": [
+    {
+      "Action": "s3:PutObject",
+      "Effect": "Allow",
+      "Principal": {
+        "AWS": "arn:aws:iam::797873946194:root"
+      },
+      "Resource": "arn:aws:s3:::terraform-access-logs-bucket/*",
+      "Sid": "Stmt1446575236270"
+    }
+  ],
+  "Version": "2012-10-17"
+}
+EOF
+}
+
+resource "aws_elb" "foo" {
+  name = "FoobarTerraform-test123"
+  availability_zones = ["us-west-2a", "us-west-2b", "us-west-2c"]
+
+  listener {
+    instance_port = 8000
+    instance_protocol = "http"
+    lb_port = 80
+    lb_protocol = "http"
+  }
+
+	access_logs {
+		interval = 5
+		bucket = "${aws_s3_bucket.acceslogs_bucket.bucket}"
+	}
+}
+`
+
 const testAccAWSELBGeneratedName = `
 resource "aws_elb" "foo" {
   availability_zones = ["us-west-2a", "us-west-2b", "us-west-2c"]

builtin/providers/aws/structure.go

@@ -239,12 +239,23 @@ func flattenAccessLog(log *elb.AccessLog) []map[string]interface{} {
 	result := make([]map[string]interface{}, 0, 1)
 
 	if log != nil {
-		result = append(result, map[string]interface{}{
-			"enabled":       *log.Enabled,
-			"interval":      *log.EmitInterval,
-			"bucket":        *log.S3BucketName,
-			"bucket_prefix": *log.S3BucketPrefix,
-		})
+		r := make(map[string]interface{})
+		// enabled is the only value we can rely on to not be nil
+		r["enabled"] = *log.Enabled
+
+		if log.S3BucketName != nil {
+			r["bucket"] = *log.S3BucketName
+		}
+
+		if log.S3BucketPrefix != nil {
+			r["bucket_prefix"] = *log.S3BucketPrefix
+		}
+
+		if log.EmitInterval != nil {
+			r["interval"] = *log.EmitInterval
+		}
+
+		result = append(result, r)
 	}
 
 	return result