<!---
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
Use `SYSTEM_PULLREQUEST_SOURCECOMMITID` when AZ pipelines checks against
a Github PR. Fix made based on investigation from @blampe in #15072Fixes#15072
## Checklist
- [ ] I have run `make tidy` to update any new dependencies
- [ ] 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. -->
- [ ] 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. -->
Adds support for suppressing the periodic "..." printing that can
disrupt normal output stream. This output is still deemed necessary to
include by default for CI systems that might otherwise cancel an update
that goes for a long time without printing anything. But we now have an
option to turn this off.
Notes:
1. We want to expose this via Automtation API, and may even want to
default to off via Automation API?
Fixes https://github.com/pulumi/pulumi/issues/14069.
Related https://github.com/pulumi/pulumi/issues/11139.
<!---
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. -->
We're moving away from referring to filestate as "self managed"
backends, preferring to refer to this as "DIY" backends going forward.
## 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
This allows pulumi's log output to contain the location of where the log
call was made from, rather than it always reporting the line in `log.go`
where the glog call was made.
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
This changes
```
I0124 20:53:35.244319 169013 log.go:81] skipping update check
I0124 20:53:36.262079 169013 log.go:81] found username for access token
```
to
```
I0129 14:08:42.918230 38925 pulumi.go:231] skipping update check
I0129 14:08:45.056866 38925 backend.go:615] found username for access token
```
## 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
- This is hard to test because it is difficult to reliably intercept
stderr from within go tests
<!---
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. -->
Co-authored-by: Paul Roberts <proberts@pulumi.com>
Turn on the golangci-lint exhaustive linter. This is the first step
towards catching more missing cases during development rather than
in tests, or in production.
This might be best reviewed commit-by-commit, as the first commit turns
on the linter with the `default-signifies-exhaustive: true` option set,
which requires a lot less changes in the current codebase.
I think it's probably worth doing the second commit as well, as that
will get us the real benefits, even though we end up with a little bit
more churn. However it means all the `switch` statements are covered,
which isn't the case after the first commit, since we do have a lot of
`default` statements that just call `assert.Fail`.
Fixes#14601
## 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. -->
github.com/golang/protobuf is marked deprecated and I was getting
increasingly triggered by the inconsistency of importing the `Empty`
type from "github.com/golang/protobuf/ptypes/empty" or
"google.golang.org/protobuf/types/known/emptypb" as "pbempty" or "empty"
or "emptypb". Similar for the struct type.
So this replaces all the Protobufs imports with ones from
"google.golang.org/protobuf", normalises the import name to always just
be the module name (emptypb), and adds the depguard linter to ensure we
don't use the deprecated package anymore.
## 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. -->
This value was never actually used or correctly implemented, it was just
being used as a sentinal value for `Set/GetGlobalColorization`. We can
just use a pointer instead.
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.
# Description
`FileCopy` silently swallows file not found errors which leads to hard
to debug issues. For instance, as described in #14694, tests with edit
steps unexpectedly run twice against the first step.
Fixes#14694Fixes#5230
Merging this will break plenty of existing tests if they're not fixed
first. [This
query](https://github.com/search?q=org%3Apulumi+path%3A_test.go+%2F%5E%5Ct*Dir%3A+*%22%2F&type=code)
is a rough indication.
## 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.
-->
- [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. -->
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
<!---
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 adds a new type `tokens.StackName` which is a relatively strongly
typed container for a stack name. The only weakly typed aspect of it is
Go will always allow the "zero" value to be created for a struct, which
for a stack name is the empty string which is invalid. To prevent
introducing unexpected empty strings when working with stack names the
`String()` method will panic for zero initialized stack names.
Apart from the zero value, all other instances of `StackName` are via
`ParseStackName` which returns a descriptive error if the string is not
valid.
This PR only updates "pkg/" to use this type. There are a number of
places in "sdk/" which could do with this type as well, but there's no
harm in doing a staggered roll out, and some parts of "sdk/" are user
facing and will probably have to stay on the current `tokens.Name` and
`tokens.QName` types.
There are two places in the system where we panic on invalid stack
names, both in the http backend. This _should_ be fine as we've had long
standing validation that stacks created in the service are valid stack
names.
Just in case people have managed to introduce invalid stack names, there
is the `PULUMI_DISABLE_VALIDATION` environment variable which will turn
off the validation _and_ panicing for stack names. Users can use that to
temporarily disable the validation and continue working, but it should
only be seen as a temporary measure. If they have invalid names they
should rename them, or if they think they should be valid raise an issue
with us to change the validation code.
## 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.
-->
- [ ] 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. -->
These changes colorize the header row of each table printed by the CLI
using the `SpecHeadline` style (bold + pinkish). This improves the
readability of the tables on rich terminals.
<!---
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
Fixes a panic that occurs when attempting to encode certain values, for
example a map that contains a nil value:
```go
bag := bagtag{
String: "something",
StringOpt: "ohmv",
MapOpt: map[string]interface{}{
"a": "something",
"b": nil, // this reflect.Value is of type Interface and IsNil is true.
},
}
_ = resource.NewPropertyMap(bag) // panic here
```
New tests are provided to cover the above scenario and to cover each
type of value, including nil cases where appropriate.
Fixes: https://github.com/pulumi/pulumi-kubernetes/issues/2622
Follow-up to: https://github.com/pulumi/pulumi/pull/9810
## 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. -->
Likewise `require.NoError` instead of `require.Nil`, and `assert.Error`
rather than `assert.NotNil`.
The error variants of these functions print the errors nicer for test
failures using `Error()` rather than `GoString()`.
For bail errors this is _much_ better than the `result.Result` days
where we now get errors like:
```
Error: Received unexpected error:
BAIL: inner error
```
instead of:
```
Error: Expected nil, but got: &simpleResult{}
```
Also print the bail error in `TestPlan.Run` so we can see the
description of it.
cmdutil has some special handling for hashicorp/go-multierror
so that multi-errors are printed cleanly in the form:
%d errors occurred:
1) foo
2) bar
...
In Go 1.20, the errors package got a native `errors.Join` function.
This adds support for errors.Join-based multi-errors to this logic.
These errors implement an `Unwrap() []error` method
which can be used to access the full list of errors.
We use that and then implement the same logic for formatting as before.
<!---
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#13831
This reverts commit 25901d95c4.
On linux, `pulumi up` hung during a `pulumi-command provider` panic
after this change was introduced.
Switch the cmdutil.ReadConsole and cmdutil.ReadConsoleNoEcho functions
to use the bubbletea library to render the prompt,
using the textinput widget provided by the accompanying bubbles library.
The resulting input widgets support arrow keys, back space,
and some basic readline-style bindings including Ctrl-A, Alt-B, etc.
I went through all uses of ReadConsole or ReadConsoleNoEcho.
Only the one in new.go had a non-compliant prompt that I had to adjust.
Note: One divergence in behavior I opted for was that
password prompts will echo '*' characters as the user is typing
and then no echo once they've accepted or canceled the value.
Previously, the prompt did not echo anything in either case.
<details>
<summary>
Introduction if you're unfamiliar with bubbletea
</summary>
bubbletea operates by modeling the widget state as
an immutable data structure that receives messages for events.
On receiving a message (key press, e.g.) the model's Update method
returns a new model instance representing its new state.
Update may also optionally return additional commands for the program,
e.g. stop running, or print something and move on.
The model's View method returns what should be drawn in the terminal
based on the model's current state.
This programming model makes it reasonably straightforward to unit test
some of the core functionality of independent widgets
as demonstrated in this PR.
</details>
Resolves#1565
---
Demos:
<details>
<summary>Plain text</summary>
![prompt-plain](https://github.com/pulumi/pulumi/assets/41730/66258fc8-f772-4d01-bc7c-1f7b116aebaa)
</details>
<details>
<summary>Secret</summary>
![prompt-secret](https://github.com/pulumi/pulumi/assets/41730/372f862e-9186-4d47-ba7d-0107c47f52f6)
</details>
<details>
<summary>Secret prompt with padding</summary>
![prompt-secret-2](https://github.com/pulumi/pulumi/assets/41730/e9b7c253-4c9d-4235-9fa6-197aa0522033)
</details>
Uses the new TerminateProcessGroup functionality introduced in #13792
to shut down plugins gracefully.
Graceful shutdown takes the following form:
- Send a termination signal (SIGINT or CTRL_BREAK_EVENT)
- Wait up to 1 second for the plugin to exit
- Kill it with SIGKILL
Note that TerminateProcessGroup kills the entire group
so we don't need a separate KillChildren and cmd.Process.Kill().
This change also deprecates cmdutil.KillChildren
since we shouldn't really be using SIGKILL as a first resort anyway.
Note that this does not modify the behavior of individual plugins.
Those will exit as usual but with a SIGINT instead of SIGKILL
terminating them.
Resolves#9780
**Background**
The result.Result type is used by our CLI implementation to communicate
how we want to exit the program.
Most `result.Result` values (built from errors with `result.FromError`)
cause the program to print the message to stderr and exit the program
with exit code -1.
The exception is `result.Bail()`, which indicates that we've already
printed the error message, and we simply need to `exit(-1)` now.
Our CLI command implementation use `cmdutil.RunResultFunc` which takes a
`func(...) result.Result` to implement this logic.
`cmdutil` additionally includes a `cmdutil.RunFunc` which takes a
`func(...) error` and wraps it in `RunResultFunc`, relying on
`result.FromError` for the conversion:
func RunFunc(run func(...) error) func(...) {
return RunResultFunc(func(...) result.Result {
if err := run(...); err != nil {
return result.FromError(err)
}
return nil
})
}
**Problem**
In CLI contexts where we're using an `error`, and we want to print an
error message to the user and exit, it's desirable to use diag.Sink to
print the message to the user with the appropriate level (error,
warning, etc.) and exit without printing anything else.
However, the only way to do that currently is by converting that
function to return `result.Result`, turn all error returns to
`result.FromError`, and then return `result.Bail()`.
**Solution**
This change introduces a `result.BailError` error that gets converted
into a `result.Bail()` when it passes through `result.FromError`.
It allows commands implementations that use `error` to continue
returning errors and still provide an ideal CLI experience.
It relies on `errors.As` for matching, so even if an intermediate layer
wraps the error with `fmt.Errorf("..: %w", ErrBail)`, we'll recognize
the request to bail.
BailError keep track of the internal error that triggered it, which
(when everything is moved off of result and onto error) means we'll
still be able to see the internal errors that triggered a bail during
debugging.
Currently debugging engine tests is pretty horrible because you often
just get back a `result.Result{err:nil}` with no information where in
the engine stack that came from.
**Testing**
Besides unit tests, this includes an end-to-end test for using
RunResultFunc with a bail error.
The test operates by putting the mock behavior in a fake test, and
re-running the test binary to execute *just that test*.
**Demonstration**
This change also ports the following commands to use BailError: cancel,
convert, env, policy rm, stack rm.
These command implementations are simple and were able to switch easily,
without bubbling into a change to a bunch of other code.
Adds a new **currently unused** function TerminateProcessGroup
that terminates all processes in a group gracefully.
It does so by first sending the process
a SIGINT on Unix systems, and CTRL_BREAK_EVENT on Windows,
and waiting a specified duration for the process to exit.
The choice of signals was very deliberate
and is documented in the comments for TerminateProcessGroup.
If the process does not exit in the given duration,
it and its child processes are forcibly terminated
with SIGKILL or equivalent.
Testing:
The core behaviors are tested against Python, Go, and Node.
The corner cases of signal handling are tested
against rogue Go processes.
The changes were experimented with in #13760.
Refs #9780
Files tagged with OS-specific build tags
should usually have a suffix in their name
indicating which OS they're narrowing to.
child_windows.go already does this,
but child.go aimed at Linux and macOS does not.
This change renames child.go to child_unix.go.
Further, it moves the go:build directives
outside the copyright block.
This adds support for SSH-style Git URLs, enabling folks to use
private repos for their templates.
For instance,
$ pulumi new git@github.com:acmecorp/templates/website
will now work as intended.
The `ssh_config` library handles finding the relevant SSH key for the
given host.
If the SSH key is protected by a password, the user will be prompted to
supply the password on-demand. (It is memoized to avoid asking multiple
times, as the template workflow requires using it more than once.) To
avoid prompting, the `PULUMI_GITSSH_PASSPHRASE` env var can be set.
Fixes#4872 and #5007.
These changes add the path to the current project relative to the root
of the VCS repository to update metadata. If VCS info is not available,
this metadata is not added.
Part of https://github.com/pulumi/home/issues/2946.
Fixes https://github.com/pulumi/pulumi/issues/12738https://github.com/pulumi/pulumi/pull/11834 turned on the prealloc
linter and changed a load of slice uses from just `var x T[]` to `x :=
make([]T, 0, preallocSize)`. This was good for performance but it turns
out there are a number of places in the codebase that treat a `nil`
slice as semnatically different to an empty slice.
Trying to test that, or even reason that through for every callsite is
untractable, so this PR replaces all expressions of the form `make([]T,
0, size)` with a call to `slice.Prealloc[T](size)`. When size is 0 that
returns a nil array, rather than an empty array.
Fixes https://github.com/pulumi/pulumi/issues/13117
This adds a new "--strict" flag to `pulumi convert` which defaults to
false. When strict is NOT set we bind the PCL with the extra options of
`SkipResourceTypechecking`, `AllowMissingVariables`, and
`AllowMissingProperties`. This will change some errors to warnings in
code generation.
The `strict` flag is sent over the gRPC interface to the Go/Node plugins
for their `GenerateProject` methods as they have to do PCL binding
plugin side currently.
13030: [sdk] Update uniseg r=pgavlin a=pgavlin
The latest version of this module dramatically improves its allocation volume and offers a friendlier API for measuring string width.
Co-authored-by: Pat Gavlin <pat@pulumi.com>
`glog.Verbose(int)` returns a bool that indicates whether or not
logging is enabled at the given level. Check that result prior to
actually calling the logger in order to avoid extra work when verbose
logging is disabled.
The current memory profile reports the live objects and their sources at
the time at which it is captured. This is almost never what we want:
instead, we want a profile of _all_ allocations over the lifetime of the
process. These changes replace the heap profile with an allocation
profile and add a parameter, --memprofilerate, to control the rate at
which allocations are profiled.
The prior commit discovered that our retry logic backs off
by a longer delay than it needs to.
It doesn't respect the initial "delay" configuration,
and instead begins with "delay * backoff".
This change fixes the issue and updates the test expectations.
This extracts the core logic in the retry package into a Retryer struct.
The Retryer struct supports an optional "After" field,
which allows injecting a custom time.After implementation.
With this, we're able to write tests for this package
without actually sleeping (and slowing down the test suite).
Note that the tests exposed two surprising behaviors:
- If the context times out or expires, we correctly return `false`,
but we return a nil error instead of `ctx.Err()`.
I've chosen to not change this at this time
because we don't know what that might break downstream.
- The initial delay is never used. We start with delay*backoff.
Fixes the following issues found by revive
included in the latest release of golangci-lint.
Full list of issues:
**pkg**
```
backend/display/object_diff.go:47:10: superfluous-else: if block ends with a break statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (revive)
backend/display/object_diff.go:716:12: redefines-builtin-id: redefinition of the built-in function delete (revive)
backend/display/object_diff.go:742:14: redefines-builtin-id: redefinition of the built-in function delete (revive)
backend/display/object_diff.go:983:10: superfluous-else: if block ends with a continue statement, so drop this else and outdent its block (revive)
backend/httpstate/backend.go:1814:4: redefines-builtin-id: redefinition of the built-in function cap (revive)
backend/httpstate/backend.go:1824:5: redefines-builtin-id: redefinition of the built-in function cap (revive)
backend/httpstate/client/client.go:444:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
backend/httpstate/client/client.go:455:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
cmd/pulumi/org.go:113:4: if-return: redundant if ...; err != nil check, just return error instead. (revive)
cmd/pulumi/util.go:216:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
codegen/docs/gen.go:428:2: redefines-builtin-id: redefinition of the built-in function copy (revive)
codegen/hcl2/model/expression.go:2151:5: redefines-builtin-id: redefinition of the built-in function close (revive)
codegen/hcl2/syntax/comments.go:151:2: redefines-builtin-id: redefinition of the built-in function close (revive)
codegen/hcl2/syntax/comments.go:329:3: redefines-builtin-id: redefinition of the built-in function close (revive)
codegen/hcl2/syntax/comments.go:381:5: redefines-builtin-id: redefinition of the built-in function close (revive)
codegen/nodejs/gen.go:1367:5: redefines-builtin-id: redefinition of the built-in function copy (revive)
codegen/python/gen_program_expressions.go:136:2: redefines-builtin-id: redefinition of the built-in function close (revive)
codegen/python/gen_program_expressions.go:142:3: redefines-builtin-id: redefinition of the built-in function close (revive)
codegen/report/report.go:126:6: redefines-builtin-id: redefinition of the built-in function panic (revive)
codegen/schema/docs_test.go:210:10: superfluous-else: if block ends with a continue statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (revive)
codegen/schema/schema.go:790:2: redefines-builtin-id: redefinition of the built-in type any (revive)
codegen/schema/schema.go:793:4: redefines-builtin-id: redefinition of the built-in type any (revive)
resource/deploy/plan.go:506:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
resource/deploy/snapshot_test.go:59:3: redefines-builtin-id: redefinition of the built-in function copy (revive)
resource/deploy/state_builder.go:108:2: redefines-builtin-id: redefinition of the built-in function copy (revive)
```
**sdk**
```
go/common/resource/plugin/context.go:142:2: redefines-builtin-id: redefinition of the built-in function copy (revive)
go/common/resource/plugin/plugin.go:142:12: superfluous-else: if block ends with a break statement, so drop this else and outdent its block (revive)
go/common/resource/properties_diff.go:114:2: redefines-builtin-id: redefinition of the built-in function len (revive)
go/common/resource/properties_diff.go:117:4: redefines-builtin-id: redefinition of the built-in function len (revive)
go/common/resource/properties_diff.go:122:4: redefines-builtin-id: redefinition of the built-in function len (revive)
go/common/resource/properties_diff.go:127:4: redefines-builtin-id: redefinition of the built-in function len (revive)
go/common/resource/properties_diff.go:132:4: redefines-builtin-id: redefinition of the built-in function len (revive)
go/common/util/deepcopy/copy.go:30:1: redefines-builtin-id: redefinition of the built-in function copy (revive)
go/common/workspace/creds.go:242:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
go/pulumi-language-go/main.go:569:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
go/pulumi-language-go/main.go:706:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
go/pulumi/run_test.go:925:2: redefines-builtin-id: redefinition of the built-in type any (revive)
go/pulumi/run_test.go:933:3: redefines-builtin-id: redefinition of the built-in type any (revive)
nodejs/cmd/pulumi-language-nodejs/main.go:778:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
python/cmd/pulumi-language-python/main.go:1011:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
python/cmd/pulumi-language-python/main.go:863:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
python/python.go:230:2: redefines-builtin-id: redefinition of the built-in function print (revive)
```
**tests**
```
integration/integration_util_test.go:282:11: superfluous-else: if block ends with a continue statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (revive)
```
Mark the non-f variants of these functions as deprecated,
pushing users to use the f variants
that require an accompanying error message.
staticcheck will prevent any new uses of these deprecated functions.
Resolves#12132
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.
Replaces all instances of `strings.Replace(s, old, new, -1)`
with the equivalent `strings.ReplaceAll(s, old, new)`.
This function has been available since Go 1.12.
Migrates all uses of contract.{Assert, AssertNoError, Require} in sdk/
to the `*f` variants that are required to provide more error context.
Step towards deprecating non-f variants entirely.
For context, `contract.Require` is similar to `contract.Assert`,
except it has a required parameter name as an argument:
func Require(cond bool, param string)
func Requiref(cond bool, param string, msg string, args ...any)
It includes the parameter name in the error message by default,
so the `msg` and `args` should only describe the constraint
without naming the parameter.
Refs #12132
We use a small logging wrapper around glog for most of our logging. The
global Errorf/Warningf/Infof methods in that module format the string
and args passed in then filter out any secrets before calling into glog
with the final string.
The wrapper also exposed a `V` method to only log if verbosity was set,
but that directly returned a glog.Verbose which mean the Infof calls to
that didn't run our secret filtering code.
This changes the logging wrapper to return a `VerboseShim` (Not a great
name, but `Verbose` was already taken in this module) which is simply a
new type over `glog.Verbose` with the same methods but it calls
`FilterString` on the inputs.
Technically a breaking change, but I think the uses of this will be
source compatible.
The implementation of `pulumi stack output` prints everything
to a global `io.Writer`.
This makes testing its output a bit hacky and unparalellizable
because we have to hijack `os.Stdout` to capture the output.
This changes it the command implementation
and all the functions it relies on
to accept an `io.Writer` as an argument.
This allows us to delete the os.Stdout hijacking hack
that was introduced in #11952,
and have those tests run in parallel, writing to an in-memory buffer.
11819: Cleanup diag error for protected resources r=dixler a=iwahbe
Change the error message for deleting protected resources so it sounds less like we tried and failed to delete the resource, and more like we chose not to.
Fixes#11816
11834: Preallocate slices with a known capacity. r=dixler a=RobbieMcKinstry
Enable the prealloc linter, which identifies slices with a known capacity, but are not preallocated, which results in unnecessary allocations and memcpys.
<!---
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. -->
## Checklist
<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] N/A: enabling a linter. I have added tests that prove my fix is effective or that my feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [ ] **N/A: not a user-facing change.** 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 Service,
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 Service API version
<!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->
11842: Changelog and go.mod updates for v3.51.1 r=dixler a=justinvp
11845: Move SecretsProvider to pkg/secrets r=dixler a=Frassle
<!---
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. -->
Trying to push DefaultSecretsProvider up past the backends resulted in a module loop between pkg/resource/stack and pkg/resource/deploy. This places SecretProvider in the secrets module to avoid that.
## Checklist
<!--- 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 - N/A Just moving interface declaration
<!---
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change - N/A Internal code changes
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
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 Service API version - No
<!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->
Co-authored-by: Ian Wahbe <ian@wahbe.com>
Co-authored-by: Robbie McKinstry <robbie@pulumi.com>
Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
Co-authored-by: Robbie McKinstry <thesnowmancometh@gmail.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Fraser Waters <fraser@pulumi.com>
11829: golangci-lint: Don't disable megacheck r=abhinav a=abhinav
As a pre-step to enabling staticcheck (#11808)
drop megacheck from the list of disabled linters.
This was suppressing the output of some other linters
that are enabled by default including gosimple and unused.
## Issues fixed
### pkg
```
cmd/pulumi/preview.go:199:4: S1011: should replace loop with `targetURNs = append(targetURNs, targets...)` (gosimple)
cmd/pulumi/preview.go:204:4: S1011: should replace loop with `replaceURNs = append(replaceURNs, replaces...)` (gosimple)
cmd/pulumi/refresh.go:248:4: S1011: should replace loop with `targetUrns = append(targetUrns, *targets...)` (gosimple)
cmd/pulumi/up.go:130:3: S1011: should replace loop with `targetURNs = append(targetURNs, targets...)` (gosimple)
cmd/pulumi/up.go:134:3: S1011: should replace loop with `replaceURNs = append(replaceURNs, replaces...)` (gosimple)
resource/stack/deployment.go:61:5: var `resourceSchema` is unused (unused)
codegen/hcl2/model/pretty/display.go:227:6: S1003: should use strings.ContainsRune(v, '\n') instead (gosimple)
codegen/hcl2/model/pretty/display.go:289:6: S1003: should use strings.ContainsRune(v, '\n') instead (gosimple)
codegen/nodejs/gen.go:110:2: S1029: should range over string, not []rune(string) (gosimple)
codegen/nodejs/gen_program.go:381:2: S1008: should use 'return makeValidIdentifier(outputName) != outputName' instead of 'if makeValidIdentifier(outputName) != outputName { return true }; return false' (gosimple)
codegen/go/gen_program_json.go:30:2: field `temps` is unused (unused)
codegen/go/gen_program_json.go:34:24: func `(*jsonSpiller).spillExpression` is unused (unused)
codegen/go/gen_program_json.go:12:6: type `jsonTemp` is unused (unused)
codegen/go/gen_program_expressions.go:542:21: func `(*generator).genRelativeTraversalExpression` is unused (unused)
codegen/go/gen_program_json.go:21:21: func `(*jsonTemp).Traverse` is unused (unused)
codegen/go/gen_program_json.go:31:2: field `count` is unused (unused)
codegen/go/gen_program.go:484:21: func `(*generator).getPkgContext` is unused (unused)
codegen/go/gen_program_json.go:17:21: func `(*jsonTemp).Type` is unused (unused)
codegen/go/gen_program_json.go:25:21: func `(*jsonTemp).SyntaxNode` is unused (unused)
codegen/go/gen_test.go:399:4: S1023: redundant break statement (gosimple)
codegen/python/gen_program.go:337:41: S1040: type assertion to the same type: destType already has type model.Type (gosimple)
backend/display/jsonmessage.go:176:27: func `(*messageRenderer).writeBlankLine` is unused (unused)
backend/display/progress.go:1239:21: S1012: should use `time.Since` instead of `time.Now().Sub` (gosimple)
codegen/hcl2/syntax/tokens.go:265:2: S1001: should use copy() instead of a loop (gosimple)
testing/integration/util.go:195:2: S1006: should use for {} instead of for true {} (gosimple)
testing/integration/program.go:456:3: S1011: should replace loop with `opts.OrderedConfig = append(opts.OrderedConfig, overrides.OrderedConfig...)` (gosimple)
testing/integration/program.go:690:3: S1038: should use t.Skipf(...) instead of t.Skip(fmt.Sprintf(...)) (gosimple)
codegen/dotnet/gen_program.go:255:21: func `(*generator).warnf` is unused (unused)
codegen/dotnet/gen.go:62:2: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
codegen/dotnet/gen_program.go:515:41: S1040: type assertion to the same type: destType already has type model.Type (gosimple)
codegen/schema/loader.go:116:9: S1039: unnecessary use of fmt.Sprintf (gosimple)
```
### sdk
```
python/cmd/pulumi-language-python/main.go:532:3: S1029: should range over string, not []rune(string) (gosimple)
go/common/resource/plugin/context.go:145:8: S1005: unnecessary assignment to the blank identifier (gosimple)
go/common/resource/plugin/context.go:144:3: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
go/common/util/env/env.go:198:9: S1025: the argument is already a string, there's no need to use fmt.Sprintf (gosimple)
go/pulumi/resource_set.go:9:22: func `resourceSet.any` is unused (unused)
go/pulumi/resource_set.go:17:22: func `resourceSet.has` is unused (unused)
go/pulumi/resource_set.go:13:22: func `resourceSet.delete` is unused (unused)
go/pulumi/types_test.go:444:12: S1040: type assertion to the same type: argsV.R already has type Resource (gosimple)
go/auto/stack.go:153:2: S1021: should merge variable declaration with assignment on next line (gosimple)
go/auto/stack.go:170:2: S1021: should merge variable declaration with assignment on next line (gosimple)
go/common/resource/asset_test.go:391:6: func `findRepositoryRoot` is unused (unused)
go/common/util/goversion/version.go:32:2: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
```
### tests
There were a couple functions marked unused
that are used by tests that are currently skipped.
I've added `//nolint:unused` for those.
Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>