Fix incorrect unique on nat_gateway-subnet edge

pkg/construct2/graph.go

@@ -68,7 +68,7 @@ func stringTo(g Graph, w io.Writer) error {
 		for t := range adjacent[id] {
 			targets = append(targets, t)
 		}
-		sort.Sort(sortedIds(targets))
+		sort.Sort(SortedIds(targets))
 
 		for _, t := range targets {
 			e := adjacent[id][t]

pkg/construct2/graph_deps.go

@@ -31,7 +31,7 @@ func DirectDownstreamDependencies(g Graph, r ResourceId) ([]ResourceId, error) {
 			ids = append(ids, e.Target)
 		}
 	}
-	sort.Sort(sortedIds(ids))
+	sort.Sort(SortedIds(ids))
 
 	return ids, nil
 }
@@ -61,7 +61,7 @@ func DirectUpstreamDependencies(g Graph, r ResourceId) ([]ResourceId, error) {
 			ids = append(ids, e.Source)
 		}
 	}
-	sort.Sort(sortedIds(ids))
+	sort.Sort(SortedIds(ids))
 
 	return ids, nil
 }
@@ -73,7 +73,7 @@ func allDependencies(deps map[ResourceId]map[ResourceId]Edge, r ResourceId) []Re
 	for d := range deps[r] {
 		stack = append(stack, d)
 	}
-	sort.Sort(sortedIds(stack))
+	sort.Sort(SortedIds(stack))
 
 	var ids []ResourceId
 	for len(stack) > 0 {
@@ -90,7 +90,7 @@ func allDependencies(deps map[ResourceId]map[ResourceId]Edge, r ResourceId) []Re
 			}
 			next = append(next, d)
 		}
-		sort.Sort(sortedIds(next))
+		sort.Sort(SortedIds(next))
 		stack = append(stack, next...)
 	}
 

pkg/construct2/graph_io.go

@@ -73,7 +73,7 @@ func (g YamlGraph) MarshalYAML() (interface{}, error) {
 		for t := range adj[source] {
 			targets = append(targets, t)
 		}
-		sort.Sort(sortedIds(targets))
+		sort.Sort(SortedIds(targets))
 		for _, target := range targets {
 			edges.Content = append(edges.Content,
 				&yaml.Node{

pkg/construct2/graph_vertices.go

@@ -6,35 +6,6 @@ import (
 	"sort"
 )
 
-// sortedIds is a helper type for sorting ResourceIds by purely their content, for use when deterministic ordering
-// is desired (when no other sources of ordering are available).
-type sortedIds []ResourceId
-
-func (s sortedIds) Len() int {
-	return len(s)
-}
-
-func ResourceIdLess(a, b ResourceId) bool {
-	if a.Provider != b.Provider {
-		return a.Provider < b.Provider
-	}
-	if a.Type != b.Type {
-		return a.Type < b.Type
-	}
-	if a.Namespace != b.Namespace {
-		return a.Namespace < b.Namespace
-	}
-	return a.Name < b.Name
-}
-
-func (s sortedIds) Less(i, j int) bool {
-	return ResourceIdLess(s[i], s[j])
-}
-
-func (s sortedIds) Swap(i, j int) {
-	s[i], s[j] = s[j], s[i]
-}
-
 // ToplogicalSort provides a stable topological ordering of resource IDs.
 // This is a modified implementation of graph.StableTopologicalSort with the primary difference
 // being any uses of the internal function `enqueueArbitrary`.
@@ -92,9 +63,9 @@ func toplogicalSort(g Graph, invertLess bool) ([]ResourceId, error) {
 
 			// Tie-break on the ID contents themselves
 			if invertLess {
-				return !sortedIds(remainingIds).Less(i, j)
+				return !SortedIds(remainingIds).Less(i, j)
 			}
-			return sortedIds(remainingIds).Less(i, j)
+			return SortedIds(remainingIds).Less(i, j)
 		})
 		enqueue(remainingIds[0])
 	}
@@ -106,7 +77,7 @@ func toplogicalSort(g Graph, invertLess bool) ([]ResourceId, error) {
 	order := make([]ResourceId, 0, len(predecessorMap))
 	visited := make(map[ResourceId]struct{})
 
-	sort.Sort(sortedIds(queue))
+	sort.Sort(SortedIds(queue))
 
 	for len(queue) > 0 {
 		currentVertex := queue[0]
@@ -137,9 +108,9 @@ func toplogicalSort(g Graph, invertLess bool) ([]ResourceId, error) {
 		}
 
 		if invertLess {
-			sort.Sort(sort.Reverse(sortedIds(frontier)))
+			sort.Sort(sort.Reverse(SortedIds(frontier)))
 		} else {
-			sort.Sort(sortedIds(frontier))
+			sort.Sort(SortedIds(frontier))
 		}
 
 		enqueue(frontier...)

pkg/construct2/id_sort.go

@@ -0,0 +1,30 @@
+package construct2
+
+// SortedIds is a helper type for sorting ResourceIds by purely their content, for use when deterministic ordering
+// is desired (when no other sources of ordering are available).
+type SortedIds []ResourceId
+
+func (s SortedIds) Len() int {
+	return len(s)
+}
+
+func ResourceIdLess(a, b ResourceId) bool {
+	if a.Provider != b.Provider {
+		return a.Provider < b.Provider
+	}
+	if a.Type != b.Type {
+		return a.Type < b.Type
+	}
+	if a.Namespace != b.Namespace {
+		return a.Namespace < b.Namespace
+	}
+	return a.Name < b.Name
+}
+
+func (s SortedIds) Less(i, j int) bool {
+	return ResourceIdLess(s[i], s[j])
+}
+
+func (s SortedIds) Swap(i, j int) {
+	s[i], s[j] = s[j], s[i]
+}

pkg/construct2/paths.go

@@ -126,7 +126,7 @@ func bellmanFord(g Graph, source ResourceId, skipEdge func(Edge) bool) (*bellman
 	for key := range adjacencyMap {
 		sortedKeys = append(sortedKeys, key)
 	}
-	sort.Sort(sortedIds(sortedKeys))
+	sort.Sort(SortedIds(sortedKeys))
 
 	for i := 0; i < len(adjacencyMap)-1; i++ {
 		for _, key := range sortedKeys {

pkg/engine2/operational_rule/operational_action.go

@@ -3,6 +3,7 @@ package operational_rule
 import (
 	"errors"
 	"fmt"
+	"sort"
 
 	"github.com/dominikbraun/graph"
 	"github.com/klothoplatform/klotho/pkg/collectionutil"
@@ -125,6 +126,10 @@ func (action *operationalResourceAction) useAvailableResources(resource *constru
 	if err != nil {
 		return err
 	}
+	resources, err := construct.ToplogicalSort(action.ruleCtx.Solution.RawView())
+	if err != nil {
+		return err
+	}
 
 	// Next we will loop through and try to use available resources if the unique flag is not set
 	for _, resourceSelector := range action.Step.Resources {
@@ -147,10 +152,6 @@ func (action *operationalResourceAction) useAvailableResources(resource *constru
 				continue
 			}
 
-			resources, err := construct.ToplogicalSort(action.ruleCtx.Solution.RawView())
-			if err != nil {
-				return err
-			}
 			for _, resId := range resources {
 				res, err := action.ruleCtx.Solution.RawView().Vertex(resId)
 				if err != nil {
@@ -239,7 +240,11 @@ func (action *operationalResourceAction) placeResources(resource *construct.Reso
 	}
 	placer := placerGen()
 	placer.SetCtx(action.ruleCtx)
-	return placer.PlaceResources(resource, action.Step, availableResources.ToSlice(), &action.numNeeded)
+	resources := availableResources.ToSlice()
+	sort.Slice(resources, func(i, j int) bool {
+		return construct.ResourceIdLess(resources[i].ID, resources[j].ID)
+	})
+	return placer.PlaceResources(resource, action.Step, resources, &action.numNeeded)
 }
 
 func (action *operationalResourceAction) doesResourceSatisfyNamespace(stepResource *construct.Resource, resource *construct.Resource) (bool, error) {
@@ -382,22 +387,25 @@ func (action *operationalResourceAction) createAndAddDependency(res, stepResourc
 }
 
 func (action *operationalResourceAction) generateResourceName(resourceToSet *construct.ResourceId, resource construct.ResourceId) error {
-	if resourceToSet.Name == "" {
-		numResources := 0
-		ids, err := construct.ToplogicalSort(action.ruleCtx.Solution.DataflowGraph())
-		if err != nil {
-			return err
-		}
-		for _, id := range ids {
-			if id.Type == resourceToSet.Type {
-				numResources++
-			}
-		}
-		if action.Step.Unique {
-			resourceToSet.Name = fmt.Sprintf("%s-%s-%d", resourceToSet.Type, resource.Name, numResources)
-		} else {
-			resourceToSet.Name = fmt.Sprintf("%s-%d", resourceToSet.Type, numResources)
+	if resourceToSet.Name != "" {
+		return nil
+	}
+	numResources := 0
+	ids, err := construct.ToplogicalSort(action.ruleCtx.Solution.DataflowGraph())
+	if err != nil {
+		return err
+	}
+	matcher := construct.ResourceId{Provider: resourceToSet.Provider, Type: resourceToSet.Type, Namespace: resourceToSet.Namespace}
+	for _, id := range ids {
+		if matcher.Matches(id) {
+			numResources++
 		}
 	}
+	if action.Step.Unique {
+		resourceToSet.Name = fmt.Sprintf("%s-%s-%d", resourceToSet.Type, resource.Name, numResources)
+	} else {
+		resourceToSet.Name = fmt.Sprintf("%s-%d", resourceToSet.Type, numResources)
+	}
+
 	return nil
 }

pkg/engine2/testdata/ecs_rds.expect.yaml

@@ -80,39 +80,47 @@ resources:
         ForceDelete: true
     aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-0-0-0-0:
     aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-1-1-1-1:
-    aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-2-2-2-2:
-    aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-3-3-3-3:
-    aws:internet_gateway:vpc-0:internet_gateway-0:
-        Vpc: aws:vpc:vpc-0
     aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-0-0-0:
         ElasticIp: aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-0-0-0-0
         Subnet: aws:subnet:vpc-0:subnet-2
-    aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-2-2-2:
-        ElasticIp: aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-2-2-2-2
-        Subnet: aws:subnet:vpc-0:subnet-2
     aws:subnet:vpc-0:subnet-2:
         AvailabilityZone: aws:availability_zone:region-0:availability_zone-0
         CidrBlock: 10.0.0.0/18
         MapPublicIpOnLaunch: false
         RouteTable: aws:route_table:route_table-subnet-2-2
         Type: public
         Vpc: aws:vpc:vpc-0
+    aws:route_table_association:subnet-2-route_table-subnet-2-2:
+        RouteTable: aws:route_table:route_table-subnet-2-2
+        Subnet: aws:subnet:vpc-0:subnet-2
+    aws:route_table:route_table-subnet-2-2:
+        Routes:
+            - CidrBlock: 0.0.0.0/0
+              Gateway: aws:internet_gateway:vpc-0:internet_gateway-0
+        Vpc: aws:vpc:vpc-0
     aws:availability_zone:region-0:availability_zone-0:
         Index: 0
         Region: aws:region:region-0
+    aws:internet_gateway:vpc-0:internet_gateway-0:
+        Vpc: aws:vpc:vpc-0
     aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-1-1-1:
         ElasticIp: aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-1-1-1-1
         Subnet: aws:subnet:vpc-0:subnet-3
-    aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-3-3-3:
-        ElasticIp: aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-3-3-3-3
-        Subnet: aws:subnet:vpc-0:subnet-3
     aws:subnet:vpc-0:subnet-3:
         AvailabilityZone: aws:availability_zone:region-0:availability_zone-1
         CidrBlock: 10.0.64.0/18
         MapPublicIpOnLaunch: false
         RouteTable: aws:route_table:route_table-subnet-3-3
         Type: public
         Vpc: aws:vpc:vpc-0
+    aws:route_table_association:subnet-3-route_table-subnet-3-3:
+        RouteTable: aws:route_table:route_table-subnet-3-3
+        Subnet: aws:subnet:vpc-0:subnet-3
+    aws:route_table:route_table-subnet-3-3:
+        Routes:
+            - CidrBlock: 0.0.0.0/0
+              Gateway: aws:internet_gateway:vpc-0:internet_gateway-0
+        Vpc: aws:vpc:vpc-0
     aws:availability_zone:region-0:availability_zone-1:
         Index: 1
         Region: aws:region:region-0
@@ -148,6 +156,9 @@ resources:
         Vpc: aws:vpc:vpc-0
     aws:route_table_association:subnet-0-route_table-subnet-0-0:
         RouteTable: aws:route_table:route_table-subnet-0-0
+        Subnet: aws:subnet:vpc-0:subnet-0
+    aws:route_table_association:subnet-1-route_table-subnet-1-1:
+        RouteTable: aws:route_table:route_table-subnet-1-1
         Subnet: aws:subnet:vpc-0:subnet-1
     aws:security_group:vpc-0:security_group-rds-instance-2-1:
         EgressRules:
@@ -186,18 +197,6 @@ resources:
             - CidrBlock: 0.0.0.0/0
               NatGateway: aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-1-1-1
         Vpc: aws:vpc:vpc-0
-    aws:route_table:route_table-subnet-2-2:
-        Routes:
-            - CidrBlock: 0.0.0.0/0
-              NatGateway: aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-2-2-2
-        Vpc: aws:vpc:vpc-0
-    aws:route_table:route_table-subnet-3-3:
-        Routes:
-            - CidrBlock: 0.0.0.0/0
-              Gateway: aws:internet_gateway:vpc-0:internet_gateway-0
-            - CidrBlock: 0.0.0.0/0
-              NatGateway: aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-3-3-3
-        Vpc: aws:vpc:vpc-0
     aws:vpc:vpc-0:
         CidrBlock: 10.0.0.0/16
         EnableDnsHostnames: true
@@ -215,26 +214,26 @@ edges:
     aws:ecs_task_definition:ecs_service_0 -> aws:region:region-0:
     aws:ecr_image:ecs_service_0-image -> aws:ecr_repo:ecr_repo-0:
     aws:iam_role:ecs_service_0-rds-instance-2 -> aws:rds_instance:rds-instance-2:
-    aws:internet_gateway:vpc-0:internet_gateway-0 -> aws:vpc:vpc-0:
     ? aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-0-0-0 -> aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-0-0-0-0
     :
     aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-0-0-0 -> aws:subnet:vpc-0:subnet-2:
-    ? aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-2-2-2 -> aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-2-2-2-2
-    :
-    aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-2-2-2 -> aws:subnet:vpc-0:subnet-2:
     aws:subnet:vpc-0:subnet-2 -> aws:availability_zone:region-0:availability_zone-0:
-    aws:subnet:vpc-0:subnet-2 -> aws:route_table_association:subnet-0-route_table-subnet-0-0:
+    aws:subnet:vpc-0:subnet-2 -> aws:route_table_association:subnet-2-route_table-subnet-2-2:
     aws:subnet:vpc-0:subnet-2 -> aws:vpc:vpc-0:
+    aws:route_table_association:subnet-2-route_table-subnet-2-2 -> aws:route_table:route_table-subnet-2-2:
+    aws:route_table:route_table-subnet-2-2 -> aws:internet_gateway:vpc-0:internet_gateway-0:
+    aws:route_table:route_table-subnet-2-2 -> aws:vpc:vpc-0:
     aws:availability_zone:region-0:availability_zone-0 -> aws:region:region-0:
+    aws:internet_gateway:vpc-0:internet_gateway-0 -> aws:vpc:vpc-0:
     ? aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-1-1-1 -> aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-1-1-1-1
     :
     aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-1-1-1 -> aws:subnet:vpc-0:subnet-3:
-    ? aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-3-3-3 -> aws:elastic_ip:elastic_ip-nat_gateway-route_table-subnet-3-3-3-3
-    :
-    aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-3-3-3 -> aws:subnet:vpc-0:subnet-3:
     aws:subnet:vpc-0:subnet-3 -> aws:availability_zone:region-0:availability_zone-1:
-    aws:subnet:vpc-0:subnet-3 -> aws:route_table_association:subnet-0-route_table-subnet-0-0:
+    aws:subnet:vpc-0:subnet-3 -> aws:route_table_association:subnet-3-route_table-subnet-3-3:
     aws:subnet:vpc-0:subnet-3 -> aws:vpc:vpc-0:
+    aws:route_table_association:subnet-3-route_table-subnet-3-3 -> aws:route_table:route_table-subnet-3-3:
+    aws:route_table:route_table-subnet-3-3 -> aws:internet_gateway:vpc-0:internet_gateway-0:
+    aws:route_table:route_table-subnet-3-3 -> aws:vpc:vpc-0:
     aws:availability_zone:region-0:availability_zone-1 -> aws:region:region-0:
     aws:rds_instance:rds-instance-2 -> aws:rds_subnet_group:rds_subnet_group-0:
     aws:rds_subnet_group:rds_subnet_group-0 -> aws:subnet:vpc-0:subnet-0:
@@ -244,21 +243,14 @@ edges:
     aws:subnet:vpc-0:subnet-0 -> aws:security_group:vpc-0:security_group-rds-instance-2-1:
     aws:subnet:vpc-0:subnet-0 -> aws:vpc:vpc-0:
     aws:subnet:vpc-0:subnet-1 -> aws:availability_zone:region-0:availability_zone-1:
-    aws:subnet:vpc-0:subnet-1 -> aws:route_table_association:subnet-0-route_table-subnet-0-0:
+    aws:subnet:vpc-0:subnet-1 -> aws:route_table_association:subnet-1-route_table-subnet-1-1:
     aws:subnet:vpc-0:subnet-1 -> aws:security_group:vpc-0:security_group-rds-instance-2-1:
     aws:subnet:vpc-0:subnet-1 -> aws:vpc:vpc-0:
     aws:route_table_association:subnet-0-route_table-subnet-0-0 -> aws:route_table:route_table-subnet-0-0:
-    aws:route_table_association:subnet-0-route_table-subnet-0-0 -> aws:route_table:route_table-subnet-1-1:
-    aws:route_table_association:subnet-0-route_table-subnet-0-0 -> aws:route_table:route_table-subnet-2-2:
-    aws:route_table_association:subnet-0-route_table-subnet-0-0 -> aws:route_table:route_table-subnet-3-3:
+    aws:route_table_association:subnet-1-route_table-subnet-1-1 -> aws:route_table:route_table-subnet-1-1:
     aws:security_group:vpc-0:security_group-rds-instance-2-1 -> aws:rds_instance:rds-instance-2:
     aws:security_group:vpc-0:security_group-rds-instance-2-1 -> aws:vpc:vpc-0:
     aws:route_table:route_table-subnet-0-0 -> aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-0-0-0:
     aws:route_table:route_table-subnet-0-0 -> aws:vpc:vpc-0:
     aws:route_table:route_table-subnet-1-1 -> aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-1-1-1:
     aws:route_table:route_table-subnet-1-1 -> aws:vpc:vpc-0:
-    aws:route_table:route_table-subnet-2-2 -> aws:nat_gateway:subnet-2:nat_gateway-route_table-subnet-2-2-2:
-    aws:route_table:route_table-subnet-2-2 -> aws:vpc:vpc-0:
-    aws:route_table:route_table-subnet-3-3 -> aws:internet_gateway:vpc-0:internet_gateway-0:
-    aws:route_table:route_table-subnet-3-3 -> aws:nat_gateway:subnet-3:nat_gateway-route_table-subnet-3-3-3:
-    aws:route_table:route_table-subnet-3-3 -> aws:vpc:vpc-0: