Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions bundle/deployplan/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ const (
Delete ActionType = "delete"
)

// Bind and BindAndUpdate adopt an existing workspace resource: Bind records it unchanged and
// BindAndUpdate also applies the config. Only the resource being bound gets them, so they are
// never compared by severity.
const (
Bind ActionType = "bind"
BindAndUpdate ActionType = "bind_and_update"
)

var actionOrder = map[ActionType]int{
Undefined: 0,
Skip: 1,
Expand Down
4 changes: 2 additions & 2 deletions bundle/deployplan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (p *Plan) CountActions() ActionCounts {
switch entry.Action {
case Create:
c.Create++
case Update, UpdateWithID, Resize:
case Update, UpdateWithID, Resize, BindAndUpdate:
c.Change++
case Delete:
// A state-only delete touches nothing in the backend and only drops the
Expand All @@ -69,7 +69,7 @@ func (p *Plan) CountActions() ActionCounts {
// A recreate counts as both a delete and a create.
c.Delete++
c.Create++
case Skip, Undefined:
case Skip, Undefined, Bind:
c.Unchanged++
}
}
Expand Down
12 changes: 11 additions & 1 deletion bundle/direct/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,18 @@ func (d *DeploymentUnit) Destroy(ctx context.Context, db *dstate.DeploymentState
func (d *DeploymentUnit) Deploy(ctx context.Context, db *dstate.DeploymentState, newState any, actionType deployplan.ActionType, planEntry *deployplan.PlanEntry) error {
ctx = log.WithPrefix(ctx, "deploying "+d.ResourceKey)
ctx = d.withResourceKey(ctx)
if actionType == deployplan.Create {
// A bound resource is not in state yet, so its id comes from the plan entry.
switch actionType {
case deployplan.Create:
return d.Create(ctx, db, newState)
case deployplan.Bind:
copyRemoteEtag(d.ResourceKey, planEntry.RemoteState, newState)
log.Infof(ctx, "Bound %s id=%#v", d.ResourceKey, planEntry.ID)
return d.saveState(ctx, db, planEntry.ID, newState, d.DependsOn)
case deployplan.BindAndUpdate:
return d.Update(ctx, db, planEntry.ID, newState, planEntry)
default:
// The remaining actions act on a resource already in state.
}

oldID := db.GetResourceID(d.ResourceKey)
Expand Down
28 changes: 17 additions & 11 deletions bundle/direct/bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ type BindResult struct {
StatePath string
}

// copyRemoteEtag copies the etag from remote state for resources that use etag-based drift
// detection (dashboards and genie spaces). The etag is not provided by the
// user; it comes from remote. If we don't store it in state, we won't
// detect remote drift correctly and the next plan shows a bogus update.
func copyRemoteEtag(resourceKey string, remoteState, newState any) {
if (strings.Contains(resourceKey, ".dashboards.") || strings.Contains(resourceKey, ".genie_spaces.")) && remoteState != nil {
etag, err := structaccess.Get(remoteState, structpath.NewStringKey(nil, "etag"))
if err == nil && etag != nil {
if etagStr, ok := etag.(string); ok && etagStr != "" {
_ = structaccess.Set(newState, structpath.NewStringKey(nil, "etag"), etagStr)
}
}
}
}

// Bind adds an existing workspace resource to a temporary state and calculates
// if there will be any changes when deploying.
//
Expand Down Expand Up @@ -137,17 +152,8 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac
dependsOn = entry.DependsOn
}

// Copy etag from remote state for resources that use etag-based drift
// detection (dashboards and genie spaces). The etag is not provided by the
// user; it comes from remote. If we don't store it in state, we won't
// detect remote drift correctly and the next plan shows a bogus update.
if (strings.Contains(resourceKey, ".dashboards.") || strings.Contains(resourceKey, ".genie_spaces.")) && entry != nil && entry.RemoteState != nil {
etag, err := structaccess.Get(entry.RemoteState, structpath.NewStringKey(nil, "etag"))
if err == nil && etag != nil {
if etagStr, ok := etag.(string); ok && etagStr != "" {
_ = structaccess.Set(sv.Value, structpath.NewStringKey(nil, "etag"), etagStr)
}
}
if entry != nil {
copyRemoteEtag(resourceKey, entry.RemoteState, sv.Value)
}

// Compact hashed_fields fields so the persisted state stays small. Not needed for
Expand Down
66 changes: 49 additions & 17 deletions bundle/direct/bundle_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,21 +269,37 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
}

dbentry, hasEntry := b.StateDB.GetResourceEntry(resourceKey)
// A bound resource is read by the id being bound; it has no state entry yet.
adopt := b.BindKey == resourceKey
// Tolerate empty-ID entries from older partial-recreate failures
// (apply.Recreate now deletes state on the way through, but pre-fix
// state files may still carry a malformed entry). Treat as missing
// and let the resource be re-created on this plan.
if !hasEntry || dbentry.ID == "" {
if !adopt && (!hasEntry || dbentry.ID == "") {
entry.Action = deployplan.Create
return true
}

savedState, err := parseState(adapter.StateType(), dbentry.State)
if err != nil {
logdiag.LogError(ctx, fmt.Errorf("%s: interpreting state: %w", errorPrefix, err))
sv, ok := b.StateCache.Load(resourceKey)
if !ok {
logdiag.LogError(ctx, fmt.Errorf("%s: internal error: no state cache entry found for %q", errorPrefix, resourceKey))
return false
}

resourceID := dbentry.ID
var savedState any
if adopt {
resourceID = b.BindID
// No saved state, so diff the config against itself: only remote drift shows below.
savedState = sv.Value
} else {
savedState, err = parseState(adapter.StateType(), dbentry.State)
if err != nil {
logdiag.LogError(ctx, fmt.Errorf("%s: interpreting state: %w", errorPrefix, err))
return false
}
}

// Compact the saved state so hashed_fields fields are hashes
compactedSavedState, err := dresources.CompactState(adapter.ResourceConfig(), savedState)
if err != nil {
Expand All @@ -297,11 +313,6 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
// for integers: compare 0 with actual object ID. As long as real object IDs are never 0 we're good.
// Once we add non-id fields or add per-field details to "bundle plan", we must read dynamic data and deal with references as first class citizen.
// This means distinguishing between 0 that are actually object ids and 0 that are there because typed struct integer cannot contain ${...} string.
sv, ok := b.StateCache.Load(resourceKey)
if !ok {
logdiag.LogError(ctx, fmt.Errorf("%s: internal error: no state cache entry found for %q", errorPrefix, resourceKey))
return false
}
// Compact a copy for comparison only; sv.Value keeps the full contents, which
// the deploy sends to the API.
localState, err := dresources.CompactState(adapter.ResourceConfig(), sv.Value)
Expand All @@ -316,13 +327,13 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
}

remoteState, err := retry.With(ctx, retry.IsTransient, func() (any, error) {
return adapter.DoRead(ctx, dbentry.ID)
return adapter.DoRead(ctx, resourceID)
})
if err != nil {
if apierr.IsMissing(err) {
remoteState = nil
} else {
logdiag.LogError(ctx, fmt.Errorf("%s: reading id=%q: %w", errorPrefix, dbentry.ID, err))
logdiag.LogError(ctx, fmt.Errorf("%s: reading id=%q: %w", errorPrefix, resourceID, err))
return false
}
}
Expand All @@ -338,15 +349,15 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
if remoteState != nil {
remoteStateComparable, err = adapter.RemapState(remoteState)
if err != nil {
logdiag.LogError(ctx, fmt.Errorf("%s: interpreting remote state id=%q: %w", errorPrefix, dbentry.ID, err))
logdiag.LogError(ctx, fmt.Errorf("%s: interpreting remote state id=%q: %w", errorPrefix, resourceID, err))
return false
}

// Compact the remapped remote on the same fields, so a hashed_fields field
// is a hash on all three sides of the diff (saved, local, remote)
remoteStateComparable, err = dresources.CompactState(adapter.ResourceConfig(), remoteStateComparable)
if err != nil {
logdiag.LogError(ctx, fmt.Errorf("%s: compacting remote state id=%q: %w", errorPrefix, dbentry.ID, err))
logdiag.LogError(ctx, fmt.Errorf("%s: compacting remote state id=%q: %w", errorPrefix, resourceID, err))
return false
}

Expand All @@ -369,11 +380,28 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
return false
}

if remoteState == nil {
switch {
case remoteState == nil && adopt:
logdiag.LogError(ctx, fmt.Errorf("%s: cannot bind to id=%q: resource not found", errorPrefix, resourceID))
return false
case remoteState == nil:
// Even if local action is "recreate" which is higher than "create", we should still pick "create" here
// because we know remote does not exist.
action = deployplan.Create
} else {
case adopt:
// Bind adopts the resource as is or updates it in place; it does not recreate or resize.
switch maxAction := getMaxAction(entry.Changes); maxAction {
case deployplan.Skip:
action = deployplan.Bind
case deployplan.Update:
action = deployplan.BindAndUpdate
default:
logdiag.LogError(ctx, fmt.Errorf("%s: cannot bind id=%q: the config differs from the resource in a field that requires %s, which bind does not apply; align the config with the existing resource first", errorPrefix, resourceID, maxAction))
return false
}
// The id is not in state yet, so the plan entry carries it to apply.
entry.ID = resourceID
default:
action = getMaxAction(entry.Changes)
}

Expand All @@ -382,7 +410,7 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
b.RemoteStateCache.Store(resourceKey, remoteState)

// Validate that resources without DoUpdate don't have update actions
if action == deployplan.Update && !adapter.HasDoUpdate() {
if (action == deployplan.Update || action == deployplan.BindAndUpdate) && !adapter.HasDoUpdate() {
logdiag.LogError(ctx, fmt.Errorf("%s: resource does not support update action but plan produced update", errorPrefix))
return false
}
Expand Down Expand Up @@ -984,7 +1012,11 @@ func (b *DeploymentBundle) LookupReferencePreDeploy(ctx context.Context, path *s

if fieldPathS == "id" {
if targetAction.KeepsID() {
id := b.StateDB.GetResourceID(targetResourceKey)
// A resource being bound is not in state yet; its plan entry carries the id.
id := targetEntry.ID
if targetAction != deployplan.Bind && targetAction != deployplan.BindAndUpdate {
id = b.StateDB.GetResourceID(targetResourceKey)
}
if id == "" {
return nil, errors.New("internal error: no db entry")
}
Expand Down
17 changes: 17 additions & 0 deletions bundle/direct/bundle_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,23 @@ func TestLookupReferencePreDeploy_FinishedJobRun(t *testing.T) {
assert.Equal(t, jobs.RunResultStateSuccess, value)
}

// A resource being bound is not in state yet, so a reference to its id (from its own
// permissions, say) resolves from the plan entry.
func TestLookupReferencePreDeploy_BoundResourceID(t *testing.T) {
for _, action := range []deployplan.ActionType{deployplan.Bind, deployplan.BindAndUpdate} {
t.Run(string(action), func(t *testing.T) {
plan := deployplan.NewPlanDirect()
plan.Plan["resources.jobs.foo"] = &deployplan.PlanEntry{ID: "123", Action: action}
b := &DeploymentBundle{Plan: plan}

value, err := b.LookupReferencePreDeploy(t.Context(), structpath.MustParsePath("resources.jobs.foo.id"))

require.NoError(t, err)
assert.Equal(t, "123", value)
})
}
}

// jobRunResultStateAction classifies the result_state drift of a run in the given
// state. Old == New because a deploy records the outcome it asked for, and only
// the remote says whether the run reached it.
Expand Down
4 changes: 4 additions & 0 deletions bundle/direct/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ type DeploymentBundle struct {
Plan *deployplan.Plan
RemoteStateCache sync.Map
StateCache structvar.Cache

// BindKey and BindID name the resource CalculatePlan adopts for `bundle deployment bind`.
BindKey string
BindID string
}

// SetRemoteState updates the remote state with type validation and marks as fresh.
Expand Down
Loading