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

provider/aws: Fixed deletion of aws_api_gateway_base_path_mapping with empty path #10177

Merged
merged 1 commit into from
Dec 12, 2016

Conversation

Ninir
Copy link
Contributor

@Ninir Ninir commented Nov 16, 2016

Fixes #9212

Here is the output when running it:

2016/11/16 23:16:49 [DEBUG] plugin: terraform: aws-provider (internal) 2016/11/16 23:16:49 [DEBUG] [aws-sdk-go] DEBUG: Request apigateway/CreateBasePathMapping Details:
2016/11/16 23:16:49 [DEBUG] plugin: terraform: ---[ REQUEST POST-SIGN ]-----------------------------
2016/11/16 23:16:49 [DEBUG] plugin: terraform: POST http://apigateway.eu-west-1.amazonaws.com/domainnames/12345.tf-acc.invalid/basepathmappings HTTP/1.1

...

2016/11/16 23:16:49 [DEBUG] plugin: terraform: 
2016/11/16 23:16:49 [DEBUG] plugin: terraform: {"basePath":"","restApiId":"c7hpqdek26","stage":"test"}
2016/11/16 23:16:49 [DEBUG] plugin: terraform: -----------------------------------------------------
2016/11/16 23:16:49 [DEBUG] plugin: terraform: aws-provider (internal) 2016/11/16 23:16:49 [DEBUG] [aws-sdk-go] DEBUG: Response apigateway/CreateBasePathMapping Details:
2016/11/16 23:16:49 [DEBUG] plugin: terraform: ---[ RESPONSE ]--------------------------------------

...

2016/11/16 23:16:49 [DEBUG] plugin: terraform: 
2016/11/16 23:16:49 [DEBUG] plugin: terraform: {"basePath":"(none)","restApiId":"c7hpqdek26","stage":"test"}
2016/11/16 23:16:49 [DEBUG] plugin: terraform: 
2016/11/16 23:16:49 [DEBUG] plugin: terraform: -----------------------------------------------------
2016/11/16 23:16:49 [DEBUG] plugin: terraform: aws-provider (internal) 2016/11/16 23:16:49 [DEBUG] [aws-sdk-go] DEBUG: Request apigateway/GetBasePathMapping Details:
2016/11/16 23:16:49 [DEBUG] plugin: terraform: ---[ REQUEST POST-SIGN ]-----------------------------
2016/11/16 23:16:49 [DEBUG] plugin: terraform: GET http://apigateway.eu-west-1.amazonaws.com/domainnames/12345.tf-acc.invalid/basepathmappings/ HTTP/1.1

...

2016/11/16 23:16:49 [DEBUG] plugin: terraform: 
2016/11/16 23:16:49 [DEBUG] plugin: terraform: 
2016/11/16 23:16:49 [DEBUG] plugin: terraform: -----------------------------------------------------
2016/11/16 23:16:49 [DEBUG] plugin: terraform: aws-provider (internal) 2016/11/16 23:16:49 [DEBUG] [aws-sdk-go] DEBUG: Response apigateway/GetBasePathMapping Details:
2016/11/16 23:16:49 [DEBUG] plugin: terraform: ---[ RESPONSE ]--------------------------------------

...

2016/11/16 23:16:49 [DEBUG] plugin: terraform: 
2016/11/16 23:16:49 [DEBUG] plugin: terraform: {"item":[{"basePath":"(none)","restApiId":"c7hpqdek26","stage":"test"}]}
2016/11/16 23:16:49 [DEBUG] plugin: terraform: 
2016/11/16 23:16:49 [DEBUG] plugin: terraform: -----------------------------------------------------

@kwilczynski
Copy link
Contributor

@Ninir hi there! Nice catch! Thank you!

@Ninir
Copy link
Contributor Author

Ninir commented Nov 21, 2016

@kwilczynski Do you have any hint about the diff issue? Still investigating, but missing something, for sure...

Testing the same test under real conditions is fine (thus compiling and testing the compiled binary), and everything destroyed correctly.

