Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark diff as an input diff when auto-diffing in the step generator #14256

Merged
merged 2 commits into from Oct 18, 2023

Conversation

Frassle
Copy link
Member

@Frassle Frassle commented Oct 16, 2023

Description

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

  • 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
  • I have added tests that prove my fix is effective or that my feature works
  • I have run make changelog and committed the changelog/pending/<file> documenting my change
  • Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Oct 16, 2023

Changelog

[uncommitted] (2023-10-18)

Bug Fixes

  • [engine] Fix automatic diffs comparing against output instead of input properties.
    #14256

Fixes #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.
@Frassle Frassle requested a review from a team October 16, 2023 13:14
@Frassle Frassle marked this pull request as ready for review October 16, 2023 13:14
pkg/resource/deploy/step_generator.go Outdated Show resolved Hide resolved
Co-authored-by: Justin Van Patten <jvp@justinvp.com>
@Frassle Frassle added this pull request to the merge queue Oct 18, 2023
Merged via the queue into master with commit 47ecd5a Oct 18, 2023
44 checks passed
@Frassle Frassle deleted the fraser/inputDiff branch October 18, 2023 11:16
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2023
### Features

- [auto/nodejs] Add support for the path option for config operations
  [#14305](#14305)

- [engine] Converters can return diagnostics from `ConvertState`.
  [#14135](#14135)


### Bug Fixes

- [cli] Tightened the parser for property paths to be less prone to
typos
  [#14257](#14257)

- [engine] Fix handling of explicit providers and --target-dependents.
  [#14238](#14238)

- [engine] Fix automatic diffs comparing against output instead of input
properties.
  [#14256](#14256)

- [sdkgen/dotnet] Fix codegen with nested modules.
  [#14297](#14297)

- [programgen/go] Fix codegen to correctly output pulumi.Array instead
of pulumi.AnyArray
  [#14299](#14299)

- [cli/new] `pulumi new` now allows users to bypass existing project
name checks.
  [#14081](#14081)

- [sdk/nodejs] Nodejs now supports unknown resource IDs.
  [#14137](#14137)

- [sdkgen/python] Fix `_configure` failing due to required args
mismatch.
  [#14281](#14281)


### Miscellaneous

- [cli] Pull in fixes from esc v0.5.6
  [#14284](#14284)

- [protobuf] Add a config as property map field to RunRequest and pass
that to the SDK
  [#14273](#14273)

- [sdk/python] updates grpcio dependency
  [#14259](#14259)
@justinvp justinvp mentioned this pull request Oct 23, 2023
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2023
### Features

- [auto/nodejs] Add support for the path option for config operations
  [#14305](#14305)

- [engine] Converters can return diagnostics from `ConvertState`.
  [#14135](#14135)


### Bug Fixes

- [cli] Tightened the parser for property paths to be less prone to
typos
  [#14257](#14257)

- [engine] Fix handling of explicit providers and --target-dependents.
  [#14238](#14238)

- [engine] Fix automatic diffs comparing against output instead of input
properties.
  [#14256](#14256)

- [sdkgen/dotnet] Fix codegen with nested modules.
  [#14297](#14297)

- [programgen/go] Fix codegen to correctly output pulumi.Array instead
of pulumi.AnyArray
  [#14299](#14299)

- [cli/new] `pulumi new` now allows users to bypass existing project
name checks.
  [#14081](#14081)

- [sdk/nodejs] Nodejs now supports unknown resource IDs.
  [#14137](#14137)

- [sdkgen/python] Fix `_configure` failing due to required args
mismatch.
  [#14281](#14281)


### Miscellaneous

- [cli] Pull in fixes from esc v0.5.6
  [#14284](#14284)

- [protobuf] Add a config as property map field to RunRequest and pass
that to the SDK
  [#14273](#14273)

- [sdk/python] updates grpcio dependency
  [#14259](#14259)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors on the diff of ecs.TaskDefinition, but applies correctly
3 participants