update tests

klothoplatform · Mar 12, 2024 · da6f9fb · da6f9fb
1 parent 36f1ab6
commit da6f9fb
Show file tree

Hide file tree

Showing 11 changed files with 103 additions and 131 deletions.
diff --git a/pkg/engine/operational_eval/graph.go b/pkg/engine/operational_eval/graph.go
@@ -175,10 +175,10 @@ func (eval *Evaluator) edgeVertices(
 ) (graphChanges, error) {
 	changes := newChanges()
 
-	opVertex := edgeVertexWithRules(
-		construct.SimpleEdge{Source: edge.Source, Target: edge.Target},
-		tmpl.OperationalRules,
-	)
+	opVertex := &edgeVertex{
+		Edge:  construct.SimpleEdge{Source: edge.Source, Target: edge.Target},
+		Rules: tmpl.OperationalRules,
+	}
 
 	return changes, changes.AddVertexAndDeps(eval, opVertex)
 }

diff --git a/pkg/engine/operational_eval/graph_test.go b/pkg/engine/operational_eval/graph_test.go
@@ -52,7 +52,8 @@ func TestEvaluator_resourceVertices(t *testing.T) {
 								Path: "prop1",
 							},
 						},
-						EdgeRules: map[construct.SimpleEdge][]knowledgebase.OperationalRule{},
+						EdgeRules:      map[construct.SimpleEdge][]knowledgebase.OperationalRule{},
+						TransformRules: map[construct.SimpleEdge]*set.HashedSet[string, knowledgebase.OperationalRule]{},
 					},
 				},
 				edges: map[Key]set.Set[Key]{},

