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.

Sign up for GitHub

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

Jump to bottom

resource/aws_cloudfront_key_group - new resource #17041

Merged

bflad merged 9 commits into hashicorp:main from shuheiktgw:add_aws_cloudfront_key_group

Apr 5, 2021

Collaborator

shuheiktgw commented Jan 10, 2021 •

edited

Loading

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

Relates #15912

Release note for CHANGELOG:

* **New Resource:** `aws_cloudfront_key_group `

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSCloudFrontKeyGroup_*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSCloudFrontKeyGroup_* -timeout 120m
=== RUN   TestAccAWSCloudFrontKeyGroup_basic
=== PAUSE TestAccAWSCloudFrontKeyGroup_basic
=== RUN   TestAccAWSCloudFrontKeyGroup_disappears
=== PAUSE TestAccAWSCloudFrontKeyGroup_disappears
=== RUN   TestAccAWSCloudFrontKeyGroup_Comment
=== PAUSE TestAccAWSCloudFrontKeyGroup_Comment
=== RUN   TestAccAWSCloudFrontKeyGroup_Items
=== PAUSE TestAccAWSCloudFrontKeyGroup_Items
=== CONT  TestAccAWSCloudFrontKeyGroup_basic
=== CONT  TestAccAWSCloudFrontKeyGroup_Items
=== CONT  TestAccAWSCloudFrontKeyGroup_Comment
=== CONT  TestAccAWSCloudFrontKeyGroup_disappears
--- PASS: TestAccAWSCloudFrontKeyGroup_disappears (38.58s)
--- PASS: TestAccAWSCloudFrontKeyGroup_basic (46.54s)
--- PASS: TestAccAWSCloudFrontKeyGroup_Items (78.10s)
--- PASS: TestAccAWSCloudFrontKeyGroup_Comment (84.76s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	86.753s

Thank you for your review! 👍

shuheiktgw requested a review from a team as a code owner

January 10, 2021 08:00

ghost added size/XL provider service/cloudfront tests labels

github-actions bot added the needs-triage label

shuheiktgw changed the title ~~wip~~ resource/aws_cloudfront_key_group - new resource

shuheiktgw marked this pull request as draft

January 10, 2021 08:01

shuheiktgw force-pushed the add_aws_cloudfront_key_group branch from 749bdd6 to 5156d19 Compare

January 10, 2021 11:00

ghost added the documentation label

shuheiktgw marked this pull request as ready for review

January 10, 2021 23:15

shuheiktgw mentioned this pull request

Support the use of CloudFront Trusted Key Group as a signer #15912

Closed

shuheiktgw added 3 commits

January 17, 2021 09:24


          Define aws_cloudfront_key_group resource

95d13c4


          Add tests for aws_cloudfront_key_group resource

d1e7ecb


          Add a doc for aws_cloudfront_key_group resource

aaa09e4

shuheiktgw force-pushed the add_aws_cloudfront_key_group branch from 5156d19 to aaa09e4 Compare

January 17, 2021 00:24

Base automatically changed from master to main

January 23, 2021 01:00

ghost commented Mar 3, 2021

@shuheiktgw it does not look that this PR could be cherry picked, it incomplete isn't it, how much work left to modify aws_cloudfront_distribution for key groups? I want to help

Collaborator Author

shuheiktgw commented Mar 4, 2021

Hi @ysz, this PR is ready for review so once this is merged, I can modify aws_cloudfront_distribution resource for key groups.

bflad added new-resource and removed needs-triage labels

bflad self-assigned this

bflad suggested changes

View reviewed changes

Contributor

bflad left a comment

Hi @shuheiktgw 👋 Thank you for submitting this. Overall looking pretty good. Please reach out if you have any questions.

aws/resource_aws_cloudfront_key_group.go Outdated

Comment on lines 31 to 34

+              			"id": {
+              				Type:     schema.TypeString,
+              				Computed: true,
+              			},

Contributor

bflad Mar 31, 2021

A computed id attribute is implicitly handled by the Terraform Plugin SDK, which is why you do not see it typically implemented in resource schemas. 👍

aws/resource_aws_cloudfront_key_group.go Outdated

+              	output, err := conn.CreateKeyGroup(input)
+              	if err != nil {
+              		return fmt.Errorf("error creating CloudFront key group: %s", err)

Contributor

bflad Mar 31, 2021

Nit: To help with some future static analysis we'll be enabling

Suggested change

      
            		return fmt.Errorf("error creating CloudFront key group: %s", err)
          
            		return fmt.Errorf("error creating CloudFront Key Group: %w", err)

aws/resource_aws_cloudfront_key_group.go Show resolved Hide resolved

aws/resource_aws_cloudfront_key_group.go Outdated

+              	output, err := conn.GetKeyGroup(input)
+              	if err != nil {
+              		if isAWSErr(err, cloudfront.ErrCodeNoSuchResource, "") {

Contributor

bflad Mar 31, 2021

Reference: #16796

Suggested change

      
            		if isAWSErr(err, cloudfront.ErrCodeNoSuchResource, "") {
          
            		if !d.IsNewResource && isAWSErr(err, cloudfront.ErrCodeNoSuchResource, "") {

aws/resource_aws_cloudfront_key_group.go Outdated

+              			d.SetId("")
+              			return nil
+              		}
+              		return err

Contributor

bflad Mar 31, 2021

Suggested change

      
            		return err
          
            		return fmt.Errorf("error reading CloudFront Key Group (%s): %w", d.Id(), err)

aws/resource_aws_cloudfront_key_group_test.go Outdated

Comment on lines 130 to 132

+              		PreCheck:     func() { testAccPreCheck(t); testAccPartitionHasServicePreCheck(cloudfront.EndpointsID, t) },
+              		Providers:    testAccProviders,
+              		CheckDestroy: testAccCheckCloudFrontKeyGroupDestroy,

Contributor

bflad Mar 31, 2021

Suggested change

      
            		PreCheck:     func() { testAccPreCheck(t); testAccPartitionHasServicePreCheck(cloudfront.EndpointsID, t) },
          
            		Providers:    testAccProviders,
          
            		CheckDestroy: testAccCheckCloudFrontKeyGroupDestroy,
          
            		PreCheck:     func() { testAccPreCheck(t); testAccPartitionHasServicePreCheck(cloudfront.EndpointsID, t) },
          
            		ErrorCheck:   testAccErrorCheck(t, cloudfront.EndpointsID),
          
            		Providers:    testAccProviders,
          
            		CheckDestroy: testAccCheckCloudFrontKeyGroupDestroy,

aws/resource_aws_cloudfront_key_group_test.go Outdated

Comment on lines 106 to 108

+              		PreCheck:     func() { testAccPreCheck(t); testAccPartitionHasServicePreCheck(cloudfront.EndpointsID, t) },
+              		Providers:    testAccProviders,
+              		CheckDestroy: testAccCheckCloudFrontKeyGroupDestroy,

Contributor

bflad Mar 31, 2021

Suggested change

      
            		PreCheck:     func() { testAccPreCheck(t); testAccPartitionHasServicePreCheck(cloudfront.EndpointsID, t) },
          
            		Providers:    testAccProviders,
          
            		CheckDestroy: testAccCheckCloudFrontKeyGroupDestroy,
          
            		PreCheck:     func() { testAccPreCheck(t); testAccPartitionHasServicePreCheck(cloudfront.EndpointsID, t) },
          
            		ErrorCheck:   testAccErrorCheck(t, cloudfront.EndpointsID),
          
            		Providers:    testAccProviders,
          
            		CheckDestroy: testAccCheckCloudFrontKeyGroupDestroy,

aws/resource_aws_cloudfront_key_group_test.go Outdated

+                items   = [aws_cloudfront_public_key.test.id, aws_cloudfront_public_key.test2.id]
+                name    = "tf-acc-test-%d"
+              }
+              `, rInt, rInt)

Contributor

bflad Mar 31, 2021

Go's fmt package supports indexed verbs, such as %[1]d that allow you not repeat parameters.

website/docs/r/cloudfront_key_group.html.markdown Outdated


		The following example below creates a CloudFront key group.

		```hcl

Contributor

bflad Mar 31, 2021

Newer linting:

Suggested change

      
            ```hcl
          
            ```terraform

aws/provider.go

@@ @@ -483,6 +483,7 @@ func Provider() *schema.Provider { @@
               			"aws_cloudformation_stack_set":                            resourceAwsCloudFormationStackSet(),
               			"aws_cloudformation_stack_set_instance":                   resourceAwsCloudFormationStackSetInstance(),
               			"aws_cloudfront_distribution":                             resourceAwsCloudFrontDistribution(),
+              			"aws_cloudfront_key_group":                                resourceAwsCloudFrontKeyGroup(),

Contributor

bflad Mar 31, 2021

We can changelog this by adding a .changelog/17041.txt file with contents such as:

```release-note:new-resource
aws_cloudfront_key_group
```

bflad added the waiting-response label

shuheiktgw added 5 commits

April 4, 2021 08:30


          Merge branch 'main' of github.com:hashicorp/terraform-provider-aws in…

6fa6b9b

…to add_aws_cloudfront_key_group


          Remove the id attribute

a4a0633


          Improve error handlings

05ddc58


          Miscellaneous changes on the tests

5b55171


          Change the format from hcl to terraform

5cdd440


          Add a changelog

92ae055

Collaborator Author

shuheiktgw commented Apr 4, 2021

$ make testacc TESTARGS='-run=TestAccAWSCloudFrontKeyGroup_'
=== RUN   TestAccAWSCloudFrontKeyGroup_Comment
=== PAUSE TestAccAWSCloudFrontKeyGroup_Comment
=== RUN   TestAccAWSCloudFrontKeyGroup_Items
=== PAUSE TestAccAWSCloudFrontKeyGroup_Items
=== CONT  TestAccAWSCloudFrontKeyGroup_basic
=== CONT  TestAccAWSCloudFrontKeyGroup_disappears
=== CONT  TestAccAWSCloudFrontKeyGroup_Items
=== CONT  TestAccAWSCloudFrontKeyGroup_Comment
--- PASS: TestAccAWSCloudFrontKeyGroup_disappears (49.65s)
--- PASS: TestAccAWSCloudFrontKeyGroup_basic (56.78s)
--- PASS: TestAccAWSCloudFrontKeyGroup_Comment (88.34s)
--- PASS: TestAccAWSCloudFrontKeyGroup_Items (90.59s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       92.640sq

Collaborator Author

shuheiktgw commented Apr 4, 2021

@bflad Thank you for your review! I believe I've fixed it up so would you review the PR again?

shuheiktgw requested a review from bflad

April 4, 2021 00:24

bflad approved these changes

View reviewed changes

Contributor

bflad left a comment

Looks great, thank you, @shuheiktgw 🚀

Output from acceptance testing in AWS Commercial:

--- PASS: TestAccAWSCloudFrontKeyGroup_disappears (10.83s)
--- PASS: TestAccAWSCloudFrontKeyGroup_basic (12.39s)
--- PASS: TestAccAWSCloudFrontKeyGroup_Comment (19.26s)
--- PASS: TestAccAWSCloudFrontKeyGroup_Items (19.63s)

Output from acceptance testing in AWS GovCloud (US):

--- SKIP: TestAccAWSCloudFrontKeyGroup_basic (1.33s)
--- SKIP: TestAccAWSCloudFrontKeyGroup_disappears (1.33s)
--- SKIP: TestAccAWSCloudFrontKeyGroup_Comment (1.33s)
--- SKIP: TestAccAWSCloudFrontKeyGroup_Items (1.33s)

bflad merged commit 29c750a into hashicorp:main

github-actions bot added this to the v3.36.0 milestone

ghost commented Apr 9, 2021

This has been released in version 3.36.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 commented May 5, 2021

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!

ghost locked as resolved and limited conversation to collaborators

breathingdust removed the waiting-response label

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation new-resource provider service/cloudfront size/XL tests