Commit Graph

195 Commits

Author SHA1 Message Date
Will Jones 2aad59df18
Spot skipped-create dependencies even when inputs don't change ()
`Create`s happen in response to new resources being present in a Pulumi
program but not in Pulumi's state snapshot. For instance, take the
following TypeScript program:

```typescript
const b = new Resource("b", { x: 2 })
const c = new Resource("c", { x: 3 }, { deletedWith: b })
```

When asked to run this program on an empty state with no arguments,
Pulumi will issue two `Create` calls -- one for `b`, and one for `c`.
The call for `c` will *depend on* `b`'s due to the need for
`deletedWith` to refer to `b`'s URN.

If instead of passing no arguments we ask Pulumi to perform a *targeted*
operation that *only targets `c`*, Pulumi will throw an error and refuse
to execute the program. This is because `c`'s `Create` would depend on
`b`'s `Create`, but since `b` has not been targeted, its `Create` will
not happen and thus `c`'s call cannot be built. Internally, we call
`b`'s omission a "skipped create", and keep track of these so that we
can trigger the error above appropriately.

So far, so good. Now, consider that we have executed the above program
with no targets, so that Pulumi's state contains both `b` and `c`, with
`c` depending on `b` via a `deletedWith` link. We now add the missing
`a` to the program and modify `b` to depend on it:

```typescript
const a = new Resource("a", { x: 1 })
const b = new Resource("b", { x: 2 }, { deletedWith: a })
const c = new Resource("c", { x: 3 }, { deletedWith: b })
```

If we were to run the program with no targets, we would expect a state
in which all of `a`, `b` and `c` existed. `c` would depend on `b` (as it
did before) and `b` would have a new dependency on `a`. Lovely.

Of course, we are not about to run the program with no targets. We are
instead interested in *targeting `b` only*. When we do, the following
sequence of events will unfold:

* `a`, not existing in the state already and not being targeted, will
yield a skipped create.
* `b` depends on a skipped create. However, its *inputs* have not
changed (`x` was `2` before, and it's still `2` after). Consequently,
`b` will have yielded a `SameStep`. We *incorrectly* assume this means
that skipped creates are not a problem (`SameStep`s mean no changes,
right?) and allow the deployment to proceed.
* We write the new `b` to the state, with a dependency on the skipped
create, yielding a snapshot integrity error.
* 💥

We might ask "why assume that `SameStep`s are safe?". From looking at
the existing test cases, it seems likely that this was designed to cover
the case where a skipped create is depended on *by another skipped
create* (which is internally represented as a `SameStep`) -- in such
cases, we don't need to error and the deployment can proceed. However,
this bug shows that there are cases where `SameStep` does not imply
skipped create. This commit thus tightens up the logic appropriately,
checking explicitly for `SameStep`s whose `IsSkippedCreate` method
returns true.
2024-10-30 16:17:30 +00:00
Will Jones dc6f275f03
Fix dependency traversal for untargeted skipped creates ()
When performing a `--target`ed operation, it is possible that the new
program introduces new resources that are not targeted. In such cases,
we can't emit `CreateStep`s (since that would not respect the
`--target`s), but we need to track the creations so that e.g. later
resources that will come to depend on these would-be resources can be
handled appropriately. The step generator maintains a list of so-called
"skipped creates" to this end.

Unfortunately, when later traversing the list of skipped creates to see
if any resources depend on them, "depend on" does not take into account
property dependencies and deleted-with relationships. This results in
snapshot integrity errors as we end up writing the broken dependencies
to the state. This commit rewrites the code to use the new
`GetAllDependencies()` method on `resource.State` to take account of the
other possible relationships, adding lifecycle tests for these new cases
and cleaning up the existing tests to be more specific about the errors
they expect.
2024-09-23 15:02:37 +00:00
Will Jones ec1e3dded1
Centralize resource state dependency traversal ()
Resources can depend on others in a number of ways -- provider
references, parent-child relationships, dependencies and property
dependencies, and deleted-with relationships. With all these linkages to
keep track of, it's easy for one to be missed and bugs to be introduced,
and indeed this has happened on numerous occasions. This commit attempts
to make it harder to introduce bugs where dependencies are missed by
adding a new `GetAllDependencies` method to `resource.State`. The idea
is not that this will make all traversals of dependencies shorter and
neater (though in many cases it does), but that it will force iterations
over a resource's dependency set to account for all possible relations
appropriately, and discard any that are not important explicitly.

> [!NOTE]
> In some cases the use of `GetAllDependencies` means that algorithms
> which previously mutated an existing set of properties are
> easier/cleaner to express by building up a new set of dependencies and
> mutating after the loop's completion. This does mean a change of
> performance characteristics in some places but I don't think these are
> significant and for the most part they are off hot paths -- `state
move`
> and `state rename` are the most obvious examples.

Fixes 
2024-09-23 08:37:34 +00:00
Fraser Waters 524a22976e
Log alias generation to V(7) ()
Part of fixing https://github.com/pulumi/pulumi/issues/17279. This logs
the generated aliases, the matched alias, and the old URN it matched to
in the V(7) debug logs.
2024-09-17 07:24:33 +00:00
Fraser Waters 49d8298e84
Fix snapshot integrity on pending replacement ()
This fixes a snapshot integrity issue with delete before replace, failed
creates, and multiple updates.

