When moving resources between projects that use the passphrase secret
manager, we automatically prompt for the secret, unless
PULUMI_CONFIG_PASSPHRASE or PULUMI_CONFIG_PASSPHRASE_FILE are set.
Unfortunately that can be a bit confusing when moving resources between
different projects, because the prompt does not specify in any way which
stack the passphrase is for.
Fix this by adding a little bit of output to the prompt, showing the
user the stack name for which they are supposed to enter the passphrase.
---------
Co-authored-by: Julien <julien@caffeine.lu>
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. -->
Fixes https://github.com/pulumi/pulumi/issues/7282
This is fairly simple, just grab the last deployment from the stack (we
should have one otherwise we wouldn't have any config to fetch either)
and pull the SecretsProviders data out the deployment data and translate
and insert it into the stack config.
## 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. -->
- [ ] I have added tests that prove my fix is effective or that my
feature works - I've manually checked this with a passphrase deployment.
Need to do the command split for "config refresh" to write up some unit
tests to cover this.
<!---
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. -->
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.
<!---
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 command is currently untested. This adds a couple of basic tests to
ensure that state and config secrets are changed.
## 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. -->
The state from a secret manager _always_ has to be something json
serialisable because we store it into the json state files. Rather than
allowing any `interface{}` to be returned here and then error'ing if it
happens to not be something that can be marshalled to JSON this changes
the interface to return a `json.RawMessage` moving the
marshalling/unmarshalling concerns into the individual implementations
that can pretty much guarantee valid structures.
Migrates all remaining usages of
`contract.Assert*` and `contract.Require*` to the f variants,
which require adding meaningful error messages.
There were a couple cases where a `testing.T` or `testing.B`
was already available.
For those, this uses t.FailNow or require.NoError.
Refs #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.
This means the secret providers just work in terms of
workspace.ProjectStack, mutate as they wish and let the higher level
work out if it should save the file or not. Rather than having each
secret manager maintain "should I save the file" code.
* Allow PULUMI_CONFIG_PASSPHRASE to be empty
This partially reverts 273fa3343d
I've left the behaviour that PULUMI_CONFIG_PASSPHRASE_FILE being empty is the same as unset because an empty file path isn't valid anyway.
* Update CHANGELOG
The bulk decryption logic in deployment deserialization did not recur
into slices or maps. This prevented the bulk decryption of nested secret
values. These changes fix that bug, improve test coverage, and refactor
the bulk decryption code for simplicity, clarity, and separation of
concerns.
Related to https://github.com/pulumi/home/issues/1842.
In #2736, caching was introduced inside the passphrase secrets provider to avoid prompting the user for a password multiple times in the same `pulumi` command. However, the cache is not used in all cases, which leads to double prompting and/or erroring if the passphrase is not set in the `PULUMI_CONFIG_PASSPHRASE` envvar. In some cases, the user is prompted for their passphrase and then the command fails because the passphrase wasn't set via the envvar.
This change addresses these issues. The net effect is that when using the passphrase secrets provider, you'll be prompted for your passphrase up-front once (if interactive and the envvar isn't set), and subsequent uses will use the cached value and won't prompt or error if the envvar isn't set.
Implementation note: This moves the prompting code inside the `passphrase` package. I was originally trying to avoid this, to avoid the package depending on `cmdutil.ReadConsoleNoEcho` and `cmdutil.Interactive`, but this lead to unnatural contortions elsewhere. It ended up being much cleaner to have the prompting happen inside the passphrase implementation. Functions that may prompt (when interactive and the envvar isn't set) include "Prompt" in their name.
Fixes: #6974
Passphrase Environment variables were set before loading the
secrets provider from state
Unfortunately, it seems that some users are using empty passphrases
and thus this newly introduced logic has broken their usecases
We now check that the environment variables are set - it doesn't
matter if they are set as empty, but the existance of an empty
environment variabe still suggests that it is an intentional
empty passphrase
This incorporates some post merge feedback from https://github.com/pulumi/pulumi/pull/6893
This doesn't change the functionality of any of the tests:
```
go test -v ./secrets/passphrase -count=1
=== RUN TestPassphraseManagerIncorrectPassphraseReturnsErrorCrypter
--- PASS: TestPassphraseManagerIncorrectPassphraseReturnsErrorCrypter (0.89s)
=== RUN TestPassphraseManagerIncorrectStateReturnsError
--- PASS: TestPassphraseManagerIncorrectStateReturnsError (0.00s)
=== RUN TestPassphraseManagerCorrectPassphraseReturnsSecretsManager
--- PASS: TestPassphraseManagerCorrectPassphraseReturnsSecretsManager (1.08s)
=== RUN TestPassphraseManagerNoEnvironmentVariablesReturnsError
--- PASS: TestPassphraseManagerNoEnvironmentVariablesReturnsError (0.00s)
PASS
ok github.com/pulumi/pulumi/pkg/v3/secrets/passphrase 2.270s
```
Fixes: #6286
When a user is using the passphrase provider and unsets the environment
variables that let them interact with the secrets provider, then would
get an error as follows:
```
▶ pulumi up -y -f
error: decrypting secret value: failed to decrypt: incorrect passphrase, please set PULUMI_CONFIG_PASSPHRASE to the correct passphrase
```
We are oging to change this error message to make it more obvious
to a user what the error is and how they need to fix it
```
▶ pulumi up -y -f
error: constructing secrets manager of type "passphrase": unable to find either `PULUMI_CONFIG_PASSPHRASE` nor `PULUMI_CONFIG_PASSPHRASE_FILE` when trying to access the Passphrase Secrets Manager. Please ensure one of these values are set to allow the operation to continue
```
Ideally, we would like to prompt the user for the passphrase at this
point rather than error, but the CLI could be in the middle of an
update operation as the same codepath is used for reading stackreference
values
* 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)
The change does two things:
- Reorders some calls in the CLI to prevent trying to create a secrets
manager twice (which would end up prompting twice).
- Adds a cache inside the passphrase secrets manager such that when
decrypting a deployment, we can re-use the one created earlier in
the update. This is sort of a hack, but is needed because otherwise
we would fail to decrypt the deployment, meaning that if you had a
secret value in your deployment *and* you were using local
passphrase encryption *and* you had not set PULUMI_CONFIG_PASSPHRASE
you would get an error asking you to do so.
Fixes#2729
This is helpful some round trip cases where we many not be able to
build the encrypter or decrypter but we will end up not needing
them. When we fail to load the manager, we return a manager that has
the correct state, but will error when it tries to preform any
operations. However, if there are no secrets in the deployment, these
methods will never be called and we'll be able to correctly roundtrip
checkpoints even without having access to the password (since there
were no secret values to decrypt or encrypt).
This change allows using the passphrase secrets manager when creating
a stack managed by the Pulumi service. `pulumi stack init`, `pulumi
new` and `pulumi up` all learned a new optional argument
`--secrets-provider` which can be set to "passphrase" to force the
passphrase based secrets provider to be used. When unset the default
secrets provider is used based on the backend (for local stacks this
is passphrase, for remote stacks, it is the key managed by the pulumi
service).
As part of this change, we also initialize the secrets manager when a
stack is created, instead of waiting for the first time a secret
config value is stored. We do this so that if an update is run using
`pulumi.secret` before any secret configuration values are used, we
already have the correct encryption method selected for a stack.
We move the implementations of our secrets managers in to
`pkg/secrets` (which is where the base64 one lives) and wire their use
up during deserialization.
It's a little unfortunate that for the passphrase based secrets
manager, we have to require `PULUMI_CONFIG_PASSPHRASE` when
constructing it from state, but we can make more progress with the
changes as they are now, and I think we can come up with some ways to
mitigate this problem a bit (at least make it only a problem for cases
where you are trying to take a stack reference to another stack that
is managed with local encryption).