[sdk/go] Embed PluginDownloadURL during codegen #8690

iwahbe · 2022-01-05T21:49:14Z

Description

Fixes # (issue)

Embed PluginDownloadURLs in resource creation requests from codegen.

Checklist

I have added tests that prove my fix is effective or that my feature works

I have updated the CHANGELOG-PENDING file with my change

Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

github-actions · 2022-01-05T21:53:16Z

Diff for pulumi-azuread with merge commit 8ef9b34

github-actions · 2022-01-05T21:53:20Z

Diff for pulumi-random with merge commit 8ef9b34

github-actions · 2022-01-05T21:55:22Z

Diff for pulumi-kubernetes with merge commit 8ef9b34

github-actions · 2022-01-05T21:57:25Z

Diff for pulumi-gcp with merge commit 8ef9b34

github-actions · 2022-01-05T22:07:35Z

Diff for pulumi-azure with merge commit 8ef9b34

github-actions · 2022-01-05T22:14:10Z

Diff for pulumi-aws with merge commit 8ef9b34

pkg/codegen/go/gen.go

justinvp · 2022-01-05T22:32:52Z

pkg/codegen/go/gen.go

+	// that if a user adds their own separate PluginDownloadURL, we won't
+	// override it.
+	if url := pkg.pkg.PluginDownloadURL; url != "" {
+		fmt.Fprintf(w, "\topts = append([]pulumi.ResourceOption{pulumi.PluginDownloadURL(%q)},  opts...)\n", url)


@pgavlin, quick question that's orthogonal to this PR, but kind of related:

Shouldn't we be setting the default version in here like we do in the other languages, or am I missing something?

pulumi/pkg/codegen/dotnet/gen.go

Line 1037 in 684d7aa

fmt.Fprintf(w, " Version = Utilities.Version,\n")

pulumi/pkg/codegen/nodejs/gen.go

Lines 868 to 870 in 684d7aa

fmt.Fprintf(w, " if (!opts.version) {\n")

fmt.Fprintf(w, " opts = pulumi.mergeOptions(opts, { version: utilities.getVersion()});\n")

fmt.Fprintf(w, " }\n")

pulumi/pkg/codegen/python/gen.go

Lines 1219 to 1220 in 684d7aa

fmt.Fprintf(w, " if opts.version is None:\n")

fmt.Fprintf(w, " opts.version = _utilities.get_version()\n")

Maybe we didn't do this for Go because the version info for Go isn't readily available? (Although, we did later add PkgVersion used in resource module registrations; so perhaps we could consider using that, if it is desirable).

Honestly, I'm not sure why we didn't do this in Go. It does seem like we should. I'm also not sure how we arrived at PkgVersion--any idea if there's a reason we didn't just embed the version statically? The reflection-based code used by PkgVersion is essentially static, I think.

We can't embed the version statically because we can't trust that the version supplied in the schema is correct. This is the same reason why #8521 exists, and is blocked. Injecting the version at codegen time would require changing how our provider makefile work.

Ah yep, thank you for the reminder. I wonder if we should switch this to using a //go:embed and a parse in init so that we can get a real version. This would be akin to the solutions we use for the other language. More concretely, we could e.g. have the Go code generator emit a dummy version.txt file that is embedded into the package, then have the Makefile update the contents of that file with the actual server. The contents of the file would be parsed into a real version during the package's init function.

If we are willing to touch every Makefile to actualize that change, I think we should instead attempt to find a safe way to embed version into the binary (if at all possible).

I think we should instead attempt to find a safe way to embed version into the binary (if at all possible).

I would love it if we could find a way to do this, but I'll admit that I'm pessimistic about making that happen any time soon. I think that taking a tactical fix--even though it requires updating Makefiles--is a chance for us to not let perfect be the enemy of better.

we could e.g. have the Go code generator emit a dummy version.txt file that is embedded into the package, then have the Makefile update the contents of that file with the actual server

I still don't quite follow how this would work for go modules. Say we generate an SDK for a foo provider, emitted at github.com/pulumi/pulumi-foo/sdk/go/foo. Versions are tagged with git tags (e.g. v1.2.3 and sdk/v1.2.3). Wouldn't we have to commit the version.txt file before tagging it? I don't see how the Makefile helps. When I use the foo module from my program, our Makefile is not involved.

That's a good point. For Go, it's the push to the git remote that's the publishing step, so even if we took my suggestion we'd still be emitting the wrong version.

github-actions · 2022-01-05T22:40:04Z

Diff for pulumi-azure-native with merge commit 8ef9b34

codecov · 2022-01-05T23:00:16Z

Codecov Report

Merging #8690 (551f2fc) into master (9a5ba09) will increase coverage by 2.54%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #8690      +/-   ##
==========================================
+ Coverage   58.83%   61.37%   +2.54%     
==========================================
  Files         557      500      -57     
  Lines       94165    85339    -8826     
  Branches     1385        0    -1385     
==========================================
- Hits        55398    52377    -3021     
+ Misses      35497    29869    -5628     
+ Partials     3270     3093     -177

Impacted Files	Coverage Δ
pkg/codegen/go/gen.go	`89.70% <100.00%> (+0.50%)`	⬆️
...dk/go/common/resource/plugin/langruntime_plugin.go	`67.29% <0.00%> (-1.89%)`	⬇️
sdk/nodejs/proto/engine_grpc_pb.js
sdk/nodejs/runtime/debuggable.ts
sdk/nodejs/runtime/closure/utils.ts
sdk/nodejs/proto/provider_grpc_pb.js
sdk/nodejs/proto/engine_pb.js
sdk/nodejs/runtime/settings.ts
sdk/nodejs/runtime/invoke.ts
sdk/nodejs/proto/resource_pb.js
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a5ba09...551f2fc. Read the comment docs.

github-actions · 2022-01-06T01:35:07Z

Diff for pulumi-azuread with merge commit c96f0e8

github-actions · 2022-01-06T01:35:08Z

Diff for pulumi-random with merge commit c96f0e8

github-actions · 2022-01-06T01:39:03Z

Diff for pulumi-kubernetes with merge commit c96f0e8

github-actions · 2022-01-06T01:40:00Z

Diff for pulumi-gcp with merge commit c96f0e8

github-actions · 2022-01-06T01:42:13Z

Diff for pulumi-random with merge commit dd1ebb0

github-actions · 2022-01-06T01:42:45Z

Diff for pulumi-azuread with merge commit dd1ebb0

github-actions · 2022-01-06T01:44:17Z

Diff for pulumi-kubernetes with merge commit dd1ebb0

github-actions · 2022-01-06T01:47:47Z

Diff for pulumi-gcp with merge commit dd1ebb0

github-actions · 2022-01-06T01:48:37Z

Diff for pulumi-azure with merge commit c96f0e8

github-actions · 2022-01-06T01:52:46Z

Diff for pulumi-aws with merge commit c96f0e8

github-actions · 2022-01-06T01:56:28Z

Diff for pulumi-azure with merge commit dd1ebb0

github-actions · 2022-01-06T01:59:16Z

Diff for pulumi-aws with merge commit dd1ebb0

github-actions · 2022-01-06T02:20:52Z

Diff for pulumi-azure-native with merge commit c96f0e8

github-actions · 2022-01-06T02:29:26Z

Diff for pulumi-azure-native with merge commit dd1ebb0

github-actions · 2022-01-06T23:50:10Z

Diff for pulumi-azuread with merge commit a67b5dd

github-actions · 2022-01-06T23:51:45Z

Diff for pulumi-kubernetes with merge commit a67b5dd

github-actions · 2022-01-06T23:53:15Z

Diff for pulumi-gcp with merge commit a67b5dd

github-actions · 2022-01-07T00:01:42Z

Diff for pulumi-azure with merge commit a67b5dd

github-actions · 2022-01-07T00:09:36Z

Diff for pulumi-aws with merge commit a67b5dd

github-actions · 2022-01-07T00:41:00Z

Diff for pulumi-azure-native with merge commit a67b5dd

pkg/codegen/go/gen.go

justinvp · 2022-01-07T18:33:37Z

...egen/internal/test/testdata/azure-native-nested-types/go/azure/documentdb/pulumiUtilities.go

+}
+
+// PkgResourceDefaultOpts provides package level defaults to pulumi.OptionResource.
+func PkgResourceDefaultOpts(opts []pulumi.ResourceOption) []pulumi.ResourceOption {


I realize the existing PkgVersion is exported (capitalized), but I doubt that was intentional. I am wondering if these new functions should be unexported (lowercase)... I don't think anyone will need to be calling these as part of an SDK's public API directly.

(Separately, it might be worth revisiting PkgVersion, making it lowercase).

I capitalized Pkg%sDefaultOpts because I assumed there was a good reason why PkgVersion was capitalized. I can change that.

...egen/internal/test/testdata/azure-native-nested-types/go/azure/documentdb/pulumiUtilities.go

justinvp · 2022-01-07T18:56:14Z

pkg/codegen/go/gen.go

@@ -3526,7 +3531,7 @@ func GeneratePackage(tool string, pkg *schema.Package) (map[string][]byte, error
 				packageRegex = fmt.Sprintf("^%s(/v\\d+)?", pkg.importBasePath)
 			}

-			_, err := fmt.Fprintf(buffer, utilitiesFile, packageRegex)
+			_, err := fmt.Fprintf(buffer, UtilitiesFile(pkg), packageRegex)


Do we need to make sure pkg.needsUtils is set to true if there are any resources or invokes? Otherwise, is it possible the utilities file won't be emitted?

insertPkgDefaultsOptsCall sets pkg.needsUtils = true.

github-actions · 2022-01-07T20:55:44Z

Diff for pulumi-random with merge commit e3a3f82

github-actions · 2022-01-07T20:56:09Z

Diff for pulumi-azuread with merge commit e3a3f82

github-actions · 2022-01-07T20:56:39Z

Diff for pulumi-kubernetes with merge commit e3a3f82

github-actions · 2022-01-07T20:59:45Z

Diff for pulumi-gcp with merge commit e3a3f82

github-actions · 2022-01-07T21:09:37Z

Diff for pulumi-azure with merge commit e3a3f82

github-actions · 2022-01-07T21:13:25Z

Diff for pulumi-aws with merge commit e3a3f82

github-actions · 2022-01-07T21:50:15Z

Diff for pulumi-azure-native with merge commit e3a3f82

justinvp

LGTM

github-actions · 2022-01-11T00:05:02Z

Diff for pulumi-random with merge commit c4490e1

github-actions · 2022-01-11T00:05:42Z

Diff for pulumi-azuread with merge commit c4490e1

github-actions · 2022-01-11T00:06:15Z

Diff for pulumi-kubernetes with merge commit c4490e1

github-actions · 2022-01-11T00:09:53Z

Diff for pulumi-gcp with merge commit c4490e1

github-actions · 2022-01-11T00:17:18Z

Diff for pulumi-azure with merge commit c4490e1

github-actions · 2022-01-11T00:25:49Z

Diff for pulumi-aws with merge commit c4490e1

github-actions · 2022-01-11T00:52:45Z

Diff for pulumi-azure-native with merge commit c4490e1

Embed PluginDownloadURL during codegen

fc7ace4

iwahbe requested a review from justinvp January 5, 2022 21:49

iwahbe self-assigned this Jan 5, 2022

justinvp reviewed Jan 5, 2022

View reviewed changes

Move default function to pulumiUtilities.go

14cb0de

iwahbe requested a review from justinvp January 6, 2022 01:33

Add change to CHANGELOG_PENDING.md

752dc54

Merge branch 'master' into iwahbe/go-codegen-embed-PluginDownloadURL

98711aa

iwahbe requested a review from pgavlin January 7, 2022 02:13

justinvp reviewed Jan 7, 2022

View reviewed changes

iwahbe added 2 commits January 7, 2022 12:49

Fix nits

b44cd84

Merge branch 'master' into iwahbe/go-codegen-embed-PluginDownloadURL

439e388

iwahbe requested a review from justinvp January 7, 2022 20:51

iwahbe changed the title ~~Embed PluginDownloadURL during codegen~~ [sdk/go] Embed PluginDownloadURL during codegen Jan 8, 2022

justinvp approved these changes Jan 10, 2022

View reviewed changes

Merge branch 'master' into iwahbe/go-codegen-embed-PluginDownloadURL

551f2fc

iwahbe merged commit 27f761d into master Jan 11, 2022

pulumi-bot deleted the iwahbe/go-codegen-embed-PluginDownloadURL branch January 11, 2022 01:38

	fmt.Fprintf(w, " if (!opts.version) {\n")
	fmt.Fprintf(w, " opts = pulumi.mergeOptions(opts, { version: utilities.getVersion()});\n")
	fmt.Fprintf(w, " }\n")

	fmt.Fprintf(w, " if opts.version is None:\n")
	fmt.Fprintf(w, " opts.version = _utilities.get_version()\n")

[sdk/go] Embed PluginDownloadURL during codegen #8690

[sdk/go] Embed PluginDownloadURL during codegen #8690

Conversation

iwahbe commented Jan 5, 2022 • edited Loading

Description

Checklist

github-actions bot commented Jan 5, 2022

github-actions bot commented Jan 5, 2022

github-actions bot commented Jan 5, 2022

github-actions bot commented Jan 5, 2022

github-actions bot commented Jan 5, 2022

github-actions bot commented Jan 5, 2022

justinvp Jan 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 5, 2022

codecov bot commented Jan 5, 2022 • edited Loading

Codecov Report

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 7, 2022

justinvp left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 11, 2022

github-actions bot commented Jan 11, 2022

github-actions bot commented Jan 11, 2022

github-actions bot commented Jan 11, 2022

github-actions bot commented Jan 11, 2022

github-actions bot commented Jan 11, 2022

github-actions bot commented Jan 11, 2022

iwahbe commented Jan 5, 2022 •

edited

Loading

justinvp Jan 5, 2022 •

edited

Loading

codecov bot commented Jan 5, 2022 •

edited

Loading