This was caused by https://github.com/pulumi/pulumi/pull/16510 where we
started removing pending replace resources as part of their replacement
steps. A well intentioned fix to stop trying to delete resources we'd
already deleted in a previous update.

The bug would manifest if the following happened:
1. An update goes to replace a resource with `deleteBeforeCreate` set.
It deletes the old resource (and saves the state as pending replace) but
then failed to create the new replacement resource.
2. A second update is run to try and create the replacement resource. At
this point the bug manifest and we delete the pending replace resource
from state, but then if the new resource fails to create again we end up
with an invalid snapshot.

The fix is very simple. If a resource is already pending replacement we
just don't issue a delete/remove step at all. The pending replace
resource will get cleaned up at the end by the create step creating the
new version.

Fixes 
Fixes 
Fixes 
Fixes 
Fixes 
2024-09-04 10:52:43 +00:00
Fraser Waters fe4e33cfc1
Cleanup old comment ()
 has been fixed for a number of years, delete before
replace operations are now safe.
2024-09-03 15:24:09 +00:00
Will Jones 217187d9a8
Propagate deleted parents of untargeted resources ()
When using `--target` to target specific resources during an update, we
use the list of targets to decide which steps to generate given a set of
resource registrations. Specifically, *prior to #16247*:

* If a registration event named a resource that was targeted, we would
process it as usual.
* If a registration event named a resource that was not targeted, we
would emit a SameStep for it.

In , this behaviour was changed so that we'd consider the entire
dependency graph of an untargeted resource, in order to avoid missing
copying resources that had been removed from the program but not
targeted (and thus whose old states would be copied over later on,
potentially *after* the states of resources depending on them). As part
of this work, parent-child relationships were not handled. This did not
manifest in errors in the cases tested, but in the presence of aliases
and other edge cases, parent-child relationships *can* result in
snapshot integrity issues and thus must be handled too. This commit
implements those changes.

Fixes  (I believe)
2024-08-30 14:31:50 +00:00
Julien 7e2ccd8757
Prepare golangci-lint upgrade ()
The latest version of golangci-lint (1.60.3) flags a bunch of new issues
in our code base. This PR addresses part of them ahead of the upgrade.

* A dynamic string passed to printf style functions as first argument,
this can lead to bad `%` interpolations. The fix is typically to use
`"%s"` as first argument and pass the dynamic string as 2nd argument.
* Using `os.ModePerm` in tests instead of more restricted file
permissions. The fix is to use 0o600 for files, or 0o700 for
directories.
* Int conversion overflows. The fix has to be done case by case,
checking that no overflow can occur.
2024-08-28 07:57:38 +00:00
Will Jones 1be4888f7d
Replace `result.Result` with native errors ()
`result.Result` is a type that was introduced to enable us to
distinguish between *expected* errors (or *bails* in Pulumi parlance)
and *unexpected* errors or exceptions. Prior to Go 1.13, this made a lot
of sense, since there was no standard story on "wrapping" errors, and
thus no way to answer the question "did I end up here (in an error path)
because I meant to, or did I end up here because of a program bug?".

With the introduction of `%w`, `interface { Unwrap() []error }` and
company in Go 1.13, a simpler interface is available: one can use
`errors.Is` and `errors.As` to ask if an error at any point wraps an
error of a certain type. With this, we can simply ask "is there a bail
at any point in this error tree?" rather than having to track this
explicitly using a type such as `result.Result`. The `IsBail` function
was introduced a while ago to this end, but its rollout was not
completed and several uses of `result.Result` remained.

This commit completes the rollout of this simplified interface,
replacing all uses of `result.Result` with native Go errors that may or
may not wrap bails, and all uses of e.g. `res.IsBail` with
`IsBail(err)`. Doing so allows us to remove `result.Result` entirely.
2024-08-22 14:39:59 +00:00
Will Jones 3f27ee9688
Don't re-delete resources that are `PendingReplacement` ()
As well as indicating that a resource's state has changes, a diff can
also indicate that those changes require the _replacement_ of the
resource, meaning that it must be recreated and not just updated. In
this scenario, there are two possible ways to replace the resource -- by
first creating another new resource before deleting the old one
("create-before-replace"), or by first deleting the old resource before
creating its replacement ("delete-before-replace").
Create-before-replace is the default since generally, if possible to
implement, it should result in fewer instances of "downtime", where a
desired resource does not exist in the system.

Should delete-before-replace be chosen, Pulumi implements this under the
hood as three steps: delete for replacement, replace, and create
replacement. To track things consistently, as well as enable resumption
of an interrupted operation, Pulumi writes a flag, `PendingReplacement`
to the state of a deleted resource that will later be cleaned up by a
completed replacement.

Should an interrupted operation be resumed, Pulumi does not currently
take `PendingReplacement` into account, and always enqueues a(nother)
delete operation. This is typically fine (albeit wasteful) since deletes
are (should) be idempotent, but unnecessary. This commit adds
@jesse-triplewhale's fix for this behaviour whereby the
`PendingReplacement` flag is simply removed before the remainder of the
required steps (replace, create replacement) are actioned as normal. It
also extends this work with some lifecycle tests for this scenario and a
few others that may arise as a result of an interrupted replacement.

Fixes 
Closes 

