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

Add pulumi state delete --target-dependents #11164

Merged
merged 1 commit into from Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion changelog/config.yaml
Expand Up @@ -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]
Expand Down
@@ -0,0 +1,4 @@
changes:
- type: feat
scope: cli/state
description: Add the --target-dependents flag to `pulumi state delete`
29 changes: 15 additions & 14 deletions pkg/cmd/pulumi/state_delete.go
Expand Up @@ -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 <resource URN>",
Expand All @@ -57,32 +58,31 @@ 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(res *resource.State) error {
cmdutil.Diag().Warningf(diag.Message(res.URN,
"deleting protected resource %s due to presence of --force"), res.URN)
return edit.UnprotectResource(nil, res)
}
}

return edit.DeleteResource(snap, res)
return edit.DeleteResource(snap, res, handleProtected, targetDepenedents)
})
if res != nil {
switch e := res.Error().(type) {
case edit.ResourceHasDependenciesError:
message := "This resource can't be safely deleted because the following resources depend on it:\n"
message := string(e.Condemned.URN) + " can't be safely deleted because the following resources depend on it:\n"
for _, dependentResource := range e.Dependencies {
depUrn := dependentResource.URN
message += fmt.Sprintf(" * %-15q (%s)\n", depUrn.Name(), depUrn)
}

message += "\nDelete those resources first before deleting this one."
message += "\nDelete those resources first or pass --target-dependents."
return result.Error(message)
case edit.ResourceProtectedError:
return result.Error(
"This resource can't be safely deleted because it is protected. " +
"Re-run this command with --force to force deletion")
return result.Errorf(
"%s can't be safely deleted because it is protected. "+
"Re-run this command with --force to force deletion", string(e.Condemned.URN))
default:
return res
}
Expand All @@ -97,5 +97,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
}
68 changes: 51 additions & 17 deletions 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.
Expand Down Expand Up @@ -29,43 +29,77 @@ 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)
if len(dependencies) != 0 {
return ResourceHasDependenciesError{Condemned: condemnedRes, Dependencies: dependencies}
deleteSet := map[resource.URN]bool{
condemnedRes.URN: true,
}

if dependencies := dg.DependingOn(condemnedRes, nil, true); len(dependencies) != 0 {
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
// not condemnedRes.
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.
Expand Down
130 changes: 124 additions & 6 deletions 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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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()

Expand All @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/resource/graph/dependency_graph.go
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions pkg/resource/graph/dependency_graph_rapid_test.go
Expand Up @@ -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
}
Expand All @@ -140,7 +138,7 @@ func expectedDependingOn(universe []*resource.State, includeChildren bool) R {
}
}
return false
}
})
}

// Verify `DependingOn` against `expectedDependingOn`. Note that
Expand Down