-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
resource/api_gateway_integration: support vpc link #3428
resource/api_gateway_integration: support vpc link #3428
Conversation
|
Would love to have this ASAP! |
Can we get this merged and relaeased ASAP please? The previous vpc_link resource is great but it's useless without the integration piece. |
FWIW we've moved on from using the integration directly and are using the
swagger import feature of aws_api_gateway_rest_api. You can set the VPC
links there, not sure if that is an option for you.
…On Tue, Feb 27, 2018 at 4:12 PM, Gary Poore ***@***.***> wrote:
Can we get this merged and relaeased ASAP please? The previous vpc_link
resource is great but it's useless without the integration piece.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3428 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFDPgSNODD77X9Xv9ifWxnauiRYL6rcjks5tZG_agaJpZM4SJTd9>
.
|
I'd like to move away from integrations aswell as they are pretty cumbersome, but AFAIK swagger import only supports Swagger2 and not the new OAS3 which all of our stuff is defined in. i guess that will come eventually. |
Yeah it only supports 2.0 and even then only a subset of it; their importer
is kind of strict. But this is an API Gateway detail, even if you define
all your models and everything in TF you have to conform to what the API
Gateway supports.
…On Wed, Feb 28, 2018 at 4:35 PM, Gary Poore ***@***.***> wrote:
I'd like to move away from integrations aswell as they are pretty
cumbersome, but AFAIK swagger import only supports Swagger2 and not the new
OAS3 which all of our stuff is defined in.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3428 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFDPgYxNe4K4gwpKZL3gV37UwccCUdnFks5tZcalgaJpZM4SJTd9>
.
|
Would like to see this PR merged soon. Thx |
07273ef
to
995c78e
Compare
995c78e
to
7382e36
Compare
7382e36
to
d06948c
Compare
resolve conflicts with master. |
d06948c
to
05a41d2
Compare
Some attention on this feature? |
Without wanting to be that guy who just +1s it, I'd like to see this feature in soon too. |
Any chance that this will get merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for this! Can you please add the two missing d.Set()
? Then hopefully this is ready to ship. 🚀
var connectionId *string | ||
if *connectionType == apigateway.ConnectionTypeVpcLink { | ||
if _, ok := d.GetOk("connection_id"); !ok { | ||
return fmt.Errorf("connection_id required when connection_type set to VPC_LINK") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can move this to a CustomizeDiff
for plan time validation at some point 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried but didn't manage to do it because of question hashicorp/terraform#17595
@@ -55,6 +55,21 @@ func resourceAwsApiGatewayIntegration() *schema.Resource { | |||
}, false), | |||
}, | |||
|
|||
"connection_type": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're missing d.Set("connection_type", integration.ConnectionType)
in the read function 😄
}, false), | ||
}, | ||
|
||
"connection_id": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're missing d.Set("connection_id", integration.ConnectionId)
in the read function 😄
just realized update is not handled at all 😱 |
…id in read function
bc1423e
to
b146a80
Compare
|
Thanks for working on this. We also really need this functionality. |
@@ -240,6 +269,8 @@ func resourceAwsApiGatewayIntegrationRead(d *schema.ResourceData, meta interface | |||
d.Set("request_parameters", aws.StringValueMap(integration.RequestParameters)) | |||
d.Set("request_parameters_in_json", aws.StringValueMap(integration.RequestParameters)) | |||
d.Set("passthrough_behavior", integration.PassthroughBehavior) | |||
d.Set("connection_type", integration.ConnectionType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this might not always be returned? Maybe we should set it to INTERNET
if nil
--- FAIL: TestAccAWSAPIGatewayIntegrationResponse_basic (110.48s)
testing.go:518: Step 0 error: After applying this step, the plan was not empty:
DIFF:
UPDATE: aws_api_gateway_integration.test
connection_type: "" => "INTERNET"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated but a bit curious though.
I think I covered the attribute check in TestAccAWSAPIGatewayIntegration_integrationType
. Is "not always" behavior caused by regional difference from aws api? If so, it looks tricky to discover.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really sure, honestly. It seems some service APIs always return everything and others don't. 🙁
727d04d
to
54c6734
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for this @loivis!! 🚀
5 tests passed (all tests)
=== RUN TestAccAWSAPIGatewayIntegration_cache_key_parameters
--- PASS: TestAccAWSAPIGatewayIntegration_cache_key_parameters (7.02s)
=== RUN TestAccAWSAPIGatewayIntegration_basic
--- PASS: TestAccAWSAPIGatewayIntegration_basic (38.42s)
=== RUN TestAccAWSAPIGatewayIntegrationResponse_basic
--- PASS: TestAccAWSAPIGatewayIntegrationResponse_basic (96.08s)
=== RUN TestAccAWSAPIGatewayIntegration_contentHandling
--- PASS: TestAccAWSAPIGatewayIntegration_contentHandling (124.82s)
=== RUN TestAccAWSAPIGatewayIntegration_integrationType
--- PASS: TestAccAWSAPIGatewayIntegration_integrationType (644.46s)
This has been released in version 1.15.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
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! |
Follow up on #2512 to enable vpc link support for resource
aws_api_gateway_integration