Skip to content

Commit 0c8d101

Browse files
authored
Fix checkForPreventDestroy to check all resources if some does not have prevent_destroy set (#3615)
## Changes Previously, `checkForPreventDestroy` aborted earlier if it encountered a resource with no prevent_destroy set, and wouldn't check the rest of the resources. Now this is fixed to check all resources. ## Why Fixes a bug in `checkForPreventDestroy` ## Tests Added a regression unit test for `checkForPreventDestroy` <!-- If your PR needs to be included in the release notes for next release, add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
1 parent 1012632 commit 0c8d101

File tree

3 files changed

+61
-2
lines changed

3 files changed

+61
-2
lines changed

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@
99
### Dependency updates
1010

1111
### Bundles
12+
* Fix checkForPreventDestroy to check all resources if some does not have prevent_destroy set ([#3615](https://github.com/databricks/cli/pull/3615))
1213

1314
### API Changes

bundle/phases/plan.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ func checkForPreventDestroy(b *bundle.Bundle, actions []deployplan.Action) error
6262
}
6363

6464
path := dyn.NewPath(dyn.Key("resources"), dyn.Key(action.Group), dyn.Key(action.Key), dyn.Key("lifecycle"), dyn.Key("prevent_destroy"))
65-
// If there is no prevent_destroy, skip
65+
// If there is no prevent_destroy, skip and check other resources
6666
preventDestroyV, err := dyn.GetByPath(root, path)
6767
if err != nil {
68-
return nil
68+
continue
6969
}
7070

7171
preventDestroy, ok := preventDestroyV.AsBool()

bundle/phases/plan_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import (
77

88
"github.com/databricks/cli/bundle"
99
"github.com/databricks/cli/bundle/config"
10+
"github.com/databricks/cli/bundle/config/resources"
1011
"github.com/databricks/cli/bundle/deployplan"
1112
"github.com/databricks/cli/libs/dyn"
13+
"github.com/databricks/databricks-sdk-go/service/apps"
14+
"github.com/databricks/databricks-sdk-go/service/jobs"
1215
"github.com/stretchr/testify/require"
1316
)
1417

@@ -62,3 +65,58 @@ func TestCheckPreventDestroyForAllResources(t *testing.T) {
6265
})
6366
}
6467
}
68+
69+
func TestCheckForPreventDestroyWhenFirstHasNoPreventDestroy(t *testing.T) {
70+
b := &bundle.Bundle{
71+
Config: config.Root{
72+
Bundle: config.Bundle{
73+
Name: "test",
74+
},
75+
Resources: config.Resources{
76+
Jobs: map[string]*resources.Job{
77+
"test_job": {
78+
JobSettings: jobs.JobSettings{
79+
Tasks: []jobs.Task{},
80+
},
81+
},
82+
},
83+
Apps: map[string]*resources.App{
84+
"test_app": {
85+
App: apps.App{
86+
Name: "Test App",
87+
},
88+
BaseResource: resources.BaseResource{
89+
Lifecycle: resources.Lifecycle{
90+
PreventDestroy: true,
91+
},
92+
},
93+
},
94+
},
95+
},
96+
},
97+
}
98+
99+
actions := []deployplan.Action{
100+
{
101+
ResourceNode: deployplan.ResourceNode{
102+
Group: "jobs",
103+
Key: "test_job",
104+
},
105+
ActionType: deployplan.ActionTypeRecreate,
106+
},
107+
{
108+
ResourceNode: deployplan.ResourceNode{
109+
Group: "apps",
110+
Key: "test_app",
111+
},
112+
ActionType: deployplan.ActionTypeRecreate,
113+
},
114+
}
115+
116+
ctx := context.Background()
117+
bundle.ApplyFuncContext(ctx, b, func(ctx context.Context, b *bundle.Bundle) {
118+
err := checkForPreventDestroy(b, actions)
119+
require.Error(t, err)
120+
require.Contains(t, err.Error(), "resource test_app has lifecycle.prevent_destroy set")
121+
})
122+
}

0 commit comments

Comments
 (0)