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

Add aws_codestarconnections_connection resource #15990

Merged

gdavison merged 10 commits into hashicorp:master from shuheiktgw:add_codestar_connections_connection_resource

Dec 15, 2020

Collaborator

shuheiktgw commented Nov 2, 2020 •

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 #15453, #15960
Closes #12637
Closes #11389

Release note for CHANGELOG:

* **New Resource:** `aws_codestarconnections_connection`

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSCodeStarConnectionsConnection_*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSCodeStarConnectionsConnection_* -timeout 120m
=== RUN   TestAccAWSCodeStarConnectionsConnection_Basic
=== PAUSE TestAccAWSCodeStarConnectionsConnection_Basic
=== RUN   TestAccAWSCodeStarConnectionsConnection_disappears
=== PAUSE TestAccAWSCodeStarConnectionsConnection_disappears
=== CONT  TestAccAWSCodeStarConnectionsConnection_disappears
=== CONT  TestAccAWSCodeStarConnectionsConnection_Basic
--- PASS: TestAccAWSCodeStarConnectionsConnection_disappears (36.78s)
--- PASS: TestAccAWSCodeStarConnectionsConnection_Basic (49.39s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	51.265s

This is the second part of a PR to add aws_codestarconnections_connection resource (the first part: #15960). This PR originally came from #12637. Thank you for your review!

shuheiktgw requested a review from a team as a code owner

November 2, 2020 23:52

ghost added size/XL provider service/codestarconnections tests labels

github-actions bot added the needs-triage label

shuheiktgw force-pushed the add_codestar_connections_connection_resource branch 3 times, most recently from b585c07 to 6942de9 Compare

November 3, 2020 00:01


          Add aws_codestarconnections_connection resource

222c479

Co-authored-by: Kévin Sénéchal <kevin.senechal@gmail.com>

shuheiktgw force-pushed the add_codestar_connections_connection_resource branch from 6942de9 to 222c479 Compare

November 3, 2020 00:07

Collaborator Author

shuheiktgw commented Nov 3, 2020

@bflad I've created the second part of the PR to support aws_codestarconnections_connection resource so would you review the PR?

shuheiktgw mentioned this pull request

Add support for CodeStar Connections #15453

Closed

5 tasks

gdavison self-assigned this

gdavison mentioned this pull request

aws_codepipeline with Github OAuth still breaking auth #15200

Closed

gdavison requested changes

View reviewed changes

Contributor

gdavison left a comment

Thanks for the PR, @shuheiktgw, it's looking good. I have a few suggestions for additions to the resource

aws/resource_aws_codestarconnections_connection.go Outdated Show resolved Hide resolved

aws/resource_aws_codestarconnections_connection.go Outdated

Comment on lines 23 to 31

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

Contributor

gdavison Dec 12, 2020

Normally, we prefer to follow the API, but in the case of "standard" outputs such as arn, it's ok to just have the arn and remove connection_arn

aws/resource_aws_codestarconnections_connection.go Outdated

+              				Computed: true,
+              			},
+              			"connection_name": {

Contributor

gdavison Dec 12, 2020

This can be renamed to name. It doesn't match the API, but it's a common pattern for resources to use the name parameter, and the resource type aws_codestarconnections_connection already mentions "connection" twice 🙂

Optionally, we can also use the name generation documented at https://github.com/hashicorp/terraform-provider-aws/blob/master/docs/contributing/contribution-checklists.md#adding-resource-name-generation-support

aws/resource_aws_codestarconnections_connection.go Outdated

+              	res, err := conn.CreateConnection(params)
+              	if err != nil {
+              		return fmt.Errorf("error creating codestar connection: %s", err)

Contributor

gdavison Dec 12, 2020

We prefer error messages to use the Go 1.13 error wrapping verb %w.
For user-facing output, we should use the styling used by AWS

Suggested change

      
            		return fmt.Errorf("error creating codestar connection: %s", err)
          
            		return fmt.Errorf("error creating CodeStar connection: %w", err)

aws/resource_aws_codestarconnections_connection.go Show resolved Hide resolved

aws/resource_aws_codestarconnections_connection.go Outdated

+              func resourceAwsCodeStarConnectionsConnectionRead(d *schema.ResourceData, meta interface{}) error {
+              	conn := meta.(*AWSClient).codestarconnectionsconn
+              	rule, err := conn.GetConnection(&codestarconnections.GetConnectionInput{

Contributor

gdavison Dec 12, 2020

rule should probably be renamed, since it doesn't return a rule. We often use resp.

Suggested change

      
            	rule, err := conn.GetConnection(&codestarconnections.GetConnectionInput{
          
            	resp, err := conn.GetConnection(&codestarconnections.GetConnectionInput{

aws/resource_aws_codestarconnections_connection.go Show resolved Hide resolved

aws/resource_aws_codestarconnections_connection_test.go Show resolved Hide resolved

gdavison removed the needs-triage label

Collaborator Author

shuheiktgw commented Dec 14, 2020

@gdavison Thank you for your review! Let me fix them quickly 👍


          Use codestarconnections.ProviderType_Values() instead

4855e64

Co-authored-by: Graham Davison <g.m.davison@computer.org>

ghost added the documentation label

shuheiktgw added 7 commits

December 15, 2020 18:34


          Remove connection_arn

2ae27e7


          Rename connection_name to name

220e9e3


          Modify error messages

56b5336


          Add a check if rule and rule.Connections is nil

5cc7321


          Rename rule to resp

c691946


          Handle a case when a connection is not found

91c5f2d


          Add TestAccAWSCodeStarConnectionsConnection_disappears

8a963ff

Collaborator Author

shuheiktgw commented Dec 15, 2020

@gdavison Thank you for your great reviews! I believe I fixed them up so would you review the PR again? 😄
I also reran the acc tests so please check the results below.

$ make testacc TESTARGS='-run=TestAccAWSCodeStarConnectionsConnection_*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSCodeStarConnectionsConnection_* -timeout 120m
=== RUN   TestAccAWSCodeStarConnectionsConnection_Basic
=== PAUSE TestAccAWSCodeStarConnectionsConnection_Basic
=== RUN   TestAccAWSCodeStarConnectionsConnection_disappears
=== PAUSE TestAccAWSCodeStarConnectionsConnection_disappears
=== CONT  TestAccAWSCodeStarConnectionsConnection_disappears
=== CONT  TestAccAWSCodeStarConnectionsConnection_Basic
--- PASS: TestAccAWSCodeStarConnectionsConnection_disappears (36.78s)
--- PASS: TestAccAWSCodeStarConnectionsConnection_Basic (49.39s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	51.265s


          Fix a lint error

51e997d

gdavison approved these changes

View reviewed changes

Contributor

gdavison left a comment

Thanks for the updates, @shuheiktgw! LGTM 🚀

--- PASS: TestAccAWSCodeStarConnectionsConnection_disappears (10.42s)
--- PASS: TestAccAWSCodeStarConnectionsConnection_Basic (14.08s)

gdavison added this to the v3.22.0 milestone

gdavison added a commit that referenced this pull request


          Merge pull request #15990 from shuheiktgw/add_codestar_connections_co…

7bdf671

…nnection_resource

gdavison merged commit 51e997d into hashicorp:master

gdavison added a commit that referenced this pull request


          Update CHANGELOG for #15990

1776cbf

shuheiktgw deleted the add_codestar_connections_connection_resource branch

December 16, 2020 10:45

dansd mentioned this pull request

Add CodePipeline GitHub version 2 source action support #16042

Closed

ghost commented Dec 18, 2020

This has been released in version 3.22.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 Jan 15, 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

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

Labels

documentation provider service/codestarconnections size/XL tests