Skip to content
Open
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
1 change: 1 addition & 0 deletions .nextchanges/bundles/state-cli-version-last-writer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The `cli_version` field in the direct engine's deployment state (`resources.json`) now records the CLI version that last wrote the state, matching the terraform engine and the field's documented meaning. Previously it kept the version of the CLI that first created the state, so it stayed stale no matter how many times a newer CLI deployed over it.
1 change: 1 addition & 0 deletions .nextchanges/bundles/state-newer-cli-version-warning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Warn when the deployment state was last written by a newer CLI version than the one running.
7 changes: 7 additions & 0 deletions acceptance/bundle/state/newer_cli_version/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
bundle:
name: test-bundle

resources:
jobs:
my_job:
name: "my job"
2 changes: 2 additions & 0 deletions acceptance/bundle/state/newer_cli_version/out.test.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions acceptance/bundle/state/newer_cli_version/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

=== State written by a newer CLI: warn, but proceed

>>> [CLI] bundle plan
Warn: State was last deployed with CLI version 99.0.0 but current version is [CLI_VERSION]
create jobs.my_job

Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged

=== State written by this CLI: no warning

>>> [CLI] bundle plan
create jobs.my_job

Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"state_version": 2,
"cli_version": "99.0.0",
"lineage": "test-lineage",
"serial": 1,
"state": {}
}
11 changes: 11 additions & 0 deletions acceptance/bundle/state/newer_cli_version/script
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
title "State written by a newer CLI: warn, but proceed\n"
mkdir -p .databricks/bundle/default
cp resources.newer.json .databricks/bundle/default/resources.json
trace $CLI bundle plan

title "State written by this CLI: no warning\n"
# The running version is only known at test time, so derive the state's
# cli_version from the binary itself rather than committing it as a fixture.
version=$($CLI version | sed 's/^Databricks CLI v//')
jq --arg v "$version" '.cli_version = $v' resources.newer.json > .databricks/bundle/default/resources.json
trace $CLI bundle plan
4 changes: 4 additions & 0 deletions acceptance/bundle/state/newer_cli_version/test.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Ignore = [".databricks"]