Co-authored-by: Jesse Grodman <jesse@triplewhale.com>
2024-06-28 23:16:20 +00:00
Luke Hoban f1e4b4ff94
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes ()
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 
* Fixes 
* Fixes  
* Not quite , but likely replaces the need for that

Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
Will Jones f71c764a4c
Clean up deployment options ()
# Description

There are a number of parts of the deployment process that require
context about and configuration for the operation being executed. For
instance:

* Source evaluation -- evaluating programs in order to emit resource
registrations
* Step generation -- processing resource registrations in order to
generate steps (create this, update that, delete the other, etc.)
* Step execution -- executing steps in order to action a deployment.

Presently, these pieces all take some form of `Options` struct or pass
explicit arguments. This is problematic for a couple of reasons:

* It could be possible for different parts of the codebase to end up
operating in different contexts/with different configurations, whether
due to different values being passed explicitly or due to missed
copying/instantiation.
* Some parts need less context/configuration than others, but still
accept full `Options`, making it hard to discern what information is
actually necessary in any given part of the process.

This commit attempts to clean things up by moving deployment options
directly into the `Deployment` itself. Since step generation and
execution already refer to a `Deployment`, they get a consistent view of
the options for free. For source evaluation, we introduce an
`EvalSourceOptions` struct for configuring just the options necessary
there. At the top level, the engine configures a single set of options
to flow through the deployment steps later on.

As part of this work, a few other things have been changed:

* Preview/dry-run parameters have been incorporated into options. This
lets up lop off another argument and mitigate a bit of "boolean
blindness". We don't appear to flip this flag within a deployment
process (indeed, all options seem to be immutable) and so having it as a
separate flag doesn't seem to buy us anything.
* Several methods representing parts of the deployment process have lost
arguments in favour of state that is already being carried on (or can be
carried on) their receiver. For instance, `deployment.run` no longer
takes actions or preview configuration. While doing so means that a
`deployment` could be run multiple times with different actions/preview
arguments, we don't currently exploit this fact anywhere, so moving this
state to the point of construction both simplifies things considerably
and reduces the possibility for error (e.g. passing different values of
`preview` when instantiating a `deployment` and subsequently calling
`run`).
* Event handlers have been split out of the options object and attached
to `Deployment` separately. This means we can talk about options at a
higher level without having to `nil` out/worry about this field and
mutate it correctly later on.
* Options are no longer mutated during deployment. Presently there
appears to be only one case of this -- when handling `ContinueOnError`
in the presence of `IgnoreChanges` (e.g. when performing a refresh).
This case has been refactored so that the mutation is no longer
necessary.

# Notes

* This change is in preparation for , where we'd like to add an
environment variable to control behaviour and having a single unified
`Options` struct would make it easier to pass this configuration down
with introducing (more) global state into deployments. Indeed, this
change should make it easier to factor global state into `Options` so
that it can be controlled and tested more easily/is less susceptible to
bugs, race conditions, etc.
* I've tweaked/extended some comments while I'm here and have learned
things the hard way (e.g. `Refresh` vs `isRefresh`). Feedback welcome on
this if we'd rather not conflate.
* This change does mean that if in future we wanted e.g. to be able to
run a `Deployment` in multiple different ways with multiple sets of
actions, we'd have to refactor. Pushing state to the point of object
construction reduces the flexibility of the code. However, since we are
not presently using that flexibility (nor is there an obvious [to my
mind] use case in the near future), this seems like a good trade-off to
guard against bugs/make it simpler to move that state around.
* I've left some other review comments in the code around
questions/changes that might be a bad idea; happy to receive feedback on
it all though!
2024-06-11 13:37:57 +00:00
Ian Wahbe 78c48204e0
Normalize plugin.Provider methods to (Context, Request) -> (Response, error) ()
Normalize methods on plugin.Provider to the form:

```go
Method(context.Context, MethodRequest) (MethodResponse, error)
```

This provides a more consistent and forwards compatible interface for
each of our methods.

---

I'm motivated to work on this because the bridge maintains a copy of
this interface: `ProviderWithContext`. This doubles the pain of dealing
with any breaking change and this PR would allow me to remove the extra
interface. I'm willing to fix consumers of `plugin.Provider` in
`pulumi/pulumi`, but I wanted to make sure that we would be willing to
merge this PR if I get it green.

<!--- 
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 # (issue)

## 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.
-->
- [ ] 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. -->
2024-06-07 19:47:49 +00:00
Will Jones 6b4805672c
Propagate deleted dependencies of untargeted resources ()
When using `--target` to target specific resources during an update, we
use the list of targets to decide which steps to generate given a set of
resource registrations. Specifically:

* If the registration event names a resource that is targeted, we
process it as usual.
* If the registration event names a resource that _is not_ targeted, we
emit a `SameStep` for it.

In the latter case, the emission of a `SameStep` means that the old
state for the resource will be copied across to the new state. This is
the desired behaviour -- the resource was not targeted and so the new
state should contain the resource exactly as it was prior to the update.
However, this presents a problem if the old state has references to
resources that either will not appear in the new state, or will appear
in the wrong place. Consider the following program in TypeScript-esque
pseudocode:

```typescript
const a = new Resource("a")
const b = new Resource("b", { dependency: a })
const c = new Resource("c")
```

Here, `b` depends on `a`, while `a` and `c` have no dependencies. We run
this program without specifying targets and obtain a state containing
`a`, `b` and `c`, with `a` appearing before `b` due to `b`'s dependency
on `a`. We now modify the program as follows:

```typescript
const b = new Resource("b")
const c = new Resource("c")
```

`a` has been removed from the program and consequently `b` no longer
depends on it. We once more run the program, this time with a `--target`
of `c`. That is to say, neither `a` nor `b` is targeted. The execution
proceeds as follows:

* `a` is not in the program, so no `RegisterResourceEvent` will be
emitted and processed for it.
* `b` is in the program, but it is not targeted. Its
`RegisterResourceEvent` will be turned into a `SameStep` and `b`'s _old
state will be copied as-is to the new state_.
* `c` is in the program and is targeted. It will be processed as normal.

At the end of execution when we come to write the snapshot, we take the
following actions:

* We first write the processed resources: `b`'s old state and `c`'s new
state.
* We then copy over any unprocessed resources from the base (previous)
snapshot. This includes `a` (which is again desirable since its deletion
should not be processed due to it not being targeted).

Our snapshot is now not topologically sorted and thus invalid: `b` has a
dependency on `a`, but `a` appears after it. Presently this bug will
manifest irrespective of the nature of the dependency: `.Dependencies`,
`.PropertyDependencies` and `.DeletedWith` are all affected.

This commit fixes this issue by traversing all untargeted resource
dependency relationships and ensuring that `SameStep`s (or better if
they have been targeted) are emitted before emitting the depending
resource's `SameStep`.

* Fixes  
* Fixes 
2024-05-23 12:31:03 +00:00
Will Jones 6dfd57d554
Check property dependencies and `DeletedWith` for target dependents ()
The `--target-dependents` flag instructs Pulumi to include dependents of
specified targets. Presently we check dependencies, parents and
providers, but not property dependencies and `DeletedWith`
relationships. This commit fixes this so that these relationships are
correctly checked.
2024-05-17 15:07:13 +00:00
Mikhail Shilkov 60adf1823d
Don't bail at preview when a protected resource needs replacement ()
<!--- 
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

Don't bail at preview when a protected resource needs replacement, just
error. This makes sure that users can see the actual diff that causes
the replacement.

Fixes 

Before:

```
$ pulumi preview
Type                                     Name                  Plan     Info
     pulumi:pulumi:Stack                      azure-native-ts-dev2           
     └─ azure-native:resources:ResourceGroup  rg                             1 error

Diagnostics:
  azure-native:resources:ResourceGroup (rg):
    error: unable to replace resource "urn:pulumi:dev2::azure-native-ts::azure-native:resources:ResourceGroup::rg"
    as it is currently marked for protection. To unprotect the resource, remove the `protect` flag from the resource in your Pulumi program and run `pulumi up`


$ pulumi preview --diff
pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev2::azure-native-ts::pulumi:pulumi:Stack::azure-native-ts-dev2]
error: unable to replace resource "urn:pulumi:dev2::azure-native-ts::azure-native:resources:ResourceGroup::rg"
as it is currently marked for protection. To unprotect the resource, remove the `protect` flag from the resource in your Pulumi program and run `pulumi up`
Resources:
    1 unchanged
```

After:

```
$ pulumi preview
Type                                     Name                  Plan        Info
     pulumi:pulumi:Stack                      azure-native-ts-dev2              1 error; 2 warnings
 +-  └─ azure-native:resources:ResourceGroup  rg                    replace     [diff: ~location]; 1 error

Diagnostics:
  azure-native:resources:ResourceGroup (rg):
    error: unable to replace resource "urn:pulumi:dev2::azure-native-ts::azure-native:resources:ResourceGroup::rg"
    as it is currently marked for protection. To unprotect the resource, remove the `protect` flag from the resource in your Pulumi program and run `pulumi up`

  pulumi:pulumi:Stack (azure-native-ts-dev2):
    error: preview failed


$ pulumi preview --diff
pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev2::azure-native-ts::pulumi:pulumi:Stack::azure-native-ts-dev2]
error: unable to replace resource "urn:pulumi:dev2::azure-native-ts::azure-native:resources:ResourceGroup::rg"
as it is currently marked for protection. To unprotect the resource, remove the `protect` flag from the resource in your Pulumi program and run `pulumi up`
    +-azure-native:resources:ResourceGroup: (replace) 🔒
        [id=/subscriptions/0282681f-7a9e-424b-80b2-96babd57a8a1/resourceGroups/rg5f8e30e4]
        [urn=urn:pulumi:dev2::azure-native-ts::azure-native:resources:ResourceGroup::rg]
        [provider=urn:pulumi:dev2::azure-native-ts::pulumi:providers:azure-native::default_2_30_0::3c957b2a-4852-439c-b211-22a115bbe89a]
      ~ location: "westeurope" => "westus2"
error: preview failed
Resources:
    +-1 to replace
    1 unchanged
```