diff --git a/pkg/engine/operational_eval/vertex_edge.go b/pkg/engine/operational_eval/vertex_edge.go
@@ -3,7 +3,6 @@ package operational_eval
 import (
 	"errors"
 	"fmt"
-	"sort"
 
 	construct "github.com/klothoplatform/klotho/pkg/construct"
 	"github.com/klothoplatform/klotho/pkg/engine/constraints"
@@ -15,18 +14,8 @@ import (
 type edgeVertex struct {
 	Edge construct.SimpleEdge
 
-	Rules map[string]knowledgebase.OperationalRule
-}
-
-func edgeVertexWithRules(edge construct.SimpleEdge, rules []knowledgebase.OperationalRule) *edgeVertex {
-	ev := &edgeVertex{
-		Edge:  edge,
-		Rules: make(map[string]knowledgebase.OperationalRule),
-	}
-	for _, rule := range rules {
-		ev.Rules[rule.Hash()] = rule
-	}
-	return ev
+	// Rules are run in order of how they exist in the template so that the order of operations handles the rules inter dependencies
+	Rules []knowledgebase.OperationalRule
 }
 
 func (ev edgeVertex) Key() Key {
@@ -87,9 +76,7 @@ func (ev *edgeVertex) UpdateFrom(other Vertex) {
 	if ev.Edge != otherEdge.Edge {
 		panic(fmt.Sprintf("cannot merge edges with different refs: %s != %s", ev.Edge, otherEdge.Edge))
 	}
-	for hash, rule := range otherEdge.Rules {
-		ev.Rules[hash] = rule
-	}
+	ev.Rules = otherEdge.Rules
 }
 
 func (ev *edgeVertex) Evaluate(eval *Evaluator) error {
@@ -104,21 +91,7 @@ func (ev *edgeVertex) Evaluate(eval *Evaluator) error {
 	}
 
 	var errs error
-	rules := make([]knowledgebase.OperationalRule, 0, len(ev.Rules))
-	// Create a slice for the keys to sort so that we can add the rules in a deterministic order
-	keys := make([]string, 0, len(ev.Rules))
-	for k := range ev.Rules {
-		keys = append(keys, k)
-	}
-
-	// Sort the keys
-	sort.Strings(keys)
-
-	// Create a slice for the sorted values
-	for _, k := range keys {
-		rules = append(rules, ev.Rules[k])
-	}
-	for _, rule := range rules {
+	for _, rule := range ev.Rules {
 		configRules := rule.ConfigurationRules
 		rule.ConfigurationRules = nil
 

diff --git a/pkg/engine/operational_eval/vertex_edge_test.go b/pkg/engine/operational_eval/vertex_edge_test.go
@@ -25,22 +25,22 @@ func Test_edgeVertex_Dependencies(t *testing.T) {
 					Source: construct.ResourceId{Name: "source"},
 					Target: construct.ResourceId{Name: "target"},
 				},
-				Rules: map[string]knowledgebase.OperationalRule{
-					"1": {
+				Rules: []knowledgebase.OperationalRule{
+					{
 						If: "First",
 					},
-					"2": {
+					{
 						If: "Second",
 					},
 				},
 			},
 			mocks: func(dcap *MockdependencyCapturer, v *edgeVertex) {
 				dcap.EXPECT().ExecuteOpRule(knowledgebase.DynamicValueData{
 					Edge: &construct.Edge{Source: v.Edge.Source, Target: v.Edge.Target},
-				}, v.Rules["1"]).Times(1)
+				}, v.Rules[0]).Times(1)
 				dcap.EXPECT().ExecuteOpRule(knowledgebase.DynamicValueData{
 					Edge: &construct.Edge{Source: v.Edge.Source, Target: v.Edge.Target},
-				}, v.Rules["2"]).Times(1)
+				}, v.Rules[1]).Times(1)
 				dcap.EXPECT().GetChanges().Times(1)
 			},
 		},

diff --git a/pkg/engine/operational_eval/vertex_property_test.go b/pkg/engine/operational_eval/vertex_property_test.go
@@ -12,6 +12,7 @@ import (
 	operational_rule "github.com/klothoplatform/klotho/pkg/engine/operational_rule"
 	knowledgebase "github.com/klothoplatform/klotho/pkg/knowledgebase"
 	"github.com/klothoplatform/klotho/pkg/knowledgebase/properties"
+	"github.com/klothoplatform/klotho/pkg/set"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	gomock "go.uber.org/mock/gomock"
@@ -208,6 +209,7 @@ func Test_propertyVertex_Dependencies(t *testing.T) {
 		v           *propertyVertex
 		constraints constraints.Constraints
 		mocks       func(dcap *MockdependencyCapturer, resource *construct.Resource, path construct.PropertyPath)
+		want        *propertyVertex
 		wantErr     bool
 	}{
 		{
@@ -236,6 +238,7 @@ func Test_propertyVertex_Dependencies(t *testing.T) {
 						If: "test",
 					},
 				).Return(nil)
+				dcap.EXPECT().GetChanges().Times(1)
 			},
 		},
 		{
@@ -263,6 +266,79 @@ func Test_propertyVertex_Dependencies(t *testing.T) {
 				}, knowledgebase.OperationalRule{
 					If: "testE",
 				}).Return(nil)
+				dcap.EXPECT().GetChanges().Return(graphChanges{
+					edges: map[Key]set.Set[Key]{},
+				}).Times(2)
+			},
+		},
+		{
+			name: "property vertex with edge rules that depend on itself",
+			v: &propertyVertex{
+				Ref: construct.PropertyRef{
+					Property: "test",
+					Resource: construct.ResourceId{Name: "test"},
+				},
+				EdgeRules: map[construct.SimpleEdge][]knowledgebase.OperationalRule{
+					{
+						Source: construct.ResourceId{Name: "test"},
+						Target: construct.ResourceId{Name: "test2"},
+					}: {
+						{
+							If: "testE",
+						},
+					},
+				},
+				TransformRules: map[construct.SimpleEdge]*set.HashedSet[string, knowledgebase.OperationalRule]{},
+			},
+			mocks: func(dcap *MockdependencyCapturer, resource *construct.Resource, path construct.PropertyPath) {
+				dcap.EXPECT().ExecuteOpRule(knowledgebase.DynamicValueData{
+					Resource: resource.ID,
+					Edge:     &graph.Edge[construct.ResourceId]{Source: construct.ResourceId{Name: "test"}, Target: construct.ResourceId{Name: "test2"}},
+				}, knowledgebase.OperationalRule{
+					If: "testE",
+				}).Return(nil)
+				dcap.EXPECT().GetChanges().Return(graphChanges{
+					edges: map[Key]set.Set[Key]{},
+				}).Times(1)
+				dcap.EXPECT().GetChanges().Return(graphChanges{
+					edges: map[Key]set.Set[Key]{
+						{Ref: construct.PropertyRef{
+							Resource: construct.ResourceId{Name: "test"},
+							Property: "test",
+						}}: set.SetOf(
+							Key{Ref: construct.PropertyRef{
+								Resource: construct.ResourceId{Name: "test"},
+								Property: "test",
+							}}),
+					},
+				}).Times(2)
+			},
+			want: &propertyVertex{
+				Ref: construct.PropertyRef{
+					Property: "test",
+					Resource: construct.ResourceId{Name: "test"},
+				},
+				EdgeRules: map[construct.SimpleEdge][]knowledgebase.OperationalRule{
+					{
+						Source: construct.ResourceId{Name: "test"},
+						Target: construct.ResourceId{Name: "test2"},
+					}: nil,
+				},
+				TransformRules: map[construct.SimpleEdge]*set.HashedSet[string, knowledgebase.OperationalRule]{
+					{
+						Source: construct.ResourceId{Name: "test"},
+						Target: construct.ResourceId{Name: "test2"},
+					}: {
+						Hasher: func(s knowledgebase.OperationalRule) string {
+							return fmt.Sprintf("%v", s)
+						},
+						M: map[string]knowledgebase.OperationalRule{
+							"{testE [] []}": {
+								If: "testE",
+							},
+						},
+					},
+				},
 			},
 		},
 		{
@@ -296,6 +372,7 @@ func Test_propertyVertex_Dependencies(t *testing.T) {
 			mocks: func(dcap *MockdependencyCapturer, resource *construct.Resource, path construct.PropertyPath) {
 				// expect no calls to ExecuteOpRule due to shouldEvalEdges returning false
 				dcap.EXPECT().ExecuteOpRule(gomock.Any(), gomock.Any()).Times(0)
+				dcap.EXPECT().GetChanges().Times(0)
 			},
 		},
 	}
@@ -327,6 +404,12 @@ func Test_propertyVertex_Dependencies(t *testing.T) {
 				return
 			}
 			assert.NoError(t, err)
+			if tt.want != nil {
+				assert.Equal(t, tt.want.EdgeRules, tt.v.EdgeRules)
+				for k, v := range tt.want.TransformRules {
+					assert.Equal(t, v.M, tt.v.TransformRules[k].M)
+				}
+			}
 			ctrl.Finish()
 		})
 	}

