From 89c168d857725c8b46456e9c6ba7ddcef698ea93 Mon Sep 17 00:00:00 2001 From: Gordon Date: Wed, 31 Jan 2024 19:21:25 -0500 Subject: [PATCH 1/8] Print errors to stdout --- cmd/engine/main.go | 14 +- create_test.sh | 8 +- pkg/engine2/cli.go | 213 +++++++++++++----- pkg/engine2/engine.go | 23 -- pkg/engine2/engine_test.go | 41 +++- pkg/engine2/errors.go | 11 - pkg/engine2/errors/errors.go | 135 +++++++++++ pkg/engine2/solution_context/decisions.go | 61 +++-- pkg/engine2/testdata/2_routes.err.json | 24 ++ pkg/engine2/testdata/cf_distribution.err.json | 1 + .../testdata/cf_distribution.expect.yaml | 4 +- .../delete_api_to_lambda_edge.err.json | 13 ++ .../delete_namespace_resource.err.json | 13 ++ .../delete_resource_and_iacdeps.err.json | 1 + pkg/engine2/testdata/ecs_rds.err.json | 68 ++++++ pkg/engine2/testdata/ecs_rds.expect.yaml | 10 +- pkg/engine2/testdata/extend_graph.err.json | 13 ++ pkg/engine2/testdata/k8s_api.err.json | 117 ++++++++++ pkg/engine2/testdata/lambda_efs.err.json | 68 ++++++ .../testdata/namespace_pathselect.err.json | 79 +++++++ pkg/engine2/testdata/remove_path.err.json | 79 +++++++ pkg/engine2/testdata/remove_path.expect.yaml | 12 +- pkg/engine2/testdata/rename.err.json | 13 ++ .../same_path_multiple_methods.err.json | 1 + pkg/engine2/testdata/single_lambda.err.json | 13 ++ pkg/engine2/testdata/static_site.err.json | 1 + pkg/engine2/testdata/static_site.expect.yaml | 4 +- pkg/engine2/testdata/vpc_import.err.json | 112 +++++++++ .../testdata/vpc_import_to_lambda.err.json | 123 ++++++++++ .../vpc_import_wo_subnets_to_lambda.err.json | 79 +++++++ 30 files changed, 1221 insertions(+), 133 deletions(-) delete mode 100644 pkg/engine2/errors.go create mode 100644 pkg/engine2/errors/errors.go create mode 100644 pkg/engine2/testdata/2_routes.err.json create mode 100644 pkg/engine2/testdata/cf_distribution.err.json create mode 100644 pkg/engine2/testdata/delete_api_to_lambda_edge.err.json create mode 100644 pkg/engine2/testdata/delete_namespace_resource.err.json create mode 100644 pkg/engine2/testdata/delete_resource_and_iacdeps.err.json create mode 100644 pkg/engine2/testdata/ecs_rds.err.json create mode 100644 pkg/engine2/testdata/extend_graph.err.json create mode 100644 pkg/engine2/testdata/k8s_api.err.json create mode 100644 pkg/engine2/testdata/lambda_efs.err.json create mode 100644 pkg/engine2/testdata/namespace_pathselect.err.json create mode 100644 pkg/engine2/testdata/remove_path.err.json create mode 100644 pkg/engine2/testdata/rename.err.json create mode 100644 pkg/engine2/testdata/same_path_multiple_methods.err.json create mode 100644 pkg/engine2/testdata/single_lambda.err.json create mode 100644 pkg/engine2/testdata/static_site.err.json create mode 100644 pkg/engine2/testdata/vpc_import.err.json create mode 100644 pkg/engine2/testdata/vpc_import_to_lambda.err.json create mode 100644 pkg/engine2/testdata/vpc_import_wo_subnets_to_lambda.err.json diff --git a/cmd/engine/main.go b/cmd/engine/main.go index 7c12ced3d..7831aece2 100644 --- a/cmd/engine/main.go +++ b/cmd/engine/main.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" engine "github.com/klothoplatform/klotho/pkg/engine2" @@ -11,16 +10,11 @@ import ( func main() { root := newRootCmd() err := root.Execute() - if err != nil { - switch err.(type) { - case engine.ConfigValidationError: - fmt.Printf("Error: %v\n", err) - os.Exit(2) - default: - fmt.Printf("Error: %v\n", err) - os.Exit(1) - } + if err == nil { + return } + // Shouldn't happen, the engine CLI should handle errors + os.Exit(1) } func newRootCmd() *cobra.Command { diff --git a/create_test.sh b/create_test.sh index c9fc01b66..37e16306f 100755 --- a/create_test.sh +++ b/create_test.sh @@ -14,12 +14,15 @@ echo "Running $name" # Run the engine echo "Using $out_dir as output directory" +set +e go run ./cmd/engine Run \ -i "$1" \ -c "$1" \ - -o "$out_dir" > $out_dir/out.log 2> $out_dir/err.log + -o "$out_dir" > $out_dir/error_details.json 2> $out_dir/err.log -echo "Successfully ran $name, copying results to testdata" +code=$? +echo "Ran $name return code $code, copying results to testdata" +set -e test_dir="pkg/engine2/testdata" @@ -30,5 +33,6 @@ fi cp "$out_dir/resources.yaml" "$test_dir/$name.expect.yaml" cp "$out_dir/dataflow-topology.yaml" "$test_dir/$name.dataflow-viz.yaml" cp "$out_dir/iac-topology.yaml" "$test_dir/$name.iac-viz.yaml" +cp "$out_dir/error_details.json" "$test_dir/$name.err.json" rm -rf $out_dir diff --git a/pkg/engine2/cli.go b/pkg/engine2/cli.go index c74f9bd92..fd9b7cceb 100644 --- a/pkg/engine2/cli.go +++ b/pkg/engine2/cli.go @@ -1,8 +1,10 @@ package engine2 import ( + "bytes" "encoding/json" "fmt" + "io" "os" "path/filepath" "reflect" @@ -15,8 +17,9 @@ import ( "github.com/klothoplatform/klotho/pkg/closenicely" construct "github.com/klothoplatform/klotho/pkg/construct2" "github.com/klothoplatform/klotho/pkg/engine2/constraints" + engine_errs "github.com/klothoplatform/klotho/pkg/engine2/errors" "github.com/klothoplatform/klotho/pkg/engine2/solution_context" - "github.com/klothoplatform/klotho/pkg/io" + kio "github.com/klothoplatform/klotho/pkg/io" knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2" "github.com/klothoplatform/klotho/pkg/knowledge_base2/reader" "github.com/klothoplatform/klotho/pkg/logging" @@ -29,9 +32,18 @@ import ( "gopkg.in/yaml.v3" ) -type EngineMain struct { - Engine *Engine -} +type ( + EngineMain struct { + Engine *Engine + } + + // RunEngineError is used to pass the desired exit code to the main function. + // It does not have any sub-errors because EngineMain is responsible for + // printing the error to stdout. + RunEngineError struct { + ExitCode int + } +) var engineCfg struct { provider string @@ -118,7 +130,10 @@ func (em *EngineMain) AddEngineCli(root *cobra.Command) { Use: "Run", Short: "Run the klotho engine", GroupID: engineGroup.ID, - RunE: em.RunEngine, + Run: func(cmd *cobra.Command, args []string) { + exitErr := em.RunEngine(cmd, args) + os.Exit(exitErr.ExitCode) + }, } flags = runCmd.Flags() @@ -256,7 +271,88 @@ func setupProfiling() func() { return func() {} } -func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) error { +func (em *EngineMain) Run(context *EngineContext) (int, []engine_errs.EngineError) { + returnCode := 0 + var engErrs []engine_errs.EngineError + + zap.S().Info("Running engine") + err := em.Engine.Run(context) + if err != nil { + returnCode = 1 + if ee, ok := err.(engine_errs.EngineError); ok { + engErrs = append(engErrs, ee) + } else { + engErrs = append(engErrs, engine_errs.InternalError{Err: engine_errs.ErrorsToTree(err)}) + } + } + + if len(context.Solutions) > 0 { + writeDebugGraphs(context.Solutions[0]) + for _, d := range context.Solutions[0].GetDecisions().GetRecords() { + d, ok := d.(solution_context.MaybeErroDecision) + if !ok { + continue + } + ee := d.AsEngineError() + if ee == nil { + continue + } + engErrs = append(engErrs, ee) + if returnCode != 1 { + returnCode = 2 + } + } + } + + return returnCode, engErrs +} + +func writeEngineErrsJson(errs []engine_errs.EngineError, out io.Writer) error { + enc := json.NewEncoder(out) + enc.SetIndent("", " ") + // NOTE: since this isn't used in a web context (it's a CLI), we can disable escaping. + enc.SetEscapeHTML(false) + + outErrs := make([]map[string]any, len(errs)) + for i, e := range errs { + outErrs[i] = e.ToJSONMap() + outErrs[i]["error_code"] = e.ErrorCode() + wrapped := errors.Unwrap(e) + if wrapped != nil { + outErrs[i]["error"] = engine_errs.ErrorsToTree(wrapped) + } + } + return enc.Encode(outErrs) +} + +func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) (exitErr RunEngineError) { + var engErrs []engine_errs.EngineError + internalError := func(err error) { + engErrs = append(engErrs, engine_errs.InternalError{Err: err}) + exitErr.ExitCode = 1 + } + + defer func() { // defer functions execute in FILO order, so this executes after the 'recover'. + err := writeEngineErrsJson(engErrs, os.Stdout) + if err != nil { + zap.S().Errorf("failed to output errors to stdout: %v", err) + } + }() + defer func() { + r := recover() + if r == nil { + return + } + zap.S().Errorf("panic: %v", r) + switch r := r.(type) { + case engine_errs.EngineError: + engErrs = append(engErrs, r) + case error: + engErrs = append(engErrs, engine_errs.InternalError{Err: r}) + default: + engErrs = append(engErrs, engine_errs.InternalError{Err: fmt.Errorf("panic: %v", r)}) + } + }() defer setupProfiling()() // Set up analytics, and hook them up to the logs @@ -264,14 +360,18 @@ func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) error { analyticsClient.AppendProperties(map[string]any{}) z, err := setupLogger(analyticsClient) if err != nil { - return err + internalError(err) + return } - defer closenicely.FuncOrDebug(z.Sync) + // nolint:errcheck + defer z.Sync() // ignore errors from sync, it's always "ERROR: sync /dev/stderr: invalid argument" + zap.ReplaceGlobals(z) err = em.AddEngine() if err != nil { - return err + internalError(err) + return } context := &EngineContext{} @@ -281,12 +381,14 @@ func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) error { zap.S().Info("Loading input graph") inputF, err := os.Open(architectureEngineCfg.inputGraph) if err != nil { - return err + internalError(err) + return } defer inputF.Close() err = yaml.NewDecoder(inputF).Decode(&input) if err != nil { - return err + internalError(fmt.Errorf("failed to decode input graph: %w", err)) + return } context.InitialState = input.Graph if architectureEngineCfg.constraints == "" { @@ -300,54 +402,57 @@ func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) error { if architectureEngineCfg.constraints != "" { runConstraints, err := constraints.LoadConstraintsFromFile(architectureEngineCfg.constraints) if err != nil { - return errors.Errorf("failed to load constraints: %s", err.Error()) + internalError(fmt.Errorf("failed to load constraints: %w", err)) + return } context.Constraints = runConstraints } + // len(engErrs) == 0 at this point so overwriting it is safe + // All other assignments prior are via 'internalError' and return + exitErr.ExitCode, engErrs = em.Run(context) + if exitErr.ExitCode == 1 { + return + } - zap.S().Info("Running engine") - err = em.Engine.Run(context) + var files []kio.File + + configErrors := new(bytes.Buffer) + err = writeEngineErrsJson(engErrs, configErrors) if err != nil { - return errors.Errorf("failed to run engine: %s", err.Error()) + internalError(fmt.Errorf("failed to write config errors: %w", err)) + return } - writeDebugGraphs(context.Solutions[0]) + files = append(files, &kio.RawFile{ + FPath: "config_errors.json", + Content: configErrors.Bytes(), + }) + zap.S().Info("Engine finished running... Generating views") - var files []io.File - files, err = em.Engine.VisualizeViews(context.Solutions[0]) + vizFiles, err := em.Engine.VisualizeViews(context.Solutions[0]) if err != nil { - return errors.Errorf("failed to generate views %s", err.Error()) + internalError(fmt.Errorf("failed to generate views %w", err)) + return } + files = append(files, vizFiles...) zap.S().Info("Generating resources.yaml") b, err := yaml.Marshal(construct.YamlGraph{Graph: context.Solutions[0].DataflowGraph()}) if err != nil { - return errors.Errorf("failed to marshal graph: %s", err.Error()) - } - files = append(files, &io.RawFile{ - FPath: "resources.yaml", - Content: b, - }, + internalError(fmt.Errorf("failed to marshal graph: %w", err)) + return + } + files = append(files, + &kio.RawFile{ + FPath: "resources.yaml", + Content: b, + }, ) - configErrors, configErr := em.Engine.getPropertyValidation(context.Solutions[0]) - if len(configErrors) > 0 { - configErrorData, err := json.Marshal(configErrors) - if err != nil { - return errors.Errorf("failed to marshal config errors: %s", err.Error()) - } - files = append(files, &io.RawFile{ - FPath: "config_errors.json", - Content: configErrorData, - }) - } - - err = io.OutputTo(files, architectureEngineCfg.outputDir) + err = kio.OutputTo(files, architectureEngineCfg.outputDir) if err != nil { - return errors.Errorf("failed to write output files: %s", err.Error()) - } - if configErr != nil { - return ConfigValidationError{Err: configErr} + internalError(fmt.Errorf("failed to write output files: %w", err)) + return } - return nil + return } func (em *EngineMain) GetValidEdgeTargets(cmd *cobra.Command, args []string) error { @@ -376,7 +481,7 @@ func (em *EngineMain) GetValidEdgeTargets(cmd *cobra.Command, args []string) err config, err := ReadGetValidEdgeTargetsConfig(getValidEdgeTargetsCfg.configFile) if err != nil { - return errors.Errorf("failed to load constraints: %s", err.Error()) + return fmt.Errorf("failed to load constraints: %w", err) } context := &GetPossibleEdgesContext{ InputGraph: inputF, @@ -386,23 +491,23 @@ func (em *EngineMain) GetValidEdgeTargets(cmd *cobra.Command, args []string) err zap.S().Info("getting valid edge targets") validTargets, err := em.Engine.GetValidEdgeTargets(context) if err != nil { - return errors.Errorf("failed to run engine: %s", err.Error()) + return fmt.Errorf("failed to run engine: %w", err) } zap.S().Info("writing output files") b, err := yaml.Marshal(validTargets) if err != nil { - return errors.Errorf("failed to marshal possible edges: %s", err.Error()) + return fmt.Errorf("failed to marshal possible edges: %w", err) } - var files []io.File - files = append(files, &io.RawFile{ + var files []kio.File + files = append(files, &kio.RawFile{ FPath: "valid_edge_targets.yaml", Content: b, }) - err = io.OutputTo(files, getValidEdgeTargetsCfg.outputDir) + err = kio.OutputTo(files, getValidEdgeTargetsCfg.outputDir) if err != nil { - return errors.Errorf("failed to write output files: %s", err.Error()) + return fmt.Errorf("failed to write output files: %w", err) } return nil } @@ -414,15 +519,19 @@ func writeDebugGraphs(sol solution_context.SolutionContext) { defer wg.Done() err := GraphToSVG(sol.KnowledgeBase(), sol.DataflowGraph(), "dataflow") if err != nil { - zap.S().Errorf("failed to write dataflow graph: %s", err.Error()) + zap.S().Errorf("failed to write dataflow graph: %w", err) } }() go func() { defer wg.Done() err := GraphToSVG(sol.KnowledgeBase(), sol.DeploymentGraph(), "iac") if err != nil { - zap.S().Errorf("failed to write iac graph: %s", err.Error()) + zap.S().Errorf("failed to write iac graph: %w", err) } }() wg.Wait() } + +func (e RunEngineError) Error() string { + return fmt.Sprintf("exit code %d", e.ExitCode) +} diff --git a/pkg/engine2/engine.go b/pkg/engine2/engine.go index ec43483a9..b94355954 100644 --- a/pkg/engine2/engine.go +++ b/pkg/engine2/engine.go @@ -1,8 +1,6 @@ package engine2 import ( - "errors" - construct "github.com/klothoplatform/klotho/pkg/construct2" "github.com/klothoplatform/klotho/pkg/engine2/constraints" "github.com/klothoplatform/klotho/pkg/engine2/solution_context" @@ -45,24 +43,3 @@ func (e *Engine) Run(context *EngineContext) error { context.Solutions = append(context.Solutions, solutionCtx) return err } - -func (e *Engine) getPropertyValidation(ctx solution_context.SolutionContext) ([]solution_context.PropertyValidationDecision, error) { - decisions := ctx.GetDecisions().GetRecords() - validationDecisions := make([]solution_context.PropertyValidationDecision, 0) - for _, decision := range decisions { - if validation, ok := decision.(solution_context.PropertyValidationDecision); ok { - if validation.Error != nil { - validationDecisions = append(validationDecisions, validation) - } - } - } - var errs error - for _, decision := range validationDecisions { - for _, c := range ctx.Constraints().Resources { - if c.Target == decision.Resource && c.Property == decision.Property.Details().Path { - errs = errors.Join(errs, decision.Error) - } - } - } - return validationDecisions, errs -} diff --git a/pkg/engine2/engine_test.go b/pkg/engine2/engine_test.go index ebc1f6447..6123e7195 100644 --- a/pkg/engine2/engine_test.go +++ b/pkg/engine2/engine_test.go @@ -2,6 +2,7 @@ package engine2 import ( "bytes" + "encoding/json" "fmt" "io" "os" @@ -10,7 +11,9 @@ import ( "testing" construct "github.com/klothoplatform/klotho/pkg/construct2" + engine_errs "github.com/klothoplatform/klotho/pkg/engine2/errors" "github.com/r3labs/diff" + "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" ) @@ -72,10 +75,22 @@ func (tc engineTestCase) Test(t *testing.T) { Constraints: inputFile.Constraints, InitialState: inputFile.Graph, } - err = main.Engine.Run(context) + _, engineErrs := main.Run(context) + // TODO find a convenient way to specify the return code in the testdata + + errDetails := new(bytes.Buffer) + err = writeEngineErrsJson(engineErrs, errDetails) if err != nil { - t.Fatal(fmt.Errorf("failed to run engine: %w", err)) + t.Fatal(fmt.Errorf("failed to write engine errors: %w", err)) + } + errDetailsFile, err := os.Open(strings.Replace(tc.inputPath, ".input.yaml", ".err.json", 1)) + if err != nil && !os.IsNotExist(err) { + t.Fatal(fmt.Errorf("failed to open error details file: %w", err)) + } + if errDetailsFile != nil { + defer errDetailsFile.Close() } + assertErrDetails(t, errDetailsFile, engineErrs) sol := context.Solutions[0] actualContent, err := yaml.Marshal(construct.YamlGraph{Graph: sol.DataflowGraph()}) @@ -138,6 +153,28 @@ func (tc engineTestCase) Test(t *testing.T) { } } +func assertErrDetails(t *testing.T, expectR io.Reader, actual []engine_errs.EngineError) { + var expectV []map[string]any + err := json.NewDecoder(expectR).Decode(&expectV) + if err != nil { + t.Fatalf("failed to read expected error details: %v", err) + return + } + actualBuf := new(bytes.Buffer) + err = writeEngineErrsJson(actual, actualBuf) + if err != nil { + t.Fatalf("failed to write actual error details to buffer: %v", err) + return + } + var actualV []map[string]any + err = json.NewDecoder(actualBuf).Decode(&actualV) + if err != nil { + t.Fatalf("failed to read actual error details from buffer: %v", err) + return + } + assert.Equal(t, expectV, actualV, "error details") +} + func assertYamlMatches(t *testing.T, expectStr, actualStr string, name string) { t.Helper() var expect, actual map[string]interface{} diff --git a/pkg/engine2/errors.go b/pkg/engine2/errors.go deleted file mode 100644 index 0f1d33650..000000000 --- a/pkg/engine2/errors.go +++ /dev/null @@ -1,11 +0,0 @@ -package engine2 - -type ( - ConfigValidationError struct { - Err error - } -) - -func (e ConfigValidationError) Error() string { - return e.Err.Error() -} diff --git a/pkg/engine2/errors/errors.go b/pkg/engine2/errors/errors.go new file mode 100644 index 000000000..6654dec05 --- /dev/null +++ b/pkg/engine2/errors/errors.go @@ -0,0 +1,135 @@ +package engine_errs + +import ( + "fmt" + "strings" +) + +type ( + EngineError interface { + error + // ToJSONMap returns a map that can be marshaled to JSON. Uses this instead of MarshalJSON to avoid + // repeated marshalling of common fields (such as 'error_code') and to allow for consistent formatting + // (eg for pretty-print). + ToJSONMap() map[string]any + ErrorCode() ErrorCode + } + + ErrorCode string + + InternalError struct { + Err error + } + + ErrorTree struct { + Chain []string `json:"chain,omitempty"` + Children []ErrorTree `json:"children,omitempty"` + } +) + +const ( + InternalErrCode ErrorCode = "internal" + ConfigInvalidCode ErrorCode = "config_invalid" + EdgeInvalidCode ErrorCode = "edge_invalid" + EdgeUnsupportedCode ErrorCode = "edge_unsupported" +) + +func (e InternalError) Error() string { + return fmt.Sprintf("internal error: %v", e.Err) +} + +func (e InternalError) ErrorCode() ErrorCode { + return InternalErrCode +} + +func (e InternalError) ToJSONMap() map[string]any { + return map[string]any{} +} + +func (e InternalError) Unwrap() error { + return e.Err +} + +type ( + chainErr interface { + error + Unwrap() error + } + joinErr interface { + error + Unwrap() []error + } +) + +func unwrapChain(err error) (chain []string, last joinErr) { + for current := err; current != nil; { + var next error + cc, ok := current.(chainErr) + if ok { + next = cc.Unwrap() + } else { + joined, ok := current.(joinErr) + if ok { + jerrs := joined.Unwrap() + if len(jerrs) == 1 { + next = jerrs[0] + } else { + last = joined + return + } + } else { + chain = append(chain, current.Error()) + return + } + } + msg := strings.TrimSuffix(strings.TrimSuffix(current.Error(), next.Error()), ": ") + if msg != "" { + chain = append(chain, msg) + } + current = next + } + return +} + +func ErrorsToTree(err error) (tree ErrorTree) { + if err == nil { + return + } + if t, ok := err.(ErrorTree); ok { + return t + } + + var joined joinErr + tree.Chain, joined = unwrapChain(err) + + if joined != nil { + errs := joined.Unwrap() + tree.Children = make([]ErrorTree, len(errs)) + for i, e := range errs { + tree.Children[i] = ErrorsToTree(e) + } + } + return +} + +func (t ErrorTree) Error() string { + sb := &strings.Builder{} + t.print(sb, 0, 0) + return sb.String() +} + +func (t ErrorTree) print(out *strings.Builder, indent int, childChar rune) { + prefix := strings.Repeat("\t", indent) + delim := "" + if childChar != 0 { + delim = string(childChar) + " " + } + fmt.Fprintf(out, "%s%s%v\n", prefix, delim, t.Chain) + for i, child := range t.Children { + char := '├' + if i == len(t.Children)-1 { + char = '└' + } + child.print(out, indent+1, char) + } +} diff --git a/pkg/engine2/solution_context/decisions.go b/pkg/engine2/solution_context/decisions.go index 44bdd0fca..5a08d1235 100644 --- a/pkg/engine2/solution_context/decisions.go +++ b/pkg/engine2/solution_context/decisions.go @@ -1,9 +1,10 @@ package solution_context import ( - "encoding/json" + "fmt" construct "github.com/klothoplatform/klotho/pkg/construct2" + engine_errs "github.com/klothoplatform/klotho/pkg/engine2/errors" knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2" ) @@ -16,10 +17,7 @@ type ( DecisionRecords interface { // AddRecord stores each decision (the what) with the context (the why) in some datastore AddRecord(context []KV, decision SolveDecision) - // // FindDecision returns the context (the why) for a given decision (the what) - // FindDecision(decision SolveDecision) []KV - // // FindContext returns the various decisions (the what) for a given context (the why) - // FindContext(key string, value any) []SolveDecision + GetRecords() []SolveDecision } @@ -29,6 +27,11 @@ type ( internal() } + MaybeErroDecision interface { + // AsEngineError returns an EngineError if the decision is an error, otherwise nil. + AsEngineError() engine_errs.EngineError + } + AddResourceDecision struct { Resource construct.ResourceId } @@ -59,6 +62,10 @@ type ( Value any Error error } + + ConfigValidationError struct { + PropertyValidationDecision + } ) func (d AddResourceDecision) internal() {} @@ -68,18 +75,36 @@ func (d RemoveDependencyDecision) internal() {} func (d SetPropertyDecision) internal() {} func (d PropertyValidationDecision) internal() {} -func (d PropertyValidationDecision) MarshalJSON() ([]byte, error) { - if d.Value != nil { - return json.Marshal(map[string]any{ - "resource": d.Resource, - "property": d.Property.Details().Path, - "value": d.Value, - "error": d.Error.Error(), - }) +func (d PropertyValidationDecision) AsEngineError() engine_errs.EngineError { + if d.Error == nil { + return nil + } + return ConfigValidationError{ + PropertyValidationDecision: d, } - return json.Marshal(map[string]any{ - "resource": d.Resource, - "property": d.Property.Details().Path, - "error": d.Error.Error(), - }) +} + +func (e ConfigValidationError) Error() string { + return fmt.Sprintf( + "config validation error on %s#%s: %v", + e.Resource, + e.Property.Details().Path, + e.PropertyValidationDecision.Error, + ) +} + +func (e ConfigValidationError) ErrorCode() engine_errs.ErrorCode { + return engine_errs.ConfigInvalidCode +} + +func (e ConfigValidationError) ToJSONMap() map[string]any { + return map[string]any{ + "resource": e.Resource, + "property": e.Property.Details().Path, + "value": e.Value, + } +} + +func (e ConfigValidationError) Unwrap() error { + return e.PropertyValidationDecision.Error } diff --git a/pkg/engine2/testdata/2_routes.err.json b/pkg/engine2/testdata/2_routes.err.json new file mode 100644 index 000000000..7402409fc --- /dev/null +++ b/pkg/engine2/testdata/2_routes.err.json @@ -0,0 +1,24 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_0-image", + "value": null + }, + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_1-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_1-image", + "value": null + } +] diff --git a/pkg/engine2/testdata/cf_distribution.err.json b/pkg/engine2/testdata/cf_distribution.err.json new file mode 100644 index 000000000..fe51488c7 --- /dev/null +++ b/pkg/engine2/testdata/cf_distribution.err.json @@ -0,0 +1 @@ +[] diff --git a/pkg/engine2/testdata/cf_distribution.expect.yaml b/pkg/engine2/testdata/cf_distribution.expect.yaml index 6382a5a96..530f73ca7 100755 --- a/pkg/engine2/testdata/cf_distribution.expect.yaml +++ b/pkg/engine2/testdata/cf_distribution.expect.yaml @@ -1,7 +1,5 @@ resources: aws:cloudfront_distribution:cloudfront_distribution_2: - ViewerCertificate: - CloudfrontDefaultCertificate: true CacheBehaviors: [] DefaultCacheBehavior: AllowedMethods: @@ -45,6 +43,8 @@ resources: Restrictions: GeoRestriction: RestrictionType: none + ViewerCertificate: + CloudfrontDefaultCertificate: true aws:api_stage:rest_api_1:cloudfront_distribution_2-rest_api_1: Deployment: aws:api_deployment:rest_api_1:api_deployment-0 RestApi: aws:rest_api:rest_api_1 diff --git a/pkg/engine2/testdata/delete_api_to_lambda_edge.err.json b/pkg/engine2/testdata/delete_api_to_lambda_edge.err.json new file mode 100644 index 000000000..4bef1e293 --- /dev/null +++ b/pkg/engine2/testdata/delete_api_to_lambda_edge.err.json @@ -0,0 +1,13 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_2-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_2-image", + "value": null + } +] diff --git a/pkg/engine2/testdata/delete_namespace_resource.err.json b/pkg/engine2/testdata/delete_namespace_resource.err.json new file mode 100644 index 000000000..4bef1e293 --- /dev/null +++ b/pkg/engine2/testdata/delete_namespace_resource.err.json @@ -0,0 +1,13 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_2-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_2-image", + "value": null + } +] diff --git a/pkg/engine2/testdata/delete_resource_and_iacdeps.err.json b/pkg/engine2/testdata/delete_resource_and_iacdeps.err.json new file mode 100644 index 000000000..fe51488c7 --- /dev/null +++ b/pkg/engine2/testdata/delete_resource_and_iacdeps.err.json @@ -0,0 +1 @@ +[] diff --git a/pkg/engine2/testdata/ecs_rds.err.json b/pkg/engine2/testdata/ecs_rds.err.json new file mode 100644 index 000000000..ec5d5d5e6 --- /dev/null +++ b/pkg/engine2/testdata/ecs_rds.err.json @@ -0,0 +1,68 @@ +[ + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-0" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-0", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-1", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-2" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-2", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-3" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-3", + "value": null + }, + { + "error": { + "chain": [ + "required property Arn is not set on resource aws:vpc:vpc-0" + ] + }, + "error_code": "config_invalid", + "property": "Arn", + "resource": "aws:vpc:vpc-0", + "value": null + }, + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:ecs_service_0-ecs_service_0" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:ecs_service_0-ecs_service_0", + "value": null + } +] diff --git a/pkg/engine2/testdata/ecs_rds.expect.yaml b/pkg/engine2/testdata/ecs_rds.expect.yaml index 83d6e2d65..1bc991251 100755 --- a/pkg/engine2/testdata/ecs_rds.expect.yaml +++ b/pkg/engine2/testdata/ecs_rds.expect.yaml @@ -182,11 +182,6 @@ resources: Protocol: "-1" ToPort: 0 IngressRules: - - Description: Allow ingress traffic from within the same security group - FromPort: 0 - Protocol: "-1" - Self: true - ToPort: 0 - CidrBlocks: - 10.0.128.0/18 Description: Allow ingress traffic from ip addresses within the subnet subnet-0 @@ -199,6 +194,11 @@ resources: FromPort: 0 Protocol: "-1" ToPort: 0 + - Description: Allow ingress traffic from within the same security group + FromPort: 0 + Protocol: "-1" + Self: true + ToPort: 0 Vpc: aws:vpc:vpc-0 aws:route_table:vpc-0:subnet-0-route_table: Routes: diff --git a/pkg/engine2/testdata/extend_graph.err.json b/pkg/engine2/testdata/extend_graph.err.json new file mode 100644 index 000000000..4bb78c936 --- /dev/null +++ b/pkg/engine2/testdata/extend_graph.err.json @@ -0,0 +1,13 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_0-image", + "value": null + } +] diff --git a/pkg/engine2/testdata/k8s_api.err.json b/pkg/engine2/testdata/k8s_api.err.json new file mode 100644 index 000000000..13209be63 --- /dev/null +++ b/pkg/engine2/testdata/k8s_api.err.json @@ -0,0 +1,117 @@ +[ + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-0" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-0", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-1", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-2" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-2", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-3" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-3", + "value": null + }, + { + "error": { + "chain": [ + "required property Arn is not set on resource aws:vpc:vpc-0" + ] + }, + "error_code": "config_invalid", + "property": "Arn", + "resource": "aws:vpc:vpc-0", + "value": null + }, + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:pod2-ecr_image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "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", + "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", + "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", + "value": "80" + } +] diff --git a/pkg/engine2/testdata/lambda_efs.err.json b/pkg/engine2/testdata/lambda_efs.err.json new file mode 100644 index 000000000..36d0f6e69 --- /dev/null +++ b/pkg/engine2/testdata/lambda_efs.err.json @@ -0,0 +1,68 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_test_app-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_test_app-image", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:lambda_test_app-test-efs-fs" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:lambda_test_app-test-efs-fs", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-1", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-2" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-2", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-3" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-3", + "value": null + }, + { + "error": { + "chain": [ + "required property Arn is not set on resource aws:vpc:vpc-0" + ] + }, + "error_code": "config_invalid", + "property": "Arn", + "resource": "aws:vpc:vpc-0", + "value": null + } +] diff --git a/pkg/engine2/testdata/namespace_pathselect.err.json b/pkg/engine2/testdata/namespace_pathselect.err.json new file mode 100644 index 000000000..ef2b11da9 --- /dev/null +++ b/pkg/engine2/testdata/namespace_pathselect.err.json @@ -0,0 +1,79 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_0-image", + "value": null + }, + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_2-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_2-image", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc_1:lambda_function_2-vpc_1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc_1:lambda_function_2-vpc_1", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc_1:subnet-1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc_1:subnet-1", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc_1:subnet-2" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc_1:subnet-2", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc_1:subnet-3" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc_1:subnet-3", + "value": null + }, + { + "error": { + "chain": [ + "required property Arn is not set on resource aws:vpc:vpc_1" + ] + }, + "error_code": "config_invalid", + "property": "Arn", + "resource": "aws:vpc:vpc_1", + "value": null + } +] diff --git a/pkg/engine2/testdata/remove_path.err.json b/pkg/engine2/testdata/remove_path.err.json new file mode 100644 index 000000000..f79832af4 --- /dev/null +++ b/pkg/engine2/testdata/remove_path.err.json @@ -0,0 +1,79 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_0-image", + "value": null + }, + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_3-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_3-image", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-0" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-0", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-1", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-2" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-2", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc-0:subnet-3" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc-0:subnet-3", + "value": null + }, + { + "error": { + "chain": [ + "required property Arn is not set on resource aws:vpc:vpc-0" + ] + }, + "error_code": "config_invalid", + "property": "Arn", + "resource": "aws:vpc:vpc-0", + "value": null + } +] diff --git a/pkg/engine2/testdata/remove_path.expect.yaml b/pkg/engine2/testdata/remove_path.expect.yaml index 71afa2c34..4e375d68e 100644 --- a/pkg/engine2/testdata/remove_path.expect.yaml +++ b/pkg/engine2/testdata/remove_path.expect.yaml @@ -215,6 +215,12 @@ resources: Protocol: "-1" ToPort: 0 IngressRules: + - CidrBlocks: + - 10.0.192.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" @@ -226,12 +232,6 @@ resources: FromPort: 0 Protocol: "-1" ToPort: 0 - - CidrBlocks: - - 10.0.192.0/18 - Description: Allow ingress traffic from ip addresses within the subnet subnet-1 - FromPort: 0 - Protocol: "-1" - ToPort: 0 Vpc: aws:vpc:vpc-0 aws:route_table:vpc-0:subnet-0-route_table: Routes: diff --git a/pkg/engine2/testdata/rename.err.json b/pkg/engine2/testdata/rename.err.json new file mode 100644 index 000000000..b1ba8fdb2 --- /dev/null +++ b/pkg/engine2/testdata/rename.err.json @@ -0,0 +1,13 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_test_app-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_test_app-image", + "value": null + } +] diff --git a/pkg/engine2/testdata/same_path_multiple_methods.err.json b/pkg/engine2/testdata/same_path_multiple_methods.err.json new file mode 100644 index 000000000..fe51488c7 --- /dev/null +++ b/pkg/engine2/testdata/same_path_multiple_methods.err.json @@ -0,0 +1 @@ +[] diff --git a/pkg/engine2/testdata/single_lambda.err.json b/pkg/engine2/testdata/single_lambda.err.json new file mode 100644 index 000000000..4bb78c936 --- /dev/null +++ b/pkg/engine2/testdata/single_lambda.err.json @@ -0,0 +1,13 @@ +[ + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function_0-image", + "value": null + } +] diff --git a/pkg/engine2/testdata/static_site.err.json b/pkg/engine2/testdata/static_site.err.json new file mode 100644 index 000000000..fe51488c7 --- /dev/null +++ b/pkg/engine2/testdata/static_site.err.json @@ -0,0 +1 @@ +[] diff --git a/pkg/engine2/testdata/static_site.expect.yaml b/pkg/engine2/testdata/static_site.expect.yaml index c0b92a69e..85768c6cd 100755 --- a/pkg/engine2/testdata/static_site.expect.yaml +++ b/pkg/engine2/testdata/static_site.expect.yaml @@ -1,7 +1,5 @@ resources: aws:cloudfront_distribution:cloudfront_distribution_1: - ViewerCertificate: - CloudfrontDefaultCertificate: true DefaultCacheBehavior: AllowedMethods: - DELETE @@ -32,6 +30,8 @@ resources: Restrictions: GeoRestriction: RestrictionType: none + ViewerCertificate: + CloudfrontDefaultCertificate: true aws:cloudfront_origin_access_identity:cloudfront_origin_access_identity-0: Comment: this is needed to set up S3 polices so that the S3 bucket is not public aws:s3_bucket_policy:s3_bucket_policy-0: diff --git a/pkg/engine2/testdata/vpc_import.err.json b/pkg/engine2/testdata/vpc_import.err.json new file mode 100644 index 000000000..77d725ab8 --- /dev/null +++ b/pkg/engine2/testdata/vpc_import.err.json @@ -0,0 +1,112 @@ +[ + { + "error": { + "chain": [ + "required property AvailabilityZone is not set on resource aws:subnet:subnet1" + ] + }, + "error_code": "config_invalid", + "property": "AvailabilityZone", + "resource": "aws:subnet:subnet1", + "value": null + }, + { + "error": { + "chain": [ + "required property AvailabilityZone is not set on resource aws:subnet:subnet2" + ] + }, + "error_code": "config_invalid", + "property": "AvailabilityZone", + "resource": "aws:subnet:subnet2", + "value": null + }, + { + "error": { + "chain": [ + "required property AvailabilityZone is not set on resource aws:subnet:subnet3" + ] + }, + "error_code": "config_invalid", + "property": "AvailabilityZone", + "resource": "aws:subnet:subnet3", + "value": null + }, + { + "error": { + "chain": [ + "required property AvailabilityZone is not set on resource aws:subnet:subnet4" + ] + }, + "error_code": "config_invalid", + "property": "AvailabilityZone", + "resource": "aws:subnet:subnet4", + "value": null + }, + { + "error": { + "chain": [ + "required property CidrBlock is not set on resource aws:vpc:vpc" + ] + }, + "error_code": "config_invalid", + "property": "CidrBlock", + "resource": "aws:vpc:vpc", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:subnet1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:subnet1", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:subnet2" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:subnet2", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:subnet3" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:subnet3", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:subnet4" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:subnet4", + "value": null + }, + { + "error": { + "chain": [ + "required property Arn is not set on resource aws:vpc:vpc" + ] + }, + "error_code": "config_invalid", + "property": "Arn", + "resource": "aws:vpc:vpc", + "value": null + } +] diff --git a/pkg/engine2/testdata/vpc_import_to_lambda.err.json b/pkg/engine2/testdata/vpc_import_to_lambda.err.json new file mode 100644 index 000000000..fd4e4eadd --- /dev/null +++ b/pkg/engine2/testdata/vpc_import_to_lambda.err.json @@ -0,0 +1,123 @@ +[ + { + "error": { + "chain": [ + "required property AvailabilityZone is not set on resource aws:subnet:subnet1" + ] + }, + "error_code": "config_invalid", + "property": "AvailabilityZone", + "resource": "aws:subnet:subnet1", + "value": null + }, + { + "error": { + "chain": [ + "required property AvailabilityZone is not set on resource aws:subnet:subnet2" + ] + }, + "error_code": "config_invalid", + "property": "AvailabilityZone", + "resource": "aws:subnet:subnet2", + "value": null + }, + { + "error": { + "chain": [ + "required property AvailabilityZone is not set on resource aws:subnet:subnet3" + ] + }, + "error_code": "config_invalid", + "property": "AvailabilityZone", + "resource": "aws:subnet:subnet3", + "value": null + }, + { + "error": { + "chain": [ + "required property AvailabilityZone is not set on resource aws:subnet:subnet4" + ] + }, + "error_code": "config_invalid", + "property": "AvailabilityZone", + "resource": "aws:subnet:subnet4", + "value": null + }, + { + "error": { + "chain": [ + "required property CidrBlock is not set on resource aws:vpc:vpc" + ] + }, + "error_code": "config_invalid", + "property": "CidrBlock", + "resource": "aws:vpc:vpc", + "value": null + }, + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function-image", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:subnet2" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:subnet2", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:subnet3" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:subnet3", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:subnet4" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:subnet4", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:subnet1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:subnet1", + "value": null + }, + { + "error": { + "chain": [ + "required property Arn is not set on resource aws:vpc:vpc" + ] + }, + "error_code": "config_invalid", + "property": "Arn", + "resource": "aws:vpc:vpc", + "value": null + } +] diff --git a/pkg/engine2/testdata/vpc_import_wo_subnets_to_lambda.err.json b/pkg/engine2/testdata/vpc_import_wo_subnets_to_lambda.err.json new file mode 100644 index 000000000..a0a436dd0 --- /dev/null +++ b/pkg/engine2/testdata/vpc_import_wo_subnets_to_lambda.err.json @@ -0,0 +1,79 @@ +[ + { + "error": { + "chain": [ + "required property CidrBlock is not set on resource aws:vpc:vpc" + ] + }, + "error_code": "config_invalid", + "property": "CidrBlock", + "resource": "aws:vpc:vpc", + "value": null + }, + { + "error": { + "chain": [ + "required property BaseImage is not set on resource aws:ecr_image:lambda_function-image" + ] + }, + "error_code": "config_invalid", + "property": "BaseImage", + "resource": "aws:ecr_image:lambda_function-image", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:lambda_function-vpc" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:lambda_function-vpc", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:subnet-1" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:subnet-1", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:subnet-2" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:subnet-2", + "value": null + }, + { + "error": { + "chain": [ + "required property Id is not set on resource aws:subnet:vpc:subnet-3" + ] + }, + "error_code": "config_invalid", + "property": "Id", + "resource": "aws:subnet:vpc:subnet-3", + "value": null + }, + { + "error": { + "chain": [ + "required property Arn is not set on resource aws:vpc:vpc" + ] + }, + "error_code": "config_invalid", + "property": "Arn", + "resource": "aws:vpc:vpc", + "value": null + } +] From febacec9f92677c3b646793d309467b790b40bf2 Mon Sep 17 00:00:00 2001 From: Gordon Date: Wed, 31 Jan 2024 19:23:44 -0500 Subject: [PATCH 2/8] Simplify by returning exit code --- pkg/engine2/cli.go | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/pkg/engine2/cli.go b/pkg/engine2/cli.go index fd9b7cceb..38e8e153c 100644 --- a/pkg/engine2/cli.go +++ b/pkg/engine2/cli.go @@ -36,13 +36,6 @@ type ( EngineMain struct { Engine *Engine } - - // RunEngineError is used to pass the desired exit code to the main function. - // It does not have any sub-errors because EngineMain is responsible for - // printing the error to stdout. - RunEngineError struct { - ExitCode int - } ) var engineCfg struct { @@ -131,8 +124,8 @@ func (em *EngineMain) AddEngineCli(root *cobra.Command) { Short: "Run the klotho engine", GroupID: engineGroup.ID, Run: func(cmd *cobra.Command, args []string) { - exitErr := em.RunEngine(cmd, args) - os.Exit(exitErr.ExitCode) + exitCode := em.RunEngine(cmd, args) + os.Exit(exitCode) }, } @@ -325,11 +318,11 @@ func writeEngineErrsJson(errs []engine_errs.EngineError, out io.Writer) error { return enc.Encode(outErrs) } -func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) (exitErr RunEngineError) { +func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) (exitCode int) { var engErrs []engine_errs.EngineError internalError := func(err error) { engErrs = append(engErrs, engine_errs.InternalError{Err: err}) - exitErr.ExitCode = 1 + exitCode = 1 } defer func() { // defer functions execute in FILO order, so this executes after the 'recover'. @@ -409,8 +402,8 @@ func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) (exitErr RunE } // len(engErrs) == 0 at this point so overwriting it is safe // All other assignments prior are via 'internalError' and return - exitErr.ExitCode, engErrs = em.Run(context) - if exitErr.ExitCode == 1 { + exitCode, engErrs = em.Run(context) + if exitCode == 1 { return } @@ -531,7 +524,3 @@ func writeDebugGraphs(sol solution_context.SolutionContext) { }() wg.Wait() } - -func (e RunEngineError) Error() string { - return fmt.Sprintf("exit code %d", e.ExitCode) -} From cc2dc7c5d7398eb66fa75968fb49803919f810d3 Mon Sep 17 00:00:00 2001 From: Gordon Date: Thu, 1 Feb 2024 09:21:44 -0500 Subject: [PATCH 3/8] Rename interface --- pkg/engine2/cli.go | 4 ++-- pkg/engine2/solution_context/decisions.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/engine2/cli.go b/pkg/engine2/cli.go index 38e8e153c..9ee9e2ee8 100644 --- a/pkg/engine2/cli.go +++ b/pkg/engine2/cli.go @@ -282,11 +282,11 @@ func (em *EngineMain) Run(context *EngineContext) (int, []engine_errs.EngineErro if len(context.Solutions) > 0 { writeDebugGraphs(context.Solutions[0]) for _, d := range context.Solutions[0].GetDecisions().GetRecords() { - d, ok := d.(solution_context.MaybeErroDecision) + d, ok := d.(solution_context.AsEngineError) if !ok { continue } - ee := d.AsEngineError() + ee := d.TryEngineError() if ee == nil { continue } diff --git a/pkg/engine2/solution_context/decisions.go b/pkg/engine2/solution_context/decisions.go index 5a08d1235..999bb33bc 100644 --- a/pkg/engine2/solution_context/decisions.go +++ b/pkg/engine2/solution_context/decisions.go @@ -27,9 +27,9 @@ type ( internal() } - MaybeErroDecision interface { - // AsEngineError returns an EngineError if the decision is an error, otherwise nil. - AsEngineError() engine_errs.EngineError + AsEngineError interface { + // TryEngineError returns an EngineError if the decision is an error, otherwise nil. + TryEngineError() engine_errs.EngineError } AddResourceDecision struct { From 3bb92383a35898f109f7b83db797de56ee69e59f Mon Sep 17 00:00:00 2001 From: Gordon Date: Thu, 1 Feb 2024 09:42:49 -0500 Subject: [PATCH 4/8] Update decisions to match the renamed interface --- pkg/engine2/cli.go | 5 +++++ pkg/engine2/solution_context/decisions.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/engine2/cli.go b/pkg/engine2/cli.go index 9ee9e2ee8..21dbee591 100644 --- a/pkg/engine2/cli.go +++ b/pkg/engine2/cli.go @@ -271,6 +271,8 @@ func (em *EngineMain) Run(context *EngineContext) (int, []engine_errs.EngineErro zap.S().Info("Running engine") err := em.Engine.Run(context) if err != nil { + // When the engine returns an error, that indicates that it halted evaluation, thus is a fatal error. + // This is returned as exit code 1, and add the details to be printed to stdout. returnCode = 1 if ee, ok := err.(engine_errs.EngineError); ok { engErrs = append(engErrs, ee) @@ -281,6 +283,9 @@ func (em *EngineMain) Run(context *EngineContext) (int, []engine_errs.EngineErro if len(context.Solutions) > 0 { writeDebugGraphs(context.Solutions[0]) + + // If there are any decisions that are engine errors, add them to the list of error details + // to be printed to stdout. These are returned as exit code 2 unless it is already code 1. for _, d := range context.Solutions[0].GetDecisions().GetRecords() { d, ok := d.(solution_context.AsEngineError) if !ok { diff --git a/pkg/engine2/solution_context/decisions.go b/pkg/engine2/solution_context/decisions.go index 999bb33bc..0005608b2 100644 --- a/pkg/engine2/solution_context/decisions.go +++ b/pkg/engine2/solution_context/decisions.go @@ -75,7 +75,7 @@ func (d RemoveDependencyDecision) internal() {} func (d SetPropertyDecision) internal() {} func (d PropertyValidationDecision) internal() {} -func (d PropertyValidationDecision) AsEngineError() engine_errs.EngineError { +func (d PropertyValidationDecision) TryEngineError() engine_errs.EngineError { if d.Error == nil { return nil } From 93d1115ce05ba5f3ff273981bf78982dddd113d7 Mon Sep 17 00:00:00 2001 From: Gordon Date: Thu, 1 Feb 2024 10:05:21 -0500 Subject: [PATCH 5/8] Regen tests after rebasing --- pkg/engine2/testdata/ecs_rds.expect.yaml | 10 +++++----- pkg/engine2/testdata/k8s_api.expect.yaml | 16 ++++++++-------- pkg/engine2/testdata/remove_path.expect.yaml | 12 ++++++------ .../testdata/vpc_import_to_lambda.err.json | 16 ++++++++-------- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/pkg/engine2/testdata/ecs_rds.expect.yaml b/pkg/engine2/testdata/ecs_rds.expect.yaml index 1bc991251..83d6e2d65 100755 --- a/pkg/engine2/testdata/ecs_rds.expect.yaml +++ b/pkg/engine2/testdata/ecs_rds.expect.yaml @@ -182,6 +182,11 @@ resources: Protocol: "-1" ToPort: 0 IngressRules: + - Description: Allow ingress traffic from within the same security group + FromPort: 0 + Protocol: "-1" + Self: true + ToPort: 0 - CidrBlocks: - 10.0.128.0/18 Description: Allow ingress traffic from ip addresses within the subnet subnet-0 @@ -194,11 +199,6 @@ resources: FromPort: 0 Protocol: "-1" ToPort: 0 - - Description: Allow ingress traffic from within the same security group - FromPort: 0 - Protocol: "-1" - Self: true - ToPort: 0 Vpc: aws:vpc:vpc-0 aws:route_table:vpc-0:subnet-0-route_table: Routes: diff --git a/pkg/engine2/testdata/k8s_api.expect.yaml b/pkg/engine2/testdata/k8s_api.expect.yaml index 13904dd3e..8d6562bc6 100755 --- a/pkg/engine2/testdata/k8s_api.expect.yaml +++ b/pkg/engine2/testdata/k8s_api.expect.yaml @@ -267,12 +267,12 @@ resources: - ec2.amazonaws.com Version: "2012-10-17" ManagedPolicies: + - 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: @@ -484,12 +484,6 @@ resources: Protocol: "-1" ToPort: 0 IngressRules: - - CidrBlocks: - - 0.0.0.0/0 - Description: Allows ingress traffic from the EKS control plane - FromPort: 9443 - Protocol: TCP - ToPort: 9443 - Description: Allow ingress traffic from within the same security group FromPort: 0 Protocol: "-1" @@ -507,6 +501,12 @@ resources: FromPort: 0 Protocol: "-1" ToPort: 0 + - CidrBlocks: + - 0.0.0.0/0 + Description: Allows ingress traffic from the EKS control plane + FromPort: 9443 + Protocol: TCP + ToPort: 9443 Vpc: aws:vpc:vpc-0 aws:route_table:vpc-0:subnet-0-route_table: Routes: diff --git a/pkg/engine2/testdata/remove_path.expect.yaml b/pkg/engine2/testdata/remove_path.expect.yaml index 4e375d68e..71afa2c34 100644 --- a/pkg/engine2/testdata/remove_path.expect.yaml +++ b/pkg/engine2/testdata/remove_path.expect.yaml @@ -215,12 +215,6 @@ resources: Protocol: "-1" ToPort: 0 IngressRules: - - CidrBlocks: - - 10.0.192.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" @@ -232,6 +226,12 @@ resources: FromPort: 0 Protocol: "-1" ToPort: 0 + - CidrBlocks: + - 10.0.192.0/18 + Description: Allow ingress traffic from ip addresses within the subnet subnet-1 + FromPort: 0 + Protocol: "-1" + ToPort: 0 Vpc: aws:vpc:vpc-0 aws:route_table:vpc-0:subnet-0-route_table: Routes: diff --git a/pkg/engine2/testdata/vpc_import_to_lambda.err.json b/pkg/engine2/testdata/vpc_import_to_lambda.err.json index fd4e4eadd..9ed6c4d6e 100644 --- a/pkg/engine2/testdata/vpc_import_to_lambda.err.json +++ b/pkg/engine2/testdata/vpc_import_to_lambda.err.json @@ -68,45 +68,45 @@ { "error": { "chain": [ - "required property Id is not set on resource aws:subnet:subnet2" + "required property Id is not set on resource aws:subnet:subnet3" ] }, "error_code": "config_invalid", "property": "Id", - "resource": "aws:subnet:subnet2", + "resource": "aws:subnet:subnet3", "value": null }, { "error": { "chain": [ - "required property Id is not set on resource aws:subnet:subnet3" + "required property Id is not set on resource aws:subnet:subnet4" ] }, "error_code": "config_invalid", "property": "Id", - "resource": "aws:subnet:subnet3", + "resource": "aws:subnet:subnet4", "value": null }, { "error": { "chain": [ - "required property Id is not set on resource aws:subnet:subnet4" + "required property Id is not set on resource aws:subnet:vpc:subnet1" ] }, "error_code": "config_invalid", "property": "Id", - "resource": "aws:subnet:subnet4", + "resource": "aws:subnet:vpc:subnet1", "value": null }, { "error": { "chain": [ - "required property Id is not set on resource aws:subnet:vpc:subnet1" + "required property Id is not set on resource aws:subnet:vpc:subnet2" ] }, "error_code": "config_invalid", "property": "Id", - "resource": "aws:subnet:vpc:subnet1", + "resource": "aws:subnet:vpc:subnet2", "value": null }, { From 20be39259c8b8d88a3d87a2a88358e0b07601c9e Mon Sep 17 00:00:00 2001 From: Gordon Date: Sat, 3 Feb 2024 22:47:28 -0500 Subject: [PATCH 6/8] Add validation error to config error json output --- pkg/engine2/solution_context/decisions.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/engine2/solution_context/decisions.go b/pkg/engine2/solution_context/decisions.go index 0005608b2..10fc0ccdc 100644 --- a/pkg/engine2/solution_context/decisions.go +++ b/pkg/engine2/solution_context/decisions.go @@ -99,9 +99,10 @@ func (e ConfigValidationError) ErrorCode() engine_errs.ErrorCode { func (e ConfigValidationError) ToJSONMap() map[string]any { return map[string]any{ - "resource": e.Resource, - "property": e.Property.Details().Path, - "value": e.Value, + "resource": e.Resource, + "property": e.Property.Details().Path, + "value": e.Value, + "validation_error": e.PropertyValidationDecision.Error.Error(), } } From 085426f4281af997516bee6c36ccef3879d48d71 Mon Sep 17 00:00:00 2001 From: Gordon Date: Sat, 3 Feb 2024 22:49:21 -0500 Subject: [PATCH 7/8] Regen test expected errs --- pkg/engine2/testdata/2_routes.err.json | 2 ++ .../testdata/delete_api_to_lambda_edge.err.json | 1 + .../testdata/delete_namespace_resource.err.json | 1 + pkg/engine2/testdata/ecs_rds.err.json | 6 ++++++ pkg/engine2/testdata/extend_graph.err.json | 1 + pkg/engine2/testdata/k8s_api.err.json | 9 +++++++++ pkg/engine2/testdata/lambda_efs.err.json | 6 ++++++ pkg/engine2/testdata/namespace_pathselect.err.json | 7 +++++++ pkg/engine2/testdata/remove_path.err.json | 7 +++++++ pkg/engine2/testdata/rename.err.json | 1 + pkg/engine2/testdata/single_lambda.err.json | 1 + pkg/engine2/testdata/vpc_import.err.json | 10 ++++++++++ pkg/engine2/testdata/vpc_import_to_lambda.err.json | 11 +++++++++++ .../testdata/vpc_import_wo_subnets_to_lambda.err.json | 7 +++++++ 14 files changed, 70 insertions(+) diff --git a/pkg/engine2/testdata/2_routes.err.json b/pkg/engine2/testdata/2_routes.err.json index 7402409fc..f011cb455 100644 --- a/pkg/engine2/testdata/2_routes.err.json +++ b/pkg/engine2/testdata/2_routes.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_0-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_1-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_1-image", "value": null } ] diff --git a/pkg/engine2/testdata/delete_api_to_lambda_edge.err.json b/pkg/engine2/testdata/delete_api_to_lambda_edge.err.json index 4bef1e293..76fc4bc07 100644 --- a/pkg/engine2/testdata/delete_api_to_lambda_edge.err.json +++ b/pkg/engine2/testdata/delete_api_to_lambda_edge.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_2-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_2-image", "value": null } ] diff --git a/pkg/engine2/testdata/delete_namespace_resource.err.json b/pkg/engine2/testdata/delete_namespace_resource.err.json index 4bef1e293..76fc4bc07 100644 --- a/pkg/engine2/testdata/delete_namespace_resource.err.json +++ b/pkg/engine2/testdata/delete_namespace_resource.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_2-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_2-image", "value": null } ] diff --git a/pkg/engine2/testdata/ecs_rds.err.json b/pkg/engine2/testdata/ecs_rds.err.json index ec5d5d5e6..10deeae60 100644 --- a/pkg/engine2/testdata/ecs_rds.err.json +++ b/pkg/engine2/testdata/ecs_rds.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-0", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-0", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-1", "value": null }, { @@ -30,6 +32,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-2", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-2", "value": null }, { @@ -41,6 +44,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-3", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-3", "value": null }, { @@ -52,6 +56,7 @@ "error_code": "config_invalid", "property": "Arn", "resource": "aws:vpc:vpc-0", + "validation_error": "required property Arn is not set on resource aws:vpc:vpc-0", "value": null }, { @@ -63,6 +68,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:ecs_service_0-ecs_service_0", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:ecs_service_0-ecs_service_0", "value": null } ] diff --git a/pkg/engine2/testdata/extend_graph.err.json b/pkg/engine2/testdata/extend_graph.err.json index 4bb78c936..1c245faca 100644 --- a/pkg/engine2/testdata/extend_graph.err.json +++ b/pkg/engine2/testdata/extend_graph.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_0-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image", "value": null } ] diff --git a/pkg/engine2/testdata/k8s_api.err.json b/pkg/engine2/testdata/k8s_api.err.json index 13209be63..32e731976 100644 --- a/pkg/engine2/testdata/k8s_api.err.json +++ b/pkg/engine2/testdata/k8s_api.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-0", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-0", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-1", "value": null }, { @@ -30,6 +32,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-2", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-2", "value": null }, { @@ -41,6 +44,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-3", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-3", "value": null }, { @@ -52,6 +56,7 @@ "error_code": "config_invalid", "property": "Arn", "resource": "aws:vpc:vpc-0", + "validation_error": "required property Arn is not set on resource aws:vpc:vpc-0", "value": null }, { @@ -63,6 +68,7 @@ "error_code": "config_invalid", "property": "BaseImage", "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 }, { @@ -83,6 +89,7 @@ "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", @@ -101,6 +108,7 @@ "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" }, { @@ -112,6 +120,7 @@ "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/engine2/testdata/lambda_efs.err.json b/pkg/engine2/testdata/lambda_efs.err.json index 36d0f6e69..86a310d9e 100644 --- a/pkg/engine2/testdata/lambda_efs.err.json +++ b/pkg/engine2/testdata/lambda_efs.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_test_app-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_test_app-image", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:lambda_test_app-test-efs-fs", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:lambda_test_app-test-efs-fs", "value": null }, { @@ -30,6 +32,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-1", "value": null }, { @@ -41,6 +44,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-2", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-2", "value": null }, { @@ -52,6 +56,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-3", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-3", "value": null }, { @@ -63,6 +68,7 @@ "error_code": "config_invalid", "property": "Arn", "resource": "aws:vpc:vpc-0", + "validation_error": "required property Arn is not set on resource aws:vpc:vpc-0", "value": null } ] diff --git a/pkg/engine2/testdata/namespace_pathselect.err.json b/pkg/engine2/testdata/namespace_pathselect.err.json index ef2b11da9..8a3a31c4e 100644 --- a/pkg/engine2/testdata/namespace_pathselect.err.json +++ b/pkg/engine2/testdata/namespace_pathselect.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_0-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_2-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_2-image", "value": null }, { @@ -30,6 +32,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc_1:lambda_function_2-vpc_1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc_1:lambda_function_2-vpc_1", "value": null }, { @@ -41,6 +44,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc_1:subnet-1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc_1:subnet-1", "value": null }, { @@ -52,6 +56,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc_1:subnet-2", + "validation_error": "required property Id is not set on resource aws:subnet:vpc_1:subnet-2", "value": null }, { @@ -63,6 +68,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc_1:subnet-3", + "validation_error": "required property Id is not set on resource aws:subnet:vpc_1:subnet-3", "value": null }, { @@ -74,6 +80,7 @@ "error_code": "config_invalid", "property": "Arn", "resource": "aws:vpc:vpc_1", + "validation_error": "required property Arn is not set on resource aws:vpc:vpc_1", "value": null } ] diff --git a/pkg/engine2/testdata/remove_path.err.json b/pkg/engine2/testdata/remove_path.err.json index f79832af4..3656a754c 100644 --- a/pkg/engine2/testdata/remove_path.err.json +++ b/pkg/engine2/testdata/remove_path.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_0-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_3-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_3-image", "value": null }, { @@ -30,6 +32,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-0", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-0", "value": null }, { @@ -41,6 +44,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-1", "value": null }, { @@ -52,6 +56,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-2", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-2", "value": null }, { @@ -63,6 +68,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc-0:subnet-3", + "validation_error": "required property Id is not set on resource aws:subnet:vpc-0:subnet-3", "value": null }, { @@ -74,6 +80,7 @@ "error_code": "config_invalid", "property": "Arn", "resource": "aws:vpc:vpc-0", + "validation_error": "required property Arn is not set on resource aws:vpc:vpc-0", "value": null } ] diff --git a/pkg/engine2/testdata/rename.err.json b/pkg/engine2/testdata/rename.err.json index b1ba8fdb2..7fe5a0323 100644 --- a/pkg/engine2/testdata/rename.err.json +++ b/pkg/engine2/testdata/rename.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_test_app-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_test_app-image", "value": null } ] diff --git a/pkg/engine2/testdata/single_lambda.err.json b/pkg/engine2/testdata/single_lambda.err.json index 4bb78c936..1c245faca 100644 --- a/pkg/engine2/testdata/single_lambda.err.json +++ b/pkg/engine2/testdata/single_lambda.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function_0-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function_0-image", "value": null } ] diff --git a/pkg/engine2/testdata/vpc_import.err.json b/pkg/engine2/testdata/vpc_import.err.json index 77d725ab8..1c8224cbe 100644 --- a/pkg/engine2/testdata/vpc_import.err.json +++ b/pkg/engine2/testdata/vpc_import.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "AvailabilityZone", "resource": "aws:subnet:subnet1", + "validation_error": "required property AvailabilityZone is not set on resource aws:subnet:subnet1", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "AvailabilityZone", "resource": "aws:subnet:subnet2", + "validation_error": "required property AvailabilityZone is not set on resource aws:subnet:subnet2", "value": null }, { @@ -30,6 +32,7 @@ "error_code": "config_invalid", "property": "AvailabilityZone", "resource": "aws:subnet:subnet3", + "validation_error": "required property AvailabilityZone is not set on resource aws:subnet:subnet3", "value": null }, { @@ -41,6 +44,7 @@ "error_code": "config_invalid", "property": "AvailabilityZone", "resource": "aws:subnet:subnet4", + "validation_error": "required property AvailabilityZone is not set on resource aws:subnet:subnet4", "value": null }, { @@ -52,6 +56,7 @@ "error_code": "config_invalid", "property": "CidrBlock", "resource": "aws:vpc:vpc", + "validation_error": "required property CidrBlock is not set on resource aws:vpc:vpc", "value": null }, { @@ -63,6 +68,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet1", "value": null }, { @@ -74,6 +80,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet2", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet2", "value": null }, { @@ -85,6 +92,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet3", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet3", "value": null }, { @@ -96,6 +104,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet4", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet4", "value": null }, { @@ -107,6 +116,7 @@ "error_code": "config_invalid", "property": "Arn", "resource": "aws:vpc:vpc", + "validation_error": "required property Arn is not set on resource aws:vpc:vpc", "value": null } ] diff --git a/pkg/engine2/testdata/vpc_import_to_lambda.err.json b/pkg/engine2/testdata/vpc_import_to_lambda.err.json index 9ed6c4d6e..9f82940de 100644 --- a/pkg/engine2/testdata/vpc_import_to_lambda.err.json +++ b/pkg/engine2/testdata/vpc_import_to_lambda.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "AvailabilityZone", "resource": "aws:subnet:subnet1", + "validation_error": "required property AvailabilityZone is not set on resource aws:subnet:subnet1", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "AvailabilityZone", "resource": "aws:subnet:subnet2", + "validation_error": "required property AvailabilityZone is not set on resource aws:subnet:subnet2", "value": null }, { @@ -30,6 +32,7 @@ "error_code": "config_invalid", "property": "AvailabilityZone", "resource": "aws:subnet:subnet3", + "validation_error": "required property AvailabilityZone is not set on resource aws:subnet:subnet3", "value": null }, { @@ -41,6 +44,7 @@ "error_code": "config_invalid", "property": "AvailabilityZone", "resource": "aws:subnet:subnet4", + "validation_error": "required property AvailabilityZone is not set on resource aws:subnet:subnet4", "value": null }, { @@ -52,6 +56,7 @@ "error_code": "config_invalid", "property": "CidrBlock", "resource": "aws:vpc:vpc", + "validation_error": "required property CidrBlock is not set on resource aws:vpc:vpc", "value": null }, { @@ -63,6 +68,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function-image", "value": null }, { @@ -74,6 +80,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:subnet3", + "validation_error": "required property Id is not set on resource aws:subnet:subnet3", "value": null }, { @@ -85,6 +92,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:subnet4", + "validation_error": "required property Id is not set on resource aws:subnet:subnet4", "value": null }, { @@ -96,6 +104,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet1", "value": null }, { @@ -107,6 +116,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet2", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet2", "value": null }, { @@ -118,6 +128,7 @@ "error_code": "config_invalid", "property": "Arn", "resource": "aws:vpc:vpc", + "validation_error": "required property Arn is not set on resource aws:vpc:vpc", "value": null } ] diff --git a/pkg/engine2/testdata/vpc_import_wo_subnets_to_lambda.err.json b/pkg/engine2/testdata/vpc_import_wo_subnets_to_lambda.err.json index a0a436dd0..b8ca8d608 100644 --- a/pkg/engine2/testdata/vpc_import_wo_subnets_to_lambda.err.json +++ b/pkg/engine2/testdata/vpc_import_wo_subnets_to_lambda.err.json @@ -8,6 +8,7 @@ "error_code": "config_invalid", "property": "CidrBlock", "resource": "aws:vpc:vpc", + "validation_error": "required property CidrBlock is not set on resource aws:vpc:vpc", "value": null }, { @@ -19,6 +20,7 @@ "error_code": "config_invalid", "property": "BaseImage", "resource": "aws:ecr_image:lambda_function-image", + "validation_error": "required property BaseImage is not set on resource aws:ecr_image:lambda_function-image", "value": null }, { @@ -30,6 +32,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:lambda_function-vpc", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:lambda_function-vpc", "value": null }, { @@ -41,6 +44,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet-1", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet-1", "value": null }, { @@ -52,6 +56,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet-2", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet-2", "value": null }, { @@ -63,6 +68,7 @@ "error_code": "config_invalid", "property": "Id", "resource": "aws:subnet:vpc:subnet-3", + "validation_error": "required property Id is not set on resource aws:subnet:vpc:subnet-3", "value": null }, { @@ -74,6 +80,7 @@ "error_code": "config_invalid", "property": "Arn", "resource": "aws:vpc:vpc", + "validation_error": "required property Arn is not set on resource aws:vpc:vpc", "value": null } ] From 15af97486f1969ea667c4d7f75bb8ad60c638d00 Mon Sep 17 00:00:00 2001 From: Gordon Date: Tue, 6 Feb 2024 12:33:25 -0500 Subject: [PATCH 8/8] Raise PathUnsupported & PathInvalid errors --- create_test.sh | 8 +- pkg/engine2/cli.go | 31 +++- pkg/engine2/edge_targets.go | 2 +- pkg/engine2/engine_test.go | 24 +-- pkg/engine2/errors/error_tree.go | 95 ++++++++++++ pkg/engine2/errors/errors.go | 140 ++++++++---------- pkg/engine2/operational_eval/evaluator.go | 2 +- pkg/engine2/operational_eval/graph.go | 2 +- .../operational_eval/vertex_path_expand.go | 71 ++++----- .../vertex_path_expand_test.go | 12 +- pkg/engine2/path_selection/debug.go | 4 +- pkg/engine2/path_selection/path_expansion.go | 75 ++++++---- pkg/engine2/path_selection/path_selection.go | 10 +- pkg/engine2/path_selection/paths.go | 8 +- .../testdata/unsupported_edge.err.json | 26 ++++ .../testdata/unsupported_edge.input.yaml | 12 ++ 16 files changed, 345 insertions(+), 177 deletions(-) create mode 100644 pkg/engine2/errors/error_tree.go create mode 100644 pkg/engine2/testdata/unsupported_edge.err.json create mode 100644 pkg/engine2/testdata/unsupported_edge.input.yaml diff --git a/create_test.sh b/create_test.sh index 37e16306f..bb5e3aa67 100755 --- a/create_test.sh +++ b/create_test.sh @@ -30,9 +30,9 @@ if [ ! "$test_dir/$name.input.yaml" -ef "$1" ]; then cp $1 "$test_dir/$name.input.yaml" fi -cp "$out_dir/resources.yaml" "$test_dir/$name.expect.yaml" -cp "$out_dir/dataflow-topology.yaml" "$test_dir/$name.dataflow-viz.yaml" -cp "$out_dir/iac-topology.yaml" "$test_dir/$name.iac-viz.yaml" -cp "$out_dir/error_details.json" "$test_dir/$name.err.json" +[ -e "$out_dir/resources.yaml" ] && cp "$out_dir/resources.yaml" "$test_dir/$name.expect.yaml" +[ -e "$out_dir/dataflow-topology.yaml" ] && cp "$out_dir/dataflow-topology.yaml" "$test_dir/$name.dataflow-viz.yaml" +[ -e "$out_dir/iac-topology.yaml" ] && cp "$out_dir/iac-topology.yaml" "$test_dir/$name.iac-viz.yaml" +[ -e "$out_dir/error_details.json" ] && cp "$out_dir/error_details.json" "$test_dir/$name.err.json" rm -rf $out_dir diff --git a/pkg/engine2/cli.go b/pkg/engine2/cli.go index 21dbee591..92f6a1a4f 100644 --- a/pkg/engine2/cli.go +++ b/pkg/engine2/cli.go @@ -264,6 +264,30 @@ func setupProfiling() func() { return func() {} } +func extractEngineErrors(err error) []engine_errs.EngineError { + if err == nil { + return nil + } + var errs []engine_errs.EngineError + queue := []error{err} + for len(queue) > 0 { + err := queue[0] + queue = queue[1:] + switch err := err.(type) { + case engine_errs.EngineError: + errs = append(errs, err) + case interface{ Unwrap() []error }: + queue = append(queue, err.Unwrap()...) + case interface{ Unwrap() error }: + queue = append(queue, err.Unwrap()) + } + } + if len(errs) == 0 { + errs = append(errs, engine_errs.InternalError{Err: err}) + } + return errs +} + func (em *EngineMain) Run(context *EngineContext) (int, []engine_errs.EngineError) { returnCode := 0 var engErrs []engine_errs.EngineError @@ -274,11 +298,8 @@ func (em *EngineMain) Run(context *EngineContext) (int, []engine_errs.EngineErro // When the engine returns an error, that indicates that it halted evaluation, thus is a fatal error. // This is returned as exit code 1, and add the details to be printed to stdout. returnCode = 1 - if ee, ok := err.(engine_errs.EngineError); ok { - engErrs = append(engErrs, ee) - } else { - engErrs = append(engErrs, engine_errs.InternalError{Err: engine_errs.ErrorsToTree(err)}) - } + engErrs = append(engErrs, extractEngineErrors(err)...) + zap.S().Errorf("Engine returned error: %v", err) } if len(context.Solutions) > 0 { diff --git a/pkg/engine2/edge_targets.go b/pkg/engine2/edge_targets.go index 94f748e9d..451cd35e3 100644 --- a/pkg/engine2/edge_targets.go +++ b/pkg/engine2/edge_targets.go @@ -116,7 +116,7 @@ func (e *Engine) EdgeCanBeExpanded(ctx *solutionContext, source construct.Resour } _, err = edgeExpander.ExpandEdge(path_selection.ExpansionInput{ - Dep: construct.ResourceEdge{ + SatisfactionEdge: construct.ResourceEdge{ Source: tempSourceResource, Target: tempTargetResource, }, diff --git a/pkg/engine2/engine_test.go b/pkg/engine2/engine_test.go index 6123e7195..4abdd8dfa 100644 --- a/pkg/engine2/engine_test.go +++ b/pkg/engine2/engine_test.go @@ -54,17 +54,8 @@ func (tc engineTestCase) Test(t *testing.T) { t.Fatal(fmt.Errorf("failed to open input file: %w", err)) } defer inputYaml.Close() - expectYaml, err := os.Open(strings.Replace(tc.inputPath, ".input.yaml", ".expect.yaml", 1)) - if err != nil { - t.Fatal(fmt.Errorf("failed to open expected output file: %w", err)) - } - defer expectYaml.Close() inputFile := tc.readGraph(t, inputYaml) - expectContent, err := io.ReadAll(expectYaml) - if err != nil { - t.Fatal(fmt.Errorf("failed to read expected output file: %w", err)) - } main := EngineMain{} err = main.AddEngine() @@ -75,7 +66,7 @@ func (tc engineTestCase) Test(t *testing.T) { Constraints: inputFile.Constraints, InitialState: inputFile.Graph, } - _, engineErrs := main.Run(context) + returnCode, engineErrs := main.Run(context) // TODO find a convenient way to specify the return code in the testdata errDetails := new(bytes.Buffer) @@ -92,12 +83,25 @@ func (tc engineTestCase) Test(t *testing.T) { } assertErrDetails(t, errDetailsFile, engineErrs) + if returnCode == 1 { + // Run resulted in a failure. After checking the error details, we're done. + return + } sol := context.Solutions[0] actualContent, err := yaml.Marshal(construct.YamlGraph{Graph: sol.DataflowGraph()}) if err != nil { t.Fatal(fmt.Errorf("failed to marshal actual output: %w", err)) } + expectYaml, err := os.Open(strings.Replace(tc.inputPath, ".input.yaml", ".expect.yaml", 1)) + if err != nil { + t.Fatal(fmt.Errorf("failed to open expected output file: %w", err)) + } + defer expectYaml.Close() + expectContent, err := io.ReadAll(expectYaml) + if err != nil { + t.Fatal(fmt.Errorf("failed to read expected output file: %w", err)) + } 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 diff --git a/pkg/engine2/errors/error_tree.go b/pkg/engine2/errors/error_tree.go new file mode 100644 index 000000000..8d89a977e --- /dev/null +++ b/pkg/engine2/errors/error_tree.go @@ -0,0 +1,95 @@ +package engine_errs + +import ( + "fmt" + "strings" +) + +type ( + ErrorTree struct { + Chain []string `json:"chain,omitempty"` + Children []ErrorTree `json:"children,omitempty"` + } + + chainErr interface { + error + Unwrap() error + } + joinErr interface { + error + Unwrap() []error + } +) + +func unwrapChain(err error) (chain []string, last joinErr) { + for current := err; current != nil; { + var next error + cc, ok := current.(chainErr) + if ok { + next = cc.Unwrap() + } else { + joined, ok := current.(joinErr) + if ok { + jerrs := joined.Unwrap() + if len(jerrs) == 1 { + next = jerrs[0] + } else { + last = joined + return + } + } else { + chain = append(chain, current.Error()) + return + } + } + msg := strings.TrimSuffix(strings.TrimSuffix(current.Error(), next.Error()), ": ") + if msg != "" { + chain = append(chain, msg) + } + current = next + } + return +} + +func ErrorsToTree(err error) (tree ErrorTree) { + if err == nil { + return + } + if t, ok := err.(ErrorTree); ok { + return t + } + + var joined joinErr + tree.Chain, joined = unwrapChain(err) + + if joined != nil { + errs := joined.Unwrap() + tree.Children = make([]ErrorTree, len(errs)) + for i, e := range errs { + tree.Children[i] = ErrorsToTree(e) + } + } + return +} + +func (t ErrorTree) Error() string { + sb := &strings.Builder{} + t.print(sb, 0, 0) + return sb.String() +} + +func (t ErrorTree) print(out *strings.Builder, indent int, childChar rune) { + prefix := strings.Repeat("\t", indent) + delim := "" + if childChar != 0 { + delim = string(childChar) + " " + } + fmt.Fprintf(out, "%s%s%v\n", prefix, delim, t.Chain) + for i, child := range t.Children { + char := '├' + if i == len(t.Children)-1 { + char = '└' + } + child.print(out, indent+1, char) + } +} diff --git a/pkg/engine2/errors/errors.go b/pkg/engine2/errors/errors.go index 6654dec05..ac9a75907 100644 --- a/pkg/engine2/errors/errors.go +++ b/pkg/engine2/errors/errors.go @@ -2,7 +2,8 @@ package engine_errs import ( "fmt" - "strings" + + construct "github.com/klothoplatform/klotho/pkg/construct2" ) type ( @@ -16,15 +17,6 @@ type ( } ErrorCode string - - InternalError struct { - Err error - } - - ErrorTree struct { - Chain []string `json:"chain,omitempty"` - Children []ErrorTree `json:"children,omitempty"` - } ) const ( @@ -34,6 +26,10 @@ const ( EdgeUnsupportedCode ErrorCode = "edge_unsupported" ) +type InternalError struct { + Err error +} + func (e InternalError) Error() string { return fmt.Sprintf("internal error: %v", e.Err) } @@ -50,86 +46,76 @@ func (e InternalError) Unwrap() error { return e.Err } -type ( - chainErr interface { - error - Unwrap() error - } - joinErr interface { - error - Unwrap() []error - } -) +type UnsupportedExpansionErr struct { + // ExpandEdge is the overall edge that is being expanded + ExpandEdge construct.SimpleEdge + // SatisfactionEdge is the specific edge that was being expanded when the error occurred + SatisfactionEdge construct.SimpleEdge + Classification string +} -func unwrapChain(err error) (chain []string, last joinErr) { - for current := err; current != nil; { - var next error - cc, ok := current.(chainErr) - if ok { - next = cc.Unwrap() - } else { - joined, ok := current.(joinErr) - if ok { - jerrs := joined.Unwrap() - if len(jerrs) == 1 { - next = jerrs[0] - } else { - last = joined - return - } - } else { - chain = append(chain, current.Error()) - return - } - } - msg := strings.TrimSuffix(strings.TrimSuffix(current.Error(), next.Error()), ": ") - if msg != "" { - chain = append(chain, msg) - } - current = next +func (e UnsupportedExpansionErr) Error() string { + if e.SatisfactionEdge.Source.IsZero() || e.ExpandEdge == e.SatisfactionEdge { + return fmt.Sprintf("unsupported expansion %s in %s", e.ExpandEdge, e.Classification) } - return + return fmt.Sprintf( + "while expanding %s, unsupported expansion of %s in %s", + e.ExpandEdge, + e.Classification, + e.SatisfactionEdge, + ) +} + +func (e UnsupportedExpansionErr) ErrorCode() ErrorCode { + return EdgeUnsupportedCode } -func ErrorsToTree(err error) (tree ErrorTree) { - if err == nil { - return +func (e UnsupportedExpansionErr) ToJSONMap() map[string]any { + m := map[string]any{ + "satisfaction_edge": e.SatisfactionEdge, + } + if !e.ExpandEdge.Source.IsZero() { + m["expand_edge"] = e.ExpandEdge } - if t, ok := err.(ErrorTree); ok { - return t + if e.Classification != "" { + m["classification"] = e.Classification } + return m +} - var joined joinErr - tree.Chain, joined = unwrapChain(err) +type InvalidPathErr struct { + // ExpandEdge is the overall edge that is being expanded + ExpandEdge construct.SimpleEdge + // SatisfactionEdge is the specific edge that was being expanded when the error occurred + SatisfactionEdge construct.SimpleEdge + Classification string +} - if joined != nil { - errs := joined.Unwrap() - tree.Children = make([]ErrorTree, len(errs)) - for i, e := range errs { - tree.Children[i] = ErrorsToTree(e) - } +func (e InvalidPathErr) Error() string { + if e.SatisfactionEdge.Source.IsZero() || e.ExpandEdge == e.SatisfactionEdge { + return fmt.Sprintf("invalid expansion %s in %s", e.ExpandEdge, e.Classification) } - return + return fmt.Sprintf( + "while expanding %s, invalid expansion of %s in %s", + e.ExpandEdge, + e.Classification, + e.SatisfactionEdge, + ) } -func (t ErrorTree) Error() string { - sb := &strings.Builder{} - t.print(sb, 0, 0) - return sb.String() +func (e InvalidPathErr) ErrorCode() ErrorCode { + return EdgeInvalidCode } -func (t ErrorTree) print(out *strings.Builder, indent int, childChar rune) { - prefix := strings.Repeat("\t", indent) - delim := "" - if childChar != 0 { - delim = string(childChar) + " " +func (e InvalidPathErr) ToJSONMap() map[string]any { + m := map[string]any{ + "satisfaction_edge": e.SatisfactionEdge, + } + if !e.ExpandEdge.Source.IsZero() { + m["expand_edge"] = e.ExpandEdge } - fmt.Fprintf(out, "%s%s%v\n", prefix, delim, t.Chain) - for i, child := range t.Children { - char := '├' - if i == len(t.Children)-1 { - char = '└' - } - child.print(out, indent+1, char) + if e.Classification != "" { + m["classification"] = e.Classification } + return m } diff --git a/pkg/engine2/operational_eval/evaluator.go b/pkg/engine2/operational_eval/evaluator.go index 25130055b..3ae1a9c60 100644 --- a/pkg/engine2/operational_eval/evaluator.go +++ b/pkg/engine2/operational_eval/evaluator.go @@ -474,7 +474,7 @@ func (eval *Evaluator) UpdateId(oldId, newId construct.ResourceId) error { } case *pathExpandVertex: if key.Edge.Source == oldId || key.Edge.Target == oldId { - vertex.Edge = UpdateEdgeId(vertex.Edge, oldId, newId) + vertex.SatisfactionEdge = UpdateEdgeId(vertex.SatisfactionEdge, oldId, newId) replaceVertex(key, vertex) // because the temp graph contains the src and target as nodes, we need to update it if it exists } diff --git a/pkg/engine2/operational_eval/graph.go b/pkg/engine2/operational_eval/graph.go index 4a37e4f44..17ff1ff86 100644 --- a/pkg/engine2/operational_eval/graph.go +++ b/pkg/engine2/operational_eval/graph.go @@ -106,7 +106,7 @@ func (eval *Evaluator) pathVertices(source, target construct.ResourceId) (graphC return fmt.Errorf("could not build temp graph for %s: %w", edge, err) } } - vertex := &pathExpandVertex{Edge: edge, Satisfication: satisfication, TempGraph: tempGraph} + vertex := &pathExpandVertex{SatisfactionEdge: edge, Satisfication: satisfication, TempGraph: tempGraph} return changes.AddVertexAndDeps(eval, vertex) } diff --git a/pkg/engine2/operational_eval/vertex_path_expand.go b/pkg/engine2/operational_eval/vertex_path_expand.go index b47462714..868a7e2d2 100644 --- a/pkg/engine2/operational_eval/vertex_path_expand.go +++ b/pkg/engine2/operational_eval/vertex_path_expand.go @@ -20,7 +20,11 @@ import ( type ( pathExpandVertex struct { - Edge construct.SimpleEdge + // ExpandEdge is the overall edge that is being expanded + ExpandEdge construct.SimpleEdge + // SatisfactionEdge is the specific edge that was being expanded when the error occurred + SatisfactionEdge construct.SimpleEdge + TempGraph construct.Graph Satisfication knowledgebase.EdgePathSatisfaction } @@ -39,7 +43,7 @@ type ( ) func (v *pathExpandVertex) Key() Key { - return Key{PathSatisfication: v.Satisfication, Edge: v.Edge} + return Key{PathSatisfication: v.Satisfication, Edge: v.SatisfactionEdge} } func (v *pathExpandVertex) Evaluate(eval *Evaluator) error { @@ -56,15 +60,15 @@ func (v *pathExpandVertex) runEvaluation(eval *Evaluator, runner expansionRunner } log := eval.Log() if len(expansions) > 1 && log.Desugar().Core().Enabled(zap.DebugLevel) { - log.Debugf("Expansion %s subexpansions:", v.Edge) + log.Debugf("Expansion %s subexpansions:", v.SatisfactionEdge) for _, expansion := range expansions { - log.Debugf(" %s -> %s", expansion.Dep.Source.ID, expansion.Dep.Target.ID) + log.Debugf(" %s -> %s", expansion.SatisfactionEdge.Source.ID, expansion.SatisfactionEdge.Target.ID) } } createExpansionErr := func(err error) error { return fmt.Errorf("could not run expansion %s -> %s <%s>: %w", - v.Edge.Source, v.Edge.Target, v.Satisfication.Classification, err, + v.SatisfactionEdge.Source, v.SatisfactionEdge.Target, v.Satisfication.Classification, err, ) } @@ -75,15 +79,15 @@ func (v *pathExpandVertex) runEvaluation(eval *Evaluator, runner expansionRunner continue } - resultStr, err := expansionResultString(result.Graph, expansion.Dep) + resultStr, err := expansionResultString(result.Graph, expansion.SatisfactionEdge) if err != nil { errs = errors.Join(errs, createExpansionErr(err)) continue } if v.Satisfication.Classification != "" { - eval.Log().Infof("Satisfied %s for %s through %s", v.Satisfication.Classification, v.Edge, resultStr) + eval.Log().Infof("Satisfied %s for %s through %s", v.Satisfication.Classification, v.SatisfactionEdge, resultStr) } else { - eval.Log().Infof("Satisfied %s -> %s through %s", v.Edge.Source, v.Edge.Target, resultStr) + eval.Log().Infof("Satisfied %s -> %s through %s", v.SatisfactionEdge.Source, v.SatisfactionEdge.Target, resultStr) } if err := runner.addResourcesAndEdges(result, expansion, v); err != nil { @@ -144,7 +148,7 @@ func (v *pathExpandVertex) addDepsFromProps( ref := construct.PropertyRef{Resource: res, Property: k} for _, dep := range dependencies { - if dep == v.Edge.Source || dep == v.Edge.Target { + if dep == v.SatisfactionEdge.Source || dep == v.SatisfactionEdge.Target { continue } resource, err := eval.Solution.RawView().Vertex(res) @@ -282,8 +286,8 @@ func (v *pathExpandVertex) Dependencies(eval *Evaluator, propCtx dependencyCaptu srcKey := v.Key() - changes.addEdges(srcKey, getDepsForPropertyRef(eval.Solution, v.Edge.Source, v.Satisfication.Source.PropertyReference)) - changes.addEdges(srcKey, getDepsForPropertyRef(eval.Solution, v.Edge.Target, v.Satisfication.Target.PropertyReference)) + changes.addEdges(srcKey, getDepsForPropertyRef(eval.Solution, v.SatisfactionEdge.Source, v.Satisfication.Source.PropertyReference)) + changes.addEdges(srcKey, getDepsForPropertyRef(eval.Solution, v.SatisfactionEdge.Target, v.Satisfication.Target.PropertyReference)) // if we have a temp graph we can analyze the paths in it for possible dependencies on property vertices // if we dont, we should return what we currently have @@ -294,17 +298,17 @@ func (v *pathExpandVertex) Dependencies(eval *Evaluator, propCtx dependencyCaptu } var errs error - srcDeps, err := construct.AllDownstreamDependencies(v.TempGraph, v.Edge.Source) + srcDeps, err := construct.AllDownstreamDependencies(v.TempGraph, v.SatisfactionEdge.Source) if err != nil { return err } - errs = errors.Join(errs, v.addDepsFromProps(eval, changes, v.Edge.Source, srcDeps)) + errs = errors.Join(errs, v.addDepsFromProps(eval, changes, v.SatisfactionEdge.Source, srcDeps)) - targetDeps, err := construct.AllUpstreamDependencies(v.TempGraph, v.Edge.Target) + targetDeps, err := construct.AllUpstreamDependencies(v.TempGraph, v.SatisfactionEdge.Target) if err != nil { return err } - errs = errors.Join(errs, v.addDepsFromProps(eval, changes, v.Edge.Target, targetDeps)) + errs = errors.Join(errs, v.addDepsFromProps(eval, changes, v.SatisfactionEdge.Target, targetDeps)) if errs != nil { return errs } @@ -323,13 +327,13 @@ func (v *pathExpandVertex) Dependencies(eval *Evaluator, propCtx dependencyCaptu func (runner *pathExpandVertexRunner) getExpansionsToRun(v *pathExpandVertex) ([]path_selection.ExpansionInput, error) { eval := runner.Eval var errs error - sourceRes, err := eval.Solution.RawView().Vertex(v.Edge.Source) + sourceRes, err := eval.Solution.RawView().Vertex(v.SatisfactionEdge.Source) if err != nil { - return nil, fmt.Errorf("could not find source resource %s: %w", v.Edge.Source, err) + return nil, fmt.Errorf("could not find source resource %s: %w", v.SatisfactionEdge.Source, err) } - targetRes, err := eval.Solution.RawView().Vertex(v.Edge.Target) + targetRes, err := eval.Solution.RawView().Vertex(v.SatisfactionEdge.Target) if err != nil { - return nil, fmt.Errorf("could not find target resource %s: %w", v.Edge.Target, err) + return nil, fmt.Errorf("could not find target resource %s: %w", v.SatisfactionEdge.Target, err) } edge := construct.ResourceEdge{Source: sourceRes, Target: targetRes} expansions, err := path_selection.DeterminePathSatisfactionInputs(eval.Solution, v.Satisfication, edge) @@ -340,12 +344,13 @@ func (runner *pathExpandVertexRunner) getExpansionsToRun(v *pathExpandVertex) ([ result := make([]path_selection.ExpansionInput, len(expansions)) for i, expansion := range expansions { input := path_selection.ExpansionInput{ - Dep: expansion.Dep, - Classification: expansion.Classification, - TempGraph: v.TempGraph, + ExpandEdge: v.ExpandEdge, + SatisfactionEdge: expansion.SatisfactionEdge, + Classification: expansion.Classification, + TempGraph: v.TempGraph, } - if expansion.Dep.Source != edge.Source || expansion.Dep.Target != edge.Target { - simple := construct.SimpleEdge{Source: expansion.Dep.Source.ID, Target: expansion.Dep.Target.ID} + if expansion.SatisfactionEdge.Source != edge.Source || expansion.SatisfactionEdge.Target != edge.Target { + simple := construct.SimpleEdge{Source: expansion.SatisfactionEdge.Source.ID, Target: expansion.SatisfactionEdge.Target.ID} tempGraph, err := path_selection.BuildPathSelectionGraph(simple, eval.Solution.KnowledgeBase(), expansion.Classification) if err != nil { errs = errors.Join(errs, fmt.Errorf("error getting expansions to run. could not build path selection graph: %w", err)) @@ -369,21 +374,21 @@ func (runner *pathExpandVertexRunner) addResourcesAndEdges( return err } if len(adj) > 2 { - _, err := eval.Solution.OperationalView().Edge(v.Edge.Source, v.Edge.Target) + _, err := eval.Solution.OperationalView().Edge(v.SatisfactionEdge.Source, v.SatisfactionEdge.Target) if err == nil { - if err := eval.Solution.OperationalView().RemoveEdge(v.Edge.Source, v.Edge.Target); err != nil { + if err := eval.Solution.OperationalView().RemoveEdge(v.SatisfactionEdge.Source, v.SatisfactionEdge.Target); err != nil { return err } } else if !errors.Is(err, graph.ErrEdgeNotFound) { return err } } else if len(adj) == 2 { - err = eval.Solution.RawView().AddEdge(expansion.Dep.Source.ID, expansion.Dep.Target.ID) + err = eval.Solution.RawView().AddEdge(expansion.SatisfactionEdge.Source.ID, expansion.SatisfactionEdge.Target.ID) if err != nil { return err } return eval.Solution.OperationalView().MakeEdgesOperational([]construct.Edge{ - {Source: expansion.Dep.Source.ID, Target: expansion.Dep.Target.ID}, + {Source: expansion.SatisfactionEdge.Source.ID, Target: expansion.SatisfactionEdge.Target.ID}, }) } @@ -450,9 +455,9 @@ func (runner *pathExpandVertexRunner) addSubExpansion( 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, - Satisfication: satisfication, + SatisfactionEdge: construct.SimpleEdge{Source: subExpand.Source, Target: subExpand.Target}, + TempGraph: expansion.TempGraph, + Satisfication: satisfication, }) } } @@ -462,8 +467,8 @@ func (runner *pathExpandVertexRunner) addSubExpansion( func (runner *pathExpandVertexRunner) consumeExpansionProperties(expansion path_selection.ExpansionInput) error { delays, err := knowledgebase.ConsumeFromResource( - expansion.Dep.Source, - expansion.Dep.Target, + expansion.SatisfactionEdge.Source, + expansion.SatisfactionEdge.Target, solution_context.DynamicCtx(runner.Eval.Solution), ) if err != nil { diff --git a/pkg/engine2/operational_eval/vertex_path_expand_test.go b/pkg/engine2/operational_eval/vertex_path_expand_test.go index 6e15f6f57..2d8e0fb4f 100644 --- a/pkg/engine2/operational_eval/vertex_path_expand_test.go +++ b/pkg/engine2/operational_eval/vertex_path_expand_test.go @@ -16,7 +16,7 @@ import ( func Test_pathExpandVertex_Key(t *testing.T) { assert := assert.New(t) v := &pathExpandVertex{ - Edge: construct.SimpleEdge{ + SatisfactionEdge: construct.SimpleEdge{ Source: construct.ResourceId{Name: "test"}, Target: construct.ResourceId{Name: "test"}, }, @@ -59,7 +59,7 @@ func Test_pathExpandVertex_runEvaluation(t *testing.T) { { name: "run evaluation", v: &pathExpandVertex{ - Edge: construct.SimpleEdge{ + SatisfactionEdge: construct.SimpleEdge{ Source: construct.ResourceId{Name: "s"}, Target: construct.ResourceId{Name: "t"}, }, @@ -69,7 +69,7 @@ func Test_pathExpandVertex_runEvaluation(t *testing.T) { }, mocks: func(mr *MockexpansionRunner, me *MockEdgeExpander, v *pathExpandVertex) { input := path_selection.ExpansionInput{ - Dep: construct.ResourceEdge{ + SatisfactionEdge: construct.ResourceEdge{ Source: sResource, Target: tResource, }, @@ -123,7 +123,7 @@ func Test_pathExpandVertex_addDepsFromProps(t *testing.T) { { name: "add deps from props", v: &pathExpandVertex{ - Edge: construct.SimpleEdge{ + SatisfactionEdge: construct.SimpleEdge{ Source: construct.ResourceId{Name: "s"}, Target: construct.ResourceId{Name: "t"}, }, @@ -210,7 +210,7 @@ func Test_pathExpandVertex_addDepsFromEdge(t *testing.T) { { name: "add deps from props", v: &pathExpandVertex{ - Edge: construct.SimpleEdge{ + SatisfactionEdge: construct.SimpleEdge{ Source: construct.ResourceId{Name: "s"}, Target: construct.ResourceId{Name: "t"}, }, @@ -419,7 +419,7 @@ func Test_pathExpandVertex_Dependencies(t *testing.T) { { name: "no temp graph", v: &pathExpandVertex{ - Edge: construct.SimpleEdge{ + SatisfactionEdge: construct.SimpleEdge{ Source: construct.ResourceId{Name: "s"}, Target: construct.ResourceId{Name: "t"}, }, diff --git a/pkg/engine2/path_selection/debug.go b/pkg/engine2/path_selection/debug.go index 3675d1671..4339dbbf9 100644 --- a/pkg/engine2/path_selection/debug.go +++ b/pkg/engine2/path_selection/debug.go @@ -33,7 +33,7 @@ func writeGraph(input ExpansionInput, working, result construct.Graph) { return } - fprefix := fmt.Sprintf("%s-%s", input.Dep.Source.ID, input.Dep.Target.ID) + fprefix := fmt.Sprintf("%s-%s", input.SatisfactionEdge.Source.ID, input.SatisfactionEdge.Target.ID) fprefix = strings.ReplaceAll(fprefix, ":", "_") // some filesystems (NTFS) don't like colons in filenames fprefix = filepath.Join(dir, fprefix) @@ -74,7 +74,7 @@ func writeGraph(input ExpansionInput, working, result construct.Graph) { rankdir = LR labelloc = t graph [ranksep = 2] -`, input.Dep.Source.ID, input.Dep.Target.ID) +`, input.SatisfactionEdge.Source.ID, input.SatisfactionEdge.Target.ID) } err = graphToDOTCluster(input.Classification, working, result, dotContent) diff --git a/pkg/engine2/path_selection/path_expansion.go b/pkg/engine2/path_selection/path_expansion.go index 3d5094ce9..3796dc94d 100644 --- a/pkg/engine2/path_selection/path_expansion.go +++ b/pkg/engine2/path_selection/path_expansion.go @@ -8,6 +8,7 @@ import ( "github.com/dominikbraun/graph" construct "github.com/klothoplatform/klotho/pkg/construct2" + engine_errs "github.com/klothoplatform/klotho/pkg/engine2/errors" "github.com/klothoplatform/klotho/pkg/engine2/solution_context" knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2" "github.com/klothoplatform/klotho/pkg/set" @@ -18,9 +19,10 @@ import ( type ( ExpansionInput struct { - Dep construct.ResourceEdge - Classification string - TempGraph construct.Graph + ExpandEdge construct.SimpleEdge + SatisfactionEdge construct.ResourceEdge + Classification string + TempGraph construct.Graph } ExpansionResult struct { Edges []graph.Edge[construct.ResourceId] @@ -41,7 +43,7 @@ func (e *EdgeExpand) ExpandEdge( ) (ExpansionResult, error) { ctx := e.Ctx tempGraph := input.TempGraph - dep := input.Dep + dep := input.SatisfactionEdge result := ExpansionResult{ Graph: construct.NewGraph(), @@ -69,10 +71,20 @@ func expandEdge( input ExpansionInput, g construct.Graph, ) ([]graph.Edge[construct.ResourceId], error) { - paths, err := graph.AllPathsBetween(input.TempGraph, input.Dep.Source.ID, input.Dep.Target.ID) + paths, err := graph.AllPathsBetween(input.TempGraph, input.SatisfactionEdge.Source.ID, input.SatisfactionEdge.Target.ID) if err != nil { return nil, err } + if len(paths) == 0 { + return nil, engine_errs.UnsupportedExpansionErr{ + ExpandEdge: input.ExpandEdge, + SatisfactionEdge: construct.SimpleEdge{ + Source: input.SatisfactionEdge.Source.ID, + Target: input.SatisfactionEdge.Target.ID, + }, + Classification: input.Classification, + } + } sort.Slice(paths, func(i, j int) bool { il, jl := len(paths[i]), len(paths[j]) if il != jl { @@ -86,10 +98,16 @@ func expandEdge( } return false }) + + undirected, err := BuildUndirectedGraph(ctx.RawView(), ctx.KnowledgeBase()) + if err != nil { + return nil, err + } + var errs error // represents id to qualified type because we dont need to do that processing more than once for _, path := range paths { - err := expandPath(ctx, input, path, g) + err := expandPath(ctx, undirected, input, path, g) if err != nil { errs = errors.Join(errs, fmt.Errorf("error expanding path %s: %w", construct.Path(path), err)) } @@ -100,14 +118,21 @@ func expandEdge( path, err := graph.ShortestPathStable( input.TempGraph, - input.Dep.Source.ID, - input.Dep.Target.ID, + input.SatisfactionEdge.Source.ID, + input.SatisfactionEdge.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), - ) + // NOTE(gg) this can't happen with the current expandPath implementation + // but may in the future. + return nil, engine_errs.InvalidPathErr{ + ExpandEdge: input.ExpandEdge, + SatisfactionEdge: construct.SimpleEdge{ + Source: input.SatisfactionEdge.Source.ID, + Target: input.SatisfactionEdge.Target.ID, + }, + Classification: input.Classification, + } } resultResources, err := renameAndReplaceInTempGraph(ctx, input, g, path) @@ -123,7 +148,7 @@ func renameAndReplaceInTempGraph( path construct.Path, ) ([]*construct.Resource, error) { var errs error - name := fmt.Sprintf("%s-%s", input.Dep.Source.ID.Name, input.Dep.Target.ID.Name) + name := fmt.Sprintf("%s-%s", input.SatisfactionEdge.Source.ID.Name, input.SatisfactionEdge.Target.ID.Name) // rename phantom nodes result := make([]*construct.Resource, len(path)) for i, id := range path { @@ -274,8 +299,11 @@ func findSubExpansionsToRun( // ExpandEdge takes a given `selectedPath` and resolves it to a path of resourceIds that can be used // for creating resources, or existing resources. +// 'undirected' is the undirected graph of the dataflow graph from 'ctx' but are a separate input to reuse +// the calculated graph for performance. func expandPath( ctx solution_context.SolutionContext, + undirected construct.Graph, input ExpansionInput, path construct.Path, resultGraph construct.Graph, @@ -324,17 +352,12 @@ func expandPath( return errs } - undirected, err := BuildUndirectedGraph(ctx.RawView(), ctx.KnowledgeBase()) - if err != nil { - return err - } - addCandidates := func(id construct.ResourceId, resource *construct.Resource, nerr error) error { matchIdx := matchesNonBoundary(id, nonBoundaryResources) if matchIdx < 0 { return nil } - valid, err := checkNamespaceValidity(ctx, resource, input.Dep.Target.ID) + valid, err := checkNamespaceValidity(ctx, resource, input.SatisfactionEdge.Target.ID) if err != nil { return errors.Join(nerr, fmt.Errorf("error checking namespace validity of %s: %w", resource.ID, err)) } @@ -350,7 +373,7 @@ func expandPath( if _, ok := candidates[matchIdx][id]; !ok { candidates[matchIdx][id] = 0 } - weight, err := determineCandidateWeight(ctx, input.Dep.Source.ID, input.Dep.Target.ID, id, resultGraph, undirected) + weight, err := determineCandidateWeight(ctx, input.SatisfactionEdge.Source.ID, input.SatisfactionEdge.Target.ID, id, resultGraph, undirected) if err != nil { return errors.Join(nerr, err) } @@ -368,7 +391,7 @@ func expandPath( } // We need to add candidates which exist in our current result graph so we can reuse them. We do this in case // we have already performed expansions to ensure the namespaces are connected, etc - err = construct.WalkGraph(resultGraph, func(id construct.ResourceId, resource *construct.Resource, nerr error) error { + err := construct.WalkGraph(resultGraph, func(id construct.ResourceId, resource *construct.Resource, nerr error) error { return addCandidates(id, resource, nerr) }) if err != nil { @@ -397,7 +420,7 @@ func expandPath( // 3. Otherwise, add it addEdge := func(source, target candidate) { weight := calculateEdgeWeight( - construct.SimpleEdge{Source: input.Dep.Source.ID, Target: input.Dep.Target.ID}, + construct.SimpleEdge{Source: input.SatisfactionEdge.Source.ID, Target: input.SatisfactionEdge.Target.ID}, source.id, target.id, source.divideWeightBy, target.divideWeightBy, input.Classification, @@ -444,7 +467,7 @@ func expandPath( for i, resCandidates := range candidates { for id, weight := range resCandidates { if i == 0 { - addEdge(candidate{id: input.Dep.Source.ID}, candidate{id: id, divideWeightBy: weight}) + addEdge(candidate{id: input.SatisfactionEdge.Source.ID}, candidate{id: id, divideWeightBy: weight}) continue } @@ -458,7 +481,7 @@ func expandPath( } if len(candidates) > 0 { for c, weight := range candidates[len(candidates)-1] { - addEdge(candidate{id: c, divideWeightBy: weight}, candidate{id: input.Dep.Target.ID}) + addEdge(candidate{id: c, divideWeightBy: weight}, candidate{id: input.SatisfactionEdge.Target.ID}) } } if errs != nil { @@ -506,9 +529,9 @@ func connectThroughNamespace(src, target *construct.Resource, ctx solution_conte continue } input := ExpansionInput{ - Dep: construct.ResourceEdge{Source: down, Target: target}, - Classification: "", - TempGraph: tg, + SatisfactionEdge: construct.ResourceEdge{Source: down, Target: target}, + Classification: "", + TempGraph: tg, } edges, err := expandEdge(ctx, input, result.Graph) if err != nil { diff --git a/pkg/engine2/path_selection/path_selection.go b/pkg/engine2/path_selection/path_selection.go index 4bc4eb34d..cb00787e3 100644 --- a/pkg/engine2/path_selection/path_selection.go +++ b/pkg/engine2/path_selection/path_selection.go @@ -27,10 +27,7 @@ func BuildPathSelectionGraph( // Check to see if there is a direct edge which satisfies the classification and if so short circuit in building the temp graph if et := kb.GetEdgeTemplate(dep.Source, dep.Target); et != nil && dep.Source.Namespace == dep.Target.Namespace { - directEdgeSatisfies := false - if collectionutil.Contains(et.Classification, classification) { - directEdgeSatisfies = true - } + directEdgeSatisfies := collectionutil.Contains(et.Classification, classification) if !directEdgeSatisfies { srcRt, err := kb.GetResourceTemplate(dep.Source) @@ -41,9 +38,8 @@ func BuildPathSelectionGraph( if err != nil { return nil, err } - if collectionutil.Contains(srcRt.Classification.Is, classification) || collectionutil.Contains(dst.Classification.Is, classification) { - directEdgeSatisfies = true - } + directEdgeSatisfies = collectionutil.Contains(srcRt.Classification.Is, classification) || + collectionutil.Contains(dst.Classification.Is, classification) } if directEdgeSatisfies { diff --git a/pkg/engine2/path_selection/paths.go b/pkg/engine2/path_selection/paths.go index 792b396ce..1a56f7a83 100644 --- a/pkg/engine2/path_selection/paths.go +++ b/pkg/engine2/path_selection/paths.go @@ -38,11 +38,11 @@ func GetPaths( return result, err } for _, expansion := range expansions { - simple := construct.SimpleEdge{Source: expansion.Dep.Source.ID, Target: expansion.Dep.Target.ID} + simple := construct.SimpleEdge{Source: expansion.SatisfactionEdge.Source.ID, Target: expansion.SatisfactionEdge.Target.ID} paths, found := pathsCache[simple] if !found { var err error - paths, err = graph.AllPathsBetween(sol.RawView(), expansion.Dep.Source.ID, expansion.Dep.Target.ID) + paths, err = graph.AllPathsBetween(sol.RawView(), expansion.SatisfactionEdge.Source.ID, expansion.SatisfactionEdge.Target.ID) if err != nil { errs = errors.Join(errs, err) continue @@ -149,8 +149,8 @@ func DeterminePathSatisfactionInputs( e := construct.ResourceEdge{Source: src, Target: target} exp := ExpansionInput{ - Dep: e, - Classification: satisfaction.Classification, + SatisfactionEdge: e, + Classification: satisfaction.Classification, } expansions = append(expansions, exp) } diff --git a/pkg/engine2/testdata/unsupported_edge.err.json b/pkg/engine2/testdata/unsupported_edge.err.json new file mode 100644 index 000000000..58fc05511 --- /dev/null +++ b/pkg/engine2/testdata/unsupported_edge.err.json @@ -0,0 +1,26 @@ +[ + { + "classification": "permissions", + "error_code": "edge_unsupported", + "satisfaction_edge": { + "Source": "aws:cloudfront_distribution:cloudfront_distribution_2", + "Target": "aws:rds_instance:mydb" + } + }, + { + "classification": "network", + "error_code": "edge_unsupported", + "satisfaction_edge": { + "Source": "aws:cloudfront_distribution:cloudfront_distribution_2", + "Target": "aws:rds_instance:mydb" + } + }, + { + "classification": "cloudfront_origin", + "error_code": "edge_unsupported", + "satisfaction_edge": { + "Source": "aws:cloudfront_distribution:cloudfront_distribution_2", + "Target": "aws:rds_instance:mydb" + } + } +] diff --git a/pkg/engine2/testdata/unsupported_edge.input.yaml b/pkg/engine2/testdata/unsupported_edge.input.yaml new file mode 100644 index 000000000..265f2c0a1 --- /dev/null +++ b/pkg/engine2/testdata/unsupported_edge.input.yaml @@ -0,0 +1,12 @@ +constraints: + - node: aws:cloudfront_distribution:cloudfront_distribution_2 + operator: add + scope: application + - node: aws:rds_instance:mydb + operator: add + scope: application + - operator: must_exist + scope: edge + target: + source: aws:cloudfront_distribution:cloudfront_distribution_2 + target: aws:rds_instance:mydb