Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more test cases, switch differ to allow slice ordering to change #802

Merged
merged 2 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions create_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ name=${name%.input}
echo "Running $name"

# Run the engine
echo "Using $out_dir as output directory"
go run ./cmd/engine Run \
-i "$1" \
-c "$1" \
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ 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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,8 @@ github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsT
github.com/prometheus/procfs v0.0.3/go.mod h1:4A/X28fw3Fc593LaREMrKMqOKvUAntwMDaekg4FpcdQ=
github.com/prometheus/procfs v0.9.0 h1:wzCHvIvM5SxWqYvwgVL7yJY8Lz3PKn49KQtpgMYJfhI=
github.com/prometheus/procfs v0.9.0/go.mod h1:+pB4zwohETzFnmlpe6yd2lSc+0/46IYZRB/chUwxUZY=
github.com/r3labs/diff v1.1.0 h1:V53xhrbTHrWFWq3gI4b94AjgEJOerO1+1l0xyHOBi8M=
github.com/r3labs/diff v1.1.0/go.mod h1:7WjXasNzi0vJetRcB/RqNl5dlIsmXcTTLmF5IoH6Xig=
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
github.com/rivo/uniseg v0.4.3/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
Expand Down
6 changes: 5 additions & 1 deletion pkg/construct2/graph_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package construct2

