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

New Resource: aws_api_gateway_vpc_link #2512

Merged
merged 8 commits into from
Feb 16, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions aws/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ func Provider() terraform.ResourceProvider {
"aws_api_gateway_stage": resourceAwsApiGatewayStage(),
"aws_api_gateway_usage_plan": resourceAwsApiGatewayUsagePlan(),
"aws_api_gateway_usage_plan_key": resourceAwsApiGatewayUsagePlanKey(),
"aws_api_gateway_vpc_link": resourceAwsApiGatewayVpcLink(),
"aws_app_cookie_stickiness_policy": resourceAwsAppCookieStickinessPolicy(),
"aws_appautoscaling_target": resourceAwsAppautoscalingTarget(),
"aws_appautoscaling_policy": resourceAwsAppautoscalingPolicy(),
Expand Down
204 changes: 204 additions & 0 deletions aws/resource_aws_api_gateway_vpc_link.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
package aws

import (
"fmt"
"log"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/apigateway"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
)

func resourceAwsApiGatewayVpcLink() *schema.Resource {
return &schema.Resource{
Create: resourceAwsApiGatewayVpcLinkCreate,
Read: resourceAwsApiGatewayVpcLinkRead,
Update: resourceAwsApiGatewayVpcLinkUpdate,
Delete: resourceAwsApiGatewayVpcLinkDelete,

Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
},
"description": {
Type: schema.TypeString,
Optional: true,
},
"target_arns": {
Type: schema.TypeSet,
Required: true,
ForceNew: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
Copy link
Member

Choose a reason for hiding this comment

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

Is there any particular reason this should be implemented as 1-1 relationship instead of reflecting the API, i.e. allowing users to assign multiple LBs to one VPC link?

},
}
}

func resourceAwsApiGatewayVpcLinkCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).apigateway

input := &apigateway.CreateVpcLinkInput{
Name: aws.String(d.Get("name").(string)),
TargetArns: expandStringList(d.Get("target_arns").(*schema.Set).List()),
}
if v, ok := d.GetOk("description"); ok {
input.Description = aws.String(v.(string))
}

resp, err := conn.CreateVpcLink(input)
if err != nil {
return err
}

d.SetId(*resp.Id)

stateConf := &resource.StateChangeConf{
Pending: []string{apigateway.VpcLinkStatusPending},
Target: []string{apigateway.VpcLinkStatusAvailable},
Refresh: apigatewayVpcLinkRefreshStatusFunc(conn, *resp.Id),
Timeout: 8 * time.Minute,
MinTimeout: 3 * time.Second,
}

_, err = stateConf.WaitForState()
if err != nil {
d.SetId("")
return fmt.Errorf("[WARN] Error waiting for APIGateway Vpc Link status to be \"%s\": %s", apigateway.VpcLinkStatusAvailable, err)
}

return nil
}

func resourceAwsApiGatewayVpcLinkRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).apigateway

input := &apigateway.GetVpcLinkInput{
VpcLinkId: aws.String(d.Id()),
}

resp, err := conn.GetVpcLink(input)
if err != nil {
if isAWSErr(err, apigateway.ErrCodeNotFoundException, "") {
log.Printf("[WARN] VPC Link %s not found, removing from state", d.Id())
d.SetId("")
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Do you mind adding our usual WARN log message here?

return nil
}
return err
}

d.Set("name", resp.Name)
d.Set("description", resp.Description)
d.Set("target_arn", flattenStringList(resp.TargetArns))
return nil
}

func resourceAwsApiGatewayVpcLinkUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).apigateway

operations := make([]*apigateway.PatchOperation, 0)

if d.HasChange("name") {
operations = append(operations, &apigateway.PatchOperation{
Op: aws.String("replace"),
Path: aws.String("/name"),
Value: aws.String(d.Get("name").(string)),
})
}

if d.HasChange("description") {
operations = append(operations, &apigateway.PatchOperation{
Op: aws.String("replace"),
Path: aws.String("/description"),
Value: aws.String(d.Get("description").(string)),
})
}

input := &apigateway.UpdateVpcLinkInput{
VpcLinkId: aws.String(d.Id()),
PatchOperations: operations,
}

_, err := conn.UpdateVpcLink(input)
if err != nil {
if isAWSErr(err, apigateway.ErrCodeNotFoundException, "") {
log.Printf("[WARN] VPC Link %s not found, removing from state", d.Id())
d.SetId("")
return nil
}
return err
}

stateConf := &resource.StateChangeConf{
Pending: []string{apigateway.VpcLinkStatusPending},
Target: []string{apigateway.VpcLinkStatusAvailable},
Refresh: apigatewayVpcLinkRefreshStatusFunc(conn, d.Id()),
Timeout: 8 * time.Minute,
MinTimeout: 3 * time.Second,
}

_, err = stateConf.WaitForState()
if err != nil {
return fmt.Errorf("[WARN] Error waiting for APIGateway Vpc Link status to be \"%s\": %s", apigateway.VpcLinkStatusAvailable, err)
}

return nil
}

func resourceAwsApiGatewayVpcLinkDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).apigateway

input := &apigateway.DeleteVpcLinkInput{
VpcLinkId: aws.String(d.Id()),
}

_, err := conn.DeleteVpcLink(input)
if err != nil {
if isAWSErr(err, apigateway.ErrCodeNotFoundException, "") {
return nil
}
return err
}

stateConf := resource.StateChangeConf{
Pending: []string{apigateway.VpcLinkStatusPending,
apigateway.VpcLinkStatusAvailable,
apigateway.VpcLinkStatusDeleting},
Target: []string{""},
Timeout: 5 * time.Minute,
MinTimeout: 1 * time.Second,
Refresh: func() (interface{}, string, error) {
resp, err := conn.GetVpcLink(&apigateway.GetVpcLinkInput{
VpcLinkId: aws.String(d.Id()),
})
if err != nil {
if isAWSErr(err, apigateway.ErrCodeNotFoundException, "") {
return 1, "", nil
}
return nil, "failed", err
}
return resp, *resp.Status, nil
},
}

if _, err := stateConf.WaitForState(); err != nil {
return err
}

return nil
}

func apigatewayVpcLinkRefreshStatusFunc(conn *apigateway.APIGateway, vl string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
input := &apigateway.GetVpcLinkInput{
VpcLinkId: aws.String(vl),
}
resp, err := conn.GetVpcLink(input)
if err != nil {
return nil, "failed", err
}
return resp, *resp.Status, nil
}
}
139 changes: 139 additions & 0 deletions aws/resource_aws_api_gateway_vpc_link_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package aws

import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/apigateway"
"github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)

func TestAccAwsAPIGatewayVpcLink_basic(t *testing.T) {
rName := acctest.RandString(5)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsAPIGatewayVpcLinkDestroy,
Steps: []resource.TestStep{
{
Config: testAccAPIGatewayVpcLinkConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsAPIGatewayVpcLinkExists("aws_api_gateway_vpc_link.test"),
resource.TestCheckResourceAttr("aws_api_gateway_vpc_link.test", "name", fmt.Sprintf("tf-apigateway-%s", rName)),
resource.TestCheckResourceAttr("aws_api_gateway_vpc_link.test", "description", "test"),
resource.TestCheckResourceAttr("aws_api_gateway_vpc_link.test", "target_arns.#", "2"),
),
},
{
Config: testAccAPIGatewayVpcLinkConfig_Update(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsAPIGatewayVpcLinkExists("aws_api_gateway_vpc_link.test"),
resource.TestCheckResourceAttr("aws_api_gateway_vpc_link.test", "name", fmt.Sprintf("tf-apigateway-update-%s", rName)),
resource.TestCheckResourceAttr("aws_api_gateway_vpc_link.test", "description", "test update"),
resource.TestCheckResourceAttr("aws_api_gateway_vpc_link.test", "target_arns.#", "2"),
),
},
},
})
}

func testAccCheckAwsAPIGatewayVpcLinkDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).apigateway

for _, rs := range s.RootModule().Resources {
if rs.Type != "aws_api_gateway_vpc_link" {
continue
}

input := &apigateway.GetVpcLinkInput{
VpcLinkId: aws.String(rs.Primary.ID),
}

_, err := conn.GetVpcLink(input)
if err != nil {
if isAWSErr(err, apigateway.ErrCodeNotFoundException, "") {
return nil
}
return err
}

return fmt.Errorf("Expected VPC Link to be destroyed, %s found", rs.Primary.ID)
}

return nil
}

func testAccCheckAwsAPIGatewayVpcLinkExists(name string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
if !ok {
return fmt.Errorf("Not found: %s", name)
}

Copy link
Member

Choose a reason for hiding this comment

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

Do you mind checking the API here also?

conn := testAccProvider.Meta().(*AWSClient).apigateway

input := &apigateway.GetVpcLinkInput{
VpcLinkId: aws.String(rs.Primary.ID),
}

_, err := conn.GetVpcLink(input)
if err != nil {
return err
}

return nil
}
}

func testAccAPIGatewayVpcLinkConfig_basis(rName string) string {
return fmt.Sprintf(`
resource "aws_lb" "test_a" {
name = "tf-lb-%s"
internal = true
load_balancer_type = "network"
subnets = ["${aws_subnet.test.id}"]
}

resource "aws_lb" "test_b" {
name = "tf-lb-%s"
internal = true
load_balancer_type = "network"
subnets = ["${aws_subnet.test.id}"]
}

resource "aws_vpc" "test" {
cidr_block = "10.10.0.0/16"
}

data "aws_availability_zones" "test" {}

resource "aws_subnet" "test" {
vpc_id = "${aws_vpc.test.id}"
cidr_block = "10.10.0.0/21"
availability_zone = "${data.aws_availability_zones.test.names[0]}"
}
`, rName, rName)
Copy link
Member

Choose a reason for hiding this comment

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

The main reason the test is failing the way it's failing is because two resources (aws_lb.test_a and aws_lb.test_b) both have the same name and the TypeSet will internally merge them into a single ARN since the ARN is the same.

Copy link
Member

Choose a reason for hiding this comment

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

Once you fix that you get more reasonable error which I think we can just present to the user:

* aws_api_gateway_vpc_link.test: BadRequestException: More than one target arn specified for vpc link.
  status code: 400, request id: d4eabe6c-0ff8-11e8-bea2-d9187e5e8757

As you rightly mentioned Amazon currently doesn't support more than 1 LB though, so we'll have to keep this test focused on single LB only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right! Thank you for correct.

}

func testAccAPIGatewayVpcLinkConfig(rName string) string {
return testAccAPIGatewayVpcLinkConfig_basis(rName) + fmt.Sprintf(`
resource "aws_api_gateway_vpc_link" "test" {
name = "tf-apigateway-%s"
description = "test"
target_arns = ["${aws_lb.test_a.arn}","${aws_lb.test_b.arn}"]
}
`, rName)
}

func testAccAPIGatewayVpcLinkConfig_Update(rName string) string {
return testAccAPIGatewayVpcLinkConfig_basis(rName) + fmt.Sprintf(`
resource "aws_api_gateway_vpc_link" "test" {
name = "tf-apigateway-update-%s"
description = "test update"
target_arns = ["${aws_lb.test_a.arn}","${aws_lb.test_b.arn}"]
}
`, rName)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also add a test with an actual API Gateway and a HTTP endpoint, so that we ensure it's all ok.
I'm actually wondering whether we need a hard dependency (using depends_on) on the HTTP integration since this resource is just creating a link between the API Gateway service & the VPC targets... 🤔
Not sure if this could result in race dependencies! 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I couldn't understand what you meant so cloud you describe more details:bow: ? @Ninir

Copy link
Member

Choose a reason for hiding this comment

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

I guess what @Ninir means is that we should add a test which tests the full setup including API Gateway integration leveraging the VPC Link.

It seems we'd also need to update other API Gateway resources to make that happen, namely to add few more attributes to aws_api_gateway_integration per http://docs.aws.amazon.com/apigateway/api-reference/resource/integration/#connectionType

I'm personally ok with shipping this resource as is and addressing the above in a separate PR to keep diffs small and easier to review - what do you think @Ninir ?

3 changes: 3 additions & 0 deletions website/aws.erb
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@
<li<%= sidebar_current("docs-aws-resource-api-gateway-usage-plan-key") %>>
<a href="/docs/providers/aws/r/api_gateway_usage_plan_key.html">aws_api_gateway_usage_plan_key</a>
</li>
<li<%= sidebar_current("docs-aws-resource-api-gateway-vpc-link") %>>
<a href="/docs/providers/aws/r/api_gateway_vpc_link.html">aws_api_gateway_vpc_link</a>
</li>
</ul>
</li>

Expand Down
Loading