klothoplatform · jhsinger-klotho · Apr 12, 2024 · Apr 9, 2024 · Apr 10, 2024 · Apr 10, 2024
diff --git a/.github/workflows/govulncheck.yaml b/.github/workflows/govulncheck.yaml
@@ -13,7 +13,7 @@ jobs:
     steps:
       - uses: actions/setup-go@v3
         with:
-          go-version: '1.22.1'
+          go-version: '1.22.2'
       - uses: actions/cache@v2
         with:
           path: |

diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
@@ -13,7 +13,7 @@ jobs:
     steps:
       - uses: actions/setup-go@v3
         with:
-          go-version: '1.21.5'
+          go-version: '1.22.2'
       - uses: actions/checkout@v3
       - uses: actions/cache@v2
         with:

diff --git a/.github/workflows/prettier.yaml b/.github/workflows/prettier.yaml
@@ -13,7 +13,7 @@ jobs:
     steps:
       - uses: actions/setup-go@v3
         with:
-          go-version: '1.21.5'
+          go-version: '1.22.2'
       - uses: actions/checkout@v3
       - name: List files to check
         run: |

diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml
@@ -25,7 +25,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@v3
         with:
-          go-version: '1.21.5'
+          go-version: '1.22.2'
 
       - name: Setup Zig
         uses: goto-bus-stop/setup-zig@v1

diff --git a/.github/workflows/run-integ-tests.yaml b/.github/workflows/run-integ-tests.yaml
@@ -97,7 +97,7 @@ jobs:
         uses: actions/checkout@v3
       - uses: actions/setup-go@v3
         with:
-          go-version: '1.21.5'
+          go-version: '1.22.2'
       - name: pre-build
         if: ${{ inputs.pre-build-script }}
         run: ${{ inputs.pre-build-script }}

diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
@@ -15,7 +15,7 @@ jobs:
     steps:
       - uses: actions/setup-go@v3
         with:
-          go-version: '1.21.5'
+          go-version: '1.22.2'
       - uses: actions/checkout@v3
       - uses: actions/cache@v2
         with:

diff --git a/cmd/kb/main.go b/cmd/kb/main.go
@@ -73,7 +73,7 @@ func (args Args) Run(ctx *kong.Context) error {
 				return fmt.Errorf("could not parse target: %w", err)
 			}
 			edge.Target.Name = "target"
-			g, err := path_selection.BuildPathSelectionGraph(edge, kb, args.Classification)
+			g, err := path_selection.BuildPathSelectionGraph(edge, kb, args.Classification, true)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/engine/edge_targets.go b/pkg/engine/edge_targets.go
@@ -102,7 +102,7 @@ func (e *Engine) EdgeCanBeExpanded(ctx *solutionContext, source construct.Resour
 			construct.SimpleEdge{
 				Source: tempSource,
 				Target: tempTarget,
-			}, ctx.KnowledgeBase(), classification)
+			}, ctx.KnowledgeBase(), classification, false)
 		if err != nil {
 			return false, cacheable, err
 		}