import (
"errors"
"fmt"

"github.com/dominikbraun/graph"
"github.com/klothoplatform/klotho/pkg/graph_addons"
Expand All @@ -23,9 +24,12 @@ func CopyEdgeProps(p graph.EdgeProperties) func(*graph.EdgeProperties) {
// references (as [ResourceId] or [PropertyRef]) of the old ID to the new ID in any resource that depends on or is
// depended on by the resource.
func ReplaceResource(g Graph, oldId ResourceId, newRes *Resource) error {
if oldId == newRes.ID {
return nil
}
err := graph_addons.ReplaceVertex(g, oldId, newRes, ResourceHasher)
if err != nil {
return err
return fmt.Errorf("could not update resource %s to %s: %w", oldId, newRes.ID, err)
}

updateId := func(path PropertyPathItem) error {
Expand Down
15 changes: 13 additions & 2 deletions pkg/construct2/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"math"
"sort"
"strings"

"github.com/dominikbraun/graph"
Expand Down Expand Up @@ -119,8 +120,17 @@ 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, edges := range adjacencyMap {
for _, key := range sortedKeys {
edges := adjacencyMap[key]
for _, edge := range edges {
if skipEdge(edge) {
continue
Expand All @@ -134,7 +144,8 @@ func bellmanFord(g Graph, source ResourceId, skipEdge func(Edge) bool) (*bellman
}
}

for _, edges := range adjacencyMap {
for _, key := range sortedKeys {
edges := adjacencyMap[key]
for _, edge := range edges {
if skipEdge(edge) {
continue
Expand Down
51 changes: 13 additions & 38 deletions pkg/engine2/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,31 @@ import (
func ApplyConstraints(ctx solution_context.SolutionContext) error {
var errs error
for _, constraint := range ctx.Constraints().Application {
errs = errors.Join(errs, applyApplicationConstraint(ctx, constraint))
err := applyApplicationConstraint(ctx, constraint)
if err != nil {
errs = errors.Join(errs, fmt.Errorf("failed to apply constraint %#v: %w", constraint, err))
}
}
if errs != nil {
return errs
}

for _, constraint := range ctx.Constraints().Edges {
errs = errors.Join(errs, applyEdgeConstraint(ctx, constraint))
err := applyEdgeConstraint(ctx, constraint)
if err != nil {
errs = errors.Join(errs, fmt.Errorf("failed to apply constraint %#v: %w", constraint, err))
}
}
if errs != nil {
return errs
}

resourceConstraints := ctx.Constraints().Resources
for i := range resourceConstraints {
errs = errors.Join(errs, applySanitization(ctx, &resourceConstraints[i]))
err := applySanitization(ctx, &resourceConstraints[i])
if err != nil {
errs = errors.Join(errs, fmt.Errorf("failed to apply constraint %#v: %w", resourceConstraints[i], err))
}
}

return nil
Expand Down Expand Up @@ -111,40 +120,6 @@ func applyEdgeConstraint(ctx solution_context.SolutionContext, constraint constr
}
}

addPath := func() error {
switch _, err := ctx.RawView().Vertex(constraint.Target.Source); {
case errors.Is(err, graph.ErrVertexNotFound):
res, err := knowledgebase.CreateResource(ctx.KnowledgeBase(), constraint.Target.Source)
if err != nil {
return fmt.Errorf("could not create source resource: %w", err)
}
err = ctx.OperationalView().AddVertex(res)
if err != nil {
return fmt.Errorf("could not add source resource %s: %w", constraint.Target.Source, err)
}

case err != nil:
return fmt.Errorf("could not get source resource %s: %w", constraint.Target.Source, err)
}

switch _, err := ctx.RawView().Vertex(constraint.Target.Target); {
case errors.Is(err, graph.ErrVertexNotFound):
res, err := knowledgebase.CreateResource(ctx.KnowledgeBase(), constraint.Target.Target)
if err != nil {
return fmt.Errorf("could not create target resource: %w", err)
}
err = ctx.OperationalView().AddVertex(res)
if err != nil {
return fmt.Errorf("could not add target resource %s: %w", constraint.Target.Target, err)
}

case err != nil:
return fmt.Errorf("could not get target resource %s: %w", constraint.Target.Target, err)
}

return ctx.OperationalView().AddEdge(constraint.Target.Source, constraint.Target.Target)
}

removePath := func() error {
paths, err := graph.AllPathsBetween(ctx.DataflowGraph(), constraint.Target.Source, constraint.Target.Target)
switch {
Expand Down Expand Up @@ -181,7 +156,7 @@ func applyEdgeConstraint(ctx solution_context.SolutionContext, constraint constr

switch constraint.Operator {
case constraints.MustExistConstraintOperator:
return addPath()
return ctx.OperationalView().AddEdge(constraint.Target.Source, constraint.Target.Target)

case constraints.MustNotExistConstraintOperator:
return removePath()
Expand Down
46 changes: 42 additions & 4 deletions pkg/engine2/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"testing"

construct "github.com/klothoplatform/klotho/pkg/construct2"
"github.com/stretchr/testify/assert"
"github.com/r3labs/diff"
"gopkg.in/yaml.v3"
)

Expand Down Expand Up @@ -83,7 +83,7 @@ func (tc engineTestCase) Test(t *testing.T) {
t.Fatal(fmt.Errorf("failed to marshal actual output: %w", err))
}

assert.Equal(t, string(expectContent), string(actualContent))
assertYamlMatches(t, string(expectContent), string(actualContent), "dataflow")

// Always visualize views even if we're not testing them to make sure that it at least succeeds
vizFiles, err := main.Engine.VisualizeViews(sol)
Expand Down Expand Up @@ -123,15 +123,53 @@ func (tc engineTestCase) Test(t *testing.T) {
if err != nil {
t.Error(fmt.Errorf("failed to read iac viz file: %w", err))
} else {
assert.Equal(t, string(iacExpect), buf.String(), "IaC topology")
// assert.Equal(t, string(iacExpect), buf.String(), "IaC topology")
assertYamlMatches(t, string(iacExpect), buf.String(), "IaC topology")
}
} else if strings.HasPrefix(f.Path(), "dataflow-") && dataflowVizFile != nil {
dataflowExpect, err := io.ReadAll(dataflowVizFile)
if err != nil {
t.Error(fmt.Errorf("failed to read dataflow viz file: %w", err))
} else {
assert.Equal(t, string(dataflowExpect), buf.String(), "dataflow topology")
// assert.Equal(t, string(dataflowExpect), buf.String(), "dataflow topology")
assertYamlMatches(t, string(dataflowExpect), buf.String(), "dataflow topology")
}
}
}
}

func assertYamlMatches(t *testing.T, expectStr, actualStr string, name string) {
t.Helper()
var expect, actual map[string]interface{}
err := yaml.Unmarshal([]byte(expectStr), &expect)
if err != nil {
t.Errorf("failed to unmarshal expected %s graph: %v", name, err)
return
}
err = yaml.Unmarshal([]byte(actualStr), &actual)
if err != nil {
t.Errorf("failed to unmarshal actual %s graph: %v", name, err)
return
}
differ, err := diff.NewDiffer(diff.SliceOrdering(false))
if err != nil {
t.Errorf("failed to create differ for %s: %v", name, err)
return
}
changes, err := differ.Diff(expect, actual)
if err != nil {
t.Errorf("failed to diff %s: %v", name, err)
return
}
for _, c := range changes {
path := strings.Join(c.Path, ".")
switch c.Type {
case diff.CREATE:
t.Errorf("[%s] %s %s: %v", name, c.Type, path, c.To)
case diff.DELETE:
t.Errorf("[%s] %s %s: %v", name, c.Type, path, c.From)
case diff.UPDATE:
t.Errorf("[%s] %s %s: %v to %v", name, c.Type, path, c.From, c.To)
}
}
}
4 changes: 4 additions & 0 deletions pkg/engine2/operational_eval/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ func (eval *Evaluator) writeGraph(prefix string) {
if debugDir := os.Getenv("KLOTHO_DEBUG_DIR"); debugDir != "" {
prefix = filepath.Join(debugDir, prefix)
}
if err := os.MkdirAll(filepath.Dir(prefix), 0755); err != nil {
zap.S().Errorf("could not create debug directory %s: %v", filepath.Dir(prefix), err)
return
}
var wg sync.WaitGroup
wg.Add(2)
go func() {
Expand Down
17 changes: 17 additions & 0 deletions pkg/engine2/testdata/ecs_rds.dataflow-viz.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
provider: aws
resources:
rds_instance/rds-instance-2:
children: aws:rds_subnet_group:rds_subnet_group-0,aws:security_group:vpc-0:security_group-rds-instance-2-1
parent: vpc/vpc-0


vpc/vpc-0:

ecs_service/ecs_service_0:
children: aws:ecs_task_definition:ecs_service_0
parent: vpc/vpc-0

ecs_service/ecs_service_0 -> rds_instance/rds-instance-2:
path: aws:ecs_task_definition:ecs_service_0,aws:iam_role:ecs_service_0-execution-role


Loading
Loading