Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix topological sort to take into account defaultProvider. #652

Merged
merged 3 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Bug Fixes-652.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: "652"
53 changes: 51 additions & 2 deletions pkg/pulumiyaml/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,19 @@ func topologicallySortedResources(t *ast.TemplateDecl, externalConfig []configNo
sorted = append(sorted, node)
}
}

// Map of package name to default provider resource and it's key.
defaultProviders := map[string]*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) {
pkg := strings.Split(node.Value.Type.Value, ":")[2]
defaultProviders[pkg] = node.key()
}

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

Expand Down Expand Up @@ -198,6 +207,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 +216,28 @@ func topologicallySortedResources(t *ast.TemplateDecl, externalConfig []configNo
return false
}
}

if resNode, ok := e.(resourceNode); ok {
pkg := ""
if resNode.Value.Type != nil {
pkg, _, ok = strings.Cut(resNode.Value.Type.Value, ":")
if !ok {
return false
}
}
defaultProviderForPackage := defaultProviders[pkg]
isDefaultProvider := resNode.Value.DefaultProvider != nil && resNode.Value.DefaultProvider.Value
if resourceNodeHasNoExplicitProvider(e) && !isDefaultProvider {
// If the resource has no explicit provider and the default provider is not set, then the
// (implicit) dependency is not yet met.
if defaultProviderForPackage != nil && !visit(defaultProviderForPackage) {
return false
}

// if the defaultProviderForPackage is not set, then it may not be needed.
}
}

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

Expand All @@ -232,13 +264,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 +297,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
18 changes: 18 additions & 0 deletions pkg/pulumiyaml/sort_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,24 @@ func TestSortUnordered(t *testing.T) {
assert.Equal(t, "my-object", names[1])
}

func TestSortMultipleDefaultProviders(t *testing.T) {
t.Parallel()

tmpl, diags, err := LoadFile("../tests/testdata/resource-ordering/Pulumi.yaml")
requireNoErrors(t, tmpl, diags)
assert.NoError(t, err)

confNodes := []configNode{}
resources, diags := topologicallySortedResources(tmpl, confNodes)
requireNoErrors(t, tmpl, diags)
names := sortedNames(resources)
assert.Len(t, names, 4)
assert.Equal(t, "provider", names[0])
assert.Equal(t, "alb", names[1])
assert.Equal(t, "testProvider", names[2])
assert.Equal(t, "echo", names[3])
}

func TestSortErrorCycle(t *testing.T) {
t.Parallel()

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 @@ -160,3 +160,13 @@ func TestParameterized(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,
})
}
29 changes: 29 additions & 0 deletions pkg/tests/testdata/resource-ordering/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: aws-yaml
runtime: yaml
plugins:
providers:
- name: testprovider
path: ../../testprovider
resources:
alb:
type: aws:alb:LoadBalancer
properties:
tags:
Name: test-lb
name: testing
subnets:
- subnet-eacf3697
- subnet-939b18f8
echo:
type: testprovider:index:Echo
provider:
defaultProvider: true
type: pulumi:providers:aws
properties:
region: us-west-2
testProvider:
defaultProvider: true
type: pulumi:providers:testprovider
properties:
testInput: ${alb.urn}

7 changes: 6 additions & 1 deletion pkg/tests/testprovider/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ var providerSchema = pschema.PackageSpec{
Description: "The provider type for the testprovider package.",
Type: "object",
},
InputProperties: map[string]pschema.PropertySpec{},
InputProperties: map[string]pschema.PropertySpec{
"testInput": {
TypeSpec: pschema.TypeSpec{Type: "string"},
Description: "A test input property.",
},
},
},

Types: map[string]pschema.ComplexTypeSpec{},
Expand Down
Loading