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(host/go): Download external plugins even if they're not imported #13455

Merged
merged 2 commits into from
Jul 12, 2023

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Jul 11, 2023

We previously changed how the Go language host retrieves information
about a Go module:

before: go list -m -json -mod=mod all
after:  go list -m -json $importPath1 $importPath1

This change made it possible for us to supprot running in vendored mode
which allowed for use of private plugins that are not go-get-able.

This uncovered a corner case in the go command's behavior
when running in module mode:

If a package is listed in the go.mod
but it's not imported by the current module,
go list -m -json $importPath will not list its Dir in the output
even if it's present in the module cache.
For example, given a go.mod file that declares a dependency
but code that doesn't import it, as of Go 1.20.5,

export GOMODCACHE=$(mktemp -d)
go mod download -json
go list -m -json $importPath

The output of go mod download will include information
about the dependency and the Dir where it was downloaded,
but go list -m will not.

Unfortunately, we can't just use go mod download
because that breaks vendoring:
vendored dependencies cannot always be redownloaded.

To resolve this issue,
we switch to a two-pass variant to gather this information:

  • Run go list -m to get whatever information we can locally.
    This will be sufficient for the majority of use cases.
  • For the handful of cases where the dependency isn't imported,
    we'll use go mod download -json to download them on-demand
    and get their location from that instead.

The go mod download step will take place only if we're in module mode.
In vendor mode, we'll work with what we have.

Resolves #13301

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Jul 11, 2023

Changelog

[uncommitted] (2023-07-11)

Bug Fixes

  • [sdk/go] Fix downloading of unimported external plugins.
    #13455

@abhinav abhinav force-pushed the abhinav/go-auto-http-plugin-13301 branch from 84f1930 to 201efec Compare July 11, 2023 22:39
@abhinav
Copy link
Contributor Author

abhinav commented Jul 11, 2023

Repro: https://github.com/pulumi/pulumi/actions/runs/5525559114/jobs/10079407337?pr=13455

=== FAIL: integration TestAutomation_externalPluginDownload_issue13301 (3.04s)
    environment.go:97: Created new test environment:  /tmp/test-env2781205917
    integration_go_test.go:964: Running command pulumi login --cloud-url file:///tmp/test-env2781205917
    integration_go_test.go:992: Running command pulumi plugin install
    integration_go_test.go:1024: 
        	Error Trace:	/home/runner/work/pulumi/pulumi/tests/integration/integration_go_test.go:1024
        	Error:      	Received unexpected error:
        	            	failed to run preview: exit status 255
        	            	code: 255
        	            	stdout: Previewing update (foo):
        	            	
        	            	 +  pulumi:pulumi:Stack issue-13301-foo create 
        	            	    pulumi:providers:hcp hcp  error: could not find latest version for provider hcp: GetLatestVersion is not supported for plugins from http sources
        	            	 +  pulumi:pulumi:Stack issue-13301-foo create 
        	            	    pulumi:providers:hcp hcp  1 error
        	            	
        	            	Diagnostics:
        	            	  pulumi:providers:hcp (hcp):
        	            	    error: could not find latest version for provider hcp: GetLatestVersion is not supported for plugins from http sources
        	            	
        	            	
        	            	
        	            	stderr: 
        	Test:       	TestAutomation_externalPluginDownload_issue13301
    integration_go_test.go:985: grpc debug log:
        {"method":"/pulumirpc.LanguageRuntime/GetPluginInfo","request":{},"response":{},"metadata":{"kind":"language","mode":"client","runtime":"go"}}
        {"method":"/pulumirpc.LanguageRuntime/GetRequiredPlugins","request":{"project":"issue-13301","pwd":"/tmp/test-env2781205917","program":"."},"response":{},"metadata":{"kind":"language","mode":"client","runtime":"go"}}
        {"method":"/pulumirpc.LanguageRuntime/GetRequiredPlugins","request":{"project":"issue-13301","pwd":"/tmp/test-env2781205917","program":"."},"response":{},"metadata":{"address":"127.0.0.1:[355](https://github.com/pulumi/pulumi/actions/runs/5525559114/jobs/10079407337?pr=13455#step:34:356)33","kind":"language","mode":"client"}}
        {"method":"/pulumirpc.ResourceMonitor/SupportsFeature","request":{"id":"resourceReferences"},"response":{"hasSupport":true},"metadata":{"mode":"server"}}
        {"method":"/pulumirpc.ResourceMonitor/SupportsFeature","request":{"id":"outputValues"},"response":{"hasSupport":true},"metadata":{"mode":"server"}}
        {"method":"/pulumirpc.ResourceMonitor/SupportsFeature","request":{"id":"deletedWith"},"response":{"hasSupport":true},"metadata":{"mode":"server"}}
        {"method":"/pulumirpc.ResourceMonitor/SupportsFeature","request":{"id":"aliasSpecs"},"response":{"hasSupport":true},"metadata":{"mode":"server"}}
        {"method":"/pulumirpc.ResourceMonitor/RegisterResource","request":{"type":"pulumi:pulumi:Stack","name":"issue-13301-foo","object":{},"acceptSecrets":true,"customTimeouts":{},"acceptResources":true},"response":{"urn":"urn:pulumi:foo::issue-13301::pulumi:pulumi:Stack::issue-13301-foo","object":{}},"metadata":{"mode":"server"}}

