Don't bail at preview when a protected resource needs replacement #15969

mikhailshilkov · 2024-04-17T15:41:03Z

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

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

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

…st error

pulumi-bot · 2024-04-17T15:41:44Z

Changelog

[uncommitted] (2024-04-18)

Bug Fixes

[engine] Display the entire preview with diff when a protected resource needs replacement
#15969

Frassle · 2024-04-17T16:40:56Z

pkg/engine/lifecycletest/pulumi_test.go

 	expectedMessage = "<{%reset%}>unable to replace resource \"urn:pulumi:test::test::pkgA:m:typA::resA\"\n" +
 		"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`<{%reset%}>\n"
 	ins = resource.NewPropertyMapFromMap(map[string]interface{}{
 		"foo": "baz",
 	})
+	_, err = TestOp(Update).Run(project, p.GetTarget(t, snap), p.Options, true, p.BackendClient, validate)
+	assert.Error(t, err)


Please don't use assert.Error, use ErrorContains or similar to check its actually the error expected.

I thought that already happens with expectedMessage. I'll double check.

expectedMessage is checking the diagnostic gets output, but we also want to check that the error is the bail error we expect matching up to that, not some other random error that might get hit in the system. We've had tests before that we thought we're testing for one error case just using asset.Error and when the system slightly changed to a different error the tests didn't catch it but users noticed.

Frassle · 2024-04-17T16:42:37Z

pkg/resource/deploy/step_generator.go

@@ -1034,7 +1034,9 @@ func (sg *stepGenerator) generateStepsFromDiff(
 					"program and run `pulumi up`", urn)
 				sg.deployment.ctx.Diag.Errorf(diag.StreamMessage(urn, message, 0))
 				sg.sawError = true
-				return nil, result.BailErrorf(message)
+				if !sg.deployment.preview {


Probably worth a comment why we have to error here if not preview.

I added a comment. I'm not exactly sure we need to bail during update. Honestly, I'm making the smallest possible change to solve the preview issue.

If we don't bail and we're doing a create before delete replacement we'll execute the create before getting to the delete error. State will still be tracking both resources but it's an awkward state to cleanup from.

...ding/20240417--engine--dont-bail-at-preview-when-a-protected-resource-needs-replacement.yaml

Don't bail at preview when a protected resource needs replacement, ju…

5370527

…st error

mikhailshilkov added 2 commits April 17, 2024 18:17

Adjust the protect lifecycle test

9cb81bb

Add a changelog entry

1c4cc6c

mikhailshilkov marked this pull request as ready for review April 17, 2024 16:23

mikhailshilkov requested a review from a team as a code owner April 17, 2024 16:23

Frassle reviewed Apr 17, 2024

View reviewed changes

Review comments

abd130d

mikhailshilkov requested a review from Frassle April 18, 2024 10:06

tgummerer approved these changes Apr 18, 2024

View reviewed changes

...ding/20240417--engine--dont-bail-at-preview-when-a-protected-resource-needs-replacement.yaml Outdated Show resolved Hide resolved

mikhailshilkov added 2 commits April 18, 2024 13:22

Improve the changelog entry

f0c4e17

Expand the comment

2610d3f

mikhailshilkov added this pull request to the merge queue Apr 18, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 18, 2024

mikhailshilkov added this pull request to the merge queue Apr 18, 2024

Merged via the queue into master with commit 60adf18 Apr 18, 2024
49 checks passed

mikhailshilkov deleted the mikhailshilkov/no-bail-on-preview-replacement-of-protected branch April 18, 2024 17:46

mikhailshilkov mentioned this pull request May 29, 2024

Diff not generated for protected resource which needs to be replaced #5749

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't bail at preview when a protected resource needs replacement #15969

Don't bail at preview when a protected resource needs replacement #15969

mikhailshilkov commented Apr 17, 2024 •

edited

pulumi-bot commented Apr 17, 2024 •

edited

Frassle Apr 17, 2024

mikhailshilkov Apr 17, 2024

Frassle Apr 17, 2024

Frassle Apr 17, 2024

mikhailshilkov Apr 18, 2024

Frassle Apr 18, 2024

Don't bail at preview when a protected resource needs replacement #15969

Don't bail at preview when a protected resource needs replacement #15969

Conversation

mikhailshilkov commented Apr 17, 2024 • edited

Description

Checklist

pulumi-bot commented Apr 17, 2024 • edited

Changelog

[uncommitted] (2024-04-18)

Bug Fixes

Frassle Apr 17, 2024

Choose a reason for hiding this comment

mikhailshilkov Apr 17, 2024

Choose a reason for hiding this comment

Frassle Apr 17, 2024

Choose a reason for hiding this comment

Frassle Apr 17, 2024

Choose a reason for hiding this comment

mikhailshilkov Apr 18, 2024

Choose a reason for hiding this comment

Frassle Apr 18, 2024

Choose a reason for hiding this comment

mikhailshilkov commented Apr 17, 2024 •

edited

pulumi-bot commented Apr 17, 2024 •

edited