Making this change has a couple of benefits.
Firstly (and honestly the main one) is it make codegen much simpler. We
just to emit a base64 string and/or other embedded byte array to the
generated SDK. Currently we need emit a full `proto.Value` expression
for each language, and that's not hard but it's also not trivial and
means more combinations of things per test per language.
Secondly it will allow providers to use more efficient encodings of
their parameter than JSON if there is one. I imagine some providers
might make the parameter value a protobuf message and parse that
(similar to what we do for transform functions), but they can easily
fallback to just treating the bytes as a JSON string if they want.
The only downside to this is the value is obfuscated in the generated
SDK and in the state file. Neither of those are really expected to be
viewed by users, so this feels like a minor loss.
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes#16072
* Fixes#16278
* Fixes#16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
<!---
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 plumbs the use of the Parameterize method and GetSchema into
get-schema (and gen-sdk partially, not that sdk gen can actually deal
with it yet properly).
It also happens to fix get-schema working better for shimless providers
(i.e. pointing to a folder and using the language host to run it).
## 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.
-->
- [ ] 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 reverts commit 75340dd942.
Fixes https://github.com/pulumi/pulumi/issues/16018.
This re-enables the locking and race detection. The locking is more
finely scoped to not be held over provider methods like Read/Update.
## 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
<!---
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. -->
Fixes https://github.com/pulumi/pulumi/issues/15984.
The bug was actually caused by a longstanding mistake in
`_collapse_providers` that didn't iterate the providers correctly if it
was passed as a `dict`. But worth having this test in full to sanity
check the other languages as well.
## 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. -->
The version we currently have doesn't support Go 1.22 properly, so it
throws a bunch of warnings locally when trying to run it with the latest
Go version installed. Just running the latest version locally also
doesn't quite work, since it throws a bunch of errors from the
perfsprint linter, which seems to have gotten stricter.
Upgrade to the latest version of golangci-lint, and fix all the errors
we're getting from it. Mostly done via `perfsprint -fix`, with some
manual changes that `perfsprint -fix` wouldn't touch.
<!---
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 PR enhances the testprovider to have a Pulumi schema, e.g. to be
able to use it with Pulumi YAML.
Related to https://github.com/pulumi/pulumi/issues/15788
## 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. -->
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 removes the dynamic resources from the transformation tests and
just uses the testprovider instead. As such we can now also write the Go
transform test in pretty much exactly the same way in the same place.
I've removed the golang_sdk test from lifecycletest has it's now covered
by this integration test.
This change fixes a regression in the step generator when looking for
old resources. When generating steps for a register resource event, we
previously looked for old resources first by the resource's URN and then
by aliases.
This regressed with #10819:
```diff
- for _, urnOrAlias := range append([]resource.URN{urn}, goal.Aliases...) {
+ aliases[urn] = struct{}{}
+ for urnOrAlias := range aliases {
```
Previously, aliases were in a slice and we always looked for the URN
first, then aliases.
With #10819, aliases changed to being stored in a map (a set). The URN
was added to the map before iterating over it, but there's no guarantee
it will be looked at first (iteration order for maps is unspecified),
and with the current behavior when there are aliases in the map, the URN
very likely won't come first.
This can lead to duplicate resources in the state (stack corruption)
when the wrong old resource is chosen.
The fix is to move back to always checking for old resources using the
URN first. We also move back to maintaining aliases in a slice for
consistent ordering.
Fixes#13848
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 change fixes the `DeletedWith` resource option in the Go, Node.js,
and Python SDKs and adds tests.
This feature was a community contribution and while there were engine
tests included with the original PR, there weren't any tests confirming
the functionality worked correctly from each SDK.
Here's a summary of the fixes:
* Go: The `DeletedWith` resource option was never usable as it accepted
a URN instead of a Resource. We discussed this internally a while back
and decided to go ahead and fix this. (Note: While changing the
signature is technically a breaking change, the feature is currently
unusable, so the change would not break anyone, so there's no need to
wait for a major version bump.)
* Node.js: The `deletedWith` resource option did not work at all from
the Node.js SDK because it was incorrectly passing the resource object
itself in the RegisterResource request, rather than the resource's
URN.
* Python: The `deleted_with` resource option did not work at all from
the Python SDK because it was incorrectly passing the resource object
itself in the RegisterResource request, rather than the resource's
URN.
A `FailsOnDelete` resource has been added to the testprovider, which
will fail when its `Delete` gRPC is called. The tests use this to ensure
`Delete` is not called for resources of this type with the `DeletedWith`
option specified.
Go treats comments that match the following regex as directives.
//[a-z0-9]+:[a-z0-9]
Comments that are directives don't show in an entity's documentation.
5a550b6951 (diff-f56160fd9fcea272966a8a1d692ad9f49206fdd8dbcbfe384865a98cd9bc2749R165)
Our code has `//nolint` directives that now show in the API Reference.
This is because these directives are in one of the following forms,
which don't get this special treatment.
// nolint:foo
//nolint: foo
This change fixes all such directives found by the regex:
`// nolint|//nolint: `.
See bottom of commit for command used for the fix.
Verification:
Here's the output of `go doc` on some entities
before and after this change.
Before
```
% go doc github.com/pulumi/pulumi/sdk/v3/go/pulumi | head -n8
package pulumi // import "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
nolint: lll, interfacer
nolint: lll, interfacer
const EnvOrganization = "PULUMI_ORGANIZATION" ...
var ErrPlugins = errors.New("pulumi: plugins requested")
```
After
```
% go doc github.com/pulumi/pulumi/sdk/v3/go/pulumi | head -n8
package pulumi // import "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
const EnvOrganization = "PULUMI_ORGANIZATION" ...
var ErrPlugins = errors.New("pulumi: plugins requested")
func BoolRef(v bool) *bool
func Float64Ref(v float64) *float64
func IntRef(v int) *int
func IsSecret(o Output) bool
```
Before
```
% go doc github.com/pulumi/pulumi/sdk/v3/go/pulumi URN_
package pulumi // import "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
func URN_(o string) ResourceOption
URN_ is an optional URN of a previously-registered resource of this type to
read from the engine. nolint: revive
```
After:
```
% go doc github.com/pulumi/pulumi/sdk/v3/go/pulumi URN_
package pulumi // import "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
func URN_(o string) ResourceOption
URN_ is an optional URN of a previously-registered resource of this type to
read from the engine.
```
Note that golangci-lint offers a 'nolintlint' linter
that finds such miuses of nolint,
but it also finds other issues so I've deferred that to a follow up PR.
Resolves#11785
Related: https://github.com/golangci/golangci-lint/issues/892
[git-generate]
FILES=$(mktemp)
rg -l '// nolint|//nolint: ' |
tee "$FILES" |
xargs perl -p -i -e '
s|// nolint|//nolint|g;
s|//nolint: |//nolint:|g;
'
rg '.go$' < "$FILES" | xargs gofmt -w -s
This allows the pulumi-language-go plugin to start up providers directly
from .go source files.
The other language providers will be extended to support this as well in
time.
* Add Attach call
* Regenerate grpc
* Start plumbing in changes
* Main doens't need a port
* Split Attach into grpc interface
* Change envvar format
* Type test for attach
* lint
* Reformat python
* Implement provider debug for nodejs
* Fix plugin close
* lint
* Add to CHANGELOG
* Set Kill
Co-authored-by: Daniel Bradley <daniel@pulumi.com>
We've had a few issues in the recent past related to pipenv oddities in
CI which lead us to temporarily globally install the Python SDK in CI.
This change removes the use of pipenv in favor of Python's built-in
venv and avoids globally installing the Python SDK.
Adds initial support for resource methods (via a new `Call` gRPC method similar to `Invoke`), with support for authoring methods from Node.js, and calling methods from Python.