When the user specifies a provider with a specific path in the
`Pulumi.yaml`, we later in the program assert that the path that was
passed in also matches with the path where we found the plugin. We do
this by using a string comparison, however that doesn't work if the path
the user passes is not clean, e.g. has a trailing slash, or has a double
slash, or some such.
Fix this by using `filepath.Clean` on the user supplied path, which
these things up.
I was also briefly wondering if this works properly if the user passes
in a path that is a symlink (it does), and wrote a test for that,
checking that behaviour.
Fixes https://github.com/pulumi/pulumi/issues/17130
Before this fix GetPluginInfo would error because stat on the expected
`pulumi-resource-exe` file would fail (because it didn't exist). This
fixes it to fallback to looking at the folder instead.
### Motivation
Pulumi plugin binaries can be downloaded by the CLI from multiple
sources. By default, it's downloaded from Pulumi's GitHub releases or
get.pulumi.com, but plugins can also specify their binary sources via
the `PluginDownloadURL` schema option. They can point to custom GitHub,
Gitlab, or HTTP locations.
Enterprise customers ask for a way to isolate the CLI from downloads
from random locations and to configure the CLI to go to their internal
pre-approved artefact location instead. This way, Pulumi can run in
"air-gapped" environments (which still have access to Cloud APIs, of
course).
Related issues:
- https://github.com/pulumi/pulumi/issues/14459
- https://github.com/pulumi/pulumi/issues/16240
Currently, there is a basic mechanism to do so via the variable
`pluginDownloadURLOverrides`, but it has two major limitations:
- The variable value is set via a compile-time flag, so it requires a
custom build of the CLI
- The overrides are based on the plugin name, so the rules must be
defined without access to the original URL, which makes it hard to
provide universal rules and still distinguish between first-party,
public third-party, or private in-house plugins
- We ignore overrides for all plugins that have `PluginDownloadURL` set
- Overrides can set a plugin replacement redirect only to HTTP(s)
addresses
### Proposal
This PR makes two sets of changes:
1. It allows passing overrides via the
`PULUMI_PLUGIN_DOWNLOAD_URL_OVERRIDES` environment variable. The
compile-time flag is still supported, but the env var takes priority.
More configuration levers could be supported, but it not clear if we
have good ones until [Support .pulumirc file for global
config](https://github.com/pulumi/pulumi/issues/13484) is implemented. I
don't expect users to want to set this via their stack configs, but I'm
curious what others think. In any case, more sources can be added later.
2. The overrides now apply based on the original download URL, not just
on plugin names. Actually, it's the base URL of a download source that
is passed to the regexp matcher. Examples of possible options are:
- `github://api.github.com/pulumi/pulumi-xyz` for a first-party plugin
(note that we don't pass `get.pulumi.com`
- `github://api.github.com/pulumiverse/pulumi-grafana` for a community
plugin that sets `PluginDownloadURL`
- `gitlab://gitlab-host/proj-name` for a community plugin hosted on
Gitlab
- `https://example.com/downloads/` for HTTP sources
So, the override
`^github://api.github.com/pulumi/pulumi-xyz=https://example.com/downloads/pulumi-xyz/`
will override the single provider URL from our GitHub releases to the
given HTTP location.
On top of that, regular expressions may contain name groups to capture
and use templated values. For example,
`^github://api.github.com/(?P<org>[^/]+)/(?P<repo>[^/]+)=https://example.com/downloads/${org}/${repo}`
captures any GitHub plugin and redirects it to its corresponding HTTP
location. Group indices are also supported: the above override can also
be written as
`^github://api.github.com/(?P<org>[^/]+)/(?P<repo>[^/]+)=https://example.com/downloads/$1/$2`,
with `$0` meaning the full match.
The override URLs have the same semantics as `PluginDownloadURL`, so
they can point to GitHub, Gitlab, HTTP, or anything we introduce in the
future.
### Impact
Technically, this is a breaking change, because name-based overrides
will stop working. However, we are fairly certain that we have a single
customer using the existing compile-time approach, and they indicated
that they don't need the name-based overrides if they have URL-based
overrides. I reviewed this PR with them and made sure they can migrate
immediately after the change is released.
Backwards compatibility is slightly tricky, because we'd need to keep
name-based override _and_ not applying them to third-party plugins. But
we can do it if necessary.
Resolve#16240
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
Overriding plugin download URLs with compilation flags was originally
added in #8798. Its intent was allowing our customers to override
download locations for all plugins, so that only trusted pre-approved
plugins could be downloaded.
Since then, we've added `PluginDownloadURL` for a package, which is the
default URL for that package's binary if it's shipped outside our Pulumi
org. Currently, `PluginDownloadURL` takes precedence over
`pluginDownloadURLOverrides`, which means it's impossible to override
third-party package binary locations.
This PR changes plugin source resolution to flip the priority of those
two. If an override matches regex, its URL will take priority over the
default `PluginDownloadURL` specified in the package.
I have added tests to verify `pluginDownloadURLOverrides` with and
without `PluginDownloadURL`. The second one fails before my change.
Resolves#16058
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [ ] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
This PR moves PluginKind to apitype to prevent circular dependencies
when adding apitype as a dependency of the workspace module.
It also re-exports PluginKind to keep backward compatibility
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [x] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
Combination of a few cleanups.
1. Don't call .Error() on errors that are being passed to "%s" format
functions. Format will call `Error()` itself.
2. Don't call assert.Error then assert.Equal/Contains, just use
assert.ErrorEqual/ErrorContains instead.
3. Use "%w" if appropriate, instead of "%v"/"%s".
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
Prompted by a comment in another review:
https://github.com/pulumi/pulumi/pull/14654#discussion_r1419995945
This lints that we don't use `fmt.Errorf` when `errors.New` will
suffice, it also covers a load of other cases where `Sprintf` is
sub-optimal.
Most of these edits were made by running `perfsprint --fix`.
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [x] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
This is a pass over all of /sdk to replace asserts that just checked we
had an error with asserts for what the error value is.
Just checking for an error is a weak test that can result in error paths
being broken and tests not detecting it.
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
Fixes https://github.com/pulumi/pulumi/issues/14680.
Updated the plugin logic (which we still use when no explict version is
given) to prefer selecting a stable version over a pre-release version
when no explict requested version is given.
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [ ] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
Allows `${NAME}` in the download url template string.
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [x] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
This PR implements the new policy transforms feature, which allows
policy packs to not only issue warnings and errors in response to policy
violations, but actually fix them by rewriting resource property state.
This can be used, for instance, to auto-tag resources, remove Internet
access on the fly, or apply encryption to storage, among other use
cases.
Upgrade to latest version of golangci-lint
and fix or opt-out the issues it caught.
The false positives are:
```
sdk/go/common/workspace/plugins_test.go:512:3: G101: Potential hardcoded credentials (gosec)
pkg/resource/deploy/builtins.go:180:2: G101: Potential hardcoded credentials (gosec)
```
The fixed issues are:
```
pkg/resource/deploy/deploytest/pluginhost.go:440:16: G601: Implicit memory aliasing in for loop. (gosec)
Version: &v.version,
^
pkg/engine/lifecycletest/alias_test.go:58:25: G601: Implicit memory aliasing in for loop. (gosec)
DeleteBeforeReplace: &r.deleteBeforeReplace,
^
```
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
Also fix an issue where if a platform was missing a checksum it would
error with "invalid checksum, expected , actual 01234".
None of the language runtimes yet return anything for this, but it's a
simple plumbing to expose it for the future.
We'll _probably_ start adding checksums to the pulumi-plugin.json files,
and then GetRequiredPlugins can simply return that data.
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [x] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
By default Pulumi will load ambient plugins from $PATH before looking in
the plugins directory or at bundled plugins.
While this is very useful for development it often causes confusion when
people have forgotten that they have plugins left on $PATH.
This makes the use of these $PATH plugins a diagnostic warning to try
and make that failure mode a little less silent.
Normal users shouldn't ever have plugins on $PATH and so won't see this
new warning.
Re-instates https://github.com/pulumi/pulumi/pull/13607 with a fix for
symlinks included.
This commit adds a check to `pulumi plugin install` to confirm
that the plugin is not bundled with Pulumi before performing the download.
Without this check, the CLI will produce a 404 error because the language
plugins aren't distributable independently of the CLI executable.
If PULUMI_DEV is set we'll skip the error and try to download anyway.
By default Pulumi will load ambient plugins from $PATH before looking in
the plugins directory or at bundled plugins.
While this is very useful for development it often causes confusion when
people have forgotten that they have plugins left on $PATH.
This makes the use of these $PATH plugins a diagnostic warning to try
and make that failure mode a little less silent.
Normal users shouldn't ever have plugins on $PATH and so won't see this
new warning.
Turns out this wasn't tested and regressed in
51cc6461df
when we started URL parsing the input string and so ended up with a text
string with "%7B" instead of "{" to replace.
This fixes the interpolation and changes a couple of the http tests to
actually use this feature so it's tested.
The retry logic wants to make at most 5 attempts
(that's what the "limit" says)
but it actually makes 6 requests.
This fixes the off-by-one error
by switching to the re-usable util/retry package
instead of re-implementing retries here.
As with util/retry.Retryer,
pluginDownloader allows overriding time.After from tests,
so that we don't sleep in tests.
All state for DownloadToFile was in a bunch of anonymous functions
that called each other.
The only way to inject new options was via new positional parameters.
This turns the core logic of DownloadToFile into
a pluginDownloader struct,
with retry and wrapper injected as fields rather than parameters.
It further breaks out all anonymous functions in DownloadToFile
into methods on that struct.
Their order has been retained to make this easy to review.
This change contains no logic changes.
Adds a failing test case for #12456
that validates the number of attempts we make
when downloading a plugin.
```
--- FAIL: TestDownloadToFile_retries (9.85s)
plugins_test.go:961:
Error Trace: [..]/pulumi/sdk/go/common/workspace/plugins_test.go:961
Error: Not equal:
expected: 30
actual : 5
Test: TestDownloadToFile_retries
Messages: number of attempts does not match number of requests
plugins_test.go:927:
Error Trace: [..]/pulumi/sdk/go/common/workspace/plugins_test.go:927
[..]/pulumi/sdk/go/common/workspace/plugins_test.go:963
Error: Not equal:
expected: 5
actual : 30
Test: TestDownloadToFile_retries
Messages: server received more requests than expected
```
Per team discussion, switching to gofumpt.
[gofumpt][1] is an alternative, stricter alternative to gofmt.
It addresses other stylistic concerns that gofmt doesn't yet cover.
[1]: https://github.com/mvdan/gofumpt
See the full list of [Added rules][2], but it includes:
- Dropping empty lines around function bodies
- Dropping unnecessary variable grouping when there's only one variable
- Ensuring an empty line between multi-line functions
- simplification (`-s` in gofmt) is always enabled
- Ensuring multi-line function signatures end with
`) {` on a separate line.
[2]: https://github.com/mvdan/gofumpt#Added-rules
gofumpt is stricter, but there's no lock-in.
All gofumpt output is valid gofmt output,
so if we decide we don't like it, it's easy to switch back
without any code changes.
gofumpt support is built into the tooling we use for development
so this won't change development workflows.
- golangci-lint includes a gofumpt check (enabled in this PR)
- gopls, the LSP for Go, includes a gofumpt option
(see [installation instrutions][3])
[3]: https://github.com/mvdan/gofumpt#installation
This change was generated by running:
```bash
gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error)
```
The following files were manually tweaked afterwards:
- pkg/cmd/pulumi/stack_change_secrets_provider.go:
one of the lines overflowed and had comments in an inconvenient place
- pkg/cmd/pulumi/destroy.go:
`var x T = y` where `T` wasn't necessary
- pkg/cmd/pulumi/policy_new.go:
long line because of error message
- pkg/backend/snapshot_test.go:
long line trying to assign three variables in the same assignment
I have included mention of gofumpt in the CONTRIBUTING.md.
Stop using io/ioutil across the entire repository.
The io/ioutil package was deprecated in Go 1.16 (2021-02)
with replacements provided in other packages.
Specifically:
ioutil.Discard => io.Discard
ioutil.NopCloser => io.NopCloser
ioutil.ReadAll => io.ReadAll
ioutil.ReadFile => os.ReadFile
ioutil.TempDir => os.MkdirTemp
ioutil.TempFile => os.CreateTemp
ioutil.WriteFile => os.WriteFile
This change switches all of these entities
across the repository.
Following this change,
the only references to ioutil are in schema files:
% rg -l ioutil
pkg/codegen/testing/test/testdata/aws-4.26.0.json
pkg/codegen/testing/test/testdata/aws-4.36.0.json
pkg/codegen/testing/test/testdata/aws-4.37.1.json
pkg/codegen/testing/test/testdata/aws-5.4.0.json
pkg/codegen/testing/test/testdata/aws-5.16.2.json
The bulk of this change was generated automatically
with manual touch ups afterwards.
This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `ioutil.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* Install missing plugins when encountering them
* changelog entry
* Update CHANGELOG_PENDING.md
Co-authored-by: Fraser Waters <fraser@pulumi.com>
* lint
* Move auto install logic to getPluginInfoAndPath
* clean up missing plugin error and its corresponding tests
* Introduce PluginInstallError which give users more context and action they could perform in case auto plugin install fails
* lint
Co-authored-by: Fraser Waters <fraser@pulumi.com>
* Add support for SHA256 checksums in PluginSpec
* Update sdk/go/common/workspace/plugins.go
Co-authored-by: Ian Wahbe <ian@wahbe.com>
* lint
Co-authored-by: Ian Wahbe <ian@wahbe.com>
PluginSpec is used to specifiy a plugin, and is what is passed to things
like "Install". PluginInfo is used to refer to an installed plugin, and
so has extra data like file sizes, and time stamps, but does not include
things like plugin download url.
This commit replaces `os.Setenv` with `t.Setenv` in tests. The
environment variable is automatically restored to its original value
when the test and all its subtests complete.
Reference: https://pkg.go.dev/testing#T.Setenv
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* demo
* modifications for serialization
* Provisionally changed plugins from map to array
* warnings for duplicate
* avoid breaking change
* avoid null pointer dereference
* added test
* Delete Pulumi.yaml
* ensurePluginsAreInstalled
* lint
* reworked NewContext and added kind
* auto-detect current project for YAML
* lint
* removed debug statement
* automatically modify local paths
* typo
* First return value of GetPluginPath was never used
* Always use the path returned from getPluginInfoAndPath in GetPluginPath
Also assert that Path is the correct directory for PluginInfo.
* address comments
* added language, analyzers
* path tweaks and cosmetic changes
* changelog + tweaks
* changed NewContextWithRoot to accept plugins instead of project
* Fix TestUnmarshalProjectWithProviderList
* Fix NewContext
* Fix comment
Co-authored-by: Fraser Waters <fraser@pulumi.com>
* Improve the plugin error to include the full name of the binary
* update changelog
* fix second error message
* PR feedback updates
* lint
* GetPluginPath shouldn't return empty path and no error
Co-authored-by: komal <komal@pulumi.com>
Co-authored-by: Fraser Waters <fraser@pulumi.com>
* Support plugins from private Pulumi repos
This uses the same environment vars that we we're using for experimental private repos. That is GITHUB_TOKEN or GITHUB_ACTOR and GITHUB_PERSONAL_ACCESS_TOKEN.
This allows us to develop plugins in private repos but continue to use our standard plugin flow.
* Add test
* Add to CHANGELOG
* Only use GITHUB_TOKEN
As described in https://docs.github.com/en/rest/overview/resources-in-the-rest-api#authentication we should be using the Authorization header.
* Fix tests
* Improve download tests
* lint
* Unauth tests need to make sure GITHUB_TOKEN is clear
* Update CHANGELOG
* Log a warning about GITHUB_PERSONAL_ACCESS_TOKEN