diff --git a/changelog/config.yaml b/changelog/config.yaml index 0b73e26b5c94..38f614dcf8d5 100644 --- a/changelog/config.yaml +++ b/changelog/config.yaml @@ -5,7 +5,7 @@ types: scopes: auto: [dotnet, go, java, nodejs, python, yaml] backend: [service, filestate] - cli: [about, config, display, engine, import, new, plugin, package] + cli: [about, config, display, engine, import, new, plugin, package, state] pkg: [testing] sdk: [dotnet, go, java, nodejs, python, yaml] components: [dotnet, go, java, nodejs, python, yaml] diff --git a/changelog/pending/20221026--cli-state--add-the-target-dependents-flag-to-pulumi-state-delete.yaml b/changelog/pending/20221026--cli-state--add-the-target-dependents-flag-to-pulumi-state-delete.yaml new file mode 100644 index 000000000000..0def3b1a37de --- /dev/null +++ b/changelog/pending/20221026--cli-state--add-the-target-dependents-flag-to-pulumi-state-delete.yaml @@ -0,0 +1,4 @@ +changes: +- type: feat + scope: cli/state + description: Add the --target-dependents flag to `pulumi state delete` diff --git a/pkg/cmd/pulumi/state_delete.go b/pkg/cmd/pulumi/state_delete.go index d81429cf187f..ea55704351b7 100644 --- a/pkg/cmd/pulumi/state_delete.go +++ b/pkg/cmd/pulumi/state_delete.go @@ -31,6 +31,7 @@ func newStateDeleteCommand() *cobra.Command { var force bool // Force deletion of protected resources var stack string var yes bool + var targetDepenedents bool cmd := &cobra.Command{ Use: "delete ", @@ -57,16 +58,14 @@ pulumi state delete 'urn:pulumi:stage::demo::eks:index:Cluster$pulumi:providers: showPrompt := !yes res := runStateEdit(ctx, stack, showPrompt, urn, func(snap *deploy.Snapshot, res *resource.State) error { - if !force { - return edit.DeleteResource(snap, res) - } - - if res.Protect { - cmdutil.Diag().Warningf(diag.RawMessage("" /*urn*/, "deleting protected resource due to presence of --force")) - res.Protect = false + var handleProtected func(*resource.State) error + if force { + handleProtected = func(s *resource.State) error { + cmdutil.Diag().Warningf(diag.RawMessage(s.URN, "deleting protected resource due to presence of --force")) + return edit.UnprotectResource(snap, res) + } } - - return edit.DeleteResource(snap, res) + return edit.DeleteResource(snap, res, handleProtected, targetDepenedents) }) if res != nil { switch e := res.Error().(type) { @@ -97,5 +96,6 @@ pulumi state delete 'urn:pulumi:stage::demo::eks:index:Cluster$pulumi:providers: "The name of the stack to operate on. Defaults to the current stack") cmd.Flags().BoolVar(&force, "force", false, "Force deletion of protected resources") cmd.Flags().BoolVarP(&yes, "yes", "y", false, "Skip confirmation prompts") + cmd.Flags().BoolVar(&targetDepenedents, "target-dependents", false, "Delete the URN and all its dependents") return cmd } diff --git a/pkg/resource/edit/operations.go b/pkg/resource/edit/operations.go index e156ba1f28e8..90e49d9bb2e9 100644 --- a/pkg/resource/edit/operations.go +++ b/pkg/resource/edit/operations.go @@ -1,4 +1,4 @@ -// Copyright 2016-2018, Pulumi Corporation. +// Copyright 2016-2022, Pulumi Corporation. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -29,21 +29,56 @@ import ( // given snapshot and pertain to the specific passed-in resource. type OperationFunc func(*deploy.Snapshot, *resource.State) error -// DeleteResource deletes a given resource from the snapshot, if it is possible to do so. A resource can only be deleted -// from a stack if there do not exist any resources that depend on it or descend from it. If such a resource does exist, -// DeleteResource will return an error instance of `ResourceHasDependenciesError`. -func DeleteResource(snapshot *deploy.Snapshot, condemnedRes *resource.State) error { +// DeleteResource deletes a given resource from the snapshot, if it is possible to do so. +// +// If targetDependents is true, dependents will also be deleted. Otherwise an error +// instance of `ResourceHasDependenciesError` will be returned. +// +// If non-nil, onProtected will be called on all protected resources planed for deletion. +// +// If a resource is marked protected, after onProtected is called, an error instance of +// `ResourceHasDependenciesError` will be returned. +func DeleteResource( + snapshot *deploy.Snapshot, condemnedRes *resource.State, + onProtected func(*resource.State) error, targetDependents bool, +) error { contract.Require(snapshot != nil, "snapshot") contract.Require(condemnedRes != nil, "state") - if condemnedRes.Protect { - return ResourceProtectedError{condemnedRes} + handleProtected := func(res *resource.State) error { + if !res.Protect { + return nil + } + var err error + if onProtected != nil { + err = onProtected(res) + } + if err == nil && res.Protect { + err = ResourceProtectedError{res} + } + return err + } + + if err := handleProtected(condemnedRes); err != nil { + return err } dg := graph.NewDependencyGraph(snapshot.Resources) - dependencies := dg.DependingOn(condemnedRes, nil, false) + deleteSet := map[resource.URN]bool{ + condemnedRes.URN: true, + } + dependencies := dg.DependingOn(condemnedRes, nil, true) if len(dependencies) != 0 { - return ResourceHasDependenciesError{Condemned: condemnedRes, Dependencies: dependencies} + if !targetDependents { + return ResourceHasDependenciesError{Condemned: condemnedRes, Dependencies: dependencies} + } + for _, dep := range dependencies { + err := handleProtected(dep) + if err != nil { + return err + } + deleteSet[dep.URN] = true + } } // If there are no resources that depend on condemnedRes, iterate through the snapshot and keep everything that's @@ -51,21 +86,20 @@ func DeleteResource(snapshot *deploy.Snapshot, condemnedRes *resource.State) err var newSnapshot []*resource.State var children []*resource.State for _, res := range snapshot.Resources { - // While iterating, keep track of the set of resources that are parented to our condemned resource. We'll only - // actually perform the deletion if this set is empty, otherwise it is not legal to delete the resource. - if res.Parent == condemnedRes.URN { + // While iterating, keep track of the set of resources that are parented to our + // condemned resource. This acts as a check on DependingOn, preventing a bug from + // introducing state corruption. + if res.Parent == condemnedRes.URN && !deleteSet[res.URN] { children = append(children, res) } - if res != condemnedRes { + if !deleteSet[res.URN] { newSnapshot = append(newSnapshot, res) } } // If there exists a resource that is the child of condemnedRes, we can't delete it. - if len(children) != 0 { - return ResourceHasDependenciesError{Condemned: condemnedRes, Dependencies: children} - } + contract.Assertf(len(children) == 0, "unexpected children in resource dependency list") // Otherwise, we're good to go. Writing the new resource list into the snapshot persists the mutations that we have // made above. diff --git a/pkg/resource/edit/operations_test.go b/pkg/resource/edit/operations_test.go index 1b44a85c12f3..fc67bc94809e 100644 --- a/pkg/resource/edit/operations_test.go +++ b/pkg/resource/edit/operations_test.go @@ -1,4 +1,4 @@ -// Copyright 2016-2018, Pulumi Corporation. +// Copyright 2016-2022, Pulumi Corporation. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ import ( "github.com/pulumi/pulumi/sdk/v3/go/common/tokens" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func NewResource(name string, provider *resource.State, deps ...resource.URN) *resource.State { @@ -84,12 +85,30 @@ func TestDeletion(t *testing.T) { c, }) - err := DeleteResource(snap, b) + err := DeleteResource(snap, b, nil, false) assert.NoError(t, err) assert.Len(t, snap.Resources, 3) assert.Equal(t, []*resource.State{pA, a, c}, snap.Resources) } +func TestDeletingDependencies(t *testing.T) { + t.Parallel() + + pA := NewProviderResource("a", "p1", "0") + a := NewResource("a", pA) + b := NewResource("b", pA) + c := NewResource("c", pA, a.URN) + d := NewResource("d", pA, c.URN) + snap := NewSnapshot([]*resource.State{ + pA, a, b, c, d, + }) + + err := DeleteResource(snap, a, nil, true) + require.NoError(t, err) + + assert.Equal(t, snap.Resources, []*resource.State{pA, b}) +} + func TestFailedDeletionProviderDependency(t *testing.T) { t.Parallel() @@ -104,7 +123,7 @@ func TestFailedDeletionProviderDependency(t *testing.T) { c, }) - err := DeleteResource(snap, pA) + err := DeleteResource(snap, pA, nil, false) assert.Error(t, err) depErr, ok := err.(ResourceHasDependenciesError) if !assert.True(t, ok) { @@ -132,7 +151,7 @@ func TestFailedDeletionRegularDependency(t *testing.T) { c, }) - err := DeleteResource(snap, a) + err := DeleteResource(snap, a, nil, false) assert.Error(t, err) depErr, ok := err.(ResourceHasDependenciesError) if !assert.True(t, ok) { @@ -158,12 +177,111 @@ func TestFailedDeletionProtected(t *testing.T) { a, }) - err := DeleteResource(snap, a) + err := DeleteResource(snap, a, nil, false) assert.Error(t, err) _, ok := err.(ResourceProtectedError) assert.True(t, ok) } +func TestDeleteProtected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + test func(t *testing.T, pA, a, b, c *resource.State, snap *deploy.Snapshot) + }{ + { + "root-protected", + func(t *testing.T, pA, a, b, c *resource.State, snap *deploy.Snapshot) { + a.Protect = true + protectedCount := 0 + err := DeleteResource(snap, a, func(s *resource.State) error { + s.Protect = false + protectedCount++ + return nil + }, false) + assert.NoError(t, err) + assert.Equal(t, protectedCount, 1) + assert.Equal(t, snap.Resources, []*resource.State{pA, b, c}) + }, + }, + { + "root-and-branch", + func(t *testing.T, pA, a, b, c *resource.State, snap *deploy.Snapshot) { + a.Protect = true + b.Protect = true + c.Protect = true + protectedCount := 0 + err := DeleteResource(snap, b, func(s *resource.State) error { + s.Protect = false + protectedCount++ + return nil + }, true) + assert.NoError(t, err) + // 2 because we only plan to delete b and c. a is protected but not + // scheduled for deletion, so we don't call the onProtect handler. + assert.Equal(t, protectedCount, 2) + assert.Equal(t, snap.Resources, []*resource.State{pA, a}) + + }, + }, + { + "branch", + func(t *testing.T, pA, a, b, c *resource.State, snap *deploy.Snapshot) { + b.Protect = true + c.Protect = true + protectedCount := 0 + err := DeleteResource(snap, c, func(s *resource.State) error { + s.Protect = false + protectedCount++ + return nil + }, false) + assert.NoError(t, err) + assert.Equal(t, protectedCount, 1) + assert.Equal(t, snap.Resources, []*resource.State{pA, a, b}) + }, + }, + { + "no-permission-root", + func(t *testing.T, pA, a, b, c *resource.State, snap *deploy.Snapshot) { + c.Protect = true + err := DeleteResource(snap, c, nil, false).(ResourceProtectedError) + assert.Equal(t, ResourceProtectedError{ + Condemned: c, + }, err) + }, + }, + { + "no-permission-branch", + func(t *testing.T, pA, a, b, c *resource.State, snap *deploy.Snapshot) { + c.Protect = true + err := DeleteResource(snap, b, nil, true).(ResourceProtectedError) + assert.Equal(t, ResourceProtectedError{ + Condemned: c, + }, err) + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + pA := NewProviderResource("a", "p1", "0") + a := NewResource("a", pA) + b := NewResource("b", pA) + c := NewResource("c", pA, b.URN) + snap := NewSnapshot([]*resource.State{ + pA, + a, + b, + c, + }) + + tt.test(t, pA, a, b, c, snap) + }) + } +} + func TestFailedDeletionParentDependency(t *testing.T) { t.Parallel() @@ -180,7 +298,7 @@ func TestFailedDeletionParentDependency(t *testing.T) { c, }) - err := DeleteResource(snap, a) + err := DeleteResource(snap, a, nil, false) assert.Error(t, err) depErr, ok := err.(ResourceHasDependenciesError) if !assert.True(t, ok) { diff --git a/pkg/resource/graph/dependency_graph.go b/pkg/resource/graph/dependency_graph.go index 5fe908804f68..4cf0f03656e1 100644 --- a/pkg/resource/graph/dependency_graph.go +++ b/pkg/resource/graph/dependency_graph.go @@ -20,6 +20,8 @@ type DependencyGraph struct { // order with respect to the snapshot dependency graph. // // The time complexity of DependingOn is linear with respect to the number of resources. +// +// includeChildren adds children as another type of (transitive) dependency. func (dg *DependencyGraph) DependingOn(res *resource.State, ignore map[resource.URN]bool, includeChildren bool) []*resource.State { // This implementation relies on the detail that snapshots are stored in a valid @@ -35,7 +37,7 @@ func (dg *DependencyGraph) DependingOn(res *resource.State, if ignore[candidate.URN] { return false } - if includeChildren && candidate.Parent == res.URN { + if includeChildren && dependentSet[candidate.Parent] { return true } for _, dependency := range candidate.Dependencies { diff --git a/pkg/resource/graph/dependency_graph_rapid_test.go b/pkg/resource/graph/dependency_graph_rapid_test.go index 85b0e9e39618..80708ff726a3 100644 --- a/pkg/resource/graph/dependency_graph_rapid_test.go +++ b/pkg/resource/graph/dependency_graph_rapid_test.go @@ -127,10 +127,8 @@ func expectedDependingOn(universe []*resource.State, includeChildren bool) R { return inverse(transitively(universe)(restrictedDependenciesOf)) } - // TODO this extends base `expectedDependingOn` with - // immediate children. Should it be a transitive closure? dependingOn := expectedDependingOn(universe, false) - return func(a, b *resource.State) bool { + return transitively(universe)(func(a, b *resource.State) bool { if dependingOn(a, b) || isParent(b, a) { return true } @@ -140,7 +138,7 @@ func expectedDependingOn(universe []*resource.State, includeChildren bool) R { } } return false - } + }) } // Verify `DependingOn` against `expectedDependingOn`. Note that