Add a body property to API Gateway RestAPI for Swagger import support #1197

lukehoban · 2017-07-20T20:16:01Z

Description

The body property allows supplying a Swagger/OpenAPI spec to a RestAPI to create the API definition using the routes and integrations defined in the spec.

Related to hashicorp/terraform#11034 and the work done in hashicorp/terraform#6175.

Provides similar behaviour to what's available in CloudFormation with: http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-restapi.html#cfn-apigateway-restapi-body.

Tests

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSAPIGatewayRestApi_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSAPIGatewayRestApi_ -timeout 120m
=== RUN   TestAccAWSAPIGatewayRestApi_basic
--- PASS: TestAccAWSAPIGatewayRestApi_basic (50.66s)
=== RUN   TestAccAWSAPIGatewayRestApi_openapi
--- PASS: TestAccAWSAPIGatewayRestApi_openapi (28.66s)
PASS
ok  	github.com/pulumi/terraform-provider-aws/aws	79.354s

lukehoban · 2017-08-01T21:56:49Z

@radeksimko Please let me know if there's any questions on concerns on this that I can help address.

The body property allows supplying an Swagger/OpenAPI spec to a RestAPI to create the API definition using the routes and integrations defined in the spec.

lukehoban · 2017-08-10T18:25:45Z

Updated tests to create valid REST API definitions and add test results to summary.

cc @radeksimko @Ninir @betabandido @apparentlymart for feedback.

Ninir

Hey @lukehoban

This looks good at first sight :)
Just left a few comments to be addressed, more for styling & debugging :)

Thanks!

Ninir · 2017-08-21T21:01:05Z

aws/resource_aws_api_gateway_rest_api.go

