Feature/add api gateway stage data source #8890

FreekingDean · 2019-06-06T05:41:07Z

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

adds new data source "aws_api_gateway_stage"

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'
 ==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccDataSourceAwsApiGatewayStage -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccDataSourceAwsApiGatewayStage
=== PAUSE TestAccDataSourceAwsApiGatewayStage
=== CONT  TestAccDataSourceAwsApiGatewayStage
--- PASS: TestAccDataSourceAwsApiGatewayStage (30.49s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	(cached)g
...

FreekingDean · 2019-06-06T05:42:27Z

One issue I ran into that I did not expect was when using depends_on for the stage instead of using the basename funkyness that is in there now I ran into this error testing:

data "aws_api_gateway_stage" "example" {
  depends_on  = ["aws_api_gateway_stage.example"]
  rest_api_id = "${aws_api_gateway_rest_api.test.id}"
  stage_name 	= "prod"
}

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccDataSourceAwsApiGatewayStage -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccDataSourceAwsApiGatewayStage
=== PAUSE TestAccDataSourceAwsApiGatewayStage
=== CONT  TestAccDataSourceAwsApiGatewayStage
--- FAIL: TestAccDataSourceAwsApiGatewayStage (31.83s)
    testing.go:568: Step 0 error: After applying this step and refreshing, the plan was not empty:
        
        DIFF:
        
        UPDATE: data.aws_api_gateway_stage.example
          current_deployment_id: "" => "<computed>"
          id:                    "" => "<computed>"
          rest_api_id:           "" => "ibu4ynzcr3"
          stage_name:            "" => "prod"

bflad

Hi @FreekingDean 👋 Thanks so much for your contribution here. I left some initial feedback items below -- please reach out if you have any questions or do not have time to implement them.

Regarding your earlier comment about depends_on: the usage of depends_on with data sources can be problematic with perpetual read updates and its usage is discouraged in general for data sources. We do have some documentation about this here, if you are interested in reading more: https://www.terraform.io/docs/configuration/data-sources.html#data-resource-dependencies

bflad · 2019-06-06T19:26:31Z

aws/data_source_aws_api_gateway_stage.go

+				Type:     schema.TypeString,
+				Required: true,
+			},
+			"current_deployment_id": {


Since the API refers to this as deployment ID, should we remove current_ from the naming?

bflad · 2019-06-06T19:27:46Z

aws/data_source_aws_api_gateway_stage.go

+	}
+
+	d.SetId(fmt.Sprintf("ags-%s-%s", restApiID, stageName))
+	d.Set("current_deployment_id", *stage.DeploymentId)


Dereferencing the string pointer via * can introduce potential panics without a nil check and is extraneous here as d.Set() automatically will handle nil 👍

Suggested change

d.Set("current_deployment_id", *stage.DeploymentId)

d.Set("deployment_id", stage.DeploymentId)

bflad · 2019-06-06T19:28:51Z

aws/data_source_aws_api_gateway_stage_test.go

+
+data "aws_api_gateway_stage" "example" {
+  rest_api_id = "${aws_api_gateway_rest_api.test.id}"
+  stage_name 	= "${basename(aws_api_gateway_stage.example.invoke_url)}"


Can we use stage_name = "${aws_api_gateway_stage.example.stage_name}" instead?

==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccDataSourceAwsApiGatewayStage -timeout 120m ? github.com/terraform-providers/terraform-provider-aws [no test files] === RUN TestAccDataSourceAwsApiGatewayStage === PAUSE TestAccDataSourceAwsApiGatewayStage === CONT TestAccDataSourceAwsApiGatewayStage --- PASS: TestAccDataSourceAwsApiGatewayStage (27.37s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 27.411s

Just to note that Acceptance tests still pass :)

aws/provider.go

FreekingDean · 2019-06-06T19:41:42Z

I believe I resolved all outstanding comments @bflad! Let me know if theres anything else you see that sticks out!

bflad

Small little things then should be good to go!

bflad · 2019-06-06T20:37:44Z

website/docs/d/api_gateway_stage.html.markdown

+  stage_name = "production"
+}
+
+resource "aws_api_gateway_stage" "production" {


Should this be named prod to match the stage_name below or vice versa? Maybe blue/green? Admittedly this example feels a little confusing to me for a use case since it doesn't show overwriting any stage settings. 😅

Hopefully this is a bit better, our use case was:
We had a system that updated & deployed an api gateway stage, but wanted to configure settings (specifically the logging settings via terraform). This would cause issues every time we applied since a deployment is required as an attribute, this change will allow us to just use the latest deployment as input to the stage resource so we dont revery the api gateway on each trerraform apply!

website/docs/d/api_gateway_stage.html.markdown

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

FreekingDean · 2019-06-06T21:36:49Z

Hopefully they are addressed!

FreekingDean · 2019-06-11T14:56:03Z

Anything needed for this?

FreekingDean · 2019-06-13T15:07:17Z

Just wanted to see if there were changes requested! :D

FreekingDean · 2019-06-18T14:48:16Z

Hey! Just wanted to do a last checkin to see if theres something I can do to help this through :)

