resource/wafv2_web_acl_logging_configuration: new resource #13892

anGie44 · 2020-06-23T08:16:47Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

resource/wafv2_web_acl_logging_configuration: new resource to start logging from a web_acl

Output from acceptance testing:

--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_basic (129.41s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_webACLDisappears (133.18s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_disappears (167.98s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_update (221.72s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeResourceARNForceNew (239.38s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeLogDestinationConfigsForceNew (225.34s)

bflad

Overall this is looking pretty great, just a few minor things and should be good to go!

Output from acceptance testing:

--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_basic (127.73s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_disappears (165.43s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeResourceArnForceNew (167.43s)
--- FAIL: TestAccAwsWafv2WebACLLoggingConfiguration_changeLogDestinationConfigsForceNew (184.56s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_update (194.59s)

I'm having trouble getting that failing test to pass, is there something special that needs to be done?

--- FAIL: TestAccAwsWafv2WebACLLoggingConfiguration_changeLogDestinationConfigsForceNew (157.92s)
testing.go:684: Step 1 error: errors during apply:
Error: WAFLimitsExceededException: AWS WAF couldn’t perform the operation because you exceeded your resource limit.

aws/resource_aws_wafv2_web_acl_logging_configuration.go

aws/resource_aws_wafv2_web_acl_logging_configuration_test.go

bflad · 2020-06-23T23:53:27Z

aws/resource_aws_wafv2_web_acl_logging_configuration_test.go

+	  "Resource": "arn:aws:iam::*:role/aws-service-role/wafv2.amazonaws.com/AWSServiceRoleForWAFV2Logging",
+	  "Condition": {"StringLike": {"iam:AWSServiceName": "wafv2.amazonaws.com"}}


Nit: To allow this configuration to work across AWS partitions, this should utilize the aws_partition data source, e.g.

data "aws_partition" "current" {} # ... "Resource": "arn:${data.aws_partition.current.partition}:iam::*:role/aws-service-role/wafv2.${data.aws_partition.current.dns_suffix}/AWSServiceRoleForWAFV2Logging", "Condition": {"StringLike": {"iam:AWSServiceName": "wafv2.${data.aws_partition.current.dns_suffix}"}}

Looks like this might have auto-resolved -- see the aws_partition data source but not it being used here. 😄

oh wow totally missed where it was used -- i'll add just in case

website/docs/r/wafv2_web_acl_logging_configuration.html.markdown

anGie44 · 2020-06-24T16:13:33Z

I'm having trouble getting that failing test to pass, is there something special that needs to be done?

hmm it might be that a WebACL can actually only have 1 logging destination even though the docs suggest you could provide multiple kinesis ARNs 🤔 https://docs.aws.amazon.com/waf/latest/APIReference/API_LoggingConfiguration.html#WAF-Type-LoggingConfiguration-LogDestinationConfigs .. I thought maybe it was the number of requests (like 1 PutLoggingConfiguration per kinesis destination), but even in the AWS console (the CLI as well), it only allows you to select one kinesis stream from the dropdown when editing the logging details of the web ACL. unless this is something supported if a quota increase is requested?

I've left the schema as-is to keep inline w/AWS but noted the limitation in the docs (also added the question in the AWS sdk repo in case I'm missing something aws/aws-sdk-go#3390).

bflad

Looks like the concurrent testing is finding a locking issue with the PutLoggingConfiguration API. We should add automatic retrying logic in aws/config.go to handle this situation.

First run:

--- FAIL: TestAccAwsWafv2WebACLLoggingConfiguration_changeResourceARNForceNew (77.91s)
testing.go:684: Step 0 error: errors during apply:
Error: error putting WAFv2 Logging Configuration for resource (arn:aws:wafv2:us-west-2:*******:regional/webacl/tf-acc-test-5540436077421452665/941ff8df-a73e-42a1-95d8-d0e49f20af54): WAFServiceLinkedRoleErrorException: AWS WAF couldn’t obtain a service-linked role (SLR) lock for your operation because the lock was in use. Retry your operation.

Next run:

--- FAIL: TestAccAwsWafv2WebACLLoggingConfiguration_changeLogDestinationConfigsForceNew (104.71s)
testing.go:684: Step 0 error: errors during apply:
Error: error putting WAFv2 Logging Configuration for resource (arn:aws:wafv2:us-west-2:*******:regional/webacl/tf-acc-test-231078216715910326/d783a407-2d2f-4568-b7f1-6254c7cf4696): WAFServiceLinkedRoleErrorException: AWS WAF couldn’t obtain a service-linked role (SLR) lock for your operation because the lock was in use. Retry your operation.

e.g. something like this near the others:

		if isAWSErr(r.Error, wafv2.ErrCodeWAFServiceLinkedRoleErrorException, "Retry your operation") {
			r.Retryable = aws.Bool(true)
		}

Otherwise, looking good 👍

aws/resource_aws_wafv2_web_acl_logging_configuration_test.go

bflad · 2020-06-24T18:15:44Z

aws/resource_aws_wafv2_web_acl_logging_configuration_test.go

+	  "Resource": "arn:aws:iam::*:role/aws-service-role/wafv2.amazonaws.com/AWSServiceRoleForWAFV2Logging",
+	  "Condition": {"StringLike": {"iam:AWSServiceName": "wafv2.amazonaws.com"}}


Looks like this might have auto-resolved -- see the aws_partition data source but not it being used here. 😄

…rs/terraform-provider-aws into f-wafv2-logging-config

itsSaad · 2020-06-25T08:36:24Z

@anGie44 Will this be able to go out with 2.68.0 release?

bflad · 2020-06-25T13:55:32Z

@itsSaad this will be released when it has a maintainer approved review, which is currently in progress. I would anticipate that it will likely be with today's release, but it may be next week.

bflad

LGTM 🚀

Output from acceptance testing:

--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_webACLDisappears (114.82s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_basic (139.50s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_disappears (150.11s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_update (188.56s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeResourceARNForceNew (214.90s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeLogDestinationConfigsForceNew (235.59s)

* origin/master: (59 commits) Update CHANGELOG for hashicorp#13935 resource/aws_batch_compute_environment: Remove resource from Terraform state when not found instead of returning error (hashicorp#13935) resource/aws_dynamodb_table: Return error instead of panic on empty CreateTable response (hashicorp#13925) Update CHANGELOG for hashicorp#13918 New Data Source: aws_efs_access_points (hashicorp#13918) tests/resource/aws_instance: Ensure sweeper has dependencies on resources that manage EC2 Instances (hashicorp#13917) Update CHANGELOG for hashicorp#13937 Update CHANGELOG for hashicorp#5448 resource/aws_cloudtrail: Handle single event selector in cloudtrail with non-default read_write_type (hashicorp#5448) Update CHANGELOG for hashicorp#13892 correct retry message to match in error handling correct import resource name accept empty string in volume_type validation Update CHANGELOG for hashicorp#4855 resource/aws_batch_compute_environment: Support fully optional desired_vcpus and wait for updates add retry error handling for SLR remove unused WebACL resource name in disappears test reference current AWS partition in iam role policy stmt remove duplicated disappears test step Update CHANGELOG for hashicorp#13926 ...

ghost · 2020-06-26T00:39:43Z

This has been released in version 2.68.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2020-07-25T17:11:08Z

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

anGie44 added 2 commits June 23, 2020 03:54

resource/web_acl_logging_configuration: new resource

14fde22

set redacted_fields to empty arr if not set

bc2caf5

anGie44 added 5 commits June 23, 2020 11:58

update iam_role_policy name reference

4a3e373

add note to docs

0184ffb

update test cases and attribute checks

ad4775b

update test cases and attribute checks

4a164ee

update disappears test and string formatting in resource read msg

efe1b31

anGie44 marked this pull request as ready for review June 23, 2020 22:29

anGie44 requested a review from a team June 23, 2020 22:29

bflad self-assigned this Jun 23, 2020

bflad added the new-resource Introduces a new resource. label Jun 23, 2020

bflad suggested changes Jun 24, 2020

View reviewed changes

CR updates

b21ae45

anGie44 requested a review from bflad June 24, 2020 17:33

bflad reviewed Jun 24, 2020

View reviewed changes

anGie44 added 5 commits June 24, 2020 16:04

remove duplicated disappears test step

4367eca

reference current AWS partition in iam role policy stmt

be6f984

remove unused WebACL resource name in disappears test

a49add5

add retry error handling for SLR

654a607

Merge branch 'f-wafv2-logging-config' of github.com:terraform-provide…

7d49a15

…rs/terraform-provider-aws into f-wafv2-logging-config

anGie44 force-pushed the f-wafv2-logging-config branch from ce5e625 to 7d49a15 Compare June 24, 2020 21:02

correct retry message to match in error handling

cb7a0b3

bflad approved these changes Jun 25, 2020

View reviewed changes

anGie44 merged commit 62468b9 into master Jun 25, 2020

anGie44 deleted the f-wafv2-logging-config branch June 25, 2020 14:28

anGie44 added this to the v2.68.0 milestone Jun 25, 2020

anGie44 added a commit that referenced this pull request Jun 25, 2020

Update CHANGELOG for #13892

355049b

hashicorp locked and limited conversation to collaborators Jul 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/wafv2_web_acl_logging_configuration: new resource #13892

resource/wafv2_web_acl_logging_configuration: new resource #13892

anGie44 commented Jun 23, 2020 •

edited

bflad left a comment

bflad Jun 23, 2020

bflad Jun 24, 2020

anGie44 Jun 24, 2020

anGie44 commented Jun 24, 2020 •

edited

bflad left a comment

bflad Jun 24, 2020

itsSaad commented Jun 25, 2020

bflad commented Jun 25, 2020

bflad left a comment

ghost commented Jun 26, 2020

ghost commented Jul 25, 2020

		"Resource": "arn:aws:iam::*:role/aws-service-role/wafv2.amazonaws.com/AWSServiceRoleForWAFV2Logging",
		"Condition": {"StringLike": {"iam:AWSServiceName": "wafv2.amazonaws.com"}}

resource/wafv2_web_acl_logging_configuration: new resource #13892

resource/wafv2_web_acl_logging_configuration: new resource #13892

Conversation

anGie44 commented Jun 23, 2020 • edited

Community Note

bflad left a comment

Choose a reason for hiding this comment

bflad Jun 23, 2020

Choose a reason for hiding this comment

bflad Jun 24, 2020

Choose a reason for hiding this comment

anGie44 Jun 24, 2020

Choose a reason for hiding this comment

anGie44 commented Jun 24, 2020 • edited

bflad left a comment

Choose a reason for hiding this comment

bflad Jun 24, 2020

Choose a reason for hiding this comment

itsSaad commented Jun 25, 2020

bflad commented Jun 25, 2020

bflad left a comment

Choose a reason for hiding this comment

ghost commented Jun 26, 2020

ghost commented Jul 25, 2020

anGie44 commented Jun 23, 2020 •

edited

anGie44 commented Jun 24, 2020 •

edited