Skip to content

Commit 44fa5c0

Browse files
gordon-klothogordon-klotho
committed
Update permissions, fix nondeterminism in edge operational rules
1 parent 5f0e942 commit 44fa5c0

29 files changed

+111
-98
lines changed

‎create_test.sh

+1
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,5 @@ fi
3838
[ -e "$out_dir/iac-topology.yaml" ] && cp "$out_dir/iac-topology.yaml" "$test_dir/$name.iac-viz.yaml"
3939
[ -e "$out_dir/error_details.json" ] && cp "$out_dir/error_details.json" "$test_dir/$name.err.json"
4040
[ -e "$out_dir/deployment_permissions_policy.json" ] && cp "$out_dir/deployment_permissions_policy.json" "$test_dir/$name.deployment-policy.json"
41+
4142
rm -rf $out_dir

‎pkg/construct/graph_io.go

+11
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,14 @@ func (e SimpleEdge) ToEdge() Edge {
197197
Target: e.Target,
198198
}
199199
}
200+
201+
func EdgeKeys[V any](m map[SimpleEdge]V) []SimpleEdge {
202+
keys := make([]SimpleEdge, 0, len(m))
203+
for k := range m {
204+
keys = append(keys, k)
205+
}
206+
sort.Slice(keys, func(i, j int) bool {
207+
return keys[i].Less(keys[j])
208+
})
209+
return keys
210+
}

‎pkg/engine/operational_eval/debug.go

+32
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/dominikbraun/graph"
1212
"github.com/klothoplatform/klotho/pkg/dot"
1313
"go.uber.org/zap"
14+
"gopkg.in/yaml.v3"
1415
)
1516

