Skip to content

Commit 608d1a9

Browse files
authored
Improve routing table association management (#1636)
* Improve routing table association management * Simplify routing table association validation logic * Refactor routing table association * Improve function name
1 parent 51799c1 commit 608d1a9

File tree

5 files changed

+133
-16
lines changed

5 files changed

+133
-16
lines changed

pkg/aws/client/client.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2085,6 +2085,38 @@ func (c *Client) DeleteRouteTableAssociation(ctx context.Context, associationId
20852085
return ignoreNotFound(err)
20862086
}
20872087

2088+
// GetRouteTableAssociationIDs gets all route table associations for the given vpc and subnet IDs.
2089+
// If no subnet IDs are given, all association IDs for the VPC are returned.
2090+
func (c *Client) GetRouteTableAssociationIDs(ctx context.Context, vpc string, subnetIDs []string) ([]string, error) {
2091+
var associationIDs []string
2092+
input := &ec2.DescribeRouteTablesInput{
2093+
Filters: []ec2types.Filter{
2094+
{
2095+
Name: aws.String("vpc-id"),
2096+
Values: []string{vpc},
2097+
},
2098+
{
2099+
Name: aws.String("association.subnet-id"),
2100+
Values: subnetIDs,
2101+
},
2102+
},
2103+
}
2104+
2105+
routeTablesOutput, err := c.EC2.DescribeRouteTables(ctx, input)
2106+
if ignoreNotFound(err) != nil {
2107+
return nil, err
2108+
}
2109+
2110+
for _, routeTable := range routeTablesOutput.RouteTables {
2111+
for _, assoc := range routeTable.Associations {
2112+
if assoc.RouteTableAssociationId != nil {
2113+
associationIDs = append(associationIDs, aws.ToString(assoc.RouteTableAssociationId))
2114+
}
2115+
}
2116+
}
2117+
return associationIDs, nil
2118+
}
2119+
20882120
// CreateIAMRole creates an IAM role resource.
20892121
func (c *Client) CreateIAMRole(ctx context.Context, role *IAMRole) (*IAMRole, error) {
20902122
input := &iam.CreateRoleInput{

pkg/aws/client/mock/mocks.go

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/aws/client/types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ type Interface interface {
146146
// Route table associations
147147
CreateRouteTableAssociation(ctx context.Context, routeTableId, subnetId string) (associationId *string, err error)
148148
DeleteRouteTableAssociation(ctx context.Context, associationId string) error
149+
GetRouteTableAssociationIDs(ctx context.Context, vpc string, subnetIDs []string) ([]string, error)
149150

150151
// Elastic IP
151152
CreateElasticIP(ctx context.Context, eip *ElasticIP) (*ElasticIP, error)

pkg/controller/infrastructure/infraflow/reconcile.go

Lines changed: 68 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,18 +1479,70 @@ func (c *FlowContext) deletePrivateRoutingTable(zoneName string) flow.TaskFn {
14791479
}
14801480
}
14811481

1482+
func (c *FlowContext) routingAssociationSpecs() []routeTableAssociationSpec {
1483+
return []routeTableAssociationSpec{
1484+
{IdentifierZoneSubnetPublic, IdentifierZoneSubnetPublicRouteTableAssoc, false},
1485+
{IdentifierZoneSubnetPrivate, IdentifierZoneSubnetPrivateRouteTableAssoc, true},
1486+
{IdentifierZoneSubnetWorkers, IdentifierZoneSubnetWorkersRouteTableAssoc, true},
1487+
}
1488+
}
1489+
1490+
// validateAndPruneRoutingTableAssocState checks whether the routing table associations stored in the state
1491+
// still exist in AWS. If not, it removes them from the state.
1492+
func (c *FlowContext) validateAndPruneRoutingTableAssocState(ctx context.Context, zoneName string, specs []routeTableAssociationSpec) error {
1493+
child := c.getSubnetZoneChild(zoneName)
1494+
log := LogFromContext(ctx)
1495+
1496+
// should validate only if at least one association ID is present in state
1497+
if !hasRouteTableAssociationInState(child.Get, specs) {
1498+
return nil
1499+
}
1500+
1501+
subnetIDs := make([]string, 0, len(specs))
1502+
for _, spec := range specs {
1503+
id := child.Get(spec.subnetKey)
1504+
if id == nil {
1505+
return fmt.Errorf("missing subnet id for key %s", spec.subnetKey)
1506+
}
1507+
subnetIDs = append(subnetIDs, *id)
1508+
}
1509+
1510+
vpc := c.state.Get(IdentifierVPC)
1511+
if vpc == nil {
1512+
return fmt.Errorf("VPC ID not found in state")
1513+
}
1514+
1515+
routeTableAssociations, err := c.client.GetRouteTableAssociationIDs(ctx, *vpc, subnetIDs)
1516+
if err != nil {
1517+
return err
1518+
}
1519+
1520+
for _, spec := range specs {
1521+
if assocID := child.Get(spec.assocKey); assocID != nil && !slices.Contains(routeTableAssociations, *assocID) {
1522+
log.Info("route table association not found in AWS, removing from state",
1523+
"AssociationID", *assocID)
1524+
child.Delete(spec.assocKey)
1525+
}
1526+
}
1527+
return nil
1528+
}
1529+
14821530
func (c *FlowContext) ensureRoutingTableAssociations(zoneName string) flow.TaskFn {
14831531
return func(ctx context.Context) error {
1484-
if err := c.ensureZoneRoutingTableAssociation(ctx, zoneName, false,
1485-
IdentifierZoneSubnetPublic, IdentifierZoneSubnetPublicRouteTableAssoc); err != nil {
1532+
specs := c.routingAssociationSpecs()
1533+
1534+
err := c.validateAndPruneRoutingTableAssocState(ctx, zoneName, specs)
1535+
if err != nil {
14861536
return err
14871537
}
1488-
if err := c.ensureZoneRoutingTableAssociation(ctx, zoneName, true,
1489-
IdentifierZoneSubnetPrivate, IdentifierZoneSubnetPrivateRouteTableAssoc); err != nil {
1490-
return err
1538+
1539+
for _, spec := range specs {
1540+
err := c.ensureZoneRoutingTableAssociation(ctx, zoneName, spec.zoneRouteTable, spec.subnetKey, spec.assocKey)
1541+
if err != nil {
1542+
return err
1543+
}
14911544
}
1492-
return c.ensureZoneRoutingTableAssociation(ctx, zoneName, true,
1493-
IdentifierZoneSubnetWorkers, IdentifierZoneSubnetWorkersRouteTableAssoc)
1545+
return nil
14941546
}
14951547
}
14961548

@@ -1570,16 +1622,16 @@ func (c *FlowContext) ensureVPCEndpointZoneRoutingTableAssociation(ctx context.C
15701622

15711623
func (c *FlowContext) deleteRoutingTableAssociations(zoneName string) flow.TaskFn {
15721624
return func(ctx context.Context) error {
1573-
if err := c.deleteZoneRoutingTableAssociation(ctx, zoneName, false,
1574-
IdentifierZoneSubnetPublic, IdentifierZoneSubnetPublicRouteTableAssoc); err != nil {
1575-
return err
1576-
}
1577-
if err := c.deleteZoneRoutingTableAssociation(ctx, zoneName, true,
1578-
IdentifierZoneSubnetPrivate, IdentifierZoneSubnetPrivateRouteTableAssoc); err != nil {
1579-
return err
1625+
specs := c.routingAssociationSpecs()
1626+
1627+
for _, spec := range specs {
1628+
err := c.deleteZoneRoutingTableAssociation(ctx, zoneName, spec.zoneRouteTable, spec.subnetKey, spec.assocKey)
1629+
if err != nil {
1630+
return err
1631+
}
15801632
}
1581-
return c.deleteZoneRoutingTableAssociation(ctx, zoneName, true,
1582-
IdentifierZoneSubnetWorkers, IdentifierZoneSubnetWorkersRouteTableAssoc)
1633+
1634+
return nil
15831635
}
15841636
}
15851637

pkg/controller/infrastructure/infraflow/utils.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,3 +329,20 @@ func BuildInfrastructureStatus(
329329

330330
return status
331331
}
332+
333+
// routeTableAssociationSpec contains the specification to associate a route table with a subnet.
334+
type routeTableAssociationSpec struct {
335+
subnetKey string
336+
assocKey string
337+
zoneRouteTable bool
338+
}
339+
340+
// hasRouteTableAssociationInState checks if any of the route table associations specified in specs exist in the state.
341+
func hasRouteTableAssociationInState(getAssociationID func(string) *string, specs []routeTableAssociationSpec) bool {
342+
for _, spec := range specs {
343+
if assocID := getAssociationID(spec.assocKey); assocID != nil {
344+
return true
345+
}
346+
}
347+
return false
348+
}

0 commit comments

Comments
 (0)