+			Body:      []byte(body.(string)),
+		})
+		if err != nil {
+			return fmt.Errorf("Error putting API Gateway specification: %s", err)


Could you use errwrap.Wrapf("Error creating API Gateway specification: {{err}}", err) there please? (notice the creating word, to make it clear we are putting by creating)

It does no harm, but I think the wrapper here isn't necessary as the only place the error will escalate to is the UI, where all that matters is string-based error message user will read, not the original error type.

Ninir · 2017-08-21T21:03:33Z

aws/resource_aws_api_gateway_rest_api.go

+	if body, ok := d.GetOk("body"); ok {
+		_, err := conn.PutRestApi(&apigateway.PutRestApiInput{
+			RestApiId: gateway.Id,
+			Mode:      aws.String("overwrite"),


Can you use the apigateway.PutModeOverwrite constant here and line 178?

Ninir · 2017-08-21T21:08:12Z

aws/resource_aws_api_gateway_rest_api_test.go

+  }
+}
+EOF
+}


Can you fix the space indentation here please? 😄 (mostly at the beginning of the JSON structure)

Ninir · 2017-08-21T21:14:09Z

aws/resource_aws_api_gateway_rest_api.go

+				Body:      []byte(body.(string)),
+			})
+			if err != nil {
+				return err


I would go for the same as line 91 here: errwrap.Wrapf("Error updating API Gateway specification: {{err}}", err). Thoughts?

It does no harm, but I think the wrapper here isn't necessary as the only place the error will escalate to is the UI, where all that matters is string-based error message user will read, not the original error type.

Ninir · 2017-08-21T21:15:17Z

aws/resource_aws_api_gateway_rest_api.go

@@ -76,6 +81,17 @@ func resourceAwsApiGatewayRestApiCreate(d *schema.ResourceData, meta interface{}

 	d.SetId(*gateway.Id)

+	if body, ok := d.GetOk("body"); ok {
+		_, err := conn.PutRestApi(&apigateway.PutRestApiInput{


Could you log in the logs that we are setting the specification?
Just for verbosity & debugging :D

Same for the update would be highly appreciated!

radeksimko · 2017-08-22T06:38:27Z

Thanks @Ninir for the review and @lukehoban for sending the PR - other than what you mentioned we should also document clearly which resources collide with this field/approach, I believe these are:

aws_api_gateway_gateway_response
aws_api_gateway_integration
aws_api_gateway_integration_response
aws_api_gateway_method
aws_api_gateway_method_response
aws_api_gateway_method_settings
aws_api_gateway_model
aws_api_gateway_resource

i.e. it should be clear to the user that they cannot use both Swagger and these resources together for the same API.

lukehoban · 2017-08-25T23:32:07Z

Thanks @Ninir and @radeksimko!

Updated PR to address feedback. Please take a look at the doc updates in particular.

radeksimko · 2017-08-29T05:49:21Z

website/docs/r/api_gateway_rest_api.html.markdown

@@ -26,6 +26,18 @@ The following arguments are supported:
 * `name` - (Required) The name of the REST API
 * `description` - (Optional) The description of the REST API
 * `binary_media_types` - (Optional) The list of binary media types supported by the RestApi. By default, the RestApi supports only UTF-8-encoded text payloads.
+* `body` - (Optional) An OpenAPI specification that defines the set of routes and integrations to create as part of the REST API.
+
+__Note__: If the `body` argument is provided, the OpenAPI specification will be used to configure the resources, methods and integrations for the Rest API. If this argument is provided, the following resources should not be configured manually for the same REST API, as updates may cause manual resource updates to be overwritten:


the following resources should not be configured manually

I'd rather say should not be managed as separate ones as that's more in-line with the language used in the rest of our docs.

radeksimko

This now LGTM except one nitpick comment, but I'll defer the final decision & push of the merge button to @Ninir as he started the review originally.

Ninir · 2017-08-29T07:32:14Z

hey @lukehoban !

Just pushed the documentation update following @radeksimko 's recommandation.
Thanks for all the work here, looks very good! :)

Now merging, cheers!

ghost · 2017-09-18T21:26:58Z

Hi All - When will this feature be available. I have v0.10.5 on my mac.

resource "aws_api_gateway_rest_api" "MyDemoAPI" {
name = "MyDemoAPI"
description = "This is my API for demonstration purposes"
body = "./api/swagger.yml"
}

when I run terraform plan , I get
1 error(s) occurred:

module.apigateway.aws_api_gateway_rest_api.MyDemoAPI: : invalid or unknown key: body

Am I missing some thing here. Please let me know

bvingerh · 2017-09-22T09:10:38Z

Same with v0.10.6: body argument not recognized.
That's the fourth release of Terraform without a release of the AWS provider (which is stuck at v0.1.4 of August 8)...

ghost · 2017-09-25T21:20:41Z

I built the terraform-provider-aws using the instructions from README. Now when I run terraform plan, the body tag is recognized. But when I apply, I get the following error. I used the same swagger with AWS SDK/CLI and it worked fine. Can you please help

$ terraform apply
module.apigateway.aws_api_gateway_rest_api.MyDemoAPI: Creating...
body: "" => "../api/swagger.json"
created_date: "" => ""
description: "" => "This is my API for demonstration purposes"
name: "" => "MyDemoAPI"
root_resource_id: "" => ""
Error applying plan:

1 error(s) occurred:

module.apigateway.aws_api_gateway_rest_api.MyDemoAPI: 1 error(s) occurred:
aws_api_gateway_rest_api.MyDemoAPI: Error creating API Gateway specification: BadRequestException: Invalid Swagger 2.0 input.
status code: 400, request id: 5ec75ec4-a235-11e7-b1f5-7b2e853b338e

woody3549 · 2017-09-26T09:53:52Z

Hi all,

Thanks for this great work.
Can you please let me know when it's going to be released ?

Thanks

Add a body property to API Gateway RestAPI for Swagger import support

ebarault · 2017-10-13T13:33:47Z

@kirangangineni : after reviewing the pull request itself i realized that the provider does not expect the body param as a path to the file as you are doing, but as the body itself.

Hence i managed having this work with the following:

data "template_file" "swagger_api" {
  template = "${file("${path.module}/swagger/api.yml")}"
}

resource "aws_api_gateway_rest_api" "MyDemoAPI" {
  name = "${format("%s", var.name == "" ? "api" : "api-${var.name}")}"
  description = "This is my API for demonstration purposes"
  body = "${data.template_file.swagger_api.rendered}"
}

This should be added in the docs here : https://www.terraform.io/docs/providers/aws/r/api_gateway_rest_api.html#body

thanks @lukehoban for bringing this feature !

Ninir · 2017-10-26T21:13:12Z

Hi folks,

the AWS provider (this repository) is release independently from the core one (https://github.com/hashicorp/terraform).
This means you can use Terraform 0.10.8 with the AWS Provider version 1.0.0, 1.1.0, etc..

the body property was added in the 1.0.0 release: to ensure you get it right, remove the .terraform/plugins directory and run terraform init again: this will get the latest version and you should be all set.

Thus being said, @ebarault is right: the body property expects the body of the file, not the path to the file. As in:

using data template

resource "aws_api_gateway_rest_api" "api" {
  ...
  body = "${data.template_file.swagger_api.rendered}"
}

using a file

resource "aws_api_gateway_rest_api" "api" {
  ...
  body = "${file("myfile.json"}"
}

using inline strings

resource "aws_api_gateway_rest_api" "api" {
  ...
  body = <<YAML
myfile
YAML
}

Tell us how it goes! :)

Willis0826 · 2018-09-13T09:13:48Z

@Ninir
Thanks for the clearly demonstration : ) But here is a typo in your post :

body = "${file("myfile.json"}"

I believe it should be ${file("...")}

ghost · 2020-04-03T17:37:03Z

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!

radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Jul 24, 2017

Add a body property to API Gateway RestAPI

4203007

The body property allows supplying an Swagger/OpenAPI spec to a RestAPI to create the API definition using the routes and integrations defined in the spec.

lukehoban force-pushed the restapi_body branch from b5793e1 to 4203007 Compare August 10, 2017 18:16

Ninir suggested changes Aug 21, 2017

View reviewed changes

radeksimko added the waiting-response Maintainers are waiting on response from community or contributor. label Aug 22, 2017

Respond to PR feedback on hashicorp#1197

26c060e

radeksimko removed the waiting-response Maintainers are waiting on response from community or contributor. label Aug 29, 2017

radeksimko reviewed Aug 29, 2017

View reviewed changes

radeksimko approved these changes Aug 29, 2017

View reviewed changes

Update api_gateway_rest_api.html.markdown

ffec7ac

Ninir approved these changes Aug 29, 2017

View reviewed changes

Ninir merged commit 0335c99 into hashicorp:master Aug 29, 2017

lukehoban deleted the restapi_body branch August 29, 2017 16:00

nbaztec pushed a commit to nbaztec/terraform-provider-aws that referenced this pull request Sep 26, 2017

Respond to PR feedback on hashicorp#1197

6e3eb22

nbaztec pushed a commit to nbaztec/terraform-provider-aws that referenced this pull request Sep 26, 2017

Merge pull request hashicorp#1197 from lukehoban/restapi_body

a62196e

Add a body property to API Gateway RestAPI for Swagger import support

apparentlymart mentioned this pull request Oct 31, 2017

provider/aws: API Gateway additional resources hashicorp/terraform#6175

Closed

eefahy mentioned this pull request Dec 13, 2017

AWS API Deploy practices sul-dlss-labs/identifier-service#23

Closed

cmharlow mentioned this pull request Feb 21, 2018

AWS API Deploy practices sul-dlss-deprecated/dataOps#36

Open

bogeylnj mentioned this pull request Mar 4, 2019

Terraform constantly updates resource policy on API Gateway #5549

Closed

ghost locked and limited conversation to collaborators Apr 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a body property to API Gateway RestAPI for Swagger import support #1197

Add a body property to API Gateway RestAPI for Swagger import support #1197

lukehoban commented Jul 20, 2017 •

edited

Loading

lukehoban commented Aug 1, 2017

lukehoban commented Aug 10, 2017

Ninir left a comment

Ninir Aug 21, 2017

radeksimko Aug 22, 2017

Ninir Aug 21, 2017

Ninir Aug 21, 2017

Ninir Aug 21, 2017

radeksimko Aug 22, 2017

Ninir Aug 21, 2017

radeksimko commented Aug 22, 2017 •

edited

Loading

lukehoban commented Aug 25, 2017

radeksimko Aug 29, 2017

radeksimko left a comment

Ninir commented Aug 29, 2017 •

edited

Loading

ghost commented Sep 18, 2017

bvingerh commented Sep 22, 2017

ghost commented Sep 25, 2017

woody3549 commented Sep 26, 2017

ebarault commented Oct 13, 2017 •

edited

Loading

Ninir commented Oct 26, 2017

Willis0826 commented Sep 13, 2018 •

edited

Loading

ghost commented Apr 3, 2020

Add a body property to API Gateway RestAPI for Swagger import support #1197

Add a body property to API Gateway RestAPI for Swagger import support #1197

Conversation

lukehoban commented Jul 20, 2017 • edited Loading

Description

Tests

lukehoban commented Aug 1, 2017

lukehoban commented Aug 10, 2017

Ninir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radeksimko commented Aug 22, 2017 • edited Loading

lukehoban commented Aug 25, 2017

Choose a reason for hiding this comment

radeksimko left a comment

Choose a reason for hiding this comment

Ninir commented Aug 29, 2017 • edited Loading

ghost commented Sep 18, 2017

bvingerh commented Sep 22, 2017

ghost commented Sep 25, 2017

woody3549 commented Sep 26, 2017

ebarault commented Oct 13, 2017 • edited Loading

Ninir commented Oct 26, 2017

using data template

using a file

using inline strings

Willis0826 commented Sep 13, 2018 • edited Loading

ghost commented Apr 3, 2020

lukehoban commented Jul 20, 2017 •

edited

Loading

radeksimko commented Aug 22, 2017 •

edited

Loading

Ninir commented Aug 29, 2017 •

edited

Loading

ebarault commented Oct 13, 2017 •

edited

Loading

Willis0826 commented Sep 13, 2018 •

edited

Loading