@kwilczynski
Copy link
Contributor

kwilczynski commented Nov 21, 2016

@Ninir let me have a look. I will report back.

@Ninir
Copy link
Contributor Author

Ninir commented Nov 21, 2016

@kwilczynski Think I'v found it... when getting the base path mapping with the empty base_path option, here is what is happening in the background (from line 90):

-----------------------------------------------------
2016/11/22 00:20:39 [DEBUG] [aws-sdk-go] DEBUG: Response apigateway/GetBasePathMapping Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 200 OK
Content-Length: 58
Content-Type: application/json
...

{"item":[{"basePath":"(none)","restApiId":"fg0279h6ji"}]}

However, the resulting mapping variable defined is empty, thus NOT setting the api_id attribute. This throws the error exposed above...

@Ninir Ninir changed the title [WIP] provider/aws: Fixed deletion of aws_api_gateway_base_path_mapping with empty path provider/aws: Fixed deletion of aws_api_gateway_base_path_mapping with empty path Nov 23, 2016
@Ninir Ninir changed the title provider/aws: Fixed deletion of aws_api_gateway_base_path_mapping with empty path [WIP] provider/aws: Fixed deletion of aws_api_gateway_base_path_mapping with empty path Nov 23, 2016
@Ninir Ninir changed the title [WIP] provider/aws: Fixed deletion of aws_api_gateway_base_path_mapping with empty path provider/aws: Fixed deletion of aws_api_gateway_base_path_mapping with empty path Nov 23, 2016
@Ninir
Copy link
Contributor Author

Ninir commented Nov 23, 2016

It should be all ok.

@stack72
Copy link
Contributor

stack72 commented Dec 12, 2016

Hi @Ninir

Thanks for the work here - this doesn't pass the tests i'm afraid

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAPIGatewayBasePath_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/12 17:47:13 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAPIGatewayBasePath_ -timeout 120m
=== RUN   TestAccAWSAPIGatewayBasePath_basic
--- FAIL: TestAccAWSAPIGatewayBasePath_basic (0.00s)
	testing.go:265: Step 0 error: Error loading configuration: Error parsing /var/folders/gd/yv86v7pd1cd5dpzjrw8702k00000gn/T/tf-test504278527/main.tf: At 49:59: expected '/' for comment
FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/aws	0.025s
make: *** [testacc] Error 1

P.

@stack72 stack72 added bug provider/aws waiting-response An issue/pull request is waiting for a response from the community labels Dec 12, 2016
@Ninir
Copy link
Contributor Author

Ninir commented Dec 12, 2016

Hi @stack72

Dang it, sorry for the overload for you! Just updated the test configuration & ran the 2 tests:

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAPIGatewayBasePath_'            
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/12 21:31:58 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAPIGatewayBasePath_ -timeout 120m
=== RUN   TestAccAWSAPIGatewayBasePath_basic
--- PASS: TestAccAWSAPIGatewayBasePath_basic (56.27s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	56.297s

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAPIGatewayEmptyBasePath_basic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/12 21:34:44 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAPIGatewayEmptyBasePath_basic -timeout 120m
=== RUN   TestAccAWSAPIGatewayEmptyBasePath_basic
--- PASS: TestAccAWSAPIGatewayEmptyBasePath_basic (21.37s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	21.395s

Thanks for the review!

@stack72
Copy link
Contributor

stack72 commented Dec 12, 2016

Hi @Ninir

This LGTM now - thanks for the fast fix :)

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAPIGatewayBasePath_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/12 22:07:14 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAPIGatewayBasePath_ -timeout 120m
=== RUN   TestAccAWSAPIGatewayBasePath_basic
--- PASS: TestAccAWSAPIGatewayBasePath_basic (40.77s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	40.794s

@stack72 stack72 merged commit 16660a4 into hashicorp:master Dec 12, 2016
@Ninir Ninir deleted the api_gw_deletion branch December 12, 2016 22:09
@ghost
Copy link

ghost commented Apr 18, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deleting AWS API GW domain base path mapping with the empty base path fails
3 participants