-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
providers/aws: Add ELB Access Logs (supersedes #3708) #3756
Conversation
- continues #3708 - adds some tests - other fixes I found along the way
@tpounds I made a few revisions, let me know if you have questions or see something I shouldn't have done 😄 |
@catsby LGTM. Thanks for picking this back up! I forgot to push the website docs change to by pull request last night. Feel free to cherry-pick it if you like. https://github.com/tpounds/terraform/commit/a93212e1dba7212f6655efbbb3456494e8d84049 |
@tpounds ah, duh, totally forgot I was supposed to add docs 😆 I cherry-picked yours and updated them to remove the |
@catsby Still LGTM. Thanks again! |
@@ -305,6 +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) | |||
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we end up with an error here - we probably want to return it, no? Seems like an error here would be a bug we'd need to fix, so better to make it obvious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense; updated in 8c32536
* master: (95 commits) Update CHANGELOG.md Update CHANGELOG.md Update CHANGELOG.md Update CHANGELOG.md upgrade a warning to error add some logging around create/update requests for IAM user Update CHANGELOG.md Update CHANGELOG.md Build using `make test` on Travis CI Update CHANGELOG.md provider/aws: Fix error format in Kinesis Firehose Update CHANGELOG.md Changes to Aws Kinesis Firehouse Docs Update CHANGELOG.md modify aws_iam_user_test to correctly check username and path for initial and changed username/path Update CHANGELOG.md Update CHANGELOG.md Prompt for input variables before context validate Removing the AWS DBInstance Acceptance Test for withoutEngine as this is now part of the checkInstanceAttributes func Making engine_version be computed in the db_instance provider ...
Looks good! 🚢 |
providers/aws: Add `access_logs` to ELB resource [GH-3756]
I've been trying to see exactly where this is resolved. Any help? Error Message
Configuration files enable_deletion_protection = false tags { /* stickiness { health_check { tags { /* stickiness { health_check { tags { /* |
* Add support for aws_wafregional_rule * Updating docs * Use singular name for predicate field * Use helpers * Use flatten func * Add test for rule without predicates * Add test for predicate changes * Require data_id for predicate * Reformat imports + remove redundant code
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. |
Builds off of #3708, adding tests and fixing some issues that I found to cause panics if not done perfectly right.