Support access logs in application_load_balancer module #55
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vandrijevik
force-pushed
the
va-support-access-logs-for-application-load-balancers
branch
from
bf8a63b
to
5f1d626
Compare
vandrijevik
force-pushed
the
va-support-access-logs-for-application-load-balancers
branch
from
c42361a
to
923cdad
Compare
|
The failures ☝️ are not related to this PR at all, and result from Amazon releasing new AMIs, and our code automatically getting the latest version. I will address those separately. |
pturley
previously approved these changes
May 15, 2018
vandrijevik
force-pushed
the
va-support-access-logs-for-application-load-balancers
branch
2 times, most recently
from
ae243fb
to
0c7c995
Compare
Due to a Terraform bug (hashicorp/terraform#4948), if Principal is set to an account id, Terraform always reports the resource as if it needs changing (because the AWS API accepts the value, but turns it into an ARN).
vandrijevik
force-pushed
the
va-support-access-logs-for-application-load-balancers
branch
from
0c7c995
to
1b68eee
Compare
fedekau
approved these changes
May 16, 2018
vandrijevik
deleted the
va-support-access-logs-for-application-load-balancers
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR supports enabling access logs for an application load balancer. Doing so creates an S3 bucket with the appropriate permissions and a rule to move logs to Glacier after 365 days, and configures the load balancer to logs to this S3 bucket.
Given that Terraform’s
access_logsblocks requires a bucket even if logging is disabled, I had to jump through a couple of hoops to make sure that this module can be used both with and without logging: I create a load balancer without logging, or one with logging based on the variableaccess_logs_enabled, because even when access logs are disabled, the API call to AWS fails if the access logs bucket is not properly configured (and we should not create an empty S3 bucket just for this case).The Hashicorp-blessed
aws-albmodule went through the same pains, and if you’re interested you can read about them here.