It is unsafe to call `print` in a Python signalhandler. Instead we set a
flag and check this in the main busy loop.
Fixes https://github.com/pulumi/pulumi/issues/16472
---------
Co-authored-by: Justin Van Patten <jvp@justinvp.com>
These changes contain some minor refactorings to conditionally disable
the use of packages that are cannot be built for `GOOS=js GOARCH=wasm`.
With these edits, `pkg/display` can be built targeting WASM.
These changes act as a safeguard to ensure that we are not adding
additional code that will _prevent_ building `pkg/display` for WASM
targets. They are not sufficient to produce a version of the display
renderer that is appropriate for actual use in a WASM environment:
- The current renderer API is not well-suited for use outside the
context of the CLI
- The current event stream format has no versioning data
- Actually building this code into a WASM module results in an
unpleasantly large file (70M uncompressed, 13M gzipped)
These changes also add a size gate for the built WASM module. The gate
is set to the 110% of the size of the WASM module as of this commit. Our
goal is to lower the size of the WASM module over time; as we do so we
will tighten this gate.
Part of #13258.
<!---
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 commit adds the `--fully-qualify-stack-names` (or `-Q` for short)
global command-line argument, which when supplied will always print
stack names in the fully-qualified form of `organization/project/stack`.
Fixes#11081.
## 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. -->
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.
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. -->
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.
<!---
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.
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.
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.
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.
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.
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.
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.
* about error string: error string should not be capitalized or end with punctuation mark
* apply suggestions from code review
* lint: fix format error and simplify the code
Co-authored-by: Fraser Waters <frassle@gmail.com>
Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
* fix: data race on global var
* fix: data race on waiting for events to close
* fix: data race on URNs in test
* chore: note spurious data race, simplify outputstate
* chore: log error on reusing a resource state, as it will cause data races
* chore: remove note, reference PR number in comment
Fixes two bugs in how padding was calculated in PrintTable.
Firstly we remove all ANSI escape codes from the string before measuring
how wide it is. Secondly we measure glyph count (using rivo/uniseg) not
byte or rune count of the string.
Together these fix the padding/alignment issues I saw when using
PrintTable with plan output. They also slightly change the layout of
"pulumi stack", for example the below is printed with current master and
has 6 characters of space for padding between SecurityGroup and
web-secgrp:
```
Current stack resources (4):
TYPE NAME
pulumi:pulumi:Stack aws-cs-webserver-test
├─ aws:ec2/securityGroup:SecurityGroup web-secgrp
├─ aws:ec2/instance:Instance web-server-www
└─ pulumi:providers:aws default_4_25_0
```
While printed with this commit you only get 2 characters of space for
padding (which is correct, the column gap is set to " "):
```
Current stack resources (4):
TYPE NAME
pulumi:pulumi:Stack aws-cs-webserver-test
├─ aws:ec2/securityGroup:SecurityGroup web-secgrp
├─ aws:ec2/instance:Instance web-server-www
└─ pulumi:providers:aws default_4_25_0
```
Rotating a passphrase requires that the old passphrase is available via
one of the `PULUMI_CONFIG_PASSPHRASE` or `PULUMI_CONFIG_PASSPHRASE_FILE`
environment variables. This confuses `readPassphrase` when reading a new
passphrase, since that function checks the aforementioned environment
variables prior to reading from the console. The overall effect is that
it is impossible to rotate the passphrase for a stack using the
passphrase provider. These changes fix this by always reading from the
console when rotating a passphrase.
* Add trace proxying to fix sub-process trace collection when tracing to files
* Better func naming in test
* Avoid dealing with Windows path nightmare
* On Windows it is go.exe of course
* Rename operation to component to better align with existing trace output
* Make `async:true` the default for `invoke` calls (#3750)
* Switch away from native grpc impl. (#3728)
* Remove usage of the 'deasync' library from @pulumi/pulumi. (#3752)
* Only retry as long as we get unavailable back. Anything else continues. (#3769)
* Handle all errors for now. (#3781)
* Do not assume --yes was present when using pulumi in non-interactive mode (#3793)
* Upgrade all paths for sdk and pkg to v2
* Backport C# invoke classes and other recent gen changes (#4288)
Adjust C# generation
* Replace IDeployment with a sealed class (#4318)
Replace IDeployment with a sealed class
* .NET: default to args subtype rather than Args.Empty (#4320)
* Adding system namespace for Dotnet code gen
This is required for using Obsolute attributes for deprecations
```
Iam/InstanceProfile.cs(142,10): error CS0246: The type or namespace name 'ObsoleteAttribute' could not be found (are you missing a using directive or an assembly reference?) [/Users/stack72/code/go/src/github.com/pulumi/pulumi-aws/sdk/dotnet/Pulumi.Aws.csproj]
Iam/InstanceProfile.cs(142,10): error CS0246: The type or namespace name 'Obsolete' could not be found (are you missing a using directive or an assembly reference?) [/Users/stack72/code/go/src/github.com/pulumi/pulumi-aws/sdk/dotnet/Pulumi.Aws.csproj]
```
* Fix the nullability of config type properties in C# codegen (#4379)