Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add parameterized provider support to YAML #616

Merged
merged 12 commits into from
Sep 26, 2024
Merged

Add parameterized provider support to YAML #616

merged 12 commits into from
Sep 26, 2024

Conversation

Frassle
Copy link
Member

@Frassle Frassle commented Aug 27, 2024

This adds parameterized provider support to YAML.

This is done via new "package declaration" files, which are generated by gen-sdk. On startup the yaml runtime searches for these files and will use them to register provider packages with the engine. These can include parameterized values, thus supporting parameterized providers.

The schema loader code paths have also been updated to use these package declarations to allow type checking of resources from parameterized packages as well.

Fixes #626.

@Frassle Frassle force-pushed the fraser/yamlparam branch 2 times, most recently from dac6515 to 01ae27b Compare August 27, 2024 11:33
@Frassle Frassle force-pushed the fraser/yamlparam branch 4 times, most recently from 3d644fa to da2797b Compare September 4, 2024 08:36
@Frassle Frassle marked this pull request as ready for review September 4, 2024 11:08
@Frassle Frassle requested a review from a team as a code owner September 4, 2024 11:08
go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
Copy link
Contributor

@lunaris lunaris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments but I think this is good to go. One thing I think of is if we'd ever want to put multiple packages in a single lock file, but I think this is fine -- we could introduce a top-level packages key and look for that first (optionally bumping the lockfile version).

@@ -0,0 +1,162 @@
// Copyright 2024, Pulumi Corporation. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self -- we should sort the licence headers in this repo

"gopkg.in/yaml.v3"
)

type ParameterizationDecl struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some doc comments on the public interface structs would be nice I reckon.

pkg/pulumiyaml/packages/file.go Show resolved Hide resolved
Parameterization *ParameterizationDecl `yaml:"parameterization,omitempty"`
}

func (p *PackageDecl) Valid() bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth changing this to Validate() error so we can have better failure reasons?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do now we've got PackageDeclartionVersion. I was worried about err'ing on too many files before.

func SearchPackageDecls(directory string) ([]PackageDecl, error) {
if directory == "" {
directory = "."
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any benefit to doing this rather than letting the caller just specify "."?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, lets be strict.

@@ -343,6 +378,14 @@ func (r *Runner) setDefaultProviders() {
contract.IgnoreError(diags)
}

// Set the runners package descriptors from the templates package decls.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Set the runners package descriptors from the templates package decls.
// Set the runner's package descriptors from the templates package decls.

// Register package refs for all packages we know upfront
packageDescriptors, err := packages.ToPackageDescriptors(r.t.Packages)
r.packageDescriptors = packageDescriptors
return err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we always set even if there is an error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not

}

var parameterization *pulumirpc.Parameterization
if pkg.Parameterization != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor but we could combine these ifs to avoid the double check (cheap as it is). Happy either way. Same is true of ToPackageDescriptors earlier too if we feel like it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

pkg/pulumiyaml/run.go Show resolved Hide resolved
pkg/server/server.go Show resolved Hide resolved
@Frassle Frassle merged commit dd0f078 into main Sep 26, 2024
6 checks passed
@Frassle Frassle deleted the fraser/yamlparam branch September 26, 2024 19:53
@pulumi-bot
Copy link

This PR has been shipped in release v1.11.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parameterized Providers: YAML support
3 participants