diff --git a/pkg/engine/testdata/k8s_api.err.json b/pkg/engine/testdata/k8s_api.err.json
@@ -10,57 +10,5 @@
     "resource": "aws:ecr_image:pod2-ecr_image",
     "validation_error": "required property BaseImage is not set on resource aws:ecr_image:pod2-ecr_image",
     "value": null
-  },
-  {
-    "error": {
-      "children": [
-        {
-          "chain": [
-            "invalid int value 80"
-          ]
-        },
-        {
-          "chain": [
-            "invalid int value 80"
-          ]
-        }
-      ]
-    },
-    "error_code": "config_invalid",
-    "property": "Object.spec.ports",
-    "resource": "kubernetes:service:eks_cluster-0:restapi4integration0-pod2",
-    "validation_error": "invalid int value 80\ninvalid int value 80",
-    "value": [
-      {
-        "name": "pod2-pod2-80",
-        "port": "80",
-        "protocol": "TCP",
-        "targetPort": "80"
-      }
-    ]
-  },
-  {
-    "error": {
-      "chain": [
-        "invalid int value 80"
-      ]
-    },
-    "error_code": "config_invalid",
-    "property": "Object.spec.ports[0].port",
-    "resource": "kubernetes:service:eks_cluster-0:restapi4integration0-pod2",
-    "validation_error": "invalid int value 80",
-    "value": "80"
-  },
-  {
-    "error": {
-      "chain": [
-        "invalid int value 80"
-      ]
-    },
-    "error_code": "config_invalid",
-    "property": "Object.spec.ports[0].targetPort",
-    "resource": "kubernetes:service:eks_cluster-0:restapi4integration0-pod2",
-    "validation_error": "invalid int value 80",
-    "value": "80"
   }
 ]
diff --git a/pkg/engine/testdata/k8s_api.expect.yaml b/pkg/engine/testdata/k8s_api.expect.yaml
@@ -197,9 +197,9 @@ resources:
             spec:
                 ports:
                     - name: pod2-pod2-80
-                      port: "80"
+                      port: 80
                       protocol: TCP
-                      targetPort: "80"
+                      targetPort: 80
                 selector:
                     KLOTHO_ID_LABEL: pod2
                     elbv2.k8s.aws/pod-readiness-gate-inject: enabled

diff --git a/pkg/engine/testdata/lambda_efs.dataflow-viz.yaml b/pkg/engine/testdata/lambda_efs.dataflow-viz.yaml
@@ -15,7 +15,6 @@ resources:
         - aws:efs_mount_target:test-efs-fs:subnet-1-test-efs-fs
         - aws:iam_role:lambda_test_app-ExecutionRole
         - aws:security_group:vpc-0:lambda_test_app-test-efs-fs
