diff --git a/pkg/model/names.go b/pkg/model/names.go index 22918af95e..b3fea3a6b3 100644 --- a/pkg/model/names.go +++ b/pkg/model/names.go @@ -194,3 +194,7 @@ func (b *KopsModelContext) LinkToUtilitySubnetInZone(zoneName string) (*awstasks func (b *KopsModelContext) NamePrivateRouteTableInZone(zoneName string) string { return "private-" + zoneName + "." + b.ClusterName() } + +func (b *KopsModelContext) LinkToPrivateRouteTableInZone(zoneName string) *awstasks.RouteTable { + return &awstasks.RouteTable{Name: s(b.NamePrivateRouteTableInZone(zoneName))} +} diff --git a/pkg/model/network.go b/pkg/model/network.go index 617d9b6386..f345814301 100644 --- a/pkg/model/network.go +++ b/pkg/model/network.go @@ -136,7 +136,7 @@ func (b *NetworkModelBuilder) Build(c *fi.ModelBuilderContext) error { // Map the Private subnet to the Private route table c.AddTask(&awstasks.RouteTableAssociation{ Name: s("private-" + subnetSpec.Name + "." + b.ClusterName()), - RouteTable: &awstasks.RouteTable{Name: s(b.NamePrivateRouteTableInZone(subnetSpec.Zone))}, + RouteTable: b.LinkToPrivateRouteTableInZone(subnetSpec.Zone), Subnet: subnet, }) @@ -153,12 +153,13 @@ func (b *NetworkModelBuilder) Build(c *fi.ModelBuilderContext) error { if err != nil { return err } + // Every NGW needs a public (Elastic) IP address, every private // subnet needs a NGW, lets create it. We tie it to a subnet // so we can track it in AWS eip := &awstasks.ElasticIP{ - Name: s(zone + "." + b.ClusterName()), - Subnet: utilitySubnet, + Name: s(zone + "." + b.ClusterName()), + AssociatedNatGatewayRouteTable: b.LinkToPrivateRouteTableInZone(zone), } c.AddTask(eip) @@ -172,7 +173,9 @@ func (b *NetworkModelBuilder) Build(c *fi.ModelBuilderContext) error { ngw := &awstasks.NatGateway{ Name: s(zone + "." + b.ClusterName()), Subnet: utilitySubnet, - ElasticIp: eip, + ElasticIP: eip, + + AssociatedRouteTable: b.LinkToPrivateRouteTableInZone(zone), } c.AddTask(ngw) diff --git a/upup/pkg/fi/cloudup/awstasks/elastic_ip.go b/upup/pkg/fi/cloudup/awstasks/elastic_ip.go index 77687b3f82..3a81224338 100644 --- a/upup/pkg/fi/cloudup/awstasks/elastic_ip.go +++ b/upup/pkg/fi/cloudup/awstasks/elastic_ip.go @@ -38,10 +38,13 @@ type ElasticIP struct { ID *string PublicIP *string - // Allow support for associated subnets - // If you need another resource to tag on (ebs volume) - // you must add it - Subnet *Subnet + // ElasticIPs don't support tags. We instead find it via a related resource. + + // TagOnSubnet tags a subnet with the ElasticIP. Deprecated: doesn't round-trip with terraform. + TagOnSubnet *Subnet + + // AssociatedNatGatewayRouteTable follows the RouteTable -> NatGateway -> ElasticIP + AssociatedNatGatewayRouteTable *RouteTable } var _ fi.CompareWithID = &ElasticIP{} @@ -73,11 +76,37 @@ func (e *ElasticIP) find(cloud awsup.AWSCloud) (*ElasticIP, error) { publicIP := e.PublicIP allocationID := e.ID - // Find via tag on foreign resource - if allocationID == nil && publicIP == nil && e.Subnet.ID != nil { + // Find via RouteTable -> NatGateway -> ElasticIP + if allocationID == nil && publicIP == nil && e.AssociatedNatGatewayRouteTable != nil { + ngw, err := findNatGatewayFromRouteTable(cloud, e.AssociatedNatGatewayRouteTable) + if err != nil { + return nil, fmt.Errorf("error finding AssociatedNatGatewayRouteTable: %v", err) + } + + if ngw == nil { + glog.V(2).Infof("AssociatedNatGatewayRouteTable not found") + } else { + if len(ngw.NatGatewayAddresses) == 0 { + return nil, fmt.Errorf("NatGateway %q has no addresses", *ngw.NatGatewayId) + } + if len(ngw.NatGatewayAddresses) > 1 { + return nil, fmt.Errorf("NatGateway %q has multiple addresses", *ngw.NatGatewayId) + } + allocationID = ngw.NatGatewayAddresses[0].AllocationId + if allocationID == nil { + return nil, fmt.Errorf("NatGateway %q has nil addresses", *ngw.NatGatewayId) + } else { + glog.V(2).Infof("Found ElasticIP AllocationID %q via NatGateway", *allocationID) + } + } + } + + // Find via tag on subnet + // TODO: Deprecated, because doesn't round-trip with terraform + if allocationID == nil && publicIP == nil && e.TagOnSubnet != nil && e.TagOnSubnet.ID != nil { var filters []*ec2.Filter filters = append(filters, awsup.NewEC2Filter("key", "AssociatedElasticIp")) - filters = append(filters, awsup.NewEC2Filter("resource-id", *e.Subnet.ID)) + filters = append(filters, awsup.NewEC2Filter("resource-id", *e.TagOnSubnet.ID)) request := &ec2.DescribeTagsInput{ Filters: filters, @@ -125,7 +154,8 @@ func (e *ElasticIP) find(cloud awsup.AWSCloud) (*ElasticIP, error) { ID: a.AllocationId, PublicIP: a.PublicIp, } - actual.Subnet = e.Subnet + actual.TagOnSubnet = e.TagOnSubnet + actual.AssociatedNatGatewayRouteTable = e.AssociatedNatGatewayRouteTable // ElasticIP don't have a Name (no tags), so we set the name to avoid spurious changes actual.Name = e.Name @@ -159,8 +189,8 @@ func (s *ElasticIP) CheckChanges(a, e, changes *ElasticIP) error { if changes.PublicIP != nil { return fi.CannotChangeField("PublicIP") } - if changes.Subnet != nil { - return fi.CannotChangeField("Subnet") + if changes.TagOnSubnet != nil { + return fi.CannotChangeField("TagOnSubnet") } if changes.ID != nil { return fi.CannotChangeField("ID") @@ -197,15 +227,15 @@ func (_ *ElasticIP) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *ElasticIP) e } // Tag the associated subnet - if e.Subnet == nil { + if e.TagOnSubnet == nil { return fmt.Errorf("Subnet not set") - } else if e.Subnet.ID == nil { + } else if e.TagOnSubnet.ID == nil { return fmt.Errorf("Subnet ID not set") } tags := make(map[string]string) tags["AssociatedElasticIp"] = *publicIp tags["AssociatedElasticIpAllocationId"] = *eipId // Leaving this in for reference, even though we don't use it - err := t.AddAWSTags(*e.Subnet.ID, tags) + err := t.AddAWSTags(*e.TagOnSubnet.ID, tags) if err != nil { return fmt.Errorf("Unable to tag subnet %v", err) } diff --git a/upup/pkg/fi/cloudup/awstasks/elastic_ip_test.go b/upup/pkg/fi/cloudup/awstasks/elastic_ip_test.go index aea166b4e9..8c01ae5ebb 100644 --- a/upup/pkg/fi/cloudup/awstasks/elastic_ip_test.go +++ b/upup/pkg/fi/cloudup/awstasks/elastic_ip_test.go @@ -47,8 +47,8 @@ func TestElasticIPCreate(t *testing.T) { CIDR: s("172.20.1.0/24"), } eip1 := &ElasticIP{ - Name: s("eip1"), - Subnet: subnet1, + Name: s("eip1"), + TagOnSubnet: subnet1, } return map[string]fi.Task{ diff --git a/upup/pkg/fi/cloudup/awstasks/natgateway.go b/upup/pkg/fi/cloudup/awstasks/natgateway.go index e6a7f12841..b2eb5d2dfd 100644 --- a/upup/pkg/fi/cloudup/awstasks/natgateway.go +++ b/upup/pkg/fi/cloudup/awstasks/natgateway.go @@ -30,9 +30,12 @@ import ( //go:generate fitask -type=NatGateway type NatGateway struct { Name *string - ElasticIp *ElasticIP + ElasticIP *ElasticIP Subnet *Subnet ID *string + + // We can't tag NatGateways, so we have to find through a surrogate + AssociatedRouteTable *RouteTable } var _ fi.CompareWithID = &NatGateway{} // Validate the IDs @@ -42,16 +45,59 @@ func (e *NatGateway) CompareWithID() *string { } func (e *NatGateway) Find(c *fi.Context) (*NatGateway, error) { + ngw, err := e.findNatGateway(c) + if err != nil { + return nil, err + } + if ngw == nil { + return nil, nil + } + + actual := &NatGateway{ + ID: ngw.NatGatewayId, + } + actual.Subnet = e.Subnet + if len(ngw.NatGatewayAddresses) == 0 { + // Not sure if this ever happens + actual.ElasticIP = nil + } else if len(ngw.NatGatewayAddresses) == 1 { + actual.ElasticIP = &ElasticIP{ID: ngw.NatGatewayAddresses[0].AllocationId} + } else { + return nil, fmt.Errorf("found multiple elastic IPs attached to NatGateway %q", aws.StringValue(ngw.NatGatewayId)) + } + + // NATGateways don't have a Name (no tags), so we set the name to avoid spurious changes + actual.Name = e.Name + + actual.AssociatedRouteTable = e.AssociatedRouteTable + + e.ID = actual.ID + return actual, nil +} + +func (e *NatGateway) findNatGateway(c *fi.Context) (*ec2.NatGateway, error) { cloud := c.Cloud.(awsup.AWSCloud) id := e.ID - // Find via tag on foreign resource + // Find via route on private route table + if id == nil && e.AssociatedRouteTable != nil { + ngw, err := findNatGatewayFromRouteTable(cloud, e.AssociatedRouteTable) + if err != nil { + return nil, err + } + if ngw != nil { + return ngw, nil + } + } + + // Find via tag on subnet + // TODO: Obsolete - we can get from the route table instead if id == nil && e.Subnet != nil { var filters []*ec2.Filter filters = append(filters, awsup.NewEC2Filter("key", "AssociatedNatgateway")) if e.Subnet.ID == nil { - glog.V(2).Infof("Unable to find subnet, bypassing Find() for NGW") + glog.V(2).Infof("Unable to find subnet, bypassing Find() for NatGateway") return nil, nil } filters = append(filters, awsup.NewEC2Filter("resource-id", *e.Subnet.ID)) @@ -74,52 +120,68 @@ func (e *NatGateway) Find(c *fi.Context) (*NatGateway, error) { } t := response.Tags[0] id = t.Value - glog.V(2).Infof("Found nat gateway via tag: %v", *id) + glog.V(2).Infof("Found NatGateway via subnet tag: %v", *id) } if id != nil { - request := &ec2.DescribeNatGatewaysInput{} - request.NatGatewayIds = []*string{id} - response, err := cloud.EC2().DescribeNatGateways(request) - if err != nil { - return nil, fmt.Errorf("error listing NAT Gateways: %v", err) - } - - if response == nil || len(response.NatGateways) == 0 { - glog.V(2).Infof("Unable to find Nat Gateways") - return nil, nil - } - if len(response.NatGateways) != 1 { - return nil, fmt.Errorf("found multiple NAT Gateways for: %v", e) - } - a := response.NatGateways[0] - actual := &NatGateway{ - ID: a.NatGatewayId, - } - actual.Subnet = e.Subnet - if len(a.NatGatewayAddresses) == 0 { - // Not sure if this ever happens - actual.ElasticIp = nil - } else if len(a.NatGatewayAddresses) == 1 { - actual.ElasticIp = &ElasticIP{ID: a.NatGatewayAddresses[0].AllocationId} - } else { - return nil, fmt.Errorf("found multiple elastic IPs attached to NatGateway %q", aws.StringValue(a.NatGatewayId)) - } - - // NATGateways don't have a Name (no tags), so we set the name to avoid spurious changes - actual.Name = e.Name - - e.ID = actual.ID - return actual, nil + return findNatGatewayById(cloud, id) } + + return nil, nil +} + +func findNatGatewayById(cloud awsup.AWSCloud, id *string) (*ec2.NatGateway, error) { + request := &ec2.DescribeNatGatewaysInput{} + request.NatGatewayIds = []*string{id} + response, err := cloud.EC2().DescribeNatGateways(request) + if err != nil { + return nil, fmt.Errorf("error listing NatGateway %q: %v", id, err) + } + + if response == nil || len(response.NatGateways) == 0 { + glog.V(2).Infof("Unable to find NatGateway %q", id) + return nil, nil + } + if len(response.NatGateways) != 1 { + return nil, fmt.Errorf("found multiple NatGateways with id %q", id) + } + return response.NatGateways[0], nil +} + +func findNatGatewayFromRouteTable(cloud awsup.AWSCloud, routeTable *RouteTable) (*ec2.NatGateway, error) { + // Find via route on private route table + if routeTable.ID != nil { + glog.V(2).Infof("trying to match NatGateway via RouteTable %s", routeTable.ID) + rt, err := routeTable.findEc2RouteTable(cloud) + if err != nil { + return nil, fmt.Errorf("error finding associated RouteTable to NatGateway: %v", err) + } + + if rt != nil { + var natGatewayIDs []*string + for _, route := range rt.Routes { + if route.NatGatewayId != nil { + natGatewayIDs = append(natGatewayIDs, route.NatGatewayId) + } + } + + if len(natGatewayIDs) == 0 { + glog.V(2).Infof("no NatGateway found in route table %s", *rt.RouteTableId) + } else if len(natGatewayIDs) > 1 { + return nil, fmt.Errorf("found multiple NatGateways in route table %s", *rt.RouteTableId) + } else { + return findNatGatewayById(cloud, natGatewayIDs[0]) + } + } + } + return nil, nil } func (s *NatGateway) CheckChanges(a, e, changes *NatGateway) error { - // New if a == nil { - if e.ElasticIp == nil { + if e.ElasticIP == nil { return fi.RequiredField("ElasticIp") } if e.Subnet == nil { @@ -129,7 +191,7 @@ func (s *NatGateway) CheckChanges(a, e, changes *NatGateway) error { // Delta if a != nil { - if changes.ElasticIp != nil { + if changes.ElasticIP != nil { return fi.CannotChangeField("ElasticIp") } if changes.Subnet != nil { @@ -170,14 +232,13 @@ func (e *NatGateway) waitAvailable(cloud awsup.AWSCloud) error { } func (_ *NatGateway) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *NatGateway) error { - // New NGW var id *string if a == nil { glog.V(2).Infof("Creating Nat Gateway") request := &ec2.CreateNatGatewayInput{} - request.AllocationId = e.ElasticIp.ID + request.AllocationId = e.ElasticIP.ID request.SubnetId = e.Subnet.ID response, err := t.Cloud.EC2().CreateNatGateway(request) if err != nil { @@ -196,6 +257,7 @@ func (_ *NatGateway) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *NatGateway) return fmt.Errorf("Subnet ID not set") } + // TODO: Obsolete - we can get from the route table instead tags := make(map[string]string) tags["AssociatedNatgateway"] = *id err := t.AddAWSTags(*e.Subnet.ID, tags) @@ -212,7 +274,7 @@ type terraformNATGateway struct { func (_ *NatGateway) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *NatGateway) error { tf := &terraformNATGateway{ - AllocationID: e.ElasticIp.TerraformLink(), + AllocationID: e.ElasticIP.TerraformLink(), SubnetID: e.Subnet.TerraformLink(), } diff --git a/upup/pkg/fi/cloudup/awstasks/routetable.go b/upup/pkg/fi/cloudup/awstasks/routetable.go index 637e2da7cb..e29dd6074d 100644 --- a/upup/pkg/fi/cloudup/awstasks/routetable.go +++ b/upup/pkg/fi/cloudup/awstasks/routetable.go @@ -42,6 +42,26 @@ func (e *RouteTable) CompareWithID() *string { func (e *RouteTable) Find(c *fi.Context) (*RouteTable, error) { cloud := c.Cloud.(awsup.AWSCloud) + rt, err := e.findEc2RouteTable(cloud) + if err != nil { + return nil, err + } + if rt == nil { + return nil, nil + } + + actual := &RouteTable{ + ID: rt.RouteTableId, + VPC: &VPC{ID: rt.VpcId}, + Name: e.Name, + } + glog.V(2).Infof("found matching RouteTable %q", *actual.ID) + e.ID = actual.ID + + return actual, nil +} + +func (e *RouteTable) findEc2RouteTable(cloud awsup.AWSCloud) (*ec2.RouteTable, error) { request := &ec2.DescribeRouteTablesInput{} if e.ID != nil { request.RouteTableIds = []*string{e.ID} @@ -62,15 +82,7 @@ func (e *RouteTable) Find(c *fi.Context) (*RouteTable, error) { } rt := response.RouteTables[0] - actual := &RouteTable{ - ID: rt.RouteTableId, - VPC: &VPC{ID: rt.VpcId}, - Name: e.Name, - } - glog.V(2).Infof("found matching RouteTable %q", *actual.ID) - e.ID = actual.ID - - return actual, nil + return rt, nil } func (e *RouteTable) Run(c *fi.Context) error { diff --git a/upup/pkg/fi/cloudup/awstasks/subnet.go b/upup/pkg/fi/cloudup/awstasks/subnet.go index d64b6df743..fe1f459fb6 100644 --- a/upup/pkg/fi/cloudup/awstasks/subnet.go +++ b/upup/pkg/fi/cloudup/awstasks/subnet.go @@ -44,6 +44,31 @@ func (e *Subnet) CompareWithID() *string { } func (e *Subnet) Find(c *fi.Context) (*Subnet, error) { + subnet, err := e.findEc2Subnet(c) + if err != nil { + return nil, err + } + + if subnet == nil { + return nil, nil + } + + actual := &Subnet{ + ID: subnet.SubnetId, + AvailabilityZone: subnet.AvailabilityZone, + VPC: &VPC{ID: subnet.VpcId}, + CIDR: subnet.CidrBlock, + Name: findNameTag(subnet.Tags), + Shared: e.Shared, + } + + glog.V(2).Infof("found matching subnet %q", *actual.ID) + e.ID = actual.ID + + return actual, nil +} + +func (e *Subnet) findEc2Subnet(c *fi.Context) (*ec2.Subnet, error) { cloud := c.Cloud.(awsup.AWSCloud) request := &ec2.DescribeSubnetsInput{} @@ -66,19 +91,7 @@ func (e *Subnet) Find(c *fi.Context) (*Subnet, error) { } subnet := response.Subnets[0] - actual := &Subnet{ - ID: subnet.SubnetId, - AvailabilityZone: subnet.AvailabilityZone, - VPC: &VPC{ID: subnet.VpcId}, - CIDR: subnet.CidrBlock, - Name: findNameTag(subnet.Tags), - Shared: e.Shared, - } - - glog.V(2).Infof("found matching subnet %q", *actual.ID) - e.ID = actual.ID - - return actual, nil + return subnet, nil } func (e *Subnet) Run(c *fi.Context) error {