Fail a create that returns no resource id

bundle/direct/apply.go

@@ -77,6 +77,10 @@ func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState,
 		// No need to prefix error, there is no ambiguity (only one operation - DoCreate) and no additional context (like id)
 		return err
 	}
+	// An empty id would be saved as-is, and the next plan would treat the resource as missing and create it again.
+	if newID == "" {
+		return errors.New("create returned an empty resource id")
+	}
 
 	log.Infof(ctx, "Created %s id=%#v", d.ResourceKey, newID)
 
@@ -216,6 +220,10 @@ func (d *DeploymentUnit) UpdateWithID(ctx context.Context, db *dstate.Deployment
 	if err != nil {
 		return fmt.Errorf("updating id=%s: %w", oldID, err)
 	}
+	// Same reason as in Create: an empty id must not replace the saved one.
+	if newID == "" {
+		return errors.New("update returned an empty resource id")
+	}
 
 	if oldID != newID {
 		log.Infof(ctx, "Updated %s id=%#v (previously %#v)", d.ResourceKey, newID, oldID)

bundle/direct/apply_test.go

@@ -0,0 +1,87 @@
+package direct
+
+import (
+	"context"
+	"path/filepath"
+	"testing"
+
+	"github.com/databricks/cli/bundle/direct/dresources"
+	"github.com/databricks/cli/bundle/direct/dstate"
+	"github.com/databricks/cli/libs/structs/structpath"
+	"github.com/databricks/databricks-sdk-go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// emptyIDResource is a fake resource whose create and update return an empty id.
+type emptyIDResource struct{}
+
+type emptyIDState struct {
+	Name string `json:"name"`
+}
+
+func (*emptyIDResource) New(client *databricks.WorkspaceClient) any {
+	return &emptyIDResource{}
+}
+
+func (*emptyIDResource) PrepareState(input *emptyIDState) *emptyIDState {
+	return input
+}
+
+func (*emptyIDResource) DoRead(ctx context.Context, id string) (*emptyIDState, error) {
+	return &emptyIDState{}, nil
+}
+
+func (*emptyIDResource) DoCreate(ctx context.Context, newState *emptyIDState) (string, *emptyIDState, error) {
+	return "", nil, nil
+}
+
+func (*emptyIDResource) DoUpdateWithID(ctx context.Context, id string, newState *emptyIDState, entry *dresources.PlanEntry) (string, *emptyIDState, error) {
+	return "", nil, nil
+}
+
+func (*emptyIDResource) OverrideChangeDesc(ctx context.Context, path *structpath.PathNode, changedesc *dresources.ChangeDesc, remoteState any) error {
+	return nil
+}
+
+func newEmptyIDDeploymentState(t *testing.T) *dstate.DeploymentState {
+	t.Helper()
+	db := &dstate.DeploymentState{}
+	path := filepath.Join(t.TempDir(), "resources.json")
+	require.NoError(t, db.Open(t.Context(), path, dstate.WritingFile, dstate.WithRecovery(true), dstate.OpenDmsArgs{}))
+	return db
+}
+
+func finalizeEmptyIDState(t *testing.T, db *dstate.DeploymentState) {
+	t.Helper()
+	_, err := db.Finalize(t.Context())
+	require.NoError(t, err)
+}
+
+func TestCreateFailsOnEmptyID(t *testing.T) {
+	adapter, err := dresources.NewAdapter((*emptyIDResource)(nil), "fake", nil)
+	require.NoError(t, err)
+	db := newEmptyIDDeploymentState(t)
+	defer finalizeEmptyIDState(t, db)
+	d := &DeploymentUnit{ResourceKey: "resources.fakes.foo", Adapter: adapter}
+
+	err = d.Create(t.Context(), db, &emptyIDState{Name: "foo"})
+	require.EqualError(t, err, "create returned an empty resource id")
+
+	_, ok := db.GetResourceEntry(d.ResourceKey)
+	assert.False(t, ok)
+}
+
+func TestUpdateWithIDFailsOnEmptyID(t *testing.T) {
+	adapter, err := dresources.NewAdapter((*emptyIDResource)(nil), "fake", nil)
+	require.NoError(t, err)
+	db := newEmptyIDDeploymentState(t)
+	defer finalizeEmptyIDState(t, db)
+	d := &DeploymentUnit{ResourceKey: "resources.fakes.foo", Adapter: adapter}
+	require.NoError(t, db.SaveState(t.Context(), d.ResourceKey, "old-id", &emptyIDState{Name: "foo"}, nil))
+
+	err = d.UpdateWithID(t.Context(), db, "old-id", &emptyIDState{Name: "bar"}, nil)
+	require.EqualError(t, err, "update returned an empty resource id")
+
+	assert.Equal(t, "old-id", db.GetResourceID(d.ResourceKey))
+}