-        - aws:security_group:vpc-0:subnet-1-test-efs-fs
         - aws:subnet:vpc-0:lambda_test_app-test-efs-fs
         - aws:subnet:vpc-0:subnet-1
 
@@ -28,7 +27,6 @@ resources:
         - aws:route_table:vpc-0:subnet-3-route_table
         - aws:security_group:vpc-0:lambda_test_app-security_group
         - aws:security_group:vpc-0:lambda_test_app-test-efs-fs
-        - aws:security_group:vpc-0:subnet-1-test-efs-fs
         - aws:subnet:vpc-0:lambda_test_app-test-efs-fs
         - aws:subnet:vpc-0:subnet-1
         - aws:subnet:vpc-0:subnet-2

diff --git a/pkg/engine/testdata/lambda_efs.expect.yaml b/pkg/engine/testdata/lambda_efs.expect.yaml
@@ -164,7 +164,7 @@ resources:
     aws:efs_mount_target:test-efs-fs:subnet-1-test-efs-fs:
         FileSystem: aws:efs_file_system:test-efs-fs
         SecurityGroups:
-            - aws:security_group:vpc-0:subnet-1-test-efs-fs
+            - aws:security_group:vpc-0:lambda_test_app-test-efs-fs
         Subnet: aws:subnet:vpc-0:subnet-1
     aws:subnet:vpc-0:subnet-1:
         AvailabilityZone: aws:availability_zone:region-0:availability_zone-0
@@ -209,30 +209,6 @@ resources:
             GLOBAL_KLOTHO_TAG: ""
             RESOURCE_NAME: lambda_test_app-test-efs-fs
         Vpc: aws:vpc:vpc-0
-    aws:security_group:vpc-0:subnet-1-test-efs-fs:
-        EgressRules:
-            - CidrBlocks:
-                - 0.0.0.0/0
-              Description: Allows all outbound IPv4 traffic
-              FromPort: 0
-              Protocol: "-1"
-              ToPort: 0
-        IngressRules:
-            - CidrBlocks:
-                - 10.0.128.0/18
-              Description: Allow ingress traffic from ip addresses within the subnet subnet-1
-              FromPort: 0
-              Protocol: "-1"
-              ToPort: 0
-            - Description: Allow ingress traffic from within the same security group
-              FromPort: 0
-              Protocol: "-1"
-              Self: true
-              ToPort: 0
-        Tags:
-            GLOBAL_KLOTHO_TAG: ""
-            RESOURCE_NAME: subnet-1-test-efs-fs
-        Vpc: aws:vpc:vpc-0
     aws:route_table:vpc-0:subnet-1-route_table:
         Routes:
             - CidrBlock: 0.0.0.0/0
@@ -329,13 +305,11 @@ edges:
     aws:subnet:vpc-0:subnet-1 -> aws:availability_zone:region-0:availability_zone-0:
     aws:subnet:vpc-0:subnet-1 -> aws:route_table_association:subnet-1-subnet-1-route_table:
     aws:subnet:vpc-0:subnet-1 -> aws:security_group:vpc-0:lambda_test_app-test-efs-fs:
-    aws:subnet:vpc-0:subnet-1 -> aws:security_group:vpc-0:subnet-1-test-efs-fs:
     aws:subnet:vpc-0:subnet-1 -> aws:vpc:vpc-0:
     aws:route_table_association:subnet-1-subnet-1-route_table -> aws:route_table:vpc-0:subnet-1-route_table:
     aws:security_group:vpc-0:lambda_test_app-test-efs-fs -> aws:efs_mount_target:test-efs-fs:lambda_test_app-test-efs-fs:
+    aws:security_group:vpc-0:lambda_test_app-test-efs-fs -> aws:efs_mount_target:test-efs-fs:subnet-1-test-efs-fs:
     aws:security_group:vpc-0:lambda_test_app-test-efs-fs -> aws:vpc:vpc-0:
-    aws:security_group:vpc-0:subnet-1-test-efs-fs -> aws:efs_mount_target:test-efs-fs:subnet-1-test-efs-fs:
-    aws:security_group:vpc-0:subnet-1-test-efs-fs -> aws:vpc:vpc-0:
     aws:route_table:vpc-0:subnet-1-route_table -> aws:nat_gateway:subnet-3:subnet-1-route_table-nat_gateway:
     aws:route_table:vpc-0:subnet-1-route_table -> aws:vpc:vpc-0:
     aws:nat_gateway:subnet-3:subnet-1-route_table-nat_gateway -> aws:elastic_ip:subnet-1-route_table-nat_gateway-elastic_ip: