fix bugs around property loading, naming, and temp graph replacement (#…

…821)

pkg/engine2/operational_eval/evaluator.go

@@ -214,6 +214,7 @@ func (eval *Evaluator) addEdge(source, target Key) error {
 			ep.Attributes[attribAddedBy] = eval.currentKey.String()
 		}
 	})
+
 	if err != nil {
 		if errors.Is(err, graph.ErrEdgeCreatesCycle) {
 			path, _ := graph.ShortestPath(eval.graph, target, source)
@@ -452,12 +453,16 @@ func (eval *Evaluator) UpdateId(oldId, newId construct.ResourceId) error {
 				vertex.Edge = UpdateEdgeId(vertex.Edge, oldId, newId)
 				replaceVertex(key, vertex)
 				// because the temp graph contains the src and target as nodes, we need to update it if it exists
-				if vertex.TempGraph != nil {
-					err := construct.ReplaceResource(vertex.TempGraph, oldId, &construct.Resource{ID: newId})
-					if err != nil {
-						errs = errors.Join(errs, err)
-						continue
-					}
+			}
+			if vertex.TempGraph != nil {
+				_, err := vertex.TempGraph.Vertex(oldId)
+				switch {
+				case errors.Is(err, graph.ErrVertexNotFound):
+					// do nothing
+				case err != nil:
+					errs = errors.Join(errs, err)
+				default:
+					errs = errors.Join(errs, construct.ReplaceResource(vertex.TempGraph, oldId, &construct.Resource{ID: newId}))
 				}
 			}
 		}

pkg/engine2/operational_eval/vertex_path_expand.go

@@ -176,6 +176,7 @@ func (v *pathExpandVertex) runExpansion(eval *Evaluator, expansion path_selectio
 
 		for _, satisfication := range pathSatisfications {
 			if satisfication.Classification == v.Satisfication.Classification {
+				// we cannot evaluate these vertices immediately because we are unsure if their dependencies have settled
 				changes.addNode(&pathExpandVertex{
 					Edge:          construct.SimpleEdge{Source: subExpand.Source, Target: subExpand.Target},
 					TempGraph:     expansion.TempGraph,

pkg/engine2/operational_rule/operational_action.go

@@ -404,8 +404,12 @@ func (action *operationalResourceAction) generateResourceName(resourceToSet *con
 		resourceToSet.Name = fmt.Sprintf("%s-%s%s", resource.Name, resourceToSet.Type, suffix)
 		return nil
 	}
+	return generateResourceName(action.ruleCtx.Solution, resourceToSet, resource)
+}
+
+func generateResourceName(sol solution_context.SolutionContext, resourceToSet *construct.ResourceId, resource construct.ResourceId) error {
 	numResources := 0
-	ids, err := construct.ToplogicalSort(action.ruleCtx.Solution.DataflowGraph())
+	ids, err := construct.ToplogicalSort(sol.DataflowGraph())
 	if err != nil {
 		return err
 	}

pkg/engine2/path_selection/path_expansion.go

@@ -117,13 +117,15 @@ func renameAndReplaceInTempGraph(
 		if strings.HasPrefix(id.Name, PHANTOM_PREFIX) {
 			oldId := id
 			id.Name = name
+			// because certain resources may be namespaced, we will check against all resource type names
+			currNames, err := getCurrNames(ctx, &id)
+			if err != nil {
+				errs = errors.Join(errs, err)
+				continue
+			}
 			for suffix := 0; suffix < 1000; suffix++ {
-				_, err := ctx.RawView().Vertex(id)
-				if err != nil && errors.Is(err, graph.ErrVertexNotFound) {
+				if !currNames.Contains(id.Name) {
 					break
-				} else if err != nil {
-					errs = errors.Join(errs, err)
-					continue
 				}
 				id.Name = fmt.Sprintf("%s-%d", name, suffix)
 			}
@@ -146,6 +148,23 @@ func renameAndReplaceInTempGraph(
 	return resultResources, errs
 }
 
+func getCurrNames(sol solution_context.SolutionContext, resourceToSet *construct.ResourceId) (set.Set[string], error) {
+	currNames := make(set.Set[string])
+	ids, err := construct.ToplogicalSort(sol.DataflowGraph())
+	if err != nil {
+		return currNames, err
+	}
+	// we cannot consider things only in the namespace because when creating a resource for an operational action
+	// it likely has not been namespaced yet and we dont know where it will be namespaced to
+	matcher := construct.ResourceId{Provider: resourceToSet.Provider, Type: resourceToSet.Type}
+	for _, id := range ids {
+		if matcher.Matches(id) {
+			currNames.Add(id.Name)
+		}
+	}
+	return currNames, nil
+}
+
 func findSubExpansionsToRun(
 	result []*construct.Resource,
 	ctx solution_context.SolutionContext,

pkg/engine2/solution_context.go

@@ -84,10 +84,11 @@ func (ctx solutionContext) LoadGraph(graph construct.Graph) error {
 		return err
 	}
 	op := ctx.OperationalView()
+	raw := ctx.RawView()
 	if err := op.AddVerticesFrom(graph); err != nil {
 		return err
 	}
-	return op.AddEdgesFrom(graph)
+	return raw.AddEdgesFrom(graph)
 }
 
 func (c solutionContext) GetDecisions() solution_context.DecisionRecords {

pkg/engine2/testdata/k8s_api.dataflow-viz.yaml

@@ -4,11 +4,11 @@ resources:
     parent: eks_cluster/eks_cluster-0
 
 
-  load_balancer/rest-api-4-integ6897f0b9:
+  load_balancer/rest-api-4-integbcc77100:
     parent: vpc/vpc-0
 
-  load_balancer/rest-api-4-integ6897f0b9 -> kubernetes:pod:eks_cluster-0/pod2:
-    path: aws:load_balancer_listener:rest_api_4_integration_0-eks_cluster-0,aws:target_group:rest-api-4-integ6897f0b9,kubernetes:target_group_binding:restapi4integration0-ekscluster-0,kubernetes:service:restapi4integration0-pod2
+  load_balancer/rest-api-4-integbcc77100 -> kubernetes:pod:eks_cluster-0/pod2:
+    path: aws:load_balancer_listener:rest_api_4_integration_0-pod2,aws:target_group:rest-api-4-integbcc77100,kubernetes:target_group_binding:restapi4integration0-pod2,kubernetes:service:restapi4integration0-pod2
 
 
   kubernetes:helm_chart:eks_cluster-0/metricsserver:
@@ -31,7 +31,7 @@ resources:
   aws:api_integration:rest_api_4/rest_api_4_integration_0:
     parent: rest_api/rest_api_4
 
-  aws:api_integration:rest_api_4/rest_api_4_integration_0 -> load_balancer/rest-api-4-integ6897f0b9:
-    path: aws:vpc_link:rest_api_4_integration_0-eks_cluster-0
+  aws:api_integration:rest_api_4/rest_api_4_integration_0 -> load_balancer/rest-api-4-integbcc77100:
+    path: aws:vpc_link:rest_api_4_integration_0-pod2
 
 

pkg/engine2/testdata/k8s_api.expect.yaml

@@ -111,26 +111,26 @@ resources:
         Resource: aws:api_resource:rest_api_4:api_resource-0
         RestApi: aws:rest_api:rest_api_4
         Route: /{proxy+}
-        Target: aws:load_balancer:rest-api-4-integ6897f0b9
+        Target: aws:load_balancer:rest-api-4-integbcc77100
         Type: HTTP_PROXY
         Uri: aws:api_integration:rest_api_4:rest_api_4_integration_0#LbUri
-        VpcLink: aws:vpc_link:rest_api_4_integration_0-eks_cluster-0
-    aws:vpc_link:rest_api_4_integration_0-eks_cluster-0:
-        Target: aws:load_balancer:rest-api-4-integ6897f0b9
-    aws:load_balancer:rest-api-4-integ6897f0b9:
+        VpcLink: aws:vpc_link:rest_api_4_integration_0-pod2
+    aws:vpc_link:rest_api_4_integration_0-pod2:
+        Target: aws:load_balancer:rest-api-4-integbcc77100
+    aws:load_balancer:rest-api-4-integbcc77100:
         Scheme: internal
         Subnets:
             - aws:subnet:vpc-0:subnet-0
             - aws:subnet:vpc-0:subnet-1
         Type: network
-    aws:load_balancer_listener:rest_api_4_integration_0-eks_cluster-0:
+    aws:load_balancer_listener:rest_api_4_integration_0-pod2:
         DefaultActions:
-            - TargetGroup: aws:target_group:rest-api-4-integ6897f0b9
+            - TargetGroup: aws:target_group:rest-api-4-integbcc77100
               Type: forward
-        LoadBalancer: aws:load_balancer:rest-api-4-integ6897f0b9
+        LoadBalancer: aws:load_balancer:rest-api-4-integbcc77100
         Port: 80
         Protocol: TCP
-    aws:target_group:rest-api-4-integ6897f0b9:
+    aws:target_group:rest-api-4-integbcc77100:
         HealthCheck:
             Enabled: true
             HealthyThreshold: 5
@@ -143,19 +143,19 @@ resources:
         Protocol: TCP
         TargetType: ip
         Vpc: aws:vpc:vpc-0
-    kubernetes:target_group_binding:restapi4integration0-ekscluster-0:
+    kubernetes:target_group_binding:restapi4integration0-pod2:
         Object:
             apiVersion: elbv2.k8s.aws/v1beta1
             kind: TargetGroupBinding
             metadata:
                 labels:
-                    KLOTHO_ID_LABEL: restapi4integration0-ekscluster-0
-                name: restapi4integration0-ekscluster-0
+                    KLOTHO_ID_LABEL: restapi4integration0-pod2
+                name: restapi4integration0-pod2
             spec:
                 serviceRef:
                     name: restapi4integration0-pod2
                     port: 80
-                targetGroupARN: aws:target_group:rest-api-4-integ6897f0b9#Arn
+                targetGroupARN: aws:target_group:rest-api-4-integbcc77100#Arn
     kubernetes:helm_chart:eks_cluster-0:aws-load-balancer-controller:
         Chart: aws-load-balancer-controller
         Cluster: aws:eks_cluster:eks_cluster-0
@@ -285,12 +285,12 @@ resources:
                         - ec2.amazonaws.com
             Version: "2012-10-17"
         ManagedPolicies:
+            - arn:aws:iam::aws:policy/AWSCloudMapFullAccess
             - arn:aws:iam::aws:policy/CloudWatchAgentServerPolicy
             - arn:aws:iam::aws:policy/AmazonSSMManagedInstanceCore
             - arn:aws:iam::aws:policy/AmazonEKSWorkerNodePolicy
             - arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly
             - arn:aws:iam::aws:policy/AmazonEKS_CNI_Policy
-            - arn:aws:iam::aws:policy/AWSCloudMapFullAccess
     aws:iam_role:pod2:
         AssumeRolePolicyDoc:
             Statement:
@@ -609,17 +609,16 @@ edges:
     aws:api_resource:rest_api_4:api_resource-0 -> aws:api_integration:rest_api_4:rest_api_4_integration_0:
     aws:api_resource:rest_api_4:api_resource-0 -> aws:api_method:rest_api_4:rest_api_4_integration_0_method:
     aws:api_method:rest_api_4:rest_api_4_integration_0_method -> aws:api_integration:rest_api_4:rest_api_4_integration_0:
-    aws:api_integration:rest_api_4:rest_api_4_integration_0 -> aws:vpc_link:rest_api_4_integration_0-eks_cluster-0:
-    aws:vpc_link:rest_api_4_integration_0-eks_cluster-0 -> aws:load_balancer:rest-api-4-integ6897f0b9:
-    aws:load_balancer:rest-api-4-integ6897f0b9 -> aws:load_balancer_listener:rest_api_4_integration_0-eks_cluster-0:
-    aws:load_balancer:rest-api-4-integ6897f0b9 -> aws:subnet:vpc-0:subnet-0:
-    aws:load_balancer:rest-api-4-integ6897f0b9 -> aws:subnet:vpc-0:subnet-1:
-    aws:load_balancer_listener:rest_api_4_integration_0-eks_cluster-0 -> aws:target_group:rest-api-4-integ6897f0b9:
-    aws:target_group:rest-api-4-integ6897f0b9 -> kubernetes:target_group_binding:restapi4integration0-ekscluster-0:
-    kubernetes:target_group_binding:restapi4integration0-ekscluster-0 -> aws:eks_cluster:eks_cluster-0:
-    ? kubernetes:target_group_binding:restapi4integration0-ekscluster-0 -> kubernetes:helm_chart:eks_cluster-0:aws-load-balancer-controller
-    :
-    kubernetes:target_group_binding:restapi4integration0-ekscluster-0 -> kubernetes:service:restapi4integration0-pod2:
+    aws:api_integration:rest_api_4:rest_api_4_integration_0 -> aws:vpc_link:rest_api_4_integration_0-pod2:
+    aws:vpc_link:rest_api_4_integration_0-pod2 -> aws:load_balancer:rest-api-4-integbcc77100:
+    aws:load_balancer:rest-api-4-integbcc77100 -> aws:load_balancer_listener:rest_api_4_integration_0-pod2:
+    aws:load_balancer:rest-api-4-integbcc77100 -> aws:subnet:vpc-0:subnet-0:
+    aws:load_balancer:rest-api-4-integbcc77100 -> aws:subnet:vpc-0:subnet-1:
+    aws:load_balancer_listener:rest_api_4_integration_0-pod2 -> aws:target_group:rest-api-4-integbcc77100:
+    aws:target_group:rest-api-4-integbcc77100 -> kubernetes:target_group_binding:restapi4integration0-pod2:
+    kubernetes:target_group_binding:restapi4integration0-pod2 -> aws:eks_cluster:eks_cluster-0:
+    kubernetes:target_group_binding:restapi4integration0-pod2 -> kubernetes:helm_chart:eks_cluster-0:aws-load-balancer-controller:
+    kubernetes:target_group_binding:restapi4integration0-pod2 -> kubernetes:service:restapi4integration0-pod2:
     kubernetes:helm_chart:eks_cluster-0:aws-load-balancer-controller -> aws:eks_cluster:eks_cluster-0:
     kubernetes:helm_chart:eks_cluster-0:aws-load-balancer-controller -> aws:region:region-0:
     ? kubernetes:helm_chart:eks_cluster-0:aws-load-balancer-controller -> kubernetes:service_account:eks_cluster-0:aws-load-balancer-controller

pkg/engine2/testdata/k8s_api.iac-viz.yaml

@@ -48,9 +48,9 @@ resources:
   aws:api_resource:rest_api_4/api_resource-0:
   aws:api_resource:rest_api_4/api_resource-0 -> rest_api/rest_api_4:
 
-  load_balancer/rest-api-4-integ6897f0b9:
-  load_balancer/rest-api-4-integ6897f0b9 -> aws:subnet:vpc-0/subnet-0:
-  load_balancer/rest-api-4-integ6897f0b9 -> aws:subnet:vpc-0/subnet-1:
+  load_balancer/rest-api-4-integbcc77100:
+  load_balancer/rest-api-4-integbcc77100 -> aws:subnet:vpc-0/subnet-0:
+  load_balancer/rest-api-4-integbcc77100 -> aws:subnet:vpc-0/subnet-1:
 
   iam_role/aws-load-balancer-controller:
   iam_role/aws-load-balancer-controller -> iam_oidc_provider/eks_cluster-0:
@@ -84,8 +84,8 @@ resources:
   aws:api_method:rest_api_4/rest_api_4_integration_0_method -> aws:api_resource:rest_api_4/api_resource-0:
   aws:api_method:rest_api_4/rest_api_4_integration_0_method -> rest_api/rest_api_4:
 
-  vpc_link/rest_api_4_integration_0-eks_cluster-0:
-  vpc_link/rest_api_4_integration_0-eks_cluster-0 -> load_balancer/rest-api-4-integ6897f0b9:
+  vpc_link/rest_api_4_integration_0-pod2:
+  vpc_link/rest_api_4_integration_0-pod2 -> load_balancer/rest-api-4-integbcc77100:
 
   kubernetes:service_account:eks_cluster-0/aws-load-balancer-controller:
   kubernetes:service_account:eks_cluster-0/aws-load-balancer-controller -> eks_cluster/eks_cluster-0:
@@ -115,9 +115,9 @@ resources:
   aws:api_integration:rest_api_4/rest_api_4_integration_0 -> aws:api_method:rest_api_4/rest_api_4_integration_0_method:
   aws:api_integration:rest_api_4/rest_api_4_integration_0 -> aws:api_resource:rest_api_4/api_resource-0:
   aws:api_integration:rest_api_4/rest_api_4_integration_0 -> rest_api/rest_api_4:
-  aws:api_integration:rest_api_4/rest_api_4_integration_0 -> vpc_link/rest_api_4_integration_0-eks_cluster-0:
+  aws:api_integration:rest_api_4/rest_api_4_integration_0 -> vpc_link/rest_api_4_integration_0-pod2:
 
-  target_group/rest-api-4-integ6897f0b9:
+  target_group/rest-api-4-integbcc77100:
 
   kubernetes:helm_chart:eks_cluster-0/aws-load-balancer-controller:
   kubernetes:helm_chart:eks_cluster-0/aws-load-balancer-controller -> eks_cluster/eks_cluster-0:
@@ -155,11 +155,11 @@ resources:
   aws:api_deployment:rest_api_4/api_deployment-0 -> aws:api_method:rest_api_4/rest_api_4_integration_0_method:
   aws:api_deployment:rest_api_4/api_deployment-0 -> rest_api/rest_api_4:
 
-  kubernetes:target_group_binding/restapi4integration0-ekscluster-0:
-  kubernetes:target_group_binding/restapi4integration0-ekscluster-0 -> eks_cluster/eks_cluster-0:
-  kubernetes:target_group_binding/restapi4integration0-ekscluster-0 -> target_group/rest-api-4-integ6897f0b9:
-  kubernetes:target_group_binding/restapi4integration0-ekscluster-0 -> kubernetes:helm_chart:eks_cluster-0/aws-load-balancer-controller:
-  kubernetes:target_group_binding/restapi4integration0-ekscluster-0 -> kubernetes:service/restapi4integration0-pod2:
+  kubernetes:target_group_binding/restapi4integration0-pod2:
+  kubernetes:target_group_binding/restapi4integration0-pod2 -> eks_cluster/eks_cluster-0:
+  kubernetes:target_group_binding/restapi4integration0-pod2 -> target_group/rest-api-4-integbcc77100:
+  kubernetes:target_group_binding/restapi4integration0-pod2 -> kubernetes:helm_chart:eks_cluster-0/aws-load-balancer-controller:
+  kubernetes:target_group_binding/restapi4integration0-pod2 -> kubernetes:service/restapi4integration0-pod2:
 
   kubernetes:manifest/fluent-bit:
   kubernetes:manifest/fluent-bit -> eks_cluster/eks_cluster-0:
@@ -191,9 +191,9 @@ resources:
   route_table_association/subnet-0-subnet-0-route_table -> route_table/subnet-0-route_table:
   route_table_association/subnet-0-subnet-0-route_table -> aws:subnet:vpc-0/subnet-0:
 
-  load_balancer_listener/rest_api_4_integration_0-eks_cluster-0:
-  load_balancer_listener/rest_api_4_integration_0-eks_cluster-0 -> load_balancer/rest-api-4-integ6897f0b9:
-  load_balancer_listener/rest_api_4_integration_0-eks_cluster-0 -> target_group/rest-api-4-integ6897f0b9:
+  load_balancer_listener/rest_api_4_integration_0-pod2:
+  load_balancer_listener/rest_api_4_integration_0-pod2 -> load_balancer/rest-api-4-integbcc77100:
+  load_balancer_listener/rest_api_4_integration_0-pod2 -> target_group/rest-api-4-integbcc77100:
 
   iam_role_policy_attachment/aws-load-balancer-controller-iam_policy-0:
   iam_role_policy_attachment/aws-load-balancer-controller-iam_policy-0 -> iam_policy/iam_policy-0:

pkg/knowledge_base2/reader/properties.go

@@ -106,6 +106,9 @@ func (p *Property) Convert() (knowledgebase.Property, error) {
 		// Skip nil pointers
 		if (srcField.Kind() == reflect.Ptr || srcField.Kind() == reflect.Interface) && srcField.IsNil() {
 			continue
+			// skip empty arrays and slices
+		} else if (srcField.Kind() == reflect.Array || srcField.Kind() == reflect.Slice) && srcField.Len() == 0 {
+			continue
 		}
 		// Handle sub properties so we can recurse down the tree
 		if fieldName == "Properties" {

pkg/templates/aws/resources/ecs_task_definition.yaml

@@ -73,7 +73,7 @@ properties:
     default_value: awsvpc
     description: The Docker networking mode to use for the containers in the task
   PortMappings:
-    type: set
+    type: list
     default_value:
       - ContainerPort: 80
         HostPort: 80