Issue 1769 add aws shield protection feature ( Part II ) #7721

parabolic · 2019-02-25T14:57:24Z

Fixes #1769
Superseeds: #1899

Changes proposed in this pull request:

create a aws_shield_protection resource.
Add documentation for aws_shield_protection resource.

Output from acceptance testing:

Note: The acceptance tests where run with TF_TEST_CLOUDFRONT_RETAIN=true environment variable set in order for it not to wait the deletion of the cloudfront distribution.

make testacc TESTARGS='-run=TestAccAWSShieldProtection'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSShieldProtection -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSShieldProtectionGlobalAccelerate
--- PASS: TestAccAWSShieldProtectionGlobalAccelerate (62.34s)
=== RUN   TestAccAWSShieldProtectionElasticIPAddress
--- PASS: TestAccAWSShieldProtectionElasticIPAddress (24.12s)
=== RUN   TestAccAWSShieldProtectionAlb
--- PASS: TestAccAWSShieldProtectionAlb (272.64s)
=== RUN   TestAccAWSShieldProtectionElb
--- PASS: TestAccAWSShieldProtectionElb (54.57s)
=== RUN   TestAccAWSShieldProtectionCloudfront
--- PASS: TestAccAWSShieldProtectionCloudfront (22.61s)
=== RUN   TestAccAWSShieldProtectionRoute53
--- PASS: TestAccAWSShieldProtectionRoute53 (59.31s)
=== RUN   TestAccAWSShieldProtection_importBasic
--- PASS: TestAccAWSShieldProtection_importBasic (25.64s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	521.280s

…ield_Protection_feature

parabolic · 2019-04-01T06:18:53Z

@bflad any update on this pr? I know you guys have been busy, but we've an open ticket on our side plus I am wondering if all is good with the PR.
Cheers!

JoshiiSinfield · 2019-04-12T10:41:27Z

Hi @parabolic @radeksimko @bflad,

I/my company would be happy to test this PR if another tester is still required?
We're keen to manage the shield protection resources through Terraform.

I'm also going to add a comment to the other PR this stemmed from around subscriptions.

Cheers,
Josh

parabolic · 2019-04-12T12:10:03Z

@JoshiiSinfield thank you for your help.
Please check my comment on the other PR and let me know if you need some more clarifications.

Cheers!

steenblik · 2019-05-03T06:27:37Z

Hi @parabolic. Are you still looking for testers? I have shield and would be happy to test. I'm keen to see this merged. Thanks!

parabolic · 2019-05-03T14:30:52Z

@accuracy27 Thanks for the offer! We don't need more testers I think we've shield and I've tested it and posted the output in the description of the PR. What we need is a response from hashicorp on how do we proceed from here.

Cheers!

bflad · 2019-05-13T22:18:53Z

Hi @parabolic 👋 We're finally ready to acceptance test this! Looks like this pull request could use some love for rebasing and running the acceptance testing with its default us-west-2 region and recent Terraform 0.12 dependency updates:

TF_ACC=1 go test ./aws -v -timeout 120m -parallel 20 -run='TestAccAWSShieldProtection'
=== RUN   TestAccAWSShieldProtectionGlobalAccelerate
--- FAIL: TestAccAWSShieldProtectionGlobalAccelerate (42.80s)
    testing.go:568: Step 0 error: errors during apply:

        Error: RequestError: send request failed
        caused by: Post https://shield.us-west-2.amazonaws.com/: dial tcp: lookup shield.us-west-2.amazonaws.com: no such host

          on /var/folders/v0/_d108fkx1pbbg4_sh864_7740000gn/T/tf-test135175294/main.tf line 11:
          (source code not available)


=== RUN   TestAccAWSShieldProtectionElasticIPAddress
--- FAIL: TestAccAWSShieldProtectionElasticIPAddress (0.89s)
    testing.go:568: Step 0 error: config is invalid: Unsupported block type: Blocks of type "tags" are not expected here. Did you mean to define argument "tags"? If so, use the equals sign to assign it a value.
=== RUN   TestAccAWSShieldProtectionAlb
--- FAIL: TestAccAWSShieldProtectionAlb (0.76s)
    testing.go:568: Step 0 error: config is invalid: Unsupported block type: Blocks of type "tags" are not expected here. Did you mean to define argument "tags"? If so, use the equals sign to assign it a value.
=== RUN   TestAccAWSShieldProtectionElb
--- FAIL: TestAccAWSShieldProtectionElb (0.77s)
    testing.go:568: Step 0 error: config is invalid: Unsupported block type: Blocks of type "tags" are not expected here. Did you mean to define argument "tags"? If so, use the equals sign to assign it a value.
=== RUN   TestAccAWSShieldProtectionCloudfront
--- FAIL: TestAccAWSShieldProtectionCloudfront (0.79s)
    testing.go:568: Step 0 error: config is invalid: 2 problems:

        - Unsupported block type: Blocks of type "tags" are not expected here. Did you mean to define argument "tags"? If so, use the equals sign to assign it a value.
        - Unsupported argument: An argument named "custom_origin_config" is not expected here. Did you mean to define a block of type "custom_origin_config"?
=== RUN   TestAccAWSShieldProtectionRoute53
--- FAIL: TestAccAWSShieldProtectionRoute53 (0.77s)
    testing.go:568: Step 0 error: config is invalid: Unsupported block type: Blocks of type "tags" are not expected here. Did you mean to define argument "tags"? If so, use the equals sign to assign it a value.
=== RUN   TestAccAWSShieldProtection_importBasic
--- FAIL: TestAccAWSShieldProtection_importBasic (1.48s)
    testing.go:568: Step 0 error: config is invalid: Unsupported block type: Blocks of type "tags" are not expected here. Did you mean to define argument "tags"? If so, use the equals sign to assign it a value.

Would you be okay if I rebased and pushed some testing changes to your branch? Otherwise I can provide line by line feedback. Thanks!

parabolic · 2019-05-14T06:52:21Z

Hi @bflad , that is awesome. Please feel free to rebase and push, and afterwards I will do the acceptance tests again. I believe that the documentation format has changed as well right ?

Cheers.

…eld SDK Region to us-east-1 References: * https://docs.aws.amazon.com/general/latest/gr/rande.html#global_accelerator_region * https://docs.aws.amazon.com/general/latest/gr/rande.html#shield_region Global AWS services sometimes only use a single region for their endpoint. As a past precedent we have hardcoded the Route 53 SDK Region to us-east-1. Following the current AWS Regions and Endpoints documentation, we do the same type of SDK configuration for Global Accelerator (endpoint is only in us-west-2) and Shield (endpoint is only in us-east-1).

Brings the acceptance testing style and configurations up to latest. Previously was failing due to Terraform 0.12 syntax changes. Changes include: * Remove provider declaration in Global Accelerator configuration (typically review feedback) * Perform `ImportState` testing in all tests instead of single import test (typically review feedback) * Perform API verification of resource creation in `testAccCheckAWSShieldProtectionExists` (typically review feedback) * Prefer `resource.ParallelTest()` over `resource.Test()` (typically review feedback) * Move `fmt.Sprintf()` usage to configurations and use underscores in test naming (typically style feedback) * Add `testAccPreCheckAWSShield` to skip testing if not subscribed to Shield

bflad · 2019-05-14T14:02:08Z

@parabolic just need permission to your fork, thanks!

 ! [remote rejected]     Issue_1769_AWS_Shield_Protection_feature -> Issue_1769_AWS_Shield_Protection_feature (permission denied)
error: failed to push some refs to 'git@github.com:parabolic/terraform-provider-aws.git'

I'll provide a "regular" review once this little bit of administrivia is handled. 😄

parabolic · 2019-05-14T15:41:29Z

Oh sorry about that @bflad, I've ticked the Allow edits from maintainers checkbox,
Cheers!

bflad · 2019-05-14T16:17:57Z

The pull request was in great shape! Should just be a few minor feedback items at this point (I'll provide in a few minutes) and then we can get this merged in once they're addressed. Thanks so much for your work here, @parabolic! 💯

Rebased on master and added two commits:

(521a9b8) provider: Hardcode Global Accelerator SDK Region to us-west-2 and Shield SDK Region to us-east-1

References:

Global AWS services sometimes only use a single region for their endpoint. As a past precedent we have hardcoded the Route 53 SDK Region to us-east-1. Following the current AWS Regions and Endpoints documentation, we do the same type of SDK configuration for Global Accelerator (endpoint is only in us-west-2) and Shield (endpoint is only in us-east-1).

(b553421) tests/resource/aws_shield_protection: Syntax and style adjustments

Brings the acceptance testing style and configurations up to latest. Previously was failing due to Terraform 0.12 syntax changes. Changes include:

Remove provider declaration in Global Accelerator configuration (typically review feedback)
Perform ImportState testing in all tests instead of single import test (typically review feedback)
Perform API verification of resource creation in testAccCheckAWSShieldProtectionExists (typically review feedback)
Prefer resource.ParallelTest() over resource.Test() (typically review feedback)
Move fmt.Sprintf() usage to configurations and use underscores in test naming (typically style feedback)
Add testAccPreCheckAWSShield to skip testing if not subscribed to Shield

Output from acceptance testing (without setting AWS_DEFAULT_REGION):

--- PASS: TestAccAWSShieldProtection_ElasticIPAddress (15.81s)
--- PASS: TestAccAWSShieldProtection_Route53 (44.65s)
--- PASS: TestAccAWSShieldProtection_Elb (44.84s)
--- PASS: TestAccAWSShieldProtection_GlobalAccelerator (112.01s)
--- PASS: TestAccAWSShieldProtection_Alb (229.90s)
--- PASS: TestAccAWSShieldProtection_Cloudfront (515.12s)

bflad

Here are the remaining feedback items, thanks @parabolic! Please reach out if you have any questions or do not have time to implement these items.

aws/resource_aws_shield_protection.go

website/docs/r/shield_protection.html.markdown

…:parabolic/terraform-provider-aws into Issue_1769_AWS_Shield_Protection_feature

--- - remove the unused import awserr - use ImportStatePassthrough - print the error when there's one - import fmt

Co-Authored-By: Brian Flad <bflad417@gmail.com>

- remove the tags and the variables - remove obsolete lines

parabolic · 2019-05-15T12:57:07Z

@bflad Thanks for the suggestions and help!
After applying the proposals, here's the output from the acceptance tests (with TF_TEST_CLOUDFRONT_RETAIN=true):

TF_ACC=1 go test ./aws -v -timeout 120m -parallel 20 -run='TestAccAWSShieldProtection'
==> Checking that code complies with gofmt requirements...
go install
=== RUN   TestAccAWSShieldProtection_GlobalAccelerator
=== PAUSE TestAccAWSShieldProtection_GlobalAccelerator
=== RUN   TestAccAWSShieldProtection_ElasticIPAddress
=== PAUSE TestAccAWSShieldProtection_ElasticIPAddress
=== RUN   TestAccAWSShieldProtection_Alb
=== PAUSE TestAccAWSShieldProtection_Alb
=== RUN   TestAccAWSShieldProtection_Elb
=== PAUSE TestAccAWSShieldProtection_Elb
=== RUN   TestAccAWSShieldProtection_Cloudfront
=== PAUSE TestAccAWSShieldProtection_Cloudfront
=== RUN   TestAccAWSShieldProtection_Route53
=== PAUSE TestAccAWSShieldProtection_Route53
=== CONT  TestAccAWSShieldProtection_GlobalAccelerator
=== CONT  TestAccAWSShieldProtection_Cloudfront
=== CONT  TestAccAWSShieldProtection_Route53
=== CONT  TestAccAWSShieldProtection_Alb
=== CONT  TestAccAWSShieldProtection_Elb
=== CONT  TestAccAWSShieldProtection_ElasticIPAddress
--- PASS: TestAccAWSShieldProtection_Cloudfront (34.38s)
--- PASS: TestAccAWSShieldProtection_ElasticIPAddress (40.75s)
--- PASS: TestAccAWSShieldProtection_Route53 (65.61s)
--- PASS: TestAccAWSShieldProtection_GlobalAccelerator (86.69s)
--- PASS: TestAccAWSShieldProtection_Elb (87.50s)
--- PASS: TestAccAWSShieldProtection_Alb (253.88s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       253.924s

bflad

Fantastic, @parabolic! LGTM 🚀

Output from acceptance testing:

--- PASS: TestAccAWSShieldProtection_ElasticIPAddress (16.26s)
--- PASS: TestAccAWSShieldProtection_Route53 (44.68s)
--- PASS: TestAccAWSShieldProtection_Elb (47.75s)
--- PASS: TestAccAWSShieldProtection_GlobalAccelerator (103.22s)
--- PASS: TestAccAWSShieldProtection_Alb (196.09s)
--- PASS: TestAccAWSShieldProtection_Cloudfront (514.49s)

bflad · 2019-05-17T13:42:22Z

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

ghost · 2020-03-30T17:15:51Z

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!

parabolic added 6 commits February 22, 2019 17:03

add the provider for shield protection

2b9f301

add the aws_shield_protection resource

a7a08ce

add the testing for the shield protection

e203a83

add the shield resource nav bar

6d33837

add the documentation file

44b9a07

Merge remote-tracking branch 'upstream/master' into Issue_1769_AWS_Sh…

024a01e

…ield_Protection_feature

parabolic changed the title ~~Issue 1769 add ws shield protection feature ( Part II )~~ Issue 1769 add aws shield protection feature ( Part II ) Feb 25, 2019

parabolic added 3 commits February 25, 2019 16:07

add a missing return

453e80f

add spaces between functions for consistency.

1692897

Merge remote-tracking branch 'upstream/master' into Issue_1769_AWS_Sh…

8a63828

…ield_Protection_feature

bflad added the new-resource Introduces a new resource. label Mar 4, 2019

parabolic mentioned this pull request Apr 14, 2019

[WIP]New Resource: aws_shield_protection, aws_shield_subscription #1899

Closed

bflad self-assigned this May 13, 2019

parabolic added 5 commits May 14, 2019 09:45

add the provider for shield protection

da53142

add the aws_shield_protection resource

8fa945b

add the testing for the shield protection

d7b8cbe

add the shield resource nav bar

7e58ec5

add the documentation file

cde6586

parabolic and others added 4 commits May 14, 2019 09:47

add a missing return

8fcb758

add spaces between functions for consistency.

e4ca542

bflad force-pushed the Issue_1769_AWS_Shield_Protection_feature branch from 1692897 to b553421 Compare May 14, 2019 16:11

bflad reviewed May 14, 2019

View reviewed changes

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label May 14, 2019

parabolic and others added 6 commits May 15, 2019 10:42

Merge branch 'Issue_1769_AWS_Shield_Protection_feature' of github.com…

0c7a09f

…:parabolic/terraform-provider-aws into Issue_1769_AWS_Shield_Protection_feature

Update the provider with the suggestions:

6ee7bad

--- - remove the unused import awserr - use ImportStatePassthrough - print the error when there's one - import fmt

Update website/docs/r/shield_protection.html.markdown

c78d5fd

Co-Authored-By: Brian Flad <bflad417@gmail.com>

Update website/docs/r/shield_protection.html.markdown

70cb118

Co-Authored-By: Brian Flad <bflad417@gmail.com>

Update website/docs/r/shield_protection.html.markdown

e34cee9

Co-Authored-By: Brian Flad <bflad417@gmail.com>

Update the documentation from the proposals:

6a53956

- remove the tags and the variables - remove obsolete lines

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label May 15, 2019

bflad approved these changes May 15, 2019

View reviewed changes

bflad added this to the v2.11.0 milestone May 15, 2019

bflad merged commit 2a7385e into hashicorp:master May 15, 2019

bflad added a commit that referenced this pull request May 15, 2019

Update CHANGELOG for #7721

cdb27ae

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 1769 add aws shield protection feature ( Part II ) #7721

Issue 1769 add aws shield protection feature ( Part II ) #7721

parabolic commented Feb 25, 2019 •

edited

Loading

parabolic commented Apr 1, 2019

JoshiiSinfield commented Apr 12, 2019

parabolic commented Apr 12, 2019

steenblik commented May 3, 2019

parabolic commented May 3, 2019 •

edited

Loading

bflad commented May 13, 2019

parabolic commented May 14, 2019

bflad commented May 14, 2019 •

edited

Loading

parabolic commented May 14, 2019 •

edited

Loading

bflad commented May 14, 2019

bflad left a comment

parabolic commented May 15, 2019

bflad left a comment

bflad commented May 17, 2019

ghost commented Mar 30, 2020

Issue 1769 add aws shield protection feature ( Part II ) #7721

Issue 1769 add aws shield protection feature ( Part II ) #7721

Conversation

parabolic commented Feb 25, 2019 • edited Loading

parabolic commented Apr 1, 2019

JoshiiSinfield commented Apr 12, 2019

parabolic commented Apr 12, 2019

steenblik commented May 3, 2019

parabolic commented May 3, 2019 • edited Loading

bflad commented May 13, 2019

parabolic commented May 14, 2019

bflad commented May 14, 2019 • edited Loading

parabolic commented May 14, 2019 • edited Loading

bflad commented May 14, 2019

bflad left a comment

Choose a reason for hiding this comment

parabolic commented May 15, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad commented May 17, 2019

ghost commented Mar 30, 2020

parabolic commented Feb 25, 2019 •

edited

Loading

parabolic commented May 3, 2019 •

edited

Loading

bflad commented May 14, 2019 •

edited

Loading

parabolic commented May 14, 2019 •

edited

Loading