diff --git a/pkg/engine/operational_eval/graph.go b/pkg/engine/operational_eval/graph.go
@@ -81,6 +81,16 @@ func (eval *Evaluator) AddEdges(es ...construct.Edge) error {
 func (eval *Evaluator) pathVertices(source, target construct.ResourceId) (graphChanges, error) {
 	changes := newChanges()
 
+	src, err := eval.Solution.RawView().Vertex(source)
+	if err != nil {
+		return changes, fmt.Errorf("failed to get source vertex for %s: %w", source, err)
+	}
+	dst, err := eval.Solution.RawView().Vertex(target)
+	if err != nil {
+		return changes, fmt.Errorf("failed to get target vertex for %s: %w", target, err)
+	}
+	requireFullBuild := dst.Imported || src.Imported
+
 	generateAndAddVertex := func(
 		edge construct.SimpleEdge,
 		kb knowledgebase.TemplateKB,
@@ -101,7 +111,7 @@ func (eval *Evaluator) pathVertices(source, target construct.ResourceId) (graphC
 		var tempGraph construct.Graph
 		if buildTempGraph {
 			var err error
-			tempGraph, err = path_selection.BuildPathSelectionGraph(edge, kb, satisfication.Classification)
+			tempGraph, err = path_selection.BuildPathSelectionGraph(edge, kb, satisfication.Classification, !requireFullBuild)
 			if err != nil {
 				return fmt.Errorf("could not build temp graph for %s: %w", edge, err)
 			}

diff --git a/pkg/engine/operational_eval/resource_template_mock_test.go b/pkg/engine/operational_eval/resource_template_mock_test.go
diff --git a/pkg/engine/operational_eval/vertex_path_expand.go b/pkg/engine/operational_eval/vertex_path_expand.go
@@ -202,7 +202,7 @@ func (v *pathExpandVertex) addDepsFromEdge(
 		return err
 	}
 
-	se := construct.SimpleEdge{Source: edge.Source, Target: edge.Target}
+	se := construct.Edge{Source: edge.Source, Target: edge.Target}
 	se.Source.Name = ""
 	se.Target.Name = ""
 
@@ -238,7 +238,7 @@ func (v *pathExpandVertex) addDepsFromEdge(
 	for i, rule := range tmpl.OperationalRules {
 		for j, cfg := range rule.ConfigurationRules {
 			var err error
-			data := knowledgebase.DynamicValueData{Edge: &edge}
+			data := knowledgebase.DynamicValueData{Edge: &se}
 			data.Resource, err = knowledgebase.ExecuteDecodeAsResourceId(dyn, cfg.Resource, data)
 
 			// We ignore the error because it just means that we cant resolve the resource yet
@@ -358,6 +358,7 @@ func (runner *pathExpandVertexRunner) getExpansionsToRun(v *pathExpandVertex) ([
 	if err != nil {
 		errs = errors.Join(errs, err)
 	}
+	requireFullBuild := sourceRes.Imported || targetRes.Imported
 
 	result := make([]path_selection.ExpansionInput, len(expansions))
 	for i, expansion := range expansions {
@@ -369,7 +370,7 @@ func (runner *pathExpandVertexRunner) getExpansionsToRun(v *pathExpandVertex) ([
 		}
 		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)
+			tempGraph, err := path_selection.BuildPathSelectionGraph(simple, eval.Solution.KnowledgeBase(), expansion.Classification, requireFullBuild)
 			if err != nil {
 				errs = errors.Join(errs, fmt.Errorf("error getting expansions to run. could not build path selection graph: %w", err))
 				continue

diff --git a/pkg/engine/operational_eval/vertex_property.go b/pkg/engine/operational_eval/vertex_property.go
@@ -163,12 +163,6 @@ func (v *propertyVertex) Evaluate(eval *Evaluator) error {
 		Data:     dynData,
 	}
 
-	// we know we cannot change properties of imported resources only users through constraints
-	// we still want to be able to update ids in case they are setting the property of a namespaced resource
-	// so we just conditionally run the edge operational rules
-	//
-	// we still need to run the resource operational rules though,
-	// to make sure dependencies exist where properties have operational rules set
 	if err := v.evaluateResourceOperational(&opCtx); err != nil {
 		return err
 	}

diff --git a/pkg/engine/operational_rule/operational_action.go b/pkg/engine/operational_rule/operational_action.go
@@ -91,7 +91,7 @@ func (action *operationalResourceAction) createUniqueResources(resource *constru
 					return err
 				}
 			}
-			if len(uids) == 1 {
+			if len(uids) == 1 && uids[0] == resource.ID {
 				res, err := action.ruleCtx.Solution.RawView().Vertex(id)
 				if err != nil {
 					return err

diff --git a/pkg/engine/path_selection/candidate_validity.go b/pkg/engine/path_selection/candidate_validity.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/dominikbraun/graph"
 	"github.com/klothoplatform/klotho/pkg/collectionutil"
 	construct "github.com/klothoplatform/klotho/pkg/construct"
 	"github.com/klothoplatform/klotho/pkg/engine/solution_context"
@@ -20,6 +21,58 @@ type (
 	}
 )
 
+// checkModifiesImportedResource checks if there is an imported resource that would be modified due to the edge
+// If there is an edge rule modifying the resource then we consider the edge to be invalid
+func checkModifiesImportedResource(
+	source, target construct.ResourceId,
+	ctx solution_context.SolutionContext,
+	et *knowledgebase.EdgeTemplate,
+) (bool, error) {
+	// see if the source resource exists in the graph
+	sourceResource, srcErr := ctx.RawView().Vertex(source)
+	// see if the target resource exists in the graph
+	targetResource, trgtErr := ctx.RawView().Vertex(target)
+	if errors.Is(srcErr, graph.ErrVertexNotFound) && errors.Is(trgtErr, graph.ErrVertexNotFound) {
+		return false, nil
+	}
+
+	if et == nil {
+		et = ctx.KnowledgeBase().GetEdgeTemplate(source, target)
+	}
+
+	checkRules := func(resources construct.ResourceList) (bool, error) {
+		if len(resources) == 0 {
+			return false, nil
+		}
+		for _, rule := range et.OperationalRules {
+			for _, config := range rule.ConfigurationRules {
+				dynamicCtx := solution_context.DynamicCtx(ctx)
+				id := construct.ResourceId{}
+				// we ignore the error since phantom resources will cause errors in the decoding of templates
+				_ = dynamicCtx.ExecuteDecode(config.Resource, knowledgebase.DynamicValueData{
+					Edge: &construct.Edge{
+						Source: source,
+						Target: target,
+					}}, &id)
+
+				if resources.MatchesAny(id) {
+					return true, nil
+				}
+			}
+		}
+		return false, nil
+	}
+
+	importedResources := construct.ResourceList{}
+	if sourceResource != nil && sourceResource.Imported {
+		importedResources = append(importedResources, source)
+	}
+	if targetResource != nil && targetResource.Imported {
+		importedResources = append(importedResources, target)
+	}
+	return checkRules(importedResources)
+}
+
 // checkCandidatesValidity checks if the candidate is valid based on the validity of its own path satisfaction rules and namespace
 func checkCandidatesValidity(
 	ctx solution_context.SolutionContext,

diff --git a/pkg/engine/path_selection/candidate_validity_test.go b/pkg/engine/path_selection/candidate_validity_test.go
@@ -3,13 +3,94 @@ package path_selection
 import (
 	"testing"
 
+	"github.com/klothoplatform/klotho/pkg/construct"
 	"github.com/klothoplatform/klotho/pkg/construct/graphtest"
 	"github.com/klothoplatform/klotho/pkg/engine/enginetesting"
 	knowledgebase "github.com/klothoplatform/klotho/pkg/knowledgebase"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
+func Test_checkModifiesImportedResource(t *testing.T) {
+	tests := []struct {
+		name   string
+		source *construct.Resource
+		target *construct.Resource
+		et     *knowledgebase.EdgeTemplate
+		mocks  func(*enginetesting.MockKB)
+		want   bool
+	}{
+		{
+			name:   "no imported resource returns true",
+			source: &construct.Resource{ID: graphtest.ParseId(t, "p:a:a")},
+			target: &construct.Resource{ID: graphtest.ParseId(t, "p:b:b")},
+			et:     &knowledgebase.EdgeTemplate{},
+			want:   false,
+		},
+		{
+			name:   "imported resource with no modifications returns true",
+			source: &construct.Resource{ID: graphtest.ParseId(t, "p:a:a"), Imported: true},
+			target: &construct.Resource{ID: graphtest.ParseId(t, "p:b:b")},
+			et: &knowledgebase.EdgeTemplate{
+				OperationalRules: []knowledgebase.OperationalRule{
+					{
+						ConfigurationRules: []knowledgebase.ConfigurationRule{
+							{
+								Resource: "{{ .Target }}",
+							},
+						},
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name:   "imported resource with modifications returns false",
+			source: &construct.Resource{ID: graphtest.ParseId(t, "p:a:a"), Imported: true},
+			target: &construct.Resource{ID: graphtest.ParseId(t, "p:b:b")},
+			et: &knowledgebase.EdgeTemplate{
+				OperationalRules: []knowledgebase.OperationalRule{
+					{
+						ConfigurationRules: []knowledgebase.ConfigurationRule{
+							{
+								Resource: "{{ .Source }}",
+							},
+						},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name:   "gets edge template if not provided",
+			source: &construct.Resource{ID: graphtest.ParseId(t, "p:a:a"), Imported: true},
+			target: &construct.Resource{ID: graphtest.ParseId(t, "p:b:b")},
+			mocks: func(kb *enginetesting.MockKB) {
+				kb.On("GetEdgeTemplate", graphtest.ParseId(t, "p:a:a"), graphtest.ParseId(t, "p:b:b")).Return(&knowledgebase.EdgeTemplate{})
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			sol := enginetesting.NewTestSolution()
+			sol.KB.On("GetResourceTemplate", mock.Anything).Return(&knowledgebase.ResourceTemplate{}, nil)
+			if tt.mocks != nil {
+				tt.mocks(&sol.KB)
+			}
+			err := sol.RawView().AddVertex(tt.source)
+			require.NoError(t, err)
+			err = sol.RawView().AddVertex(tt.target)
+			require.NoError(t, err)
+
+			got, err := checkModifiesImportedResource(tt.source.ID, tt.target.ID, sol, tt.et)
+			require.NoError(t, err)
+			require.Equal(t, tt.want, got)
+			sol.KB.AssertExpectations(t)
+		})
+	}
+}
+
 func Test_checkAsTargetValidity(t *testing.T) {
 	type testResource struct {
 		id    string

diff --git a/pkg/engine/path_selection/path_expansion.go b/pkg/engine/path_selection/path_expansion.go
@@ -308,8 +308,18 @@ func expandPath(
 	path construct.Path,
 	resultGraph construct.Graph,
 ) error {
+
 	if len(path) == 2 {
-		return nil
+		modifiesImport, err := checkModifiesImportedResource(input.SatisfactionEdge.Source.ID,
+			input.SatisfactionEdge.Target.ID, ctx, nil)
+		if err != nil {
+			return err
+		}
+		if modifiesImport {
+			// Because the direct edge will cause modifications to an imported resource, we need to remove the direct edge
+			return input.TempGraph.RemoveEdge(input.SatisfactionEdge.Source.ID,
+				input.SatisfactionEdge.Target.ID)
+		}
 	}
 	zap.S().Debugf("Resolving path %s", path)
 
@@ -434,10 +444,17 @@ func expandPath(
 		if !tmpl.Unique.CanAdd(edges, source.id, target.id) {
 			return
 		}
-
+		modifiesImport, err := checkModifiesImportedResource(source.id, target.id, ctx, tmpl)
+		if err != nil {
+			errs = errors.Join(errs, err)
+			return
+		}
+		if modifiesImport {
+			return
+		}
 		// if the edge doesnt exist in the actual graph and there is any uniqueness constraint,
 		// then we need to check uniqueness validity
-		_, err := ctx.RawView().Edge(source.id, target.id)
+		_, err = ctx.RawView().Edge(source.id, target.id)
 		if errors.Is(err, graph.ErrEdgeNotFound) {
 			if tmpl.Unique.Source || tmpl.Unique.Target {
 				valid, err := checkUniquenessValidity(ctx, source.id, target.id)
@@ -527,7 +544,7 @@ func connectThroughNamespace(src, target *construct.Resource, ctx solution_conte
 			continue
 		}
 		// if we have a namespace resource that is not the same as the target namespace resource
-		tg, err := BuildPathSelectionGraph(construct.SimpleEdge{Source: res, Target: target.ID}, kb, "")
+		tg, err := BuildPathSelectionGraph(construct.SimpleEdge{Source: res, Target: target.ID}, kb, "", true)
 		if err != nil {
 			continue
 		}