From c1bf5c6a740e5242e426ee0f75d8ddaeea3f2447 Mon Sep 17 00:00:00 2001 From: Gordon Date: Fri, 8 Dec 2023 17:34:38 -0500 Subject: [PATCH] Use ShortestPathStable and always add graphstate vertices These resolve engine behaving non-deterministically --- .vscode/settings.json | 3 +- compare.sh | 23 ++++ go.mod | 4 +- go.sum | 4 +- pkg/construct2/dot.go | 116 ++++++++++++++++++ pkg/construct2/paths.go | 30 ++--- pkg/engine2/cli.go | 47 +++++-- .../operational_eval/dependency_capture.go | 74 ++++++----- pkg/engine2/operational_eval/eval.go | 60 ++++++++- pkg/engine2/operational_eval/evaluator.go | 8 +- .../operational_eval/vertex_path_expand.go | 2 +- pkg/engine2/path_selection/path_expansion.go | 31 ++++- pkg/engine2/testdata/ecs_rds.expect.yaml | 46 +++---- pkg/engine2/testdata/ecs_rds.iac-viz.yaml | 28 ++--- .../testdata/k8s_api.dataflow-viz.yaml | 2 +- pkg/engine2/testdata/k8s_api.expect.yaml | 53 ++++---- pkg/knowledge_base2/dynamic_value.go | 2 +- pkg/knowledge_base2/graph.go | 4 +- pkg/knowledge_base2/kb.go | 12 +- pkg/knowledge_base2/resource_template.go | 11 +- 20 files changed, 413 insertions(+), 147 deletions(-) create mode 100755 compare.sh create mode 100644 pkg/construct2/dot.go diff --git a/.vscode/settings.json b/.vscode/settings.json index 1f322f0c5..ae5ec7f9b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -17,5 +17,6 @@ "pkg/engine": true, "pkg/knowledge_base": true, "**/node_modules": true - } + }, + "go.testTimeout": "5m" } diff --git a/compare.sh b/compare.sh new file mode 100755 index 000000000..c3278b46c --- /dev/null +++ b/compare.sh @@ -0,0 +1,23 @@ +#!/bin/sh + +# This script takes in an input yaml, and runs the engine on it twice. +# It saves the log output in out.log and out2.log. +# It then filters the logs for important messages and outputs to out.queue.log and out2.queue.log. +# These files can then be diffed to see if the engine is behaving the same way on the same input. + +file=$1 +if [ ! -f "$file" ]; then + file="./pkg/engine2/testdata/$1" +fi + +rm out.log out2.log +export NO_COLOR=1 +export COLUMNS=80 +go run ./cmd/engine Run -i "$file" -o "./out/$(basename $file .yaml)" -v 2> out.log +sleep 2 +go run ./cmd/engine Run -i "$file" -o "./out/$(basename $file .yaml)2" -v 2> out2.log + +rm out.queue.log out2.queue.log +grep -E -e 'op: dequeue|eval|poll-deps' -e 'Satisfied' out.log > out.queue.log +grep -E -e 'op: dequeue|eval|poll-deps' -e 'Satisfied' out2.log > out2.queue.log + diff --git a/go.mod b/go.mod index b1dd49db2..e31344efe 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/pborman/ansi v1.0.0 github.com/pelletier/go-toml/v2 v2.0.8-0.20230509155657-d34104d49374 github.com/pkg/errors v0.9.1 + github.com/r3labs/diff v1.1.0 github.com/schollz/progressbar/v3 v3.13.0 github.com/smacker/go-tree-sitter v0.0.0-20220209044044-0d3022e933c3 github.com/spf13/cobra v1.6.1 @@ -47,7 +48,6 @@ require ( github.com/kr/pretty v0.3.0 // indirect github.com/mattn/go-sqlite3 v2.0.3+incompatible // indirect github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db // indirect - github.com/r3labs/diff v1.1.0 // indirect github.com/rivo/uniseg v0.4.4 // indirect golang.org/x/mod v0.9.0 // indirect golang.org/x/oauth2 v0.4.0 // indirect @@ -55,7 +55,7 @@ require ( ) replace ( - github.com/dominikbraun/graph => github.com/klothoplatform/graph v0.24.6 + github.com/dominikbraun/graph => github.com/klothoplatform/graph v0.24.7 // github.com/dominikbraun/graph => github.com/klothoplatform/graph v0.24.3 github.com/smacker/go-tree-sitter => github.com/klothoplatform/go-tree-sitter v0.1.1 diff --git a/go.sum b/go.sum index bbe3ac97a..6ac0dedd5 100644 --- a/go.sum +++ b/go.sum @@ -383,8 +383,8 @@ github.com/klauspost/compress v1.15.14 h1:i7WCKDToww0wA+9qrUZ1xOjp218vfFo3nTU6UH github.com/klauspost/compress v1.15.14/go.mod h1:QPwzmACJjUTFsnSHH934V6woptycfrDDJnH7hvFVbGM= github.com/klothoplatform/go-tree-sitter v0.1.1 h1:UO9bDCP6jJfKHUPv0P+8wLM6FJ4tCRNu3Hj2EQE51wk= github.com/klothoplatform/go-tree-sitter v0.1.1/go.mod h1:q99oHDsbP0xRwmn7Vmob8gbSMNyvJ83OauXPSuHQuKE= -github.com/klothoplatform/graph v0.24.6 h1:eLI4Pr9aqk6trjqbxlX3K9aOCdAnTxO+8Nc/wEpZSlw= -github.com/klothoplatform/graph v0.24.6/go.mod h1:yOjYyogZLY1LSG9E33JWZJiq5k83Qy2C6POAuiViluc= +github.com/klothoplatform/graph v0.24.7 h1:eE3or9a8a5xgk0WaQPYVEUzbvaXafbc38+PpYpl3t1E= +github.com/klothoplatform/graph v0.24.7/go.mod h1:yOjYyogZLY1LSG9E33JWZJiq5k83Qy2C6POAuiViluc= github.com/klothoplatform/yaml/v3 v3.0.1 h1:9POX/TEuMqoXsMJ6ypU2FdaRtnt47bxx+klOQ0PSuys= github.com/klothoplatform/yaml/v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= diff --git a/pkg/construct2/dot.go b/pkg/construct2/dot.go new file mode 100644 index 000000000..2a3955537 --- /dev/null +++ b/pkg/construct2/dot.go @@ -0,0 +1,116 @@ +package construct2 + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "sort" + + "github.com/klothoplatform/klotho/pkg/dot" +) + +func dotAttributes(r *Resource) map[string]string { + a := make(map[string]string) + a["label"] = r.ID.String() + a["shape"] = "box" + return a +} + +func dotEdgeAttributes(e ResourceEdge) map[string]string { + a := make(map[string]string) + _ = e.Source.WalkProperties(func(path PropertyPath, nerr error) error { + v := path.Get() + if v == e.Target.ID { + a["label"] = path.String() + return StopWalk + } + return nil + }) + return a +} + +func GraphToDOT(g Graph, out io.Writer) error { + ids, err := ToplogicalSort(g) + if err != nil { + return err + } + nodes, err := ResolveIds(g, ids) + if err != nil { + return err + } + var errs error + printf := func(s string, args ...any) { + _, err := fmt.Fprintf(out, s, args...) + errs = errors.Join(errs, err) + } + printf(`digraph { + rankdir = TB +`) + for _, n := range nodes { + printf(" %q%s\n", n.ID, dot.AttributesToString(dotAttributes(n))) + } + + topoIndex := func(id ResourceId) int { + for i, id2 := range ids { + if id2 == id { + return i + } + } + return -1 + } + edges, err := g.Edges() + if err != nil { + return err + } + sort.Slice(edges, func(i, j int) bool { + ti, tj := topoIndex(edges[i].Source), topoIndex(edges[j].Source) + if ti != tj { + return ti < tj + } + ti, tj = topoIndex(edges[i].Target), topoIndex(edges[j].Target) + return ti < tj + }) + for _, e := range edges { + edge, err := g.Edge(e.Source, e.Target) + if err != nil { + errs = errors.Join(errs, err) + continue + } + printf(" %q -> %q%s\n", e.Source, e.Target, dot.AttributesToString(dotEdgeAttributes(edge))) + } + printf("}\n") + return errs +} + +func GraphToSVG(g Graph, prefix string) error { + if debugDir := os.Getenv("KLOTHO_DEBUG_DIR"); debugDir != "" { + prefix = filepath.Join(debugDir, prefix) + } + f, err := os.Create(prefix + ".gv") + if err != nil { + return err + } + defer f.Close() + + dotContent := new(bytes.Buffer) + err = GraphToDOT(g, io.MultiWriter(f, dotContent)) + if err != nil { + return fmt.Errorf("could not render graph to file %s: %v", prefix+".gv", err) + } + + svgContent, err := dot.ExecPan(bytes.NewReader(dotContent.Bytes())) + if err != nil { + return fmt.Errorf("could not run 'dot' for %s: %v", prefix+".gv", err) + } + + svgFile, err := os.Create(prefix + ".gv.svg") + if err != nil { + return fmt.Errorf("could not create file %s: %v", prefix+".gv.svg", err) + } + defer svgFile.Close() + _, err = fmt.Fprint(svgFile, svgContent) + return err +} diff --git a/pkg/construct2/paths.go b/pkg/construct2/paths.go index da26044cf..aa5d81cc3 100644 --- a/pkg/construct2/paths.go +++ b/pkg/construct2/paths.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "math" - "sort" "strings" "github.com/dominikbraun/graph" @@ -120,37 +119,38 @@ func bellmanFord(g Graph, source ResourceId, skipEdge func(Edge) bool) (*bellman } dist[source] = 0 - // Sort the keys to ensure deterministic results. It adds +O(N) to the runtime, but - // when it's already O(N * E), it doesn't matter. - sortedKeys := make([]ResourceId, 0, len(adjacencyMap)) - for key := range adjacencyMap { - sortedKeys = append(sortedKeys, key) - } - sort.Sort(SortedIds(sortedKeys)) - for i := 0; i < len(adjacencyMap)-1; i++ { - for _, key := range sortedKeys { - edges := adjacencyMap[key] + for key, edges := range adjacencyMap { for _, edge := range edges { if skipEdge(edge) { continue } - newDist := dist[key] + edge.Properties.Weight + edgeWeight := edge.Properties.Weight + if !g.Traits().IsWeighted { + edgeWeight = 1 + } + + newDist := dist[key] + edgeWeight if newDist < dist[edge.Target] { dist[edge.Target] = newDist prev[edge.Target] = key + } else if newDist == dist[edge.Target] && ResourceIdLess(key, prev[edge.Target]) { + prev[edge.Target] = key } } } } - for _, key := range sortedKeys { - edges := adjacencyMap[key] + for _, edges := range adjacencyMap { for _, edge := range edges { if skipEdge(edge) { continue } - if newDist := dist[edge.Source] + edge.Properties.Weight; newDist < dist[edge.Target] { + edgeWeight := edge.Properties.Weight + if !g.Traits().IsWeighted { + edgeWeight = 1 + } + if newDist := dist[edge.Source] + edgeWeight; newDist < dist[edge.Target] { return nil, errors.New("graph contains a negative-weight cycle") } } diff --git a/pkg/engine2/cli.go b/pkg/engine2/cli.go index 513fe51ca..6c8f8ca16 100644 --- a/pkg/engine2/cli.go +++ b/pkg/engine2/cli.go @@ -8,12 +8,14 @@ import ( "reflect" "runtime/pprof" "strings" + "sync" "github.com/iancoleman/strcase" "github.com/klothoplatform/klotho/pkg/analytics" "github.com/klothoplatform/klotho/pkg/closenicely" construct "github.com/klothoplatform/klotho/pkg/construct2" "github.com/klothoplatform/klotho/pkg/engine2/constraints" + "github.com/klothoplatform/klotho/pkg/engine2/solution_context" "github.com/klothoplatform/klotho/pkg/io" knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2" "github.com/klothoplatform/klotho/pkg/knowledge_base2/reader" @@ -264,8 +266,11 @@ func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) error { if err != nil { return err } - var input construct.YamlGraph + + context := &EngineContext{} + if architectureEngineCfg.inputGraph != "" { + var input FileFormat zap.S().Info("Loading input graph") inputF, err := os.Open(architectureEngineCfg.inputGraph) if err != nil { @@ -276,18 +281,21 @@ func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) error { if err != nil { return err } + context.InitialState = input.Graph + if architectureEngineCfg.constraints == "" { + context.Constraints = input.Constraints + } } else { - input.Graph = construct.NewGraph() + context.InitialState = construct.NewGraph() } zap.S().Info("Loading constraints") - runConstraints, err := constraints.LoadConstraintsFromFile(architectureEngineCfg.constraints) - if err != nil { - return errors.Errorf("failed to load constraints: %s", err.Error()) - } - context := &EngineContext{ - InitialState: input.Graph, - Constraints: runConstraints, + if architectureEngineCfg.constraints != "" { + runConstraints, err := constraints.LoadConstraintsFromFile(architectureEngineCfg.constraints) + if err != nil { + return errors.Errorf("failed to load constraints: %s", err.Error()) + } + context.Constraints = runConstraints } zap.S().Info("Running engine") @@ -295,6 +303,7 @@ func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) error { if err != nil { return errors.Errorf("failed to run engine: %s", err.Error()) } + writeDebugGraphs(context.Solutions[0]) zap.S().Info("Engine finished running... Generating views") var files []io.File files, err = em.Engine.VisualizeViews(context.Solutions[0]) @@ -407,3 +416,23 @@ func (em *EngineMain) GetValidEdgeTargets(cmd *cobra.Command, args []string) err } return nil } + +func writeDebugGraphs(sol solution_context.SolutionContext) { + wg := sync.WaitGroup{} + wg.Add(2) + go func() { + defer wg.Done() + err := construct.GraphToSVG(sol.DataflowGraph(), "dataflow") + if err != nil { + zap.S().Errorf("failed to write dataflow graph: %s", err.Error()) + } + }() + go func() { + defer wg.Done() + err := construct.GraphToSVG(sol.DeploymentGraph(), "iac") + if err != nil { + zap.S().Errorf("failed to write iac graph: %s", err.Error()) + } + }() + wg.Wait() +} diff --git a/pkg/engine2/operational_eval/dependency_capture.go b/pkg/engine2/operational_eval/dependency_capture.go index 564ec71b2..5d6afa5c2 100644 --- a/pkg/engine2/operational_eval/dependency_capture.go +++ b/pkg/engine2/operational_eval/dependency_capture.go @@ -9,7 +9,6 @@ import ( construct "github.com/klothoplatform/klotho/pkg/construct2" knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2" - "go.uber.org/zap" ) // fauxConfigContext acts like a [knowledgebase.DynamicValueContext] but replaces the [FieldValue] function @@ -52,18 +51,12 @@ func (ctx *fauxConfigContext) ExecuteDecode(tmpl string, data knowledgebase.Dyna if err != nil { return fmt.Errorf("could not parse template: %w", err) } - err = ctx.inner.ExecuteTemplateDecode(t, data, value) - if err != nil { - zap.S().Debugf("ignoring error from ExecuteTemplateDecode during deps calculation on %s: %s", ctx.propRef, err) - } + _ = ctx.inner.ExecuteTemplateDecode(t, data, value) return nil } func (ctx *fauxConfigContext) ExecuteValue(v any, data knowledgebase.DynamicValueData) { - _, err := knowledgebase.TransformToPropertyValue(ctx.propRef.Resource, ctx.propRef.Property, v, ctx, data) - if err != nil { - zap.S().Debugf("ignoring error from TransformToPropertyValue during deps calculation on %s: %s", ctx.propRef, err) - } + _, _ = knowledgebase.TransformToPropertyValue(ctx.propRef.Resource, ctx.propRef.Property, v, ctx, data) } func (ctx *fauxConfigContext) Execute(v any, data knowledgebase.DynamicValueData) error { @@ -78,13 +71,10 @@ func (ctx *fauxConfigContext) Execute(v any, data knowledgebase.DynamicValueData // Ignore execution errors for when the zero value is invalid due to other assumptions // if there is an error with the template, this will be caught later when actually processing it. - err = tmpl.Execute( + _ = tmpl.Execute( io.Discard, // we don't care about the results, just the side effect of appending to propCtx.refs data, ) - if err != nil { - zap.S().Debugf("ignoring error from template execution during deps calculation: %s", err) - } return nil } @@ -284,16 +274,20 @@ func emptyValue(tmpl *knowledgebase.ResourceTemplate, property string) (any, err } func (ctx *fauxConfigContext) HasUpstream(selector any, resource construct.ResourceId) (bool, error) { - has, innerErr := ctx.inner.HasUpstream(selector, resource) - if innerErr == nil && has { - return true, nil - } - selId, err := knowledgebase.TemplateArgToRID(selector) if err != nil { return false, err } + has, innerErr := ctx.inner.HasUpstream(selector, resource) + if innerErr == nil && has { + ctx.addGraphState(&graphStateVertex{ + repr: graphStateRepr(fmt.Sprintf("hasUpstream(%s, %s)", selId, resource)), + Test: func(g construct.Graph) (ReadyPriority, error) { return ReadyNow, nil }, + }) + return true, nil + } + ctx.addGraphState(&graphStateVertex{ repr: graphStateRepr(fmt.Sprintf("hasUpstream(%s, %s)", selId, resource)), Test: func(g construct.Graph) (ReadyPriority, error) { @@ -314,16 +308,20 @@ func (ctx *fauxConfigContext) HasUpstream(selector any, resource construct.Resou } func (ctx *fauxConfigContext) Upstream(selector any, resource construct.ResourceId) (construct.ResourceId, error) { - up, innerErr := ctx.inner.Upstream(selector, resource) - if innerErr == nil && !up.IsZero() { - return up, nil - } - selId, err := knowledgebase.TemplateArgToRID(selector) if err != nil { return construct.ResourceId{}, err } + up, innerErr := ctx.inner.Upstream(selector, resource) + if innerErr == nil && !up.IsZero() { + ctx.addGraphState(&graphStateVertex{ + repr: graphStateRepr(fmt.Sprintf("Upstream(%s, %s)", selId, resource)), + Test: func(g construct.Graph) (ReadyPriority, error) { return ReadyNow, nil }, + }) + return up, nil + } + ctx.addGraphState(&graphStateVertex{ repr: graphStateRepr(fmt.Sprintf("Upstream(%s, %s)", selId, resource)), Test: func(g construct.Graph) (ReadyPriority, error) { @@ -356,16 +354,20 @@ func (ctx *fauxConfigContext) AllUpstream(selector any, resource construct.Resou } func (ctx *fauxConfigContext) HasDownstream(selector any, resource construct.ResourceId) (bool, error) { - has, innerErr := ctx.inner.HasDownstream(selector, resource) - if innerErr == nil && has { - return true, nil - } - selId, err := knowledgebase.TemplateArgToRID(selector) if err != nil { return false, err } + has, innerErr := ctx.inner.HasDownstream(selector, resource) + if innerErr == nil && has { + ctx.addGraphState(&graphStateVertex{ + repr: graphStateRepr(fmt.Sprintf("hasDownstream(%s, %s)", selId, resource)), + Test: func(g construct.Graph) (ReadyPriority, error) { return ReadyNow, nil }, + }) + return true, nil + } + ctx.addGraphState(&graphStateVertex{ repr: graphStateRepr(fmt.Sprintf("hasDownstream(%s, %s)", selId, resource)), Test: func(g construct.Graph) (ReadyPriority, error) { @@ -386,18 +388,22 @@ func (ctx *fauxConfigContext) HasDownstream(selector any, resource construct.Res } func (ctx *fauxConfigContext) Downstream(selector any, resource construct.ResourceId) (construct.ResourceId, error) { - down, innerErr := ctx.inner.Downstream(selector, resource) - if innerErr == nil && !down.IsZero() { - return down, nil - } - selId, err := knowledgebase.TemplateArgToRID(selector) if err != nil { return construct.ResourceId{}, err } + down, innerErr := ctx.inner.Downstream(selector, resource) + if innerErr == nil && !down.IsZero() { + ctx.addGraphState(&graphStateVertex{ + repr: graphStateRepr(fmt.Sprintf("Downstream(%s, %s)", selId, resource)), + Test: func(g construct.Graph) (ReadyPriority, error) { return ReadyNow, nil }, + }) + return down, nil + } + ctx.addGraphState(&graphStateVertex{ - repr: graphStateRepr(fmt.Sprintf("downstream(%s, %s)", selId, resource)), + repr: graphStateRepr(fmt.Sprintf("Downstream(%s, %s)", selId, resource)), Test: func(g construct.Graph) (ReadyPriority, error) { downstream, err := knowledgebase.Downstream(g, ctx.KB(), resource, knowledgebase.FirstFunctionalLayer) if err != nil { diff --git a/pkg/engine2/operational_eval/eval.go b/pkg/engine2/operational_eval/eval.go index eed10a351..b27630094 100644 --- a/pkg/engine2/operational_eval/eval.go +++ b/pkg/engine2/operational_eval/eval.go @@ -8,6 +8,7 @@ import ( "github.com/dominikbraun/graph" "github.com/klothoplatform/klotho/pkg/graph_addons" "github.com/klothoplatform/klotho/pkg/set" + "go.uber.org/zap" ) func (eval *Evaluator) Evaluate() error { @@ -51,12 +52,12 @@ func (eval *Evaluator) Evaluate() error { log.Debugf("Evaluating %s", k) evaluated.Add(k) eval.currentKey = &k + errs = errors.Join(errs, graph_addons.RemoveVertexAndEdges(eval.unevaluated, v.Key())) err = v.Evaluate(eval) if err != nil { eval.errored.Add(k) errs = errors.Join(errs, fmt.Errorf("failed to evaluate %s: %w", k, err)) } - errs = errors.Join(errs, graph_addons.RemoveVertexAndEdges(eval.unevaluated, v.Key())) } if errs != nil { @@ -69,12 +70,60 @@ func (eval *Evaluator) Evaluate() error { } } +func (eval *Evaluator) printUnevaluated() { + log := eval.Log().With("op", "poll-deps") + if !log.Desugar().Core().Enabled(zap.DebugLevel) { + return + } + adj, err := eval.unevaluated.AdjacencyMap() + if err != nil { + log.Errorf("Could not get adjacency map: %s", err) + return + } + keys := make([]Key, 0, len(adj)) + for k := range adj { + keys = append(keys, k) + } + sort.SliceStable(keys, func(i, j int) bool { + return keys[i].Less(keys[j]) + }) + log.Debugf("Unevaluated vertices: %d", len(keys)) + for _, k := range keys { + srcStr := fmt.Sprintf("%s (%d)", k, len(adj[k])) + srcV, err := eval.unevaluated.Vertex(k) + if err != nil { + srcStr += fmt.Sprintf(" [error: %s]", err) + } else { + if cond, ok := srcV.(conditionalVertex); ok { + vReady, err := cond.Ready(eval) + if err != nil { + srcStr += fmt.Sprintf(" [error: %s]", err) + } else { + srcStr += fmt.Sprintf(" [%s]", vReady) + } + } + } + log.Debug(srcStr) + ts := make([]Key, 0, len(adj[k])) + for t := range adj[k] { + ts = append(ts, t) + } + sort.SliceStable(ts, func(i, j int) bool { + return ts[i].Less(ts[j]) + }) + for _, t := range ts { + log.Debugf(" - %s", t) + } + } +} + func (eval *Evaluator) pollReady() ([]Vertex, error) { log := eval.Log().With("op", "dequeue") adj, err := eval.unevaluated.AdjacencyMap() if err != nil { return nil, err } + eval.printUnevaluated() var readyKeys []Key @@ -115,6 +164,10 @@ func (eval *Evaluator) pollReady() ([]Vertex, error) { for i, prio := range readyPriorities { if len(prio) > 0 && ready == nil { ready = prio + sort.SliceStable(ready, func(i, j int) bool { + a, b := ready[i].Key(), ready[j].Key() + return a.Less(b) + }) log.Debugf("Dequeued [%s]: %d", ReadyPriority(i), len(ready)) for _, v := range ready { log.Debugf(" - %s", v.Key()) @@ -124,11 +177,6 @@ func (eval *Evaluator) pollReady() ([]Vertex, error) { } } - sort.SliceStable(ready, func(i, j int) bool { - a, b := ready[i].Key(), ready[j].Key() - return a.Less(b) - }) - return ready, errs } diff --git a/pkg/engine2/operational_eval/evaluator.go b/pkg/engine2/operational_eval/evaluator.go index 33cb3fcd7..6eec1ede4 100644 --- a/pkg/engine2/operational_eval/evaluator.go +++ b/pkg/engine2/operational_eval/evaluator.go @@ -201,8 +201,10 @@ func (eval *Evaluator) isEvaluated(k Key) (bool, error) { } func (eval *Evaluator) addEdge(source, target Key) error { + log := eval.Log().With("op", "deps") _, err := eval.graph.Edge(source, target) if err == nil { + log.Debugf(" -> %s ✓", target) return nil } @@ -235,8 +237,6 @@ func (eval *Evaluator) addEdge(source, target Key) error { }) } - log := eval.Log().With("op", "deps") - _, err = eval.unevaluated.Vertex(target) switch { case errors.Is(err, graph.ErrVertexNotFound): @@ -270,6 +270,10 @@ func (eval *Evaluator) addEdge(source, target Key) error { } func (eval *Evaluator) enqueue(changes graphChanges) error { + if len(changes.nodes) == 0 && len(changes.edges) == 0 { + // short circuit when there's nothing to change + return nil + } log := eval.Log().With("op", "enqueue") var errs error diff --git a/pkg/engine2/operational_eval/vertex_path_expand.go b/pkg/engine2/operational_eval/vertex_path_expand.go index c38ccbdaa..454c3ab10 100644 --- a/pkg/engine2/operational_eval/vertex_path_expand.go +++ b/pkg/engine2/operational_eval/vertex_path_expand.go @@ -50,7 +50,7 @@ func expansionResultString(result construct.Graph, dep construct.ResourceEdge) ( sb := new(strings.Builder) handled := make(set.Set[construct.SimpleEdge]) - path, err := graph.ShortestPath(result, dep.Source.ID, dep.Target.ID) + path, err := graph.ShortestPathStable(result, dep.Source.ID, dep.Target.ID, construct.ResourceIdLess) if err != nil { return "", fmt.Errorf("expansion result does not contain path from %s to %s: %w", dep.Source, dep.Target, err) } diff --git a/pkg/engine2/path_selection/path_expansion.go b/pkg/engine2/path_selection/path_expansion.go index 4f05ecd3d..a2c98d9af 100644 --- a/pkg/engine2/path_selection/path_expansion.go +++ b/pkg/engine2/path_selection/path_expansion.go @@ -3,6 +3,7 @@ package path_selection import ( "errors" "fmt" + "sort" "strings" "github.com/dominikbraun/graph" @@ -62,18 +63,38 @@ func expandEdge( if err != nil { return nil, err } + sort.Slice(paths, func(i, j int) bool { + il, jl := len(paths[i]), len(paths[j]) + if il != jl { + return il < jl + } + pi, pj := paths[i], paths[j] + for k := 0; k < il; k++ { + if pi[k] != pj[k] { + return construct.ResourceIdLess(pi[k], pj[k]) + } + } + return false + }) var errs error // represents id to qualified type because we dont need to do that processing more than once for _, path := range paths { - errs = errors.Join(errs, ExpandPath(ctx, input, path, g)) + errs = errors.Join(errs, expandPath(ctx, input, path, g)) } if errs != nil { return nil, errs } - path, err := graph.ShortestPath(input.TempGraph, input.Dep.Source.ID, input.Dep.Target.ID) + path, err := graph.ShortestPathStable( + input.TempGraph, + input.Dep.Source.ID, + input.Dep.Target.ID, + construct.ResourceIdLess, + ) if err != nil { - return nil, errors.Join(errs, fmt.Errorf("could not find shortest path between %s and %s: %w", input.Dep.Source.ID, input.Dep.Target.ID, err)) + return nil, errors.Join(errs, + fmt.Errorf("could not find shortest path between %s and %s: %w", input.Dep.Source.ID, input.Dep.Target.ID, err), + ) } resultResources, err := renameAndReplaceInTempGraph(ctx, input, g, path) @@ -264,7 +285,7 @@ func handleProperties( // ExpandEdge takes a given `selectedPath` and resolves it to a path of resourceIds that can be used // for creating resources, or existing resources. -func ExpandPath( +func expandPath( ctx solution_context.SolutionContext, input ExpansionInput, path construct.Path, @@ -273,7 +294,7 @@ func ExpandPath( if len(path) == 2 { return nil } - zap.S().Debugf("Expanding path %s", path) + zap.S().Debugf("Resolving path %s", path) type candidate struct { id construct.ResourceId diff --git a/pkg/engine2/testdata/ecs_rds.expect.yaml b/pkg/engine2/testdata/ecs_rds.expect.yaml index 6df23a665..d9b3d1b62 100755 --- a/pkg/engine2/testdata/ecs_rds.expect.yaml +++ b/pkg/engine2/testdata/ecs_rds.expect.yaml @@ -34,7 +34,7 @@ resources: rds-instance-2_RDS_ENDPOINT: aws:rds_instance:rds-instance-2#Endpoint rds-instance-2_RDS_PASSWORD: aws:rds_instance:rds-instance-2#Password rds-instance-2_RDS_USERNAME: aws:rds_instance:rds-instance-2#Username - ExecutionRole: aws:iam_role:ecs_service_0-rds-instance-2 + ExecutionRole: aws:iam_role:ecs_service_0-execution-role Image: aws:ecr_image:ecs_service_0-image LogGroup: aws:log_group:ecs_service_0-log-group Memory: "512" @@ -46,12 +46,12 @@ resources: Region: aws:region:region-0 RequiresCompatibilities: - FARGATE - TaskRole: aws:iam_role:ecs_service_0-rds-instance-2 + TaskRole: aws:iam_role:ecs_service_0-execution-role aws:ecr_image:ecs_service_0-image: Context: . Dockerfile: ecs_service_0-image.Dockerfile Repo: aws:ecr_repo:ecr_repo-0 - aws:iam_role:ecs_service_0-rds-instance-2: + aws:iam_role:ecs_service_0-execution-role: AssumeRolePolicyDoc: Statement: - Action: @@ -80,8 +80,8 @@ resources: ForceDelete: true aws:elastic_ip:subnet-0-route_table-nat_gateway-elastic_ip: aws:elastic_ip:subnet-1-route_table-nat_gateway-elastic_ip: - aws:nat_gateway:subnet-2:subnet-1-route_table-nat_gateway: - ElasticIp: aws:elastic_ip:subnet-1-route_table-nat_gateway-elastic_ip + aws:nat_gateway:subnet-2:subnet-0-route_table-nat_gateway: + ElasticIp: aws:elastic_ip:subnet-0-route_table-nat_gateway-elastic_ip Subnet: aws:subnet:vpc-0:subnet-2 aws:subnet:vpc-0:subnet-2: AvailabilityZone: aws:availability_zone:region-0:availability_zone-0 @@ -103,8 +103,8 @@ resources: Region: aws:region:region-0 aws:internet_gateway:vpc-0:internet_gateway-0: Vpc: aws:vpc:vpc-0 - aws:nat_gateway:subnet-3:subnet-0-route_table-nat_gateway: - ElasticIp: aws:elastic_ip:subnet-0-route_table-nat_gateway-elastic_ip + aws:nat_gateway:subnet-3:subnet-1-route_table-nat_gateway: + ElasticIp: aws:elastic_ip:subnet-1-route_table-nat_gateway-elastic_ip Subnet: aws:subnet:vpc-0:subnet-3 aws:subnet:vpc-0:subnet-3: AvailabilityZone: aws:availability_zone:region-0:availability_zone-1 @@ -169,6 +169,12 @@ resources: Protocol: "-1" ToPort: 0 IngressRules: + - CidrBlocks: + - 10.0.128.0/18 + Description: Allow ingress traffic from ip addresses within the subnet subnet-0 + FromPort: 0 + Protocol: "-1" + ToPort: 0 - CidrBlocks: - 10.0.192.0/18 Description: Allow ingress traffic from ip addresses within the subnet subnet-1 @@ -180,22 +186,16 @@ resources: Protocol: "-1" Self: true ToPort: 0 - - CidrBlocks: - - 10.0.128.0/18 - Description: Allow ingress traffic from ip addresses within the subnet subnet-0 - FromPort: 0 - Protocol: "-1" - ToPort: 0 Vpc: aws:vpc:vpc-0 aws:route_table:subnet-0-route_table: Routes: - CidrBlock: 0.0.0.0/0 - NatGateway: aws:nat_gateway:subnet-3:subnet-0-route_table-nat_gateway + NatGateway: aws:nat_gateway:subnet-2:subnet-0-route_table-nat_gateway Vpc: aws:vpc:vpc-0 aws:route_table:subnet-1-route_table: Routes: - CidrBlock: 0.0.0.0/0 - NatGateway: aws:nat_gateway:subnet-2:subnet-1-route_table-nat_gateway + NatGateway: aws:nat_gateway:subnet-3:subnet-1-route_table-nat_gateway Vpc: aws:vpc:vpc-0 aws:vpc:vpc-0: CidrBlock: 10.0.0.0/16 @@ -209,13 +209,13 @@ edges: aws:ecs_service:ecs_service_0 -> aws:subnet:vpc-0:subnet-0: aws:ecs_service:ecs_service_0 -> aws:subnet:vpc-0:subnet-1: aws:ecs_task_definition:ecs_service_0 -> aws:ecr_image:ecs_service_0-image: - aws:ecs_task_definition:ecs_service_0 -> aws:iam_role:ecs_service_0-rds-instance-2: + aws:ecs_task_definition:ecs_service_0 -> aws:iam_role:ecs_service_0-execution-role: aws:ecs_task_definition:ecs_service_0 -> aws:log_group:ecs_service_0-log-group: 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:nat_gateway:subnet-2:subnet-1-route_table-nat_gateway -> aws:elastic_ip:subnet-1-route_table-nat_gateway-elastic_ip: - aws:nat_gateway:subnet-2:subnet-1-route_table-nat_gateway -> aws:subnet:vpc-0:subnet-2: + aws:iam_role:ecs_service_0-execution-role -> aws:rds_instance:rds-instance-2: + aws:nat_gateway:subnet-2:subnet-0-route_table-nat_gateway -> aws:elastic_ip:subnet-0-route_table-nat_gateway-elastic_ip: + aws:nat_gateway:subnet-2:subnet-0-route_table-nat_gateway -> 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-2-subnet-2-route_table: aws:subnet:vpc-0:subnet-2 -> aws:vpc:vpc-0: @@ -224,8 +224,8 @@ edges: aws:route_table:subnet-2-route_table -> 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:subnet-0-route_table-nat_gateway -> aws:elastic_ip:subnet-0-route_table-nat_gateway-elastic_ip: - aws:nat_gateway:subnet-3:subnet-0-route_table-nat_gateway -> aws:subnet:vpc-0:subnet-3: + aws:nat_gateway:subnet-3:subnet-1-route_table-nat_gateway -> aws:elastic_ip:subnet-1-route_table-nat_gateway-elastic_ip: + aws:nat_gateway:subnet-3:subnet-1-route_table-nat_gateway -> 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-3-subnet-3-route_table: aws:subnet:vpc-0:subnet-3 -> aws:vpc:vpc-0: @@ -248,7 +248,7 @@ edges: aws:route_table_association:subnet-1-subnet-1-route_table -> aws:route_table:subnet-1-route_table: aws:security_group:vpc-0:rds-instance-2-security_group -> aws:rds_instance:rds-instance-2: aws:security_group:vpc-0:rds-instance-2-security_group -> aws:vpc:vpc-0: - aws:route_table:subnet-0-route_table -> aws:nat_gateway:subnet-3:subnet-0-route_table-nat_gateway: + aws:route_table:subnet-0-route_table -> aws:nat_gateway:subnet-2:subnet-0-route_table-nat_gateway: aws:route_table:subnet-0-route_table -> aws:vpc:vpc-0: - aws:route_table:subnet-1-route_table -> aws:nat_gateway:subnet-2:subnet-1-route_table-nat_gateway: + aws:route_table:subnet-1-route_table -> aws:nat_gateway:subnet-3:subnet-1-route_table-nat_gateway: aws:route_table:subnet-1-route_table -> aws:vpc:vpc-0: diff --git a/pkg/engine2/testdata/ecs_rds.iac-viz.yaml b/pkg/engine2/testdata/ecs_rds.iac-viz.yaml index f3b3b104c..f29d858b6 100755 --- a/pkg/engine2/testdata/ecs_rds.iac-viz.yaml +++ b/pkg/engine2/testdata/ecs_rds.iac-viz.yaml @@ -29,16 +29,16 @@ resources: elastic_ip/subnet-1-route_table-nat_gateway-elastic_ip: - 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 -> vpc/vpc-0: - - elastic_ip/subnet-0-route_table-nat_gateway-elastic_ip: - 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 -> vpc/vpc-0: + elastic_ip/subnet-0-route_table-nat_gateway-elastic_ip: + + 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 -> vpc/vpc-0: + ecr_repo/ecr_repo-0: rds_instance/rds-instance-2: @@ -48,13 +48,13 @@ resources: aws:internet_gateway:vpc-0/internet_gateway-0: aws:internet_gateway:vpc-0/internet_gateway-0 -> vpc/vpc-0: - aws:nat_gateway:subnet-2/subnet-1-route_table-nat_gateway: - aws:nat_gateway:subnet-2/subnet-1-route_table-nat_gateway -> elastic_ip/subnet-1-route_table-nat_gateway-elastic_ip: - aws:nat_gateway:subnet-2/subnet-1-route_table-nat_gateway -> aws:subnet:vpc-0/subnet-2: + aws:nat_gateway:subnet-3/subnet-1-route_table-nat_gateway: + aws:nat_gateway:subnet-3/subnet-1-route_table-nat_gateway -> elastic_ip/subnet-1-route_table-nat_gateway-elastic_ip: + aws:nat_gateway:subnet-3/subnet-1-route_table-nat_gateway -> aws:subnet:vpc-0/subnet-3: - aws:nat_gateway:subnet-3/subnet-0-route_table-nat_gateway: - aws:nat_gateway:subnet-3/subnet-0-route_table-nat_gateway -> elastic_ip/subnet-0-route_table-nat_gateway-elastic_ip: - aws:nat_gateway:subnet-3/subnet-0-route_table-nat_gateway -> aws:subnet:vpc-0/subnet-3: + aws:nat_gateway:subnet-2/subnet-0-route_table-nat_gateway: + aws:nat_gateway:subnet-2/subnet-0-route_table-nat_gateway -> elastic_ip/subnet-0-route_table-nat_gateway-elastic_ip: + aws:nat_gateway:subnet-2/subnet-0-route_table-nat_gateway -> aws:subnet:vpc-0/subnet-2: ecr_image/ecs_service_0-image: ecr_image/ecs_service_0-image -> ecr_repo/ecr_repo-0: @@ -73,11 +73,11 @@ resources: route_table/subnet-2-route_table -> vpc/vpc-0: route_table/subnet-1-route_table: - route_table/subnet-1-route_table -> aws:nat_gateway:subnet-2/subnet-1-route_table-nat_gateway: + route_table/subnet-1-route_table -> aws:nat_gateway:subnet-3/subnet-1-route_table-nat_gateway: route_table/subnet-1-route_table -> vpc/vpc-0: route_table/subnet-0-route_table: - route_table/subnet-0-route_table -> aws:nat_gateway:subnet-3/subnet-0-route_table-nat_gateway: + route_table/subnet-0-route_table -> aws:nat_gateway:subnet-2/subnet-0-route_table-nat_gateway: route_table/subnet-0-route_table -> vpc/vpc-0: ecs_cluster/ecs_cluster-0: diff --git a/pkg/engine2/testdata/k8s_api.dataflow-viz.yaml b/pkg/engine2/testdata/k8s_api.dataflow-viz.yaml index e1a2a5a47..630b6d3ce 100755 --- a/pkg/engine2/testdata/k8s_api.dataflow-viz.yaml +++ b/pkg/engine2/testdata/k8s_api.dataflow-viz.yaml @@ -5,7 +5,7 @@ resources: load_balancer/rest-api-4-integ6897f0b9: - parent: vpc/vpc-0 + parent: eks_cluster/eks_cluster-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 diff --git a/pkg/engine2/testdata/k8s_api.expect.yaml b/pkg/engine2/testdata/k8s_api.expect.yaml index a17b0637f..a43e5e6df 100755 --- a/pkg/engine2/testdata/k8s_api.expect.yaml +++ b/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-integbcc77100 + Target: aws:load_balancer:rest-api-4-integ6897f0b9 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-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: + 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: 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-pod2: + aws:load_balancer_listener:rest_api_4_integration_0-eks_cluster-0: DefaultActions: - - TargetGroup: aws:target_group:rest-api-4-integbcc77100 + - TargetGroup: aws:target_group:rest-api-4-integ6897f0b9 Type: forward - LoadBalancer: aws:load_balancer:rest-api-4-integbcc77100 + LoadBalancer: aws:load_balancer:rest-api-4-integ6897f0b9 Port: 80 Protocol: TCP - aws:target_group:rest-api-4-integbcc77100: + aws:target_group:rest-api-4-integ6897f0b9: HealthCheck: Enabled: true HealthyThreshold: 5 @@ -143,19 +143,19 @@ resources: Protocol: TCP TargetType: ip Vpc: aws:vpc:vpc-0 - kubernetes:target_group_binding:restapi4integration0-pod2: + kubernetes:target_group_binding:restapi4integration0-ekscluster-0: Object: apiVersion: elbv2.k8s.aws/v1beta1 kind: TargetGroupBinding metadata: labels: - KLOTHO_ID_LABEL: restapi4integration0-pod2 - name: restapi4integration0-pod2 + KLOTHO_ID_LABEL: restapi4integration0-ekscluster-0 + name: restapi4integration0-ekscluster-0 spec: serviceRef: name: restapi4integration0-pod2 port: 80 - targetGroupARN: aws:target_group:rest-api-4-integbcc77100#Arn + targetGroupARN: aws:target_group:rest-api-4-integ6897f0b9#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 + - arn:aws:iam::aws:policy/CloudWatchAgentServerPolicy + - arn:aws:iam::aws:policy/AmazonSSMManagedInstanceCore aws:iam_role:pod2: AssumeRolePolicyDoc: Statement: @@ -609,16 +609,17 @@ 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-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: + 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: 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 diff --git a/pkg/knowledge_base2/dynamic_value.go b/pkg/knowledge_base2/dynamic_value.go index 225210ca5..3ef559fa9 100644 --- a/pkg/knowledge_base2/dynamic_value.go +++ b/pkg/knowledge_base2/dynamic_value.go @@ -496,7 +496,7 @@ func (ctx DynamicValueContext) ShortestPath(source, destination any) (construct. if err != nil { return nil, err } - return graph.ShortestPath(ctx.Graph, srcId, dstId) + return graph.ShortestPathStable(ctx.Graph, srcId, dstId, construct.ResourceIdLess) } // FieldValue returns the value of `field` on `resource` in json diff --git a/pkg/knowledge_base2/graph.go b/pkg/knowledge_base2/graph.go index 6f3cf0925..190d83c46 100644 --- a/pkg/knowledge_base2/graph.go +++ b/pkg/knowledge_base2/graph.go @@ -282,12 +282,12 @@ func IsOperationalResourceSideEffect(dag construct.Graph, kb TemplateKB, rid, si // 2. Is the property set with the resource that we are checking for if ruleSatisfied { if step.Direction == DirectionUpstream { - resources, err := graph.ShortestPath(dag, sideEffect, rid) + resources, err := graph.ShortestPathStable(dag, sideEffect, rid, construct.ResourceIdLess) if len(resources) == 0 || err != nil { continue } } else { - resources, err := graph.ShortestPath(dag, rid, sideEffect) + resources, err := graph.ShortestPathStable(dag, rid, sideEffect, construct.ResourceIdLess) if len(resources) == 0 || err != nil { continue } diff --git a/pkg/knowledge_base2/kb.go b/pkg/knowledge_base2/kb.go index 64ae6cf4d..7517756e0 100644 --- a/pkg/knowledge_base2/kb.go +++ b/pkg/knowledge_base2/kb.go @@ -176,12 +176,20 @@ func (kb *KnowledgeBase) HasFunctionalPath(from, to construct.ResourceId) bool { // For resources that can reference themselves, such as aws:api_resource return true } - path, err := graph.ShortestPath(kb.underlying, from.QualifiedTypeName(), to.QualifiedTypeName()) + path, err := graph.ShortestPathStable( + kb.underlying, + from.QualifiedTypeName(), + to.QualifiedTypeName(), + func(a, b string) bool { return a < b }, + ) if errors.Is(err, graph.ErrTargetNotReachable) { return false } if err != nil { - zap.S().Errorf("error in finding shortes path from %s to %s: %v", from.QualifiedTypeName(), to.QualifiedTypeName(), err) + zap.S().Errorf( + "error in finding shortes path from %s to %s: %v", + from.QualifiedTypeName(), to.QualifiedTypeName(), err, + ) return false } for _, id := range path[1 : len(path)-1] { diff --git a/pkg/knowledge_base2/resource_template.go b/pkg/knowledge_base2/resource_template.go index 38043097c..56ad8ac9c 100644 --- a/pkg/knowledge_base2/resource_template.go +++ b/pkg/knowledge_base2/resource_template.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "reflect" + "sort" "strings" "github.com/klothoplatform/klotho/pkg/collectionutil" @@ -342,7 +343,15 @@ func (tmpl ResourceTemplate) LoopProperties(res *construct.Resource, addProp fun var errs error for len(queue) > 0 { props, queue = queue[0], queue[1:] - for _, prop := range props { + + propKeys := make([]string, 0, len(props)) + for k := range props { + propKeys = append(propKeys, k) + } + sort.Strings(propKeys) + + for _, key := range propKeys { + prop := props[key] err := addProp(prop) if err != nil { if errors.Is(err, ErrStopWalk) {