implement getting plugins from remote git sources #17827
Conversation
Allow users go get and run plugins from remote git sources. This should work with all currently supported sources, such as GitHub and GitLab (for the correct URL splitting).
sdk/go/common/workspace/plugins.go
Outdated
| path string | ||
| } | ||
|
|
||
| func newGitSource(url *url.URL) (*gitSource, error) { |
There was a problem hiding this comment.
We could name that newGitHTTPSSource since it always forces https.
| } | ||
|
|
||
| tw.Close() | ||
| zip.Close() |
There was a problem hiding this comment.
Should these be defered before the potential error return?
There was a problem hiding this comment.
I wanted to do that, but we can't. From https://go.dev/ref/spec#Defer_statements:
deferred functions are executed after any result parameters are set by that return statement but before the function returns to its caller.
So this means that tarbuf.Len() would be calculated before we do tw.Close(), which in turn means that the length can be off.
This is probably worth a comment though.
| @@ -1378,7 +1378,7 @@ func (host *nodeLanguageHost) RunPlugin( | |||
| return err | |||
| } | |||
|
|
|||
| env := os.Environ() | |||
| env := req.Env | |||
There was a problem hiding this comment.
The changes in this file look to be unrelated
There was a problem hiding this comment.
Yeah, let me pull these out.
| tw.Close() | ||
| zip.Close() | ||
|
|
||
| return io.NopCloser(tarbuf), int64(tarbuf.Len()), nil |
There was a problem hiding this comment.
It's a bit of a shame this builds the tarbuf in memory and then returns a reader to it, but it's probably not that easy to make this stream straight from the temp dir.
There was a problem hiding this comment.
Yeah, this is the best I found so far.
There was a problem hiding this comment.
Kinda loosing the point of this being an io.Reader, it was like that so we could see the download happening and do a progress bar. This isn't going to show any progress till the whole clone is finished.
There was a problem hiding this comment.
Progress bar is always going to be kinda tricky with Git Repos. I don't think go-git supports anything like that. Also the way git works it downloads the objects first, and can only then open the repo and splat the files somewhere, that's not possible during transport.
Even the GitHub API to download contents of the repo (e.g. https://github.com/pulumi/pulumi/archive/refs/heads/master.zip) won't give the length upfront, so that also wouldn't help us get a progress bar here.
3fd9eb0
to
8d4a6dd
Compare
sdk/go/common/workspace/plugins.go
Outdated
| @@ -2278,7 +2378,7 @@ func getCandidateExtensions() []string { | |||
| // pluginRegexp matches plugin directory names: pulumi-KIND-NAME-VERSION. | |||
| var pluginRegexp = regexp.MustCompile( | |||
| "^(?P<Kind>[a-z]+)-" + // KIND | |||
| "(?P<Name>[a-zA-Z0-9-]*[a-zA-Z0-9])-" + // NAME | |||
| "(?P<Name>[a-zA-Z0-9-_.]*[a-zA-Z0-9])-" + // NAME | |||
There was a problem hiding this comment.
We're allowing dots to start names now?
There was a problem hiding this comment.
I meant to only allow it within the name for URLs. E.g. github.com/pulumi/provider-bla gets turned into github.com_pulumi_provider-bla. Not sure we should even allow - at the beginning of the name, but since we always did I guess it's fine.
There was a problem hiding this comment.
Hrm expressing not allowing . at the start of the regex is pretty awkward now. [a-zA-Z0-9-][a-zA-Z0-9-_.]*[a-zA-Z0-9])- would force at least two character names which we didn't. Though maybe that's fine, I doubt any providers with just a single character name exist currently anyway.
| @@ -1745,7 +1755,7 @@ func TestNewPluginSpec(t *testing.T) { | |||
| Name: "github.com_pulumi_pulumi-example", | |||
There was a problem hiding this comment.
Thinking about this more while looking at it. Why is the name the whole url? Shouldn't it just be pulumi/pulumi-example, or maybe even just pulumi-example?
There was a problem hiding this comment.
To be able to distinguish the plugin. If it's just pulumi-example, it could come from gitlab.com/tgummerer/pulumi-example, but have the same name as github.com/pulumi/pulumi-example when that's downloaded using the current mechanisms. Which would get very confusing, especially as we're using this name on the file system to find the plugin again.
There was a problem hiding this comment.
this is going to end up with plugin names that don't match the package names they provide though. I think there's parts of the system that might depend on that.
There was a problem hiding this comment.
Yeah, I think we'll have to fix those though, otherwise this is getting messy with disambiguating 🤔
| tw.Close() | ||
| zip.Close() | ||
|
|
||
| return io.NopCloser(tarbuf), int64(tarbuf.Len()), nil |
There was a problem hiding this comment.
Kinda loosing the point of this being an io.Reader, it was like that so we could see the download happening and do a progress bar. This isn't going to show any progress till the whole clone is finished.
5435b9a
to
e353d9d
Compare
Allow users go get and run plugins from remote git sources. This should work with all currently supported sources, such as GitHub and GitLab (for the correct URL splitting).
This does not currently support direct links (e.g. just pasting https://github.com/pulumi/pulumi-tls-self-signed-cert/tree/mikhailshilkov/boilerplateless/lib), or just getting the latest version of a plugin (the version needs to always be specified). These can be implemented separately later.
Depends on #17798