# The warning is emitted when reading the direct engine's resources.json.
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Deployment complete!
>>> print_state.py
{
"state_version": 2,
"cli_version": "0.0.0-test",
"cli_version": "[CLI_VERSION]",
"lineage": "test-lineage",
"serial": 2,
"state": {
Expand Down
37 changes: 32 additions & 5 deletions bundle/direct/dstate/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,15 @@ type DeploymentState struct {
}

type Header struct {
StateVersion int `json:"state_version"`
CLIVersion string `json:"cli_version"`
Lineage string `json:"lineage"`
Serial int `json:"serial"`
StateVersion int `json:"state_version"`

// CLIVersion is the version of the CLI that last wrote this state. It is
// refreshed from the WAL header on every deploy that commits changes, so it
// tracks the most recent writer rather than the CLI that created the state.
CLIVersion string `json:"cli_version"`

Lineage string `json:"lineage"`
Serial int `json:"serial"`

// Features maps each feature flag this state depends on to a (currently empty)
// value. This CLI writes no features; it only reads the field to detect a state
Expand Down Expand Up @@ -184,6 +189,18 @@ func (db *DeploymentState) GetResourceID(key string) string {
return db.stateIDs[key]
}

// StateCLIVersion returns the CLI version that last wrote the state, or an empty
// string if the state does not record one (a fresh state that this CLI has not
// written yet). It is the version stored in the on-disk header, not the running
// build's version.
func (db *DeploymentState) StateCLIVersion() string {
db.AssertOpenedForReadOrWrite()
db.mu.Lock()
defer db.mu.Unlock()

return db.Data.CLIVersion
}

// GetOrInitLineage returns the deployment lineage, generating and storing a new
// one if the state does not have one yet. It is the single place the lineage is
// initialized, shared so the direct deployment engine (when it writes state, via
Expand Down Expand Up @@ -338,7 +355,10 @@ func (db *DeploymentState) mergeWalIntoState(ctx context.Context) (bool, error)
scanner.Buffer(make([]byte, 0, initialBufferSize), maxWalEntrySize)
lineNumber := 0
var corruptedLines [][]byte
var newSerial int
var (
newSerial int
newCLIVersion string
)

for scanner.Scan() {
lineNumber++
Expand All @@ -363,6 +383,7 @@ func (db *DeploymentState) mergeWalIntoState(ctx context.Context) (bool, error)
return false, fmt.Errorf("WAL serial (%d) is ahead of expected (%d), state may be corrupted", header.Serial, expectedSerial)
}
newSerial = header.Serial
newCLIVersion = header.CLIVersion
} else {
var entry WALEntry
if err := json.Unmarshal(line, &entry); err != nil {
Expand Down Expand Up @@ -405,8 +426,14 @@ func (db *DeploymentState) mergeWalIntoState(ctx context.Context) (bool, error)
// for it leaves the in-memory serial ahead of the persisted one, so the
// next deploy writes its WAL header at serial+2 and recovery rejects it as
// "ahead of expected". See acceptance/bundle/deploy/wal/header-only-wal.
//
// The CLI version moves with the serial for the same reason: it records the
// CLI that last wrote the state, so it is only accurate once that write is
// persisted. Without this the field keeps the version of the CLI that first
// created the state, no matter how many times a newer CLI deploys over it.
if hasEntries {
db.Data.Serial = newSerial
db.Data.CLIVersion = newCLIVersion
}

return hasEntries, nil
Expand Down
47 changes: 47 additions & 0 deletions bundle/direct/dstate/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"path/filepath"
"testing"

"github.com/databricks/cli/internal/build"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -101,6 +102,52 @@ func TestPanicOnDoubleOpen(t *testing.T) {
mustFinalize(t, &db)
}

// TestCLIVersionRecordsLastWriter pins that cli_version tracks the CLI that last
// wrote the state, not the one that created it. Previously the field was only set
// when the state was first created: the WAL header carried the deploying CLI's
// version but replay dropped it, so a state stayed pinned to its original writer
// no matter how many times a newer CLI deployed over it.
func TestCLIVersionRecordsLastWriter(t *testing.T) {
path := filepath.Join(t.TempDir(), "state.json")

// A state written by some older CLI.
seed := `{"state_version":2,"cli_version":"0.1.2","lineage":"test-lineage","serial":1,"state":{}}`
require.NoError(t, os.WriteFile(path, []byte(seed), 0o600))

var db DeploymentState
require.NoError(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true)))
require.NoError(t, db.SaveState("resources.jobs.my_job", "123", map[string]string{"k": "v"}, nil))
mustFinalize(t, &db)

var reopened DeploymentState
require.NoError(t, reopened.Open(t.Context(), path, WithRecovery(false), WithWrite(false)))
assert.Equal(t, build.GetInfo().Version, reopened.Data.CLIVersion)
assert.Equal(t, 2, reopened.Data.Serial)
mustFinalize(t, &reopened)
}

// TestHeaderOnlyWALDoesNotUpdateCLIVersion is the counterpart to the serial
// invariant below: a deploy that commits nothing does not persist a state file,
// so it must not claim to have written one.
func TestHeaderOnlyWALDoesNotUpdateCLIVersion(t *testing.T) {
path := filepath.Join(t.TempDir(), "state.json")
walPath := path + walSuffix

seed := `{"state_version":2,"cli_version":"0.1.2","lineage":"test-lineage","serial":1,"state":{}}`
require.NoError(t, os.WriteFile(path, []byte(seed), 0o600))

header := Header{Lineage: "test-lineage", Serial: 2, StateVersion: currentStateVersion, CLIVersion: build.GetInfo().Version}
headerLine, err := json.Marshal(header)
require.NoError(t, err)
require.NoError(t, os.WriteFile(walPath, append(headerLine, '\n'), 0o600))

var recovered DeploymentState
require.NoError(t, recovered.Open(t.Context(), path, WithRecovery(true), WithWrite(false)))
assert.Equal(t, "0.1.2", recovered.Data.CLIVersion, "a header-only WAL wrote no state, so the version must not move")
assert.Equal(t, 1, recovered.Data.Serial)
mustFinalize(t, &recovered)
}

func TestHeaderOnlyWALRecoveryDoesNotAdvanceSerial(t *testing.T) {
path := filepath.Join(t.TempDir(), "state.json")
walPath := path + walSuffix
Expand Down
24 changes: 24 additions & 0 deletions cmd/bundle/utils/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/telemetry/protos"
"github.com/spf13/cobra"
"golang.org/x/mod/semver"
)

type ProcessOptions struct {
Expand Down Expand Up @@ -215,6 +216,16 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle
logdiag.LogError(ctx, err)
return b, stateDesc, root.ErrAlreadyPrinted
}

// Warn when the state was last written by a newer CLI than the one
// running now. The state schema version is a hard gate (dstate.Open
// rejects a too-new state_version), but a state can be written by a
// newer CLI that shares this schema; that is allowed, and this only
// hints that a downgrade may be unintended.
currentVersion := build.GetInfo().Version
if stateVersion := b.DeploymentBundle.StateDB.StateCLIVersion(); isNewerVersion(stateVersion, currentVersion) {
log.Warnf(ctx, "State was last deployed with CLI version %s but current version is %s", stateVersion, currentVersion)
}
}

// These are not safe in plan/deploy because they insert empty config settings for deleted resources.
Expand Down Expand Up @@ -396,6 +407,19 @@ func ResolveEngineSetting(ctx context.Context, b *bundle.Bundle) (engine.EngineS
return engine.EngineSetting{}, nil
}

// isNewerVersion reports whether the state's recorded CLI version is strictly
// newer than the running build. Both are bare versions without a leading "v".
// An empty stateVersion (state not written by any CLI yet) or an unparseable
// version returns false, so we never warn on missing or malformed data.
func isNewerVersion(stateVersion, currentVersion string) bool {
sv := "v" + stateVersion
cv := "v" + currentVersion
if !semver.IsValid(sv) || !semver.IsValid(cv) {
return false
}
return semver.Compare(sv, cv) > 0
}

func rejectDefinitions(ctx context.Context, b *bundle.Bundle) {
if b.Config.Definitions != nil {
v := dyn.GetValue(b.Config.Value(), "definitions")
Expand Down
44 changes: 44 additions & 0 deletions cmd/bundle/utils/process_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package utils

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestIsNewerVersion(t *testing.T) {
tests := []struct {
name string
state string
current string
want bool
}{
{"state newer major", "1.0.0", "0.300.0", true},
{"state newer minor", "0.301.0", "0.300.0", true},
{"state newer patch", "0.300.1", "0.300.0", true},
{"same version", "0.300.0", "0.300.0", false},
{"state older", "0.299.0", "0.300.0", false},
// A dev build is built from main, so its version is the next release with a
// -dev prerelease: newer than the last release, older than the release it
// will become. Deploying with a dev build after a state written by the last
// release is the normal case for a CLI developer and must not warn.
{"state from last release, dev current", "0.300.0", "0.301.0-dev+abc123", false},
// A released CLI reading a state written by a dev build of the same upcoming
// release does warn: that build may have written fields this CLI lacks.
{"dev state, released current", "0.301.0-dev+abc123", "0.300.0", true},
// A prerelease sorts below its own release per semver.
{"prerelease below release", "0.300.0-rc1", "0.300.0", false},
{"release above prerelease", "0.300.0", "0.300.0-rc1", true},
// Missing or malformed data must never produce a warning.
{"empty state version", "", "0.300.0", false},
{"empty current version", "0.300.0", "", false},
{"malformed state version", "not-a-version", "0.300.0", false},
{"malformed current version", "0.300.0", "not-a-version", false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, isNewerVersion(tt.state, tt.current))
})
}
}
Loading