## 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. -->
- [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. -->
2024-04-18 16:45:52 +00:00
Fraser Waters 616e911c0f
Use a generic wrapper around `sync.Map` ()
This removes the type casts necessary when using `sync.Map` and ensures
correctness without needing to check other use sites.
2024-04-09 10:56:25 +00:00
Fraser Waters cdd14d21f6
Check for protect in replace chains ()
<!--- 
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/15763.

When doing a replace chain we need to check if the resources implicated
for deletion in the chain are actually ok to delete (i.e. they don't
have `protect` set). This change fixes the step generator to check that
when building the delete chain, and adds a test for it as well.

Sadly because of the nature of the system you have to `state unprotect`
these resources if you actually want them replaced, you can't just
change `protect` in the resource options in the program. This is because
at the point we're processing the "to be replaced" resource we've only
got the old state for the implicated resource because we won't have got
it's new goal state yet because it depends on the resource we're
currently processing.

## 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. -->
2024-03-25 20:07:36 +00:00
Thomas Gummerer 8099d314a7
destroy: implement --continue-on-error ()
Implement a `--continue-on-error` flag for `pulumi destroy`. This makes
sure we continue processing destroys even if errors occur. We will only
continue for resources which do not depend on the failed resource, to
make sure we always have a valid snapshot available, and to not leave
any orphaned resources behind.

Resources that fail to destroy will still continue to be managed by
pulumi, and the process ends in an error to indicate to the user that
there were problems and the destroy didn't succeed cleanly.

The output will continue to be the same as for failed destroys, except
we now may succeed in destroying more resources than before.

/cc https://github.com/pulumi/pulumi/issues/3304
2024-03-22 09:22:40 +00:00
Thomas Gummerer bc70928f67
Remove trustDependencies option ()
This option is always true these days, and we don't expect to set it
false for anything. Remove the flag for a bit of code cleanup.
2024-03-21 10:14:07 +00:00
Fraser Waters 08aad5997c
Fix panic when changing untargetted provider versions ()
<!--- 
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/15704.

When doing a targeted run the source evaluator isn't aware of targets
but it is responsible for registering default providers. As such on
getting a resource event with a new provider version (e.g 1.0 -> 2.0) it
will send of a registration for the new version it's seen, which as a
default provider the step generator will accept and add to state (this
is probably ok).
However when the step generator runs for the resource using this
provider it will see it's not targetted and ignore its new goal state
just reusing its old state. This old state will be referring to the old
version of the provider (e.g "default_aws_1_0_0" rather than
"default_aws_2_0_0"), which was causing a panic in the step generator
when trying to build the overall stack state for StackAnalyze as the old
provider had never been registered.

We now catch this situation when generating the same step for a
non-targeted resource and error out that this isn't supported.

## 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. -->
2024-03-20 09:44:49 +00:00
Justin Van Patten 7ebcc42455
Downgrade remediate policy violations to mandatory ()
Policy violations should not have a remediate enforcement level. The
Policy SDK currently downgrades the level from remediate to mandatory
for resource policy violations, but isn't currently doing that for stack
policies. A change to the Policy SDK is in-progress to do that.

This change applies the same behavior to the engine. If a resource
policy still has a violation after running remediations and the level is
remediate, "downgrade" the level to mandatory. Similarly, if a stack
policy has a violation with a remediate level, downgrade it to
mandatory.

This avoids a panic when getting a policy violation from a stack policy
and the enforcement level is remediate.

Related: https://github.com/pulumi/pulumi-policy/pull/339

Fixes https://github.com/pulumi/pulumi-policy/issues/332
2024-03-07 15:10:36 +00:00
Thomas Gummerer 898a682ef6
Make sure non-targeted resources are not updated ()
When the `--target` option is used, resources that already exist in the
snapshot, but aren't directly targeted should not be updated at all.
Internally in the engine, this is done by turning them into a
`SameStep`, meaning no updates will actually be preformed, and we will
make it look like the resource stayed the same.

However, we currently still write the "new" state of the resource (e.g.
updated dependencies, inputs, etc.) into the snapshot. This is mostly
fine as long as the new dependencies already exist. If a dependency on a
resource is that doesn't already exist is added however this breaks.
Since the resource that's being depended on doesn't exist in the
snapshot and isn't targeted, we won't create it. At the same time we're
adding a dependency on that virtually non-existing resource, which makes
the snapshot invalid.

Since we're in `--target` mode, we should do what we promised the user,
and only update the targeted resources, nothing else. Introduce a new
`NonTargetedSameStep` here, which does exactly that. It's essentially
the same as a `SameStep`, but we always use the *old* state instead of
the new one when writing it out. Since the resource is not targeted,
this leaves it in the same state as before.

Fixes 
Fixes 
2024-03-05 07:49:11 +00:00
Fraser Waters 2f01e691a4
Use slices.Reverse ()
We have generics and the slices package now, use it to save having to
think if a reversing for loop is correct or not.
2024-02-22 11:43:26 +00:00
Fraser Waters 16d9f4c167
Enable perfsprint linter ()
<!--- 
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. -->
2023-12-12 12:19:42 +00:00
Fraser Waters bbaf871ad1
Check qualified type for root stackness ()
With the introduction of component programs there will be mulitple
"pulumi:pulumi:stack" resources in a program. We should therefore check
against the qualified type of resources to see if they are the root
stack, not just their direct type.
2023-12-04 10:36:51 +00:00
Fraser Waters 29bf5d694b
Replace ResourceSet with a generic set type ()
<!--- 
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've got a few "Set" types written up from before generics. We should
be able to replace them all with just one generic set container now.
This replaces `graph.ResourceSet` (which is only used in pkg) with a
generic set type (github.com/deckarep/golang-set).
2023-12-03 23:20:43 +00:00
Fraser Waters d58ec60a2a
Error if a resource's parent is a skipped create ()
<!--- 
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/14531.

We had a check in the step generator to not execute steps for a resource
that had a dependency which was a skipped create. But this didn't check
for parent or provider dependencies.

## 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. -->
2023-12-02 22:37:12 +00:00
Fraser Waters 516979770f
Allow anything in resource names ()
<!--- 
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/13968.
Fixes https://github.com/pulumi/pulumi/issues/8949.

This requires changing the parsing of URN's slightly, it is _very_
likely that
providers will need to update to handle URNs like this correctly.

This changes resource names to be `string` not `QName`. We never
validated this before and it turns out that users have put all manner of
text for resource names so we just updating the system to correctly
reflect that.

## 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. -->
2023-11-20 08:59:00 +00:00
Fraser Waters c39756e6de
Tests and fix for --target-dependents with explicit providers ()
<!--- 
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/13591.

This changes the logic for providers to always be targeted, this means
they can be skipped from --targets lists most of the time.

Because they don't need to be in the --targets list it makes the
behaviour of --target-dependents much more useful. If you want to update
a resource and it's children but it has an explicit provider you can
just --targets the resource.

If you want to use --target-dependents to target _all_ the resources
managed by an explicit provider that will work if the provider is in
--targets.

## 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. -->
2023-10-18 13:10:22 +00:00
Fraser Waters 47ecd5a11d
Mark diff as an input diff when auto-diffing in the step generator ()
<!--- 
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/14040

When a provider returns `DiffUnknown` the step generator calculates a
simple diff based on the old and new inputs.

We were not correctly marking that this is an input diff, and so when
reconstructing objects from the detailed diff later in
`TranslateDetailedDiff` we we're looking at the old output properties
rather than the old input properties.

## 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. -->

---------

Co-authored-by: Justin Van Patten <jvp@justinvp.com>
2023-10-18 10:33:04 +00:00
Joe Duffy 96a9a77167
Policy remediations feature ()
This PR implements the new policy transforms feature, which allows
policy packs to not only issue warnings and errors in response to policy
violations, but actually fix them by rewriting resource property state.
This can be used, for instance, to auto-tag resources, remove Internet
access on the fly, or apply encryption to storage, among other use
cases.
2023-10-09 18:31:17 +00:00
Fraser Waters fffc52ce08
Cleanup use of result.Result in step_generator.go ()
Continued result cleanup.
2023-10-06 15:57:27 +00:00
Fraser Waters e263e15363
Replace use of result.Result in step generator ()
More cleanup of result.Result, this time a pass through the step
generator.
2023-09-25 12:25:23 +00:00
Justin Van Patten b0048b35eb
[engine] Add extra comment on old resource lookup ()
Add a comment closer to where we actually lookup an old resource, noting
that we should check the URN first before aliases and why.

Follow-up from postmortem discussion on
https://github.com/pulumi/pulumi/issues/13848
2023-09-11 06:54:02 +00:00
Justin Van Patten 4c0b08d8ae
[engine] Check for old resources first by URN and then aliases ()
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 :

```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 , 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 
2023-09-06 12:17:02 +00:00
Fraser Waters 7dbb5a68a2 Test and fix --target-dependents 2023-07-24 22:02:06 +01:00
bors[bot] 8e83ce445d
Merge
13480: Fix resoloution of aliased parent aliases. r=Frassle 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. -->

Fixes https://github.com/pulumi/pulumi/issues/13324.

When collapsing an alias that specifies a parent we need to use the parents alias if it had one. 

This is so that for example given a state with three resources:
```
urn:pulumi:test::test::prog:index:myStandardType::resA
urn:pulumi:test::test::prog:index:myType::resB
urn:pulumi:test::test::prog:index:myType$pkgAⓂ️typA::resC
```
Which is then updated such that resB and resC are re-parented to resA we end up with the following alias specifications:
```
resB: { NoParent = True}
resC: { Parent = "urn:pulumi:test::test::prog:index:myStandardType$prog:index:myType::resB
```
Note that resC's alias uses the _new_ URN for resB. If we do a simple collapse of that alias we'd end up trying to find an old URN of `urn:pulumi:test::test::prog:index:myStandardType$prog:index:myType$pkgAⓂ️typA::resC` but actually it's old URN would have been `urn:pulumi:test::test::prog:index:myType$pkgAⓂ️typA::resC`.

This change fixes alias to URN collapse to take into account if the parent was aliased so we get the correct old URN.

## 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. -->


Co-authored-by: Fraser Waters <fraser@pulumi.com>
2023-07-17 16:28:39 +00:00
Fraser Waters 0b00a089c1 Fix resoloution of aliased parent aliases.
Fixes https://github.com/pulumi/pulumi/issues/13324.
2023-07-13 16:27:17 +01:00
Pat Gavlin 948bb36e7e [engine] Add support for source positions
These changes add support for passing source position information in
gRPC metadata and recording the source position that corresponds to a
resource registration in the statefile.

Enabling source position information in the resource model can provide
substantial benefits, including but not limited to:

- Better errors from the Pulumi CLI
- Go-to-defintion for resources in state
- Editor integration for errors, etc. from `pulumi preview`

Source positions are (file, line) or (file, line, column) tuples
represented as URIs. The line and column are stored in the fragment
portion of the URI as "line(,column)?". The scheme of the URI and the
form of its path component depends on the context in which it is
generated or used:

- During an active update, the URI's scheme is `file` and paths are
  absolute filesystem paths. This allows consumers to easily access
  arbitrary files that are available on the host.
- In a statefile, the URI's scheme is `project` and paths are relative
  to the project root. This allows consumers to resolve source positions
  relative to the project file in different contexts irrespective of the
  location of the project itself (e.g. given a project-relative path and
  the URL of the project's root on GitHub, one can build a GitHub URL for
  the source position).

During an update, source position information may be attached to gRPC
calls as "source-position" metadata. This allows arbitrary calls to be
associated with source positions without changes to their protobuf
payloads. Modifying the protobuf payloads is also a viable approach, but
is somewhat more invasive than attaching metadata, and requires changes
to every call signature.

Source positions should reflect the position in user code that initiated
a resource model operation (e.g. the source position passed with
`RegisterResource` for `pet` in the example above should be the source
position in `index.ts`, _not_ the source position in the Pulumi SDK). In
general, the Pulumi SDK should be able to infer the source position of
the resource registration, as the relationship between a resource
registration and its corresponding user code should be static per SDK.

Source positions in state files will be stored as a new `registeredAt`
property on each resource. This property is optional.
2023-07-10 14:35:40 -07:00
Fraser Waters 571fadae3f Use slice.Prealloc instead of make([]T, 0, ...)
Fixes https://github.com/pulumi/pulumi/issues/12738

https://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.
2023-06-29 11:27:50 +01:00
bors[bot] db579129f4
Merge
13139: Send old inputs to diff and update r=Frassle 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. -->

Fixes https://github.com/pulumi/pulumi/issues/5973

Both Azure and Kubernetes set an extra field on created resource outputs (called "__inputs") that doesn't exist in the inputs to the corresponding Pulumi resources. Their Diff logic needs the subset of inputs that are managed by Pulumi to compare these values to live state in subsequent updates, while not showing diffs for fields that we don't manage directly. This change adds that data to the RPC protocol directly so these providers no longer need to manage these extra "__input" fields.

This adds two new properties to the RPC interfaces with providers. Firstly we pass a new bool flag to Configure to tell the provider if it will get sent old inputs as part of Diff. Secondly we now pass the old inputs and the old outputs to Diff and Update as opposed to just the old outputs.
    
The flag passed to Configure isn't strictly needed. Providers should be able to tell that the old inputs aren't being sent to Diff and Updae by virtue of them being `null/nil/None` rather than an empty map. For DiffConfig this is the only way to detect this case because it's called before Configure. But its still useful to send this explictly to Configure to allow providers to return a configure error that they need to be ran against a newer engine version.

There is one fairly major failure case that can happen with this feature and providers who update to make use of this feature should ensure they call this out in their changelog.
That is if a provider has been saving old inputs in outputs as a way to get access to them in diff, and then updates to use these new protocol properties instead and stops saving the inputs in outputs _and then_ the user downgrades back to an old provider version things will probably get _very_ confused.

## 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. -->


Co-authored-by: Fraser Waters <fraser@pulumi.com>
2023-06-23 07:14:31 +00:00
Fraser Waters 841c99a9dd Send old inputs to diff and update
Fixes https://github.com/pulumi/pulumi/issues/5973

This adds two new properties to the RPC interfaces with providers.
Firstly we pass a new bool flag to Configure to tell the provider if it
will get sent old inputs as part of Diff.
Secondly we now pass the old inputs and the old outputs to Diff and
Update as opposed to just the old outputs.

The flag passed to Configure isn't strictly needed. Providers should be
able to tell that the old inputs aren't being sent to Diff and Updae by
virtue of them being `null/nil/None` rather than an empty map. For
DiffConfig this is the only way to detect this case because it's called
before Configure. But its still useful to send this explictly to
Configure to allow providers to return a configure error that they need
to be ran against a newer engine version.

There is one fairly major failure case that can happen with this feature
and providers who update to make use of this feature should ensure they
call this out in their changelog.
That is if a provider has been saving old inputs in outputs as a way to
get access to them in diff, and then updates to use these new protocol
properties instead and stops saving the inputs in outputs _and then_ the
user downgrades back to an old provider version things will probably get
_very_ confused.
2023-06-21 21:04:50 +01:00
Fraser Waters c881441890 Fix wildcards and indexers in IgnoreChanges
Fixes https://github.com/pulumi/pulumi/issues/12581

Also fixes ignoreChanges[idx] resting values to zero.

Before this change PropertiesPath.Delete would "delete" locations from
an array by just writing null to them but not changing the size of the
array.

This could result in a deployment that looked like it should have been a
same ending up as an update. For example given a resource `X` with a
property `foo` set to `[1]` if you changed the program to set `foo` to
`[1, 2]` and set `ignoreChanges` to `foo[1]` the deployment would do an
update with `foo` set to `[1, 0]`.

For PropertiesPath.Reset this now errors that the path is invalid, this
is similar to how we error on invalid paths if intermediate objects are
missing.
2023-06-20 09:45:02 +01:00
Justin Van Patten 64a2288883 [engine] Fix aliasing children
There is an issue with how the engine computes the aliases when the
resource is a child and doesn't have `Parent` set on the alias spec
(and the parent doesn't have any aliases).

```python
class FooResource(pulumi.ComponentResource):
    def __init__(self, name, opts=None):
        super().__init__("my:module:FooResource", name, None, opts)

class ComponentResource(pulumi.ComponentResource):
    def __init__(self, name, opts=None):
        super().__init__("my:module:ComponentResource", name, None, opts)
        FooResource("childrenamed", pulumi.ResourceOptions(
            parent=self,
            aliases=[pulumi.Alias(name="child")]
        ))
```

In the example above, `ComponentResource` has a child `FooResource`
which was renamed from `child` to `childrenamed`.

The engine does not compute the correct alias:

```
expected: urn:pulumi:stack::project::my:module:ComponentResource$my:module:FooResource::child
  actual: urn:pulumi:stack::project::my:module:FooResource::child
```

The problem is due to:

117955ce14/pkg/resource/deploy/step_generator.go (L370-L382)

... and:

117955ce14/sdk/go/common/resource/alias.go (L24-L26)

Because the alias spec doesn't have `parent` specified, the parent type
is not being included the computed alias URN.

Existing tests such as https://github.com/pulumi/pulumi/tree/master/tests/integration/aliases/python/rename_component_and_child
didn't catch the problem because the alias specifies both the `name` and
`parent`:

117955ce14/tests/integration/aliases/python/rename_component_and_child/step2/__main__.py (L15)

In this case, specifying `parent` on the alias shouldn't be necessary.
However, even after removing `parent` from the alias spec, the test
still succeeds because the parent itself has an alias:

117955ce14/tests/integration/aliases/python/rename_component_and_child/step2/__main__.py (L18)

... and parent aliases are inherited as part of a child's aliases, so we
still get an alias that works from the inheritance.

If we change the test to make no changes to the parent such that it
doesn't have any aliases, then we get the failure as we'd expect.

A similar problem will happen when retyping a child.

**Fix**

The fix involves using the child's parent in the calculated alias URN
when `Parent` isn't specified for the alias.

As part of this, we need to properly handled `NoParent` because right
now the engine is not correctly using it. The struct representing an
alias in the engine does not have a `NoParent` field:

117955ce14/sdk/go/common/resource/alias.go (L8-L15)

And therefore does not copy it over in the gRPC request:

117955ce14/pkg/resource/deploy/source_eval.go (L1082-L1088)

Instead, the `Alias` struct has an incorrect `NoParent` method which
returns `true` if the `Parent` field has a value of `""`:

117955ce14/sdk/go/common/resource/alias.go (L24-L26)
2023-06-14 05:19:17 -07:00
Justin Van Patten e77f9362aa Refactor: Split out `generateAliases` method
Split out the "generate aliases" functionality from `generateSteps` to
its own method, to make it possible to test just `generateAliases`.
2023-06-14 05:19:17 -07:00
Kyle Dixler 2b803b4ab8
Consolidated Target parameters
Consolidated `Target` parameters to a single variable. The deployment
executor is not well aware of the overall update that is going on and
runs individual resource operations. The consolidation minimizes
leakage.

Moved `--target` validation for `destroy` and `refresh` into `pkg/engine`.

Fixed  where `checkTargets()` would check the `prev` Snapshot for
resources after `rebuildBaseState()` was called. This would mutate prev
by removing resources from the snapshot. This caused deleted resources
on targeted updates not to be found and be reported as an error due to
having an unknown target.
2023-06-08 08:42:03 -07:00
Fraser Waters 72381ba976 Fix inconsistency in calling DiffConfig
Diff/DiffConfig is currently always called with the old outputs and new inputs,
except for this one case in step_generator where we called DiffConfig
with the old inputs instead of old outputs.

For providers inputs and outputs are always the same (Configure doesn't
allow providers to return any properties, so the engine enforces that
outputs=inputs) so using inputs or outputs here doesn't have any effect,
but less confusing to not have this inconsistency.
2023-06-01 11:45:35 +01:00
Kyle Dixler 23d9c20730
Fix --replace behavior regression
This fixes a regression where `--replace` was incorrectly considered a
targeted update. It removes a helper function indicating such, but the
incorrect logic is retained at the remaining usage site to avoid changes
in behavior.
2023-05-26 12:29:01 -07:00
bors[bot] c25b8483f9
Merge
12657: Don't load providers at startup r=Frassle 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. -->

This changes the provider registry to no longer load all the providers
from the old state on startup (in `NewRegistry`) instead the load logic
has been moved to the `Same` method. The step_executor and
step_generator have been fixed up to ensure that for cases where a
resource might not have had it's provider created yet (i.e. for DBR'ing
the old version of a resource, for refreshes or deletes) they ask the
`Deployment` to look up the provider in the old state and `Same` it in the
registry.

All of the above means we only load providers we're going to use (even
taking --targets into account).

One fix mot done in this change is to auto-update providers for deletes.
That is given a program state with two resources both using V1 of a
provider, if you run the program to update one of those resource to use
V2 of the provider but to delete the other resource currently we'll
still load V1 to do that delete. It _might_ be possible (although this
is definitly questionable) to see that another resource changed it's
provider from V1 to V2 and to just assume the same change should have
happened to the deleted resource.
This could be helpful for not loading old provider versions at all, but
can be done in two passes now pretty easily. Just run `up` without any
program changes except for the SDK version bump to update all the
provider references to V2 of the provider, then do another `up` that
deletes the second resource.
    
Fixes https://github.com/pulumi/pulumi/issues/12177.

## Checklist

<!--- 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. -->


Co-authored-by: Fraser Waters <fraser@pulumi.com>
2023-05-22 10:57:34 +00:00