@abhinav abhinav force-pushed the abhinav/go-auto-http-plugin-13301 branch 2 times, most recently from 6160ec5 to 0d9aee0 Compare July 11, 2023 23:18
@abhinav abhinav changed the title WIP: Fix #13301 fix(host/go): Download external plugins even if they're not imported Jul 11, 2023
@abhinav abhinav marked this pull request as ready for review July 11, 2023 23:19
Adds a regression of the bug using code provided by @phillipedwards.
The reproduction is a bit complicated because of the very specific
scenario we need to replicate here.

Additionally, we need an untidy go.mod file for this,
so we need to teach scripts/tidy.sh how to ignore files.
We previously changed how the Go language host retrieves information
about a Go module:

    before: go list -m -json -mod=mod all
    after:  go list -m -json $importPath1 $importPath1

This change made it possible for us to supprot running in vendored mode
which allowed for use of private plugins that are not go-get-able.

This uncovered a corner case in the `go` command's behavior
when running in module mode:

If a package is listed in the go.mod
but it's not imported by the current module,
`go list -m -json $importPath` will not list its Dir in the output
even if it's present in the module cache.
For example, given a go.mod file that declares a dependency
but code that doesn't import it, as of Go 1.20.5,

    export GOMODCACHE=$(mktemp -d)
    go mod download -json
    go list -m -json $importPath

The output of `go mod download` will include information
about the dependency and the `Dir` where it was downloaded,
but `go list -m` will not.

Unfortunately, we can't just use `go mod download`
because that breaks vendoring:
vendored dependencies cannot always be redownloaded.

To resolve this issue,
we switch to a two-pass variant to gather this information:

- Run `go list -m` to get whatever information we can locally.
  This will be sufficient for the majority of use cases.
- For the handful of cases where the dependency isn't imported,
  we'll use `go mod download -json` to download them on-demand
  and get their location from that instead.

The `go mod download` step will take place only if we're in module mode.
In vendor mode, we'll work with what we have.

Resolves #13301
@abhinav abhinav force-pushed the abhinav/go-auto-http-plugin-13301 branch from 0d9aee0 to a244de9 Compare July 11, 2023 23:22
@abhinav
Copy link
Contributor Author

abhinav commented Jul 11, 2023

Note that this fix depends on #13447.
Thanks to @Frassle for fixing.

Copy link
Member

@Frassle Frassle left a comment

Choose a reason for hiding this comment

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

LGTM

@abhinav
Copy link
Contributor Author

abhinav commented Jul 11, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Jul 12, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit f0858e6 into master Jul 12, 2023
50 checks passed
@bors bors bot deleted the abhinav/go-auto-http-plugin-13301 branch July 12, 2023 00:27
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.

[go] http plugins fail to install for go programs when go dependencies are not present
3 participants