Skip to content
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.

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

API Gateway deployment resource, field stage_name not required #2918 #6459

Merged

Conversation

sousmangoosta
Copy link
Contributor

Fixes #2918

Changes proposed in this pull request:

  • Field stage_name changed from required to optional

@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/apigateway Issues and PRs that pertain to the apigateway service. labels Nov 14, 2018
@bflad bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Nov 14, 2018
@bflad
Copy link
Contributor

bflad commented Nov 14, 2018

Hi @sousmangoosta 👋 Thanks for submitting this! Would you be able to include a new acceptance test and test configuration that covers not setting this argument?

@sousmangoosta
Copy link
Contributor Author

Hi @bflad , I'll try doing that, I found a bug for this change.
When I change from stage for example "default" to no one, the plan create a map like that :
"stage_name":*terraform.ResourceAttrDiff{Old:"default", New:"", NewComputed:false, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:true, Sensitive:false, Type:0x0}

But on apply, the map does not contain stage_name, so terraform complaints with Mismatch reason: attribute mismatch: stage_name

@sousmangoosta
Copy link
Contributor Author

Hey @bflad, I don't have this bug anymore, so I will add new acceptance test, and push when ready.

@sousmangoosta sousmangoosta force-pushed the api_gateway_deployment_optional_stage branch from cd6aecb to 2ba99cd Compare November 20, 2018 13:56
@ghost ghost added size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. and removed size/XS Managed by automation to categorize the size of a PR. labels Nov 20, 2018
@sousmangoosta
Copy link
Contributor Author

@bflad acceptance tests has been added.

make testacc TESTARGS='-run=TestAccAWSAPIGatewayDeployment' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSAPIGatewayDeployment -timeout 120m ? github.com/terraform-providers/terraform-provider-aws [no test files] === RUN TestAccAWSAPIGatewayDeployment_basic === PAUSE TestAccAWSAPIGatewayDeployment_basic === RUN TestAccAWSAPIGatewayDeployment_basicEmptyStage === PAUSE TestAccAWSAPIGatewayDeployment_basicEmptyStage === RUN TestAccAWSAPIGatewayDeployment_namedToEmptyStage === PAUSE TestAccAWSAPIGatewayDeployment_namedToEmptyStage === RUN TestAccAWSAPIGatewayDeployment_createBeforeDestoryUpdate === PAUSE TestAccAWSAPIGatewayDeployment_createBeforeDestoryUpdate === CONT TestAccAWSAPIGatewayDeployment_basic === CONT TestAccAWSAPIGatewayDeployment_createBeforeDestoryUpdate === CONT TestAccAWSAPIGatewayDeployment_namedToEmptyStage === CONT TestAccAWSAPIGatewayDeployment_basicEmptyStage --- PASS: TestAccAWSAPIGatewayDeployment_basic (33.58s) --- PASS: TestAccAWSAPIGatewayDeployment_namedToEmptyStage (66.86s) --- PASS: TestAccAWSAPIGatewayDeployment_basicEmptyStage (102.53s) --- PASS: TestAccAWSAPIGatewayDeployment_createBeforeDestoryUpdate (159.43s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 159.467s

@sousmangoosta sousmangoosta force-pushed the api_gateway_deployment_optional_stage branch from 2ba99cd to 6b2c28d Compare November 22, 2018 11:03
@ghost ghost added the documentation Introduces or discusses updates to documentation. label Nov 22, 2018
@sousmangoosta
Copy link
Contributor Author

@bflad : Is there someone to review this one ?

@anthonyagresta
Copy link

Commenting to bump this PR, ran into this issue today.

@mkirlin
Copy link

mkirlin commented Feb 11, 2019

Just wanted to put another bump in here. I'm working around this with an apply-import-apply chain of commands, but it would be nice to have this actually optional as displayed in the docs.

@jpadhye
Copy link

jpadhye commented Mar 4, 2019

Hit this issue at work. Giving a bump

@simsinght
Copy link

also running into this issue. bump.

@jvanwagner
Copy link

@bflad @nywilken @mikecook any progress on possibly reviewing/approving this? We just hit this issue at our company and seems like a lot of others ran across it as well.

@bflad bflad added this to the v2.3.0 milestone Mar 15, 2019
@bflad bflad self-assigned this Mar 15, 2019
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sousmangoosta 👋 Thanks for this contribution and sorry for the delay. After refactoring the testing changes to match the newer testing on master, acceptance testing passes.

--- PASS: TestAccAWSAPIGatewayDeployment_StageName_EmptyString (20.75s)
--- PASS: TestAccAWSAPIGatewayDeployment_Description (55.58s)
--- PASS: TestAccAWSAPIGatewayDeployment_createBeforeDestoryUpdate (56.30s)
--- PASS: TestAccAWSAPIGatewayDeployment_Variables (117.39s)
--- PASS: TestAccAWSAPIGatewayDeployment_StageName (138.02s)
--- PASS: TestAccAWSAPIGatewayDeployment_StageDescription (201.81s)
--- PASS: TestAccAWSAPIGatewayDeployment_basic (211.81s)

@bflad bflad merged commit 6b2c28d into hashicorp:master Mar 15, 2019
bflad added a commit that referenced this pull request Mar 15, 2019
@bflad
Copy link
Contributor

bflad commented Mar 21, 2019

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

@sousmangoosta sousmangoosta deleted the api_gateway_deployment_optional_stage branch March 24, 2019 11:59
@ghost
Copy link

ghost commented Mar 30, 2020

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 ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/apigateway Issues and PRs that pertain to the apigateway service. size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error creating API Gateway Stage: ConflictException: Stage already exists
7 participants