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: Support for aws_nat_gateway #4381

Merged
merged 1 commit into from
Dec 18, 2015
Merged

provider/aws: Support for aws_nat_gateway #4381

merged 1 commit into from
Dec 18, 2015

Conversation

jen20
Copy link
Contributor

@jen20 jen20 commented Dec 18, 2015

Example usage:

provider "aws" {
    region = "us-west-2"
}

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

resource "aws_subnet" "private" {
    vpc_id = "${aws_vpc.test.id}"
    cidr_block = "10.0.1.0/24"
    map_public_ip_on_launch = false
}

resource "aws_internet_gateway" "gw" {
    vpc_id = "${aws_vpc.test.id}"
}

resource "aws_eip" "nat_gateway" {
    vpc = true
}

resource "aws_nat_gateway" "gateway" {
    allocation_id = "${aws_eip.nat_gateway.id}"
    subnet_id = "${aws_subnet.private.id}"
}

output "vpc_id" {
    value = "${aws_vpc.test.id}"
}

output "subnet_id" {
    value = "${aws_subnet.private.id}"
}

output "igw_id" {
    value = "${aws_internet_gateway.gw.id}"
}

output "eip_id" {
    value = "${aws_eip.nat_gateway.id}"
}

output "nat_public_ip" {
    value = "${aws_nat_gateway.gateway.public_ip}"
}

output "nat_private_ip" {
    value = "${aws_nat_gateway.gateway.private_ip}"
}

output "nat_nic_id" {
    value = "${aws_nat_gateway.gateway.network_interface_id}"
}

@jen20
Copy link
Contributor Author

jen20 commented Dec 18, 2015

Will (eventually) fix #4374.

return err
}
if ngRaw == nil {
d.SetId("")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we insert a debug log here, indicating that nat gateway d.Id() wasn't found? Otherwise, in a plan/refresh situation where this line hits, we just remove it from the state file and don't tell anyone why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@jen20 jen20 changed the title [WIP] provider/aws: Initial support for aws_nat_gateway provider/aws: Support for aws_nat_gateway Dec 18, 2015
@jen20
Copy link
Contributor Author

jen20 commented Dec 18, 2015

Acceptance test output:

$ make testacc TEST=./builtin/providers/aws TESTARGS="-run AWSNatGateway"
==> Checking that code complies with gofmt requirements...
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run AWSNatGateway -timeout 90m
=== RUN TestAccAWSNatGateway_basic
--- PASS: TestAccAWSNatGateway_basic (382.31s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    382.328s

@phinze
Copy link
Contributor

phinze commented Dec 18, 2015

LGTM!

jen20 added a commit that referenced this pull request Dec 18, 2015
provider/aws: Support for aws_nat_gateway
@jen20 jen20 merged commit ceee811 into master Dec 18, 2015
@jen20 jen20 deleted the f-nat-gateway branch December 18, 2015 21:00
@conorgil
Copy link

@jen20 thanks for getting this done so quickly! It will really save my team tons of time and effort. I'm really impressed with the turn around time.

@gosuri
Copy link

gosuri commented Dec 18, 2015

👍 Insane turnaround

@antonbabenko
Copy link
Contributor

👍 This was fast! So fast, that AWS CloudFormation does not support it yet.

@jgross206
Copy link

(thumbsup) Awesome

@ketzacoatl
Copy link
Contributor

GREAT WORK!

@dalethestirling
Copy link

👍 thanks guys this will be a huge help to us

@ajlanghorn
Copy link
Contributor

❤️ ❤️ ❤️ ❤️ ❤️

@worldspawn
Copy link

When were u planning on including this in a release?

@rajinders
Copy link

We need to create nat gateway soon. It will be good to know when this feature will be included in a release.

@odise
Copy link
Contributor

odise commented Jan 4, 2016

Guys, it seems that this change breaks as_route_table resources on regions that doesn't support NatGateways yet (like eu-central). When deploying such a resource AWS complains:

aws_route_table.main: InvalidParameter: NatGatewayId is not a valid parameter.
    status code: 400, request id:

Resource:

resource "aws_route_table" "main" {
  vpc_id = "${aws_vpc.vpc.id}"
  route {
    cidr_block = "0.0.0.0/0"
    gateway_id = "${aws_internet_gateway.gateway.id}"
  }
}

@odise
Copy link
Contributor

odise commented Jan 4, 2016

After investigating a bit further setting NatGatewayId in resource_aws_route_table.go to nil if it is not set should do the trick. Tests passes for eu-central-1 region afterwards.

Suggestion:

diff --git i/builtin/providers/aws/resource_aws_route_table.go w/builtin/providers/aws/resource_aws_route_table.go
index 752b771..dc31504 100644
--- i/builtin/providers/aws/resource_aws_route_table.go
+++ w/builtin/providers/aws/resource_aws_route_table.go
@@ -285,12 +285,15 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error
                // Then loop through all the newly configured routes and create them
                for _, route := range nrs.List() {
                        m := route.(map[string]interface{})
-
+                       natGateway := aws.String(m["nat_gateway_id"].(string))
+                       if len(*natGateway) == 0 {
+                               natGateway = nil
+                       }
                        opts := ec2.CreateRouteInput{
                                RouteTableId:           aws.String(d.Id()),
                                DestinationCidrBlock:   aws.String(m["cidr_block"].(string)),
                                GatewayId:              aws.String(m["gateway_id"].(string)),
-                               NatGatewayId:           aws.String(m["nat_gateway_id"].(string)),
+                               NatGatewayId:           natGateway,
                                InstanceId:             aws.String(m["instance_id"].(string)),
                                VpcPeeringConnectionId: aws.String(m["vpc_peering_connection_id"].(string)),
                                NetworkInterfaceId:     aws.String(m["network_interface_id"].(string)),

@jen20
Copy link
Contributor Author

jen20 commented Jan 4, 2016

@odise good catch - this definitely needs to be fixed prior to release. I'll open up a new issue for it.

@willejs
Copy link

willejs commented Jan 6, 2016

When is this up for release? 😸

@ketzacoatl
Copy link
Contributor

"as soon as testing and the release process it complete" - See the mailing list for the original confirmation.

@willejs
Copy link

willejs commented Jan 6, 2016

thanks @ketzacoatl

@mslayton-abra
Copy link

FWIW, I created an aws_nat_gateway as part of a deployment in us-west-2.
Though nat gateways are supported in this region, the following errors are reported by
terraform when creating the aws_route_table resources that use the new nat object:

  • aws_route_table.this_table: InvalidInstanceID.Malformed: Invalid id: "nat-0b4111d31363d694c"
    status code: 400, request id:
  • aws_route_table.this_table: InvalidInstanceID.Malformed: Invalid id: "nat-0b4111d31363d694c"
    status code: 400, request id:
  • aws_route_table.this_table: InvalidInstanceID.Malformed: Invalid id: "nat-0b4111d31363d694c"
    status code: 400, request id:

I was using the wrong syntax in my route table definition.
You must use: nat_gateway_id, not gateway_id. FYI.

@ghost
Copy link

ghost commented Apr 28, 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 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.