Skip to content

Commit

Permalink
Fix topological sort to take into account defaultProvider.
Browse files Browse the repository at this point in the history
The topological sort will not sort default providers first as is, so
this adds in a check to detect it and not allow implicit providers to be
complete until the default provider is declared.

Remote-Ref: brandonpollack23/f91181f4
Bug: #604
  • Loading branch information
brandonpollack23 committed Sep 24, 2024
1 parent ce4239e commit 485f11b
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 2 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Bug Fixes-643.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
component: runtime
kind: Bug Fixes
body: Fix default component detection when declared later in file
time: 2024-09-24T15:31:34.068526213+09:00
custom:
PR: "643"
43 changes: 41 additions & 2 deletions pkg/pulumiyaml/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,17 @@ func topologicallySortedResources(t *ast.TemplateDecl, externalConfig []configNo
sorted = append(sorted, node)
}
}

var defaultProvider *ast.StringExpr
for _, kvp := range t.Resources.Entries {
rname, r := kvp.Key.Value, kvp.Value
node := resourceNode(kvp)

// Check if the resource is a default provider
if resourceIsDefaultProvider(node) {
defaultProvider = node.key()
}

cdiags := checkUniqueNode(intermediates, node)
diags = append(diags, cdiags...)

Expand Down Expand Up @@ -198,6 +205,7 @@ func topologicallySortedResources(t *ast.TemplateDecl, externalConfig []configNo
}
if !visited[name.Value] {
visiting[name.Value] = true

for _, mname := range dependencies[name.Value] {
if mname.Value == PulumiVarName {
continue
Expand All @@ -206,6 +214,20 @@ func topologicallySortedResources(t *ast.TemplateDecl, externalConfig []configNo
return false
}
}

if resourceNodeHasNoExplicitProvider(e) && defaultProvider != name {
if defaultProvider == nil {
diags.Extend(ast.ExprError(name, "resource has no explicit provider and no default provider set", ""))
return false
}

// If the resource has no explicit provider and the default provider is not set, then the
// (implicit) dependency is not yet met.
if !visit(defaultProvider) {
return false
}
}

visited[name.Value] = true
visiting[name.Value] = false

Expand All @@ -232,13 +254,29 @@ func topologicallySortedResources(t *ast.TemplateDecl, externalConfig []configNo
return sorted, diags
}

// resourceIsDefaultProvider returns true if the node is a default provider, otherwise false.
func resourceIsDefaultProvider(res resourceNode) bool {
return res.Value.DefaultProvider != nil && res.Value.DefaultProvider.Value
}

// resourceNodeHasNoExplicitProvider returns true if the node is a resource
// node and has no explicit provider set, otherwise false.
func resourceNodeHasNoExplicitProvider(graphNode graphNode) bool {
if res, ok := graphNode.(resourceNode); ok {
return res.Value.Options.Provider == nil
}

return false
}

func checkUniqueNode(intermediates map[string]graphNode, node graphNode) syntax.Diagnostics {
var diags syntax.Diagnostics

key := node.key()
name := key.Value
if name == PulumiVarName {
return syntax.Diagnostics{ast.ExprError(key, fmt.Sprintf("%s %s uses the reserved name pulumi", node.valueKind(), name), "")}
return syntax.Diagnostics{ast.ExprError(key,
fmt.Sprintf("%s %s uses the reserved name pulumi", node.valueKind(), name), "")}
}

if other, found := intermediates[name]; found {
Expand All @@ -249,7 +287,8 @@ func checkUniqueNode(intermediates map[string]graphNode, node graphNode) syntax.
if node.valueKind() == other.valueKind() {
diags.Extend(ast.ExprError(key, fmt.Sprintf("found duplicate %s %s", node.valueKind(), name), ""))
} else {
diags.Extend(ast.ExprError(key, fmt.Sprintf("%s %s cannot have the same name as %s %s", node.valueKind(), name, other.valueKind(), name), ""))
diags.Extend(ast.ExprError(key, fmt.Sprintf(
"%s %s cannot have the same name as %s %s", node.valueKind(), name, other.valueKind(), name), ""))
}
return diags
}
Expand Down
10 changes: 10 additions & 0 deletions pkg/tests/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,13 @@ func TestLocalPlugin(t *testing.T) {
},
})
}

//nolint:paralleltest // uses parallel programtest
func TestResourceOrderingWithDefaultProvider(t *testing.T) {
integration.ProgramTest(t,
&integration.ProgramTestOptions{
Dir: filepath.Join("testdata", "resource-ordering"),
SkipUpdate: true,
SkipEmptyPreviewUpdate: true,
})
}
17 changes: 17 additions & 0 deletions pkg/tests/testdata/resource-ordering/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: aws-yaml
runtime: yaml
resources:
alb:
type: aws:alb:LoadBalancer
properties:
tags:
Name: test-lb
name: testing
subnets:
- subnet-eacf3697
- subnet-939b18f8
provider:
defaultProvider: true
type: pulumi:providers:aws
properties:
region: us-west-2

0 comments on commit 485f11b

Please sign in to comment.