1617
const (
@@ -88,3 +89,34 @@ func writeGraph(eval *Evaluator, filename string, toDot func(*Evaluator, io.Writ
8889
defer svgFile.Close()
8990
fmt.Fprint(svgFile, svgContent)
9091
}
92+
93+
func (eval *Evaluator) writeExecOrder() {
94+
path := "exec-order.yaml"
95+
if debugDir := os.Getenv("KLOTHO_DEBUG_DIR"); debugDir != "" {
96+
path = filepath.Join(debugDir, path)
97+
}
98+
if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
99+
zap.S().Errorf("could not create debug directory %s: %v", filepath.Dir(path), err)
100+
return
101+
}
102+
103+
f, err := os.Create(path)
104+
if err != nil {
105+
zap.S().Errorf("could not create file %s: %v", path, err)
106+
return
107+
}
108+
defer f.Close()
109+
110+
order := make([][]string, len(eval.evaluatedOrder))
111+
for i, group := range eval.evaluatedOrder {
112+
order[i] = make([]string, len(group))
113+
for j, key := range group {
114+
order[i][j] = key.String()
115+
}
116+
}
117+
118+
err = yaml.NewEncoder(f).Encode(order)
119+
if err != nil {
120+
zap.S().Errorf("could not write exec order to file %s: %v", path, err)
121+
}
122+
}

‎pkg/engine/operational_eval/dot.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"io"
7+
"slices"
78
"strings"
89

910
"github.com/klothoplatform/klotho/pkg/dot"
@@ -89,7 +90,7 @@ func toRanks(eval *Evaluator) ([]evalRank, error) {
8990
var noDeps []Key
9091
var onlyDownstream []Key
9192
var hasUpstream []Key
92-
for key := range keys {
93+
for _, key := range keys {
9394
switch {
9495
case len(pred[key]) == 0 && len(adj[key]) == 0:
9596
noDeps = append(noDeps, key)
@@ -115,14 +116,14 @@ func toRanks(eval *Evaluator) ([]evalRank, error) {
115116
}
116117
}
117118
} else {
118-
rank.SubRanks = [][]Key{keys.ToSlice()}
119+
rank.SubRanks = [][]Key{keys}
119120
}
120121
}
121122
var unevaluated []Key
122123
for key := range pred {
123124
evaluated := false
124125
for _, keys := range eval.evaluatedOrder {
125-
if keys.Contains(key) {
126+
if slices.Contains(keys, key) {
126127
evaluated = true
127128
break
128129
}
@@ -240,7 +241,7 @@ func graphToDOT(eval *Evaluator, out io.Writer) error {
240241

241242
evalOrder := make(map[Key]int)
242243
for i, keys := range eval.evaluatedOrder {
243-
for key := range keys {
244+
for _, key := range keys {
244245
evalOrder[key] = i
245246
}
246247
}

‎pkg/engine/operational_eval/eval.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ import (
99
"github.com/dominikbraun/graph"
1010
construct "github.com/klothoplatform/klotho/pkg/construct"
1111
"github.com/klothoplatform/klotho/pkg/graph_addons"
12-
"github.com/klothoplatform/klotho/pkg/set"
1312
"go.uber.org/zap"
1413
)
1514

1615
func (eval *Evaluator) Evaluate() error {
1716
defer eval.writeGraph("property_deps")
17+
defer eval.writeExecOrder()
1818
for {
1919
size, err := eval.unevaluated.Order()
2020
if err != nil {
@@ -26,8 +26,7 @@ func (eval *Evaluator) Evaluate() error {
2626

2727
// add to evaluatedOrder so that in popReady it has the correct group number
2828
// which is based on `len(eval.evaluatedOrder)`
29-
evaluated := make(set.Set[Key])
30-
eval.evaluatedOrder = append(eval.evaluatedOrder, evaluated)
29+
eval.evaluatedOrder = append(eval.evaluatedOrder, []Key{})
3130

3231
ready, err := eval.pollReady()
3332
if err != nil {
@@ -52,7 +51,7 @@ func (eval *Evaluator) Evaluate() error {
5251
continue
5352
}
5453
log.Debugf("Evaluating %s", k)
55-
evaluated.Add(k)
54+
eval.evaluatedOrder[len(eval.evaluatedOrder)-1] = append(eval.evaluatedOrder[len(eval.evaluatedOrder)-1], k)
5655
eval.currentKey = &k
5756
errs = errors.Join(errs, graph_addons.RemoveVertexAndEdges(eval.unevaluated, v.Key()))
5857
err = v.Evaluate(eval)

‎pkg/engine/operational_eval/evaluator.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ type (
2323

2424
unevaluated Graph
2525

26-
evaluatedOrder []set.Set[Key]
26+
evaluatedOrder [][]Key
2727
errored set.Set[Key]
2828

2929
currentKey *Key
@@ -185,6 +185,10 @@ func (r ReadyPriority) String() string {
185185
}
186186
}
187187

188+
func (eval *Evaluator) EvalutedOrder() [][]Key {
189+
return eval.evaluatedOrder
190+
}
191+
188192
func (eval *Evaluator) Log() *zap.SugaredLogger {
189193
if eval.log == nil {
190194
eval.log = zap.S().Named("engine.opeval")
@@ -501,15 +505,14 @@ func (eval *Evaluator) UpdateId(oldId, newId construct.ResourceId) error {
501505
}
502506

503507
for i, keys := range eval.evaluatedOrder {
504-
for key := range keys {
508+
for j, key := range keys {
505509
oldKey := key
506510
if key.Ref.Resource == oldId {
507511
key.Ref.Resource = newId
508512
}
509513
key.Edge = UpdateEdgeId(key.Edge, oldId, newId)
510514
if key != oldKey {
511-
eval.evaluatedOrder[i].Remove(oldKey)
512-
eval.evaluatedOrder[i].Add(key)
515+
eval.evaluatedOrder[i][j] = key
513516
}
514517
}
515518
}

‎pkg/engine/operational_eval/vertex_property.go

+9-6
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ func (prop *propertyVertex) Dependencies(eval *Evaluator, propCtx dependencyCapt
6363
current_edges[k] = v
6464
}
6565

66-
for edge, rule := range prop.EdgeRules {
66+
for _, edge := range construct.EdgeKeys(prop.EdgeRules) {
67+
rule := prop.EdgeRules[edge]
68+
6769
edgeData := knowledgebase.DynamicValueData{
6870
Resource: prop.Ref.Resource,
6971
Edge: &construct.Edge{Source: edge.Source, Target: edge.Target},
@@ -337,8 +339,8 @@ func (v *propertyVertex) evaluateEdgeOperational(
337339
) error {
338340
oldId := v.Ref.Resource
339341
var errs error
340-
for edge, rules := range v.EdgeRules {
341-
for _, rule := range rules {
342+
for _, edge := range construct.EdgeKeys(v.EdgeRules) {
343+
for _, rule := range v.EdgeRules[edge] {
342344
// In case one of the previous rules changed the ID, update it
343345
edge = UpdateEdgeId(edge, oldId, res.ID)
344346

@@ -365,8 +367,9 @@ func (v *propertyVertex) evaluateTransforms(
365367
) error {
366368
var errs error
367369
oldId := v.Ref.Resource
368-
for edge, rules := range v.TransformRules {
369-
for _, rule := range rules.ToSlice() {
370+
for _, edge := range construct.EdgeKeys(v.TransformRules) {
371+
rules := v.TransformRules[edge].ToSlice()
372+
for _, rule := range rules {
370373
// In case one of the previous rules changed the ID, update it
371374
edge = UpdateEdgeId(edge, oldId, res.ID)
372375
opCtx.SetData(knowledgebase.DynamicValueData{
@@ -502,7 +505,7 @@ func addConfigurationRuleToPropertyVertex(
502505
))
503506
}
504507
continue
505-
} else if err != nil {
508+
} else if unevalErr != nil {
506509
errs = errors.Join(errs, fmt.Errorf("could not get existing unevaluated vertex for %s: %w", ref, err))
507510
continue
508511
}

‎pkg/engine/testdata/2_routes.deployment-policy.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@
4444
"lambda:TagResource",
4545
"lambda:UntagResource",
4646
"lambda:UpdateFunctionConfiguration",
47-
"logs:*LogGroup",
48-
"logs:*LogGroups",
47+
"logs:*LogGroup*",
4948
"logs:PutRetentionPolicy"
5049
],
5150
"Effect": "Allow",

‎pkg/engine/testdata/delete_api_to_lambda_edge.deployment-policy.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@
4242
"lambda:TagResource",
4343
"lambda:UntagResource",
4444
"lambda:UpdateFunctionConfiguration",
45-
"logs:*LogGroup",
46-
"logs:*LogGroups",
45+
"logs:*LogGroup*",
4746
"logs:PutRetentionPolicy"
4847
],
4948
"Effect": "Allow",

‎pkg/engine/testdata/delete_namespace_resource.deployment-policy.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@
2424
"lambda:TagResource",
2525
"lambda:UntagResource",
2626
"lambda:UpdateFunctionConfiguration",
27-
"logs:*LogGroup",
28-
"logs:*LogGroups",
27+
"logs:*LogGroup*",
2928
"logs:PutRetentionPolicy"
3029
],
3130
"Effect": "Allow",

‎pkg/engine/testdata/delete_resource_and_iacdeps.deployment-policy.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@
2020
"apigateway:UpdateResource",
2121
"apigateway:UpdateRestApi",
2222
"apigateway:UpdateStage",
23-
"ecs:*Cluster",
24-
"ecs:*Clusters",
23+
"ecs:*Cluster*",
2524
"ecs:ListTagsForResource",
2625
"ecs:TagResource",
2726
"ecs:UntagResource",

‎pkg/engine/testdata/ecs_rds.deployment-policy.json

+3-7
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
"ec2:*Route",
1919
"ec2:*RouteTable*",
2020
"ec2:*SecurityGroup*",
21-
"ec2:*Subnet",
22-
"ec2:*Subnets",
21+
"ec2:*Subnet*",
2322
"ec2:*Tags",
2423
"ec2:*Vpc",
2524
"ec2:*Vpc*",
@@ -30,7 +29,6 @@
3029
"ec2:DescribeRegions",
3130
"ec2:DisassociateRouteTable",
3231
"ec2:ModifySecurityGroupRules",
33-
"ec2:ModifySubnetAttribute",
3432
"ec2:ModifyVpcAttribute",
3533
"ec2:ReplaceRouteTableAssociation",
3634
"ec2:RevokeSecurityGroupEgress",
@@ -42,8 +40,7 @@
4240
"ecr:Get*",
4341
"ecr:List*",
4442
"ecr:TagResource",
45-
"ecs:*Cluster",
46-
"ecs:*Clusters",
43+
"ecs:*Cluster*",
4744
"ecs:*Service",
4845
"ecs:*TaskDefinition",
4946
"ecs:Describe*",
@@ -61,8 +58,7 @@
6158
"iam:UntagRole",
6259
"iam:Update*",
6360
"kms:RetireGrant",
64-
"logs:*LogGroup",
65-
"logs:*LogGroups",
61+
"logs:*LogGroup*",
6662
"logs:PutRetentionPolicy",
6763
"rds:*DBInstance",
6864
"rds:AddTagsToResource",

‎pkg/engine/testdata/ecs_rds.expect.yaml

+6-6
Original file line numberDiff line numberDiff line change
@@ -129,42 +129,42 @@ resources:
129129
Properties:
130130
Annotations:
131131
Alarms:
132-
- aws:cloudwatch_alarm:ecs_service_0-RunningTaskCount#Arn
132+
- aws:cloudwatch_alarm:ecs_service_0-CPUUtilization#Arn
133133
Region: aws:region:region-0#Name
134134
Type: metric
135135
Width: 6
136136
- Height: 6
137137
Properties:
138138
Alarms:
139-
- aws:cloudwatch_alarm:ecs_service_0-RunningTaskCount#Arn
139+
- aws:cloudwatch_alarm:ecs_service_0-CPUUtilization#Arn
140140
Type: alarm
141141
Width: 6
142142
- Height: 6
143143
Properties:
144144
Annotations:
145145
Alarms:
146-
- aws:cloudwatch_alarm:ecs_service_0-CPUUtilization#Arn
146+
- aws:cloudwatch_alarm:ecs_service_0-MemoryUtilization#Arn
147147
Region: aws:region:region-0#Name
148148
Type: metric
149149
Width: 6
150150
- Height: 6
151151
Properties:
152152
Alarms:
153-
- aws:cloudwatch_alarm:ecs_service_0-CPUUtilization#Arn
153+
- aws:cloudwatch_alarm:ecs_service_0-MemoryUtilization#Arn
154154
Type: alarm
155155
Width: 6
156156
- Height: 6
157157
Properties:
158158
Annotations:
159159
Alarms:
160-
- aws:cloudwatch_alarm:ecs_service_0-MemoryUtilization#Arn
160+
- aws:cloudwatch_alarm:ecs_service_0-RunningTaskCount#Arn
161161
Region: aws:region:region-0#Name
162162
Type: metric
163163
Width: 6
164164
- Height: 6
165165
Properties:
166166
Alarms:
167-
- aws:cloudwatch_alarm:ecs_service_0-MemoryUtilization#Arn
167+
- aws:cloudwatch_alarm:ecs_service_0-RunningTaskCount#Arn
168168
Type: alarm
169169
Width: 6
170170
aws:ecr_image:ecs_service_0-ecs_service_0:

‎pkg/engine/testdata/extend_graph.deployment-policy.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@
4747
"lambda:TagResource",
4848
"lambda:UntagResource",
4949
"lambda:UpdateFunctionConfiguration",
50-
"logs:*LogGroup",
51-
"logs:*LogGroups",
50+
"logs:*LogGroup*",
5251
"logs:PutRetentionPolicy"
5352
],
5453
"Effect": "Allow",

‎pkg/engine/testdata/idempotent_constraints.deployment-policy.json

+3-7
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
"ec2:*Route",
1919
"ec2:*RouteTable*",
2020
"ec2:*SecurityGroup*",
21-
"ec2:*Subnet",
22-
"ec2:*Subnets",
21+
"ec2:*Subnet*",
2322
"ec2:*Tags",
2423
"ec2:*Vpc",
2524
"ec2:*Vpc*",
@@ -30,7 +29,6 @@
3029
"ec2:DescribeRegions",
3130
"ec2:DisassociateRouteTable",
3231
"ec2:ModifySecurityGroupRules",
33-
"ec2:ModifySubnetAttribute",
3432
"ec2:ModifyVpcAttribute",
3533
"ec2:ReplaceRouteTableAssociation",
3634
"ec2:RevokeSecurityGroupEgress",
@@ -42,8 +40,7 @@
4240
"ecr:Get*",
4341
"ecr:List*",
4442
"ecr:TagResource",
45-
"ecs:*Cluster",
46-
"ecs:*Clusters",
43+
"ecs:*Cluster*",
4744
"ecs:*Service",
4845
"ecs:*TaskDefinition",
4946
"ecs:Describe*",
@@ -61,8 +58,7 @@
6158
"iam:UntagRole",
6259
"iam:Update*",
6360
"kms:RetireGrant",
64-
"logs:*LogGroup",
65-
"logs:*LogGroups",
61+
"logs:*LogGroup*",
6662
"logs:PutRetentionPolicy",
6763
"rds:*DBInstance",
6864
"rds:AddTagsToResource",

0 commit comments

Comments
 (0)