-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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: Support for aws_nat_gateway #4381
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,181 @@ | ||
package aws | ||
|
||
import ( | ||
"fmt" | ||
"log" | ||
"strings" | ||
"time" | ||
|
||
"github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/aws/awserr" | ||
"github.com/aws/aws-sdk-go/service/ec2" | ||
"github.com/hashicorp/terraform/helper/resource" | ||
"github.com/hashicorp/terraform/helper/schema" | ||
) | ||
|
||
func resourceAwsNatGateway() *schema.Resource { | ||
return &schema.Resource{ | ||
Create: resourceAwsNatGatewayCreate, | ||
Read: resourceAwsNatGatewayRead, | ||
Delete: resourceAwsNatGatewayDelete, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"allocation_id": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"subnet_id": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"network_interface_id": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Computed: true, | ||
}, | ||
|
||
"private_ip": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Computed: true, | ||
}, | ||
|
||
"public_ip": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Computed: true, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func resourceAwsNatGatewayCreate(d *schema.ResourceData, meta interface{}) error { | ||
conn := meta.(*AWSClient).ec2conn | ||
|
||
// Create the NAT Gateway | ||
createOpts := &ec2.CreateNatGatewayInput{ | ||
AllocationId: aws.String(d.Get("allocation_id").(string)), | ||
SubnetId: aws.String(d.Get("subnet_id").(string)), | ||
} | ||
|
||
log.Printf("[DEBUG] Create NAT Gateway: %s", *createOpts) | ||
natResp, err := conn.CreateNatGateway(createOpts) | ||
if err != nil { | ||
return fmt.Errorf("Error creating NAT Gateway: %s", err) | ||
} | ||
|
||
// Get the ID and store it | ||
ng := natResp.NatGateway | ||
d.SetId(*ng.NatGatewayId) | ||
log.Printf("[INFO] NAT Gateway ID: %s", d.Id()) | ||
|
||
// Wait for the NAT Gateway to become available | ||
log.Printf("[DEBUG] Waiting for NAT Gateway (%s) to become available", d.Id()) | ||
stateConf := &resource.StateChangeConf{ | ||
Pending: []string{"pending"}, | ||
Target: "available", | ||
Refresh: NGStateRefreshFunc(conn, d.Id()), | ||
Timeout: 10 * time.Minute, | ||
} | ||
|
||
if _, err := stateConf.WaitForState(); err != nil { | ||
return fmt.Errorf("Error waiting for NAT Gateway (%s) to become available: %s", d.Id(), err) | ||
} | ||
|
||
// Update our attributes and return | ||
return resourceAwsNatGatewayRead(d, meta) | ||
} | ||
|
||
func resourceAwsNatGatewayRead(d *schema.ResourceData, meta interface{}) error { | ||
conn := meta.(*AWSClient).ec2conn | ||
|
||
// Refresh the NAT Gateway state | ||
ngRaw, state, err := NGStateRefreshFunc(conn, d.Id())() | ||
if err != nil { | ||
return err | ||
} | ||
if ngRaw == nil || strings.ToLower(state) == "deleted" { | ||
log.Printf("[INFO] Removing %s from Terraform state as it is not found or in the deleted state.", d.Id()) | ||
d.SetId("") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we insert a debug log here, indicating that nat gateway There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
return nil | ||
} | ||
|
||
// Set NAT Gateway attributes | ||
ng := ngRaw.(*ec2.NatGateway) | ||
address := ng.NatGatewayAddresses[0] | ||
d.Set("network_interface_id", address.NetworkInterfaceId) | ||
d.Set("private_ip", address.PrivateIp) | ||
d.Set("public_ip", address.PublicIp) | ||
|
||
return nil | ||
} | ||
|
||
func resourceAwsNatGatewayDelete(d *schema.ResourceData, meta interface{}) error { | ||
conn := meta.(*AWSClient).ec2conn | ||
deleteOpts := &ec2.DeleteNatGatewayInput{ | ||
NatGatewayId: aws.String(d.Id()), | ||
} | ||
log.Printf("[INFO] Deleting NAT Gateway: %s", d.Id()) | ||
|
||
_, err := conn.DeleteNatGateway(deleteOpts) | ||
if err != nil { | ||
ec2err, ok := err.(awserr.Error) | ||
if !ok { | ||
return err | ||
} | ||
|
||
if ec2err.Code() == "NatGatewayNotFound" { | ||
return nil | ||
} | ||
|
||
return err | ||
} | ||
|
||
stateConf := &resource.StateChangeConf{ | ||
Pending: []string{"deleting"}, | ||
Target: "deleted", | ||
Refresh: NGStateRefreshFunc(conn, d.Id()), | ||
Timeout: 30 * time.Minute, | ||
Delay: 10 * time.Second, | ||
MinTimeout: 10 * time.Second, | ||
} | ||
|
||
_, stateErr := stateConf.WaitForState() | ||
if stateErr != nil { | ||
return fmt.Errorf("Error waiting for NAT Gateway (%s) to delete: %s", d.Id(), err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// NGStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch | ||
// a NAT Gateway. | ||
func NGStateRefreshFunc(conn *ec2.EC2, id string) resource.StateRefreshFunc { | ||
return func() (interface{}, string, error) { | ||
opts := &ec2.DescribeNatGatewaysInput{ | ||
NatGatewayIds: []*string{aws.String(id)}, | ||
} | ||
resp, err := conn.DescribeNatGateways(opts) | ||
if err != nil { | ||
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "NatGatewayNotFound" { | ||
resp = nil | ||
} else { | ||
log.Printf("Error on NGStateRefresh: %s", err) | ||
return nil, "", err | ||
} | ||
} | ||
|
||
if resp == nil { | ||
// Sometimes AWS just has consistency issues and doesn't see | ||
// our instance yet. Return an empty state. | ||
return nil, "", nil | ||
} | ||
|
||
ng := resp.NatGateways[0] | ||
return ng, *ng.State, nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
package aws | ||
|
||
import ( | ||
"fmt" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/aws/awserr" | ||
"github.com/aws/aws-sdk-go/service/ec2" | ||
"github.com/hashicorp/terraform/helper/resource" | ||
"github.com/hashicorp/terraform/terraform" | ||
) | ||
|
||
func TestAccAWSNatGateway_basic(t *testing.T) { | ||
var natGateway ec2.NatGateway | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testAccCheckNatGatewayDestroy, | ||
Steps: []resource.TestStep{ | ||
resource.TestStep{ | ||
Config: testAccNatGatewayConfig, | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckNatGatewayExists("aws_nat_gateway.gateway", &natGateway), | ||
), | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func testAccCheckNatGatewayDestroy(s *terraform.State) error { | ||
conn := testAccProvider.Meta().(*AWSClient).ec2conn | ||
|
||
for _, rs := range s.RootModule().Resources { | ||
if rs.Type != "aws_nat_gateway" { | ||
continue | ||
} | ||
|
||
// Try to find the resource | ||
resp, err := conn.DescribeNatGateways(&ec2.DescribeNatGatewaysInput{ | ||
NatGatewayIds: []*string{aws.String(rs.Primary.ID)}, | ||
}) | ||
if err == nil { | ||
if len(resp.NatGateways) > 0 && strings.ToLower(*resp.NatGateways[0].State) != "deleted" { | ||
return fmt.Errorf("still exists") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// Verify the error is what we want | ||
ec2err, ok := err.(awserr.Error) | ||
if !ok { | ||
return err | ||
} | ||
if ec2err.Code() != "NatGatewayNotFound" { | ||
return err | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func testAccCheckNatGatewayExists(n string, ng *ec2.NatGateway) resource.TestCheckFunc { | ||
return func(s *terraform.State) error { | ||
rs, ok := s.RootModule().Resources[n] | ||
if !ok { | ||
return fmt.Errorf("Not found: %s", n) | ||
} | ||
|
||
if rs.Primary.ID == "" { | ||
return fmt.Errorf("No ID is set") | ||
} | ||
|
||
conn := testAccProvider.Meta().(*AWSClient).ec2conn | ||
resp, err := conn.DescribeNatGateways(&ec2.DescribeNatGatewaysInput{ | ||
NatGatewayIds: []*string{aws.String(rs.Primary.ID)}, | ||
}) | ||
if err != nil { | ||
return err | ||
} | ||
if len(resp.NatGateways) == 0 { | ||
return fmt.Errorf("NatGateway not found") | ||
} | ||
|
||
*ng = *resp.NatGateways[0] | ||
|
||
return nil | ||
} | ||
} | ||
|
||
const testAccNatGatewayConfig = ` | ||
resource "aws_vpc" "vpc" { | ||
cidr_block = "10.0.0.0/16" | ||
} | ||
|
||
resource "aws_subnet" "private" { | ||
vpc_id = "${aws_vpc.vpc.id}" | ||
cidr_block = "10.0.1.0/24" | ||
map_public_ip_on_launch = false | ||
} | ||
|
||
resource "aws_subnet" "public" { | ||
vpc_id = "${aws_vpc.vpc.id}" | ||
cidr_block = "10.0.2.0/24" | ||
map_public_ip_on_launch = true | ||
} | ||
|
||
resource "aws_internet_gateway" "gw" { | ||
vpc_id = "${aws_vpc.vpc.id}" | ||
} | ||
|
||
resource "aws_eip" "nat_gateway" { | ||
vpc = true | ||
} | ||
|
||
// Actual SUT | ||
resource "aws_nat_gateway" "gateway" { | ||
allocation_id = "${aws_eip.nat_gateway.id}" | ||
subnet_id = "${aws_subnet.public.id}" | ||
|
||
depends_on = ["aws_internet_gateway.gw"] | ||
} | ||
|
||
resource "aws_route_table" "private" { | ||
vpc_id = "${aws_vpc.vpc.id}" | ||
|
||
route { | ||
cidr_block = "0.0.0.0/0" | ||
nat_gateway_id = "${aws_nat_gateway.gateway.id}" | ||
} | ||
} | ||
|
||
resource "aws_route_table_association" "private" { | ||
subnet_id = "${aws_subnet.private.id}" | ||
route_table_id = "${aws_route_table.private.id}" | ||
} | ||
|
||
resource "aws_route_table" "public" { | ||
vpc_id = "${aws_vpc.vpc.id}" | ||
|
||
route { | ||
cidr_block = "0.0.0.0/0" | ||
gateway_id = "${aws_internet_gateway.gw.id}" | ||
} | ||
} | ||
|
||
resource "aws_route_table_association" "public" { | ||
subnet_id = "${aws_subnet.public.id}" | ||
route_table_id = "${aws_route_table.public.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.
when looking this over, I was initially confused at the name choice for
allocation_id
instead of something likeelastic_ip_address
oreip
. However, I see that the AWS Go SDK uses the wordallocation_id
as well. It makes sense to remain consistent with the AWS Go SDK, but do you have any idea why they chose the nameallocation_id
? It is not nearly as intuitive as most of the other names for fields in the aws_resources used throughout Terraform. </2 cents>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.
Digging through the Go SDK, I found this line, which defines the
AllocationId
argument.I did some searching and found the AWS CLI docs for allocating an EIP and the naming convention is already defined there in the response output.
I understand why the naming is what it is now, but I still think many folks will find it non-intuitive. What are your thought on changing it to
eip_allocation_id
to give it some more context?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 certainly understand this concern, and did briefly consider
eip_allocation_id
, but currently my (weakly held) opinion is that we should stick as closely to the AWS terminology as possible. @phinze or @catsby may have some input on this?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.
Agreed about the concern - we default to the upstream API's terminology for a few reasons:
For these reasons, the standard I've been using is to consider name changes at the Terraform level "only when there is significant UX benefit to Terraform users".
Applying that standard here, I believe it's going to be clear enough from documentation and context that it's best for us to stick with the upstream name.