FreekingDean · 2019-06-25T15:38:17Z

Just want to bump this again to see what I can do to help!

FreekingDean · 2019-07-09T22:50:02Z

Any updates to this?

jlambert121 · 2019-07-17T18:18:36Z

Is there anything blocking this? This is something that is impacting us!

FreekingDean · 2019-09-25T16:08:40Z

Fixed the merge conflict!

FreekingDean · 2020-04-13T17:38:12Z

Closing this to clean up my open PRs

ghost · 2020-05-14T17:10:07Z

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!

FreekingDean force-pushed the feature/add-api-gateway-stage-data-source branch from 1a07282 to 6ee79d8 Compare June 6, 2019 13:06

bflad added the new-data-source Introduces a new data source. label Jun 6, 2019

bflad reviewed Jun 6, 2019

View reviewed changes

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

ghost added size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. and removed size/M Managed by automation to categorize the size of a PR. labels Jun 6, 2019

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

bflad reviewed Jun 6, 2019

View reviewed changes

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

FreekingDean and others added 10 commits June 6, 2019 17:33

Added datasource for api gateway stage

2920fa1

Use basename instead of depends on

64ab055

Update documentation

ce8eb75

Added link to stage

d8b0280

Do not de reference pointer

1180b7e

current_deployment_id -> deployment_id

6897325

Use stage name over basename

5f8e403

Update website/docs/d/api_gateway_stage.html.markdown

1ef86c7

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

Update website/docs/d/api_gateway_stage.html.markdown

df20884

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

Add a variable to stage for documentation

6a4a6b2

FreekingDean force-pushed the feature/add-api-gateway-stage-data-source branch from 3bd3b01 to 6a4a6b2 Compare June 6, 2019 21:33

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

aeschright requested a review from a team June 28, 2019 18:14

Merge branch 'master' into feature/add-api-gateway-stage-data-source

17a97a9

FreekingDean requested review from bflad and removed request for a team September 25, 2019 16:08

FreekingDean closed this Apr 13, 2020

FreekingDean deleted the feature/add-api-gateway-stage-data-source branch April 13, 2020 17:38

hashicorp locked and limited conversation to collaborators May 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/add api gateway stage data source #8890

Feature/add api gateway stage data source #8890

FreekingDean commented Jun 6, 2019

FreekingDean commented Jun 6, 2019 •

edited

bflad left a comment

bflad Jun 6, 2019

bflad Jun 6, 2019

bflad Jun 6, 2019

FreekingDean Jun 6, 2019

FreekingDean commented Jun 6, 2019

bflad left a comment

bflad Jun 6, 2019

FreekingDean Jun 6, 2019

FreekingDean commented Jun 6, 2019

FreekingDean commented Jun 11, 2019

FreekingDean commented Jun 13, 2019

FreekingDean commented Jun 18, 2019

FreekingDean commented Jun 25, 2019

FreekingDean commented Jul 9, 2019

jlambert121 commented Jul 17, 2019

FreekingDean commented Sep 25, 2019

FreekingDean commented Apr 13, 2020

ghost commented May 14, 2020

	d.Set("current_deployment_id", *stage.DeploymentId)
	d.Set("deployment_id", stage.DeploymentId)

Feature/add api gateway stage data source #8890

Feature/add api gateway stage data source #8890

Conversation

FreekingDean commented Jun 6, 2019

Community Note

FreekingDean commented Jun 6, 2019 • edited

bflad left a comment

Choose a reason for hiding this comment

bflad Jun 6, 2019

Choose a reason for hiding this comment

bflad Jun 6, 2019

Choose a reason for hiding this comment

bflad Jun 6, 2019

Choose a reason for hiding this comment

FreekingDean Jun 6, 2019

Choose a reason for hiding this comment

FreekingDean commented Jun 6, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad Jun 6, 2019

Choose a reason for hiding this comment

FreekingDean Jun 6, 2019

Choose a reason for hiding this comment

FreekingDean commented Jun 6, 2019

FreekingDean commented Jun 11, 2019

FreekingDean commented Jun 13, 2019

FreekingDean commented Jun 18, 2019

FreekingDean commented Jun 25, 2019

FreekingDean commented Jul 9, 2019

jlambert121 commented Jul 17, 2019

FreekingDean commented Sep 25, 2019

FreekingDean commented Apr 13, 2020

ghost commented May 14, 2020

FreekingDean commented Jun 6, 2019 •

edited