Skip to content

Commit a535e8c

Browse files
denikIsaac
denik
and
Isaac
authored
direct: allow clearing genie space description and secret comment (#6789)
Clearing a genie space's `description` or a secret's `comment` by removing it from config left the old value in place — the field is `omitempty`, so it was dropped from the update request and every subsequent plan re-proposed the change. Force-send both on update (the same fix #6343 applied to the UC `comment`), and teach the testserver's genie/secret update handlers to honor an explicit empty value as a clear rather than treating it as "not provided". Verified against a real AWS workspace: `UpdateSpace` accepts `{"description": ""}` and `UpdateSecret` accepts `{"comment": ""}` as clears, and the `no_drift` invariant's removal cycle (added in #6773) converges for both. This flips `genie_spaces.description` and `secrets.comment` from `CANNOT_REMOVE` to `CAN_REMOVE`. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <[email protected]>
1 parent 70688d8 commit a535e8c

11 files changed

Lines changed: 36 additions & 12 deletions

File tree

‎.nextchanges/bundles/clear-genie-description-secret-comment.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* direct: Allow clearing a genie space's `description` and a secret's `comment` by removing them from configuration. ([#6789](https://github.com/databricks/cli/pull/6789))

‎acceptance/bundle/invariant/configs/genie_space.yml.tmpl‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ resources:
66
foo:
77
warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID
88
title: test-genie-space-$UNIQUE_NAME
9-
# CANNOT_REMOVE: description is not force-sent on update, so clearing it drops the
10-
# field from the request and the old value drifts back on every subsequent plan.
11-
description: This is a test genie space
9+
description: This is a test genie space # CAN_REMOVE
1210
# Structured (inline) serialized_space is marshalled to a JSON string by
1311
# ConfigureGenieSpaceSerializedSpace; this config doubles as a regression
1412
# guard that the normalization produces a drift-free deploy. Kept minimal

‎acceptance/bundle/invariant/configs/secret.yml.tmpl‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ resources:
1212
schema_name: default
1313
name: test-secret-$UNIQUE_NAME
1414
value: ${var.secret_value}
15-
# CANNOT_REMOVE: comment is not force-sent on update, so clearing it drops the field
16-
# from the request and the old value drifts back on every subsequent plan.
17-
comment: This is a test secret
15+
comment: This is a test secret # CAN_REMOVE
1816
grants:
1917
- principal: account users
2018
privileges:

‎acceptance/bundle/resources/genie_spaces/change-serialized-space/output.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
3636
"method": "PATCH",
3737
"path": "/api/2.0/genie/spaces/[MY_SPACE_ID]",
3838
"body": {
39+
"description": "",
3940
"parent_path": "/Workspace/Users/[USERNAME]/.bundle/change-serialized-space-[UNIQUE_NAME]/default/resources",
4041
"serialized_space": "{\"version\":1,\"instructions\":{\"text_instructions\":[{\"id\":\"a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4\",\"content\":[\"Summarize sales revenue by month\"]}]}}\n",
4142
"title": "test genie space",

‎acceptance/bundle/resources/secrets/remove-expire-time/output.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
4444
},
4545
"body": {
4646
"catalog_name": "main",
47+
"comment": "",
4748
"name": "test_secret_[UNIQUE_NAME]",
4849
"schema_name": "default",
4950
"value": "secret-value"

‎acceptance/bundle/resources/secrets/update-expire-time/output.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
4444
},
4545
"body": {
4646
"catalog_name": "main",
47+
"comment": "",
4748
"expire_time": "[EXPIRE_TIME_AFTER]",
4849
"name": "test_secret_[UNIQUE_NAME]",
4950
"schema_name": "default",

‎bundle/direct/dresources/genie_space.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"net/http"
8+
"slices"
89
"strings"
910

1011
"github.com/databricks/cli/bundle/config/resources"
@@ -177,6 +178,11 @@ func (r *ResourceGenieSpace) DoCreate(ctx context.Context, config *resources.Gen
177178
return createResp.SpaceId, responseToGenieSpaceConfig(createResp, serializedSpace), nil
178179
}
179180

181+
// description is force-sent on update so that clearing it in config clears it on the space.
182+
// Without this the omitempty field is dropped from the request and the old value drifts back on
183+
// every plan. Verified against a real workspace: UpdateSpace accepts {"description": ""} as a clear.
184+
var genieSpaceForceSend = []string{"Description"}
185+
180186
func (r *ResourceGenieSpace) DoUpdate(ctx context.Context, id string, config *resources.GenieSpaceConfig, _ *PlanEntry) (*resources.GenieSpaceConfig, error) {
181187
serializedSpace, err := prepareGenieSpaceRequest(config)
182188
if err != nil {
@@ -198,7 +204,7 @@ func (r *ResourceGenieSpace) DoUpdate(ctx context.Context, id string, config *re
198204
SerializedSpace: serializedSpace,
199205
Etag: "",
200206

201-
ForceSendFields: utils.FilterFields[dashboards.GenieUpdateSpaceRequest](config.ForceSendFields),
207+
ForceSendFields: utils.FilterFields[dashboards.GenieUpdateSpaceRequest](append(slices.Clone(genieSpaceForceSend), config.ForceSendFields...)),
202208
})
203209
if err != nil {
204210
return nil, err

‎bundle/direct/dresources/genie_space_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,9 @@ func TestGenieSpaceDoUpdateRoundTripsEtag(t *testing.T) {
122122
// the response is persisted, for drift detection on the next plan.
123123
m.GetMockGenieAPI().EXPECT().
124124
UpdateSpace(ctx, dashboards.GenieUpdateSpaceRequest{
125-
SpaceId: "space-id",
126-
Title: "new",
125+
SpaceId: "space-id",
126+
Title: "new",
127+
ForceSendFields: []string{"Description"},
127128
}).
128129
Return(&dashboards.GenieSpace{
129130
SpaceId: "space-id",
@@ -159,6 +160,7 @@ func TestGenieSpaceDoUpdateAlwaysSendsSerializedSpace(t *testing.T) {
159160
SpaceId: "space-id",
160161
Title: "new",
161162
SerializedSpace: "{\"converge\":\"me\"}",
163+
ForceSendFields: []string{"Description"},
162164
}).
163165
Return(&dashboards.GenieSpace{
164166
SpaceId: "space-id",

‎bundle/direct/dresources/secret.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package dresources
22

33
import (
44
"context"
5+
"slices"
56

67
"github.com/databricks/cli/bundle/config/resources"
78
"github.com/databricks/cli/bundle/deployplan"
@@ -86,11 +87,18 @@ func (r *ResourceSecret) DoCreate(ctx context.Context, state *catalog.Secret) (s
8687
return response.FullName, response, nil
8788
}
8889

90+
// comment is force-sent on update so that clearing it in config clears it on the secret. The
91+
// update_mask is "*" and the backend merges, so an omitempty comment would be dropped from the
92+
// body and the old value would drift back. Verified against a real workspace: {"comment": ""} clears it.
93+
var secretForceSend = []string{"Comment"}
94+
8995
// DoUpdate updates the secret in place and returns remote state.
9096
func (r *ResourceSecret) DoUpdate(ctx context.Context, id string, state *catalog.Secret, _ *PlanEntry) (*catalog.Secret, error) {
97+
secret := *state
98+
secret.ForceSendFields = utils.FilterFields[catalog.Secret](append(slices.Clone(secretForceSend), state.ForceSendFields...))
9199
response, err := r.client.SecretsUc.UpdateSecret(ctx, catalog.UpdateSecretRequest{
92100
FullName: id,
93-
Secret: *state,
101+
Secret: secret,
94102
UpdateMask: fieldmask.FieldMask{
95103
Paths: []string{"*"},
96104
},

‎libs/testserver/genie_spaces.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ func (s *FakeWorkspace) GenieSpaceUpdate(req Request) Response {
159159
}
160160
}
161161

162+
// Which fields the caller actually sent, so an explicit "description": "" (the CLI
163+
// force-sends it) clears the value instead of being treated as "not provided".
164+
fields, _ := parseUpdateFields(req.Body)
165+
162166
// Optimistic concurrency: if the caller sent an etag, it must match the
163167
// current one. Empty etag means apply unconditionally.
164168
if updateReq.Etag != "" && updateReq.Etag != genieSpace.Etag {
@@ -174,7 +178,7 @@ func (s *FakeWorkspace) GenieSpaceUpdate(req Request) Response {
174178
if updateReq.Title != "" {
175179
genieSpace.Title = updateReq.Title
176180
}
177-
if updateReq.Description != "" {
181+
if _, ok := fields["description"]; ok {
178182
genieSpace.Description = updateReq.Description
179183
}
180184
if updateReq.WarehouseId != "" {

0 commit comments

Comments
 (0)