Sync registered model aliases via SetAlias/DeleteAlias in direct mode

[varundeepsaini]

Fixes #4012

Summary

• The Update API ignores the aliases field on registered models, so aliases defined in bundle config were silently not applied in direct deployment mode.
• Sync aliases after create/update using explicit SetAlias/DeleteAlias API calls.
• Enable IncludeAliases on read so drift detection picks up alias changes.

Test plan

• Unit tests for create with aliases, update add/modify/delete aliases, remove all aliases, and no-op when unchanged
• Existing TestAll/registered_models CRUD test passes

[denik]

Do we need to wrap the error? Our errors already printed with context such as API endpoint.

[denik]

Can these be rewritten as acceptance test? You can capture requests there, run them against cloud, and also read remote state with databricks CLI.

[denik]

As a general guideline, DoUpdate should not read remote state. We already have it in the framework and we can pass it if it's needed. However, in this case we can probably get by reading passed Changes?

[simonfaltum]

Review (automated, 2 agents)

Verdict: Not ready yet | 1 Critical | 2 Major | 2 Gap(Major) | 2 Nit | 2 Suggestion

[Critical] Alias ordering causes infinite drift (never converges)

bundle/direct/dresources/registered_model.go (syncAliases)

syncAliases converts slices to maps, destroying order. RemapState returns aliases as-is from the backend. Without KeyedSlices() for aliases, the planner compares by index. If the backend returns aliases in a different order, direct mode plans updates forever. The acceptance test sorts aliases with sort_by(.alias_name), masking this.

Suggestion: Implement KeyedSlices() keyed by alias_name, or sort aliases consistently in RemapState. Add an idempotence test (unchanged redeploy is a no-op).

[Major] DoCreate/DoUpdate return stale state (aliases missing)

bundle/direct/dresources/registered_model.go:72,112

Both methods return the API response captured before syncAliases runs. The Create API doesn't apply aliases and the Update request sets Aliases: nil, so saved deployment state will have aliases: null. System self-heals on next deploy via DoRead, but bundle summary will show stale data.

Suggestion: After syncAliases, do a final DoRead, or merge aliases into the response before returning.

[Major] syncAliases failure after model mutation leaves diverged state

bundle/direct/dresources/registered_model.go:68-70,109-112

If SetAlias/DeleteAlias fails after the model is already created/updated, the workspace is mutated but the state file is unchanged. On create, this leaves an untracked registered model.

Suggestion: Move alias reconciliation to a post-save hook, or persist state before running alias side effects.

[Gap (Major)] No test for DoUpdate with Changes parameter

bundle/direct/dresources/registered_model_test.go

The primary production update path (changes.HasChange(pathAliases)) is not tested. Tests exercise syncAliases directly but not the Changes gating logic.

[Gap (Major)] No idempotence test (unchanged redeploy)

acceptance/bundle/resources/registered_models/aliases/script

Test only redeploys after config changes. Never verifies an unchanged deploy is a no-op.

[Nit] Unnecessary remote fetch in DoCreate

Model was just created, aliases are guaranteed empty. Pass empty slice instead of nil to avoid wasted API call.

[Nit] pathAliases variable style

Use var (...) block with descriptive comment, matching model_serving_endpoint.go pattern.

[Suggestion] Error wrapping in syncAliases

Wrap errors with context: fmt.Errorf("syncing aliases for %s: %w", fullName, err)

[Suggestion] Unnecessary remote fetch in DoCreate

Pass []catalog.RegisteredModelAlias{} instead of nil to skip the GET call.

[varundeepsaini]

@simonfaltum

have made the changes had a few thoughts on

• Stale state from DoCreate/DoUpdate: we save the local config (newState), not the API response . So missing aliases in the response don't affect state or bundle summary. The return value is only used for cross-resource refs, and nothing references registered_model.aliases.
  it would be corrrect to return the latest data but that would add a API call for no reason (as nothing is using it), do i stiill add it ?

[github-actions]

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

• PR number: 4637
• Commit SHA: 4256d018d8b7433299c88996b0f6c09067f99909

Checks will be approved automatically on success.

[simonfaltum]

No, don't add the extra API call. You're right that nothing references registered_model.aliases from the return value, and the framework persists local config (not remote state) anyway. The next plan cycle will fetch fresh state via DoRead. An extra GET call here would just add latency for no benefit. What you have is fine.

[simonfaltum]

@denik Would you do the final approval since you did the original review?