Enable/Disable Option For ELB Access logs

[sharmaansh21]

Fixes #7721
/cc @stack72

[jen20]

Hi @optimisticanshul! I considered adding this to ALBs last week - my question is why you would have an enabled option vs just adding or removing the block?

[sharmaansh21]

@jen20 I agree with the reason provided in #7721

[jen20]

Hi @optimisticanshul - the reason seems valid - one thing I am concerned about here is that this may not be backwards compatible (because of the boolean defaulting to true). It'd be a lot easier to merge if there were some acceptance tests around the change - perhaps one that creates an ELB with no access logs, adds disabled, enables, disables and then removes the access logs block. Is this something you might be able to take a look at?

[sharmaansh21]

@jen20 Yes i can do that, but i don't think this will break backward compatibility because of https://github.com/optimisticanshul/terraform/blob/master/builtin/providers/aws/resource_aws_elb.go#L523 this set enabled to true.

[stack72]

Hi @optimisticanshul

This LGTM!

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSELB_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/29 21:01:30 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSELB_ -timeout 120m
=== RUN   TestAccAWSELB_importBasic
--- PASS: TestAccAWSELB_importBasic (34.61s)
=== RUN   TestAccAWSELB_basic
--- PASS: TestAccAWSELB_basic (34.46s)
=== RUN   TestAccAWSELB_fullCharacterRange
--- PASS: TestAccAWSELB_fullCharacterRange (32.31s)
=== RUN   TestAccAWSELB_AccessLogs
--- PASS: TestAccAWSELB_AccessLogs (102.09s)
=== RUN   TestAccAWSELB_generatedName
--- PASS: TestAccAWSELB_generatedName (32.34s)
=== RUN   TestAccAWSELB_availabilityZones
--- PASS: TestAccAWSELB_availabilityZones (52.91s)
=== RUN   TestAccAWSELB_tags
--- PASS: TestAccAWSELB_tags (54.24s)
=== RUN   TestAccAWSELB_iam_server_cert
--- PASS: TestAccAWSELB_iam_server_cert (41.59s)
=== RUN   TestAccAWSELB_InstanceAttaching
--- PASS: TestAccAWSELB_InstanceAttaching (174.95s)
=== RUN   TestAccAWSELB_HealthCheck
--- PASS: TestAccAWSELB_HealthCheck (32.78s)
=== RUN   TestAccAWSELB_Timeout
--- PASS: TestAccAWSELB_Timeout (31.13s)
=== RUN   TestAccAWSELB_ConnectionDraining
--- PASS: TestAccAWSELB_ConnectionDraining (29.83s)
=== RUN   TestAccAWSELB_SecurityGroups
--- PASS: TestAccAWSELB_SecurityGroups (64.35s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    717.613s

One thing, this didn't have docs - so i added them d330ac4

Thanks for the work here :)

Paul

[krushi90]

Thanks for working on this :)

[sharmaansh21]

@krushi90 welcome 🎉

[brikis98]

Is there an equivalent of this for the ALB (aws_alb)?

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.