provider/aws: Allow disabled access_log in ELB (#11120)

* provider/aws: Save disabled ELB accesslogs to state

Save any explicitly disabled access_log to state. Do not save disabled
access_logs if they are not in the configuration.

* test that fails on master
This commit is contained in:
Clint 2017-01-09 17:10:58 -06:00 committed by Paul Stack
parent 5a9a44a36e
commit c0d0f711d9
3 changed files with 155 additions and 36 deletions

View File

@ -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

View File

@ -82,9 +82,11 @@ 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",
@ -99,15 +101,62 @@ func TestAccAWSELB_AccessLogs(t *testing.T) {
},
resource.TestStep{
Config: testAccAWSELBAccessLogsOn,
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", "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", "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",
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSELBDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSELBAccessLogs,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSELBExists("aws_elb.foo", &conf),
),
},
resource.TestStep{
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", 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 = <<EOF
@ -1013,7 +1064,7 @@ resource "aws_s3_bucket" "acceslogs_bucket" {
"Principal": {
"AWS": "arn:aws:iam::797873946194:root"
},
"Resource": "arn:aws:s3:::terraform-access-logs-bucket/*",
"Resource": "arn:aws:s3:::%s/*",
"Sid": "Stmt1446575236270"
}
],
@ -1037,7 +1088,55 @@ resource "aws_elb" "foo" {
bucket = "${aws_s3_bucket.acceslogs_bucket.bucket}"
}
}
`
`, r, r)
}
func testAccAWSELBAccessLogsDisabled(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 = "%s"
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:::%s/*",
"Sid": "Stmt1446575236270"
}
],
"Version": "2012-10-17"
}
EOF
}
resource "aws_elb" "foo" {
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}"
enabled = false
}
}
`, r, r)
}
const testAccAWSELBGeneratedName = `
resource "aws_elb" "foo" {

View File

@ -360,27 +360,29 @@ func expandElastiCacheParameters(configured []interface{}) ([]*elasticache.Param
func flattenAccessLog(l *elb.AccessLog) []map[string]interface{} {
result := make([]map[string]interface{}, 0, 1)
if l != nil && *l.Enabled {
r := make(map[string]interface{})
if l.S3BucketName != nil {
r["bucket"] = *l.S3BucketName
}
if l.S3BucketPrefix != nil {
r["bucket_prefix"] = *l.S3BucketPrefix
}
if l.EmitInterval != nil {
r["interval"] = *l.EmitInterval
}
if l.Enabled != nil {
r["enabled"] = *l.Enabled
}
result = append(result, r)
if l == nil {
return nil
}
r := make(map[string]interface{})
if l.S3BucketName != nil {
r["bucket"] = *l.S3BucketName
}
if l.S3BucketPrefix != nil {
r["bucket_prefix"] = *l.S3BucketPrefix
}
if l.EmitInterval != nil {
r["interval"] = *l.EmitInterval
}
if l.Enabled != nil {
r["enabled"] = *l.Enabled
}
result = append(result, r)
return result
}