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
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM alpine:3.22 as builder
FROM alpine:3.22@sha256:55ae5d250caebc548793f321534bc6a8ef1d116f334f18f4ada1b2daad3251b2 as builder

RUN ["apk", "add", "jq"]
RUN ["apk", "add", "bash"]
Expand All @@ -13,7 +13,7 @@ ARG ARCH
RUN /build/docker/setup.sh

# Start from a fresh base image, to remove any build artifacts and scripts.
FROM alpine:3.22
FROM alpine:3.22@sha256:55ae5d250caebc548793f321534bc6a8ef1d116f334f18f4ada1b2daad3251b2

ENV DATABRICKS_TF_EXEC_PATH "/app/bin/terraform"
ENV DATABRICKS_TF_CLI_CONFIG_FILE "/app/config/config.tfrc"
Expand Down
8 changes: 8 additions & 0 deletions acceptance/bundle/telemetry/deploy-compute-type/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ Deployment complete!
"key": "python_wheel_wrapper_is_set",
"value": false
},
{
"key": "validation_passed",
"value": true
},
{
"key": "skip_artifact_cleanup",
"value": false
Expand Down Expand Up @@ -79,6 +83,10 @@ Deployment complete!
"key": "python_wheel_wrapper_is_set",
"value": false
},
{
"key": "validation_passed",
"value": true
},
{
"key": "skip_artifact_cleanup",
"value": false
Expand Down
4 changes: 4 additions & 0 deletions acceptance/bundle/telemetry/deploy-experimental/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ Deployment complete!
"key": "python_wheel_wrapper_is_set",
"value": false
},
{
"key": "validation_passed",
"value": true
},
{
"key": "skip_artifact_cleanup",
"value": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ Deployment complete!
"key": "python_wheel_wrapper_is_set",
"value": false
},
{
"key": "validation_passed",
"value": true
},
{
"key": "skip_artifact_cleanup",
"value": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ Deployment complete!
"key": "python_wheel_wrapper_is_set",
"value": false
},
{
"key": "validation_passed",
"value": true
},
{
"key": "skip_artifact_cleanup",
"value": false
Expand Down
8 changes: 8 additions & 0 deletions acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ Deployment complete!
"key": "python_wheel_wrapper_is_set",
"value": false
},
{
"key": "validation_passed",
"value": true
},
{
"key": "skip_artifact_cleanup",
"value": false
Expand Down Expand Up @@ -80,6 +84,10 @@ Deployment complete!
"key": "python_wheel_wrapper_is_set",
"value": true
},
{
"key": "validation_passed",
"value": true
},
{
"key": "skip_artifact_cleanup",
"value": true
Expand Down
4 changes: 4 additions & 0 deletions acceptance/bundle/telemetry/deploy/out.telemetry.txt
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@
"key": "python_wheel_wrapper_is_set",
"value": false
},
{
"key": "validation_passed",
"value": true
},
{
"key": "skip_artifact_cleanup",
"value": false
Expand Down
1 change: 1 addition & 0 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type Metrics struct {
PythonUpdatedResourcesCount int64
ExecutionTimes []protos.IntMapEntry
LocalCacheMeasurementsMs []protos.IntMapEntry // Local cache measurements stored as milliseconds
DeployPlanMetrics []protos.IntMapEntry // Deployment plan and graph metrics (direct engine)
}

// SetBoolValue sets the value of a boolean metric.
Expand Down
38 changes: 38 additions & 0 deletions bundle/config/mutator/resolve_variable_references.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,18 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
// We rewrite it here to make the resolution logic simpler.
varPath := dyn.NewPath(dyn.Key("var"))

// Detect nested variable references like ${var.foo_${var.bar}} and log
// a telemetry event if found. These patterns are not supported by the
// interpolation regex and silently fail to resolve.
m.detectNestedVariableReferences(b)

var diags diag.Diagnostics
maxRounds := 1 + m.extraRounds
roundsUsed := 0

for round := range maxRounds {
hasUpdates, newDiags := m.resolveOnce(b, prefixes, varPath)
roundsUsed = round + 1

diags = diags.Extend(newDiags)

Expand All @@ -180,6 +187,13 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
b.Metrics.SetBoolValue("artifacts_reference_used", true)
}

if roundsUsed > 1 {
b.Metrics.SetBoolValue("variable_resolution_rounds_gt_1", true)
}
if roundsUsed > 3 {
b.Metrics.SetBoolValue("variable_resolution_rounds_gt_3", true)
}

return diags
}

Expand Down Expand Up @@ -294,6 +308,30 @@ func (m *resolveVariableReferences) selectivelyMutate(b *bundle.Bundle, fn func(
})
}

// errNestedVarRefFound is a sentinel error used to short-circuit WalkReadOnly
// once a nested variable reference is detected.
var errNestedVarRefFound = errors.New("nested variable reference found")

// detectNestedVariableReferences walks the bundle configuration and checks for
// nested variable references like ${var.foo_${var.bar}}. These patterns are not
// supported and are tracked via telemetry to understand how common they are.
func (m *resolveVariableReferences) detectNestedVariableReferences(b *bundle.Bundle) {
err := dyn.WalkReadOnly(b.Config.Value(), func(_ dyn.Path, v dyn.Value) error {
s, ok := v.AsString()
if !ok {
return nil
}

if dynvar.ContainsNestedVariableReference(s) {
return errNestedVarRefFound
}
return nil
})
if err == errNestedVarRefFound {
b.Metrics.SetBoolValue("nested_var_reference_used", true)
}
}

func getAllKeys(root dyn.Value) ([]string, error) {
var keys []string

Expand Down
85 changes: 85 additions & 0 deletions bundle/config/mutator/resolve_variable_references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,46 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/config/variable"
"github.com/databricks/cli/libs/telemetry/protos"
"github.com/databricks/databricks-sdk-go/service/pipelines"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestResolveVariableReferencesDetectsNestedVarRef(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Name: "${var.env_${var.region}}",
},
},
}

diags := bundle.Apply(t.Context(), b, ResolveVariableReferencesWithoutResources())
// The nested reference won't resolve, but we should still detect it.
_ = diags

assert.Contains(t, b.Metrics.BoolValues, protos.BoolMapEntry{Key: "nested_var_reference_used", Value: true})
}

func TestResolveVariableReferencesNoNestedVarRef(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Name: "${var.env}",
},
},
}

diags := bundle.Apply(t.Context(), b, ResolveVariableReferencesWithoutResources())
_ = diags

for _, entry := range b.Metrics.BoolValues {
assert.NotEqual(t, "nested_var_reference_used", entry.Key)
}
}

func TestResolveVariableReferencesWithSourceLinkedDeployment(t *testing.T) {
testCases := []struct {
enabled bool
Expand Down Expand Up @@ -63,3 +99,52 @@ func TestResolveVariableReferencesWithSourceLinkedDeployment(t *testing.T) {
testCase.assert(t, b)
}
}

func TestResolveVariableReferencesRoundsNoReferences(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Name: "literal-name",
},
},
}

diags := bundle.Apply(t.Context(), b, ResolveVariableReferencesWithoutResources())
require.NoError(t, diags.Error())

// No references means a single round with no updates, so gt_1 should not be set.
for _, entry := range b.Metrics.BoolValues {
assert.NotEqual(t, "variable_resolution_rounds_gt_1", entry.Key)
assert.NotEqual(t, "variable_resolution_rounds_gt_3", entry.Key)
}
}

func TestResolveVariableReferencesRoundsGt1MultiRound(t *testing.T) {
// Set up a chain: bundle.name -> var.a -> var.b -> literal.
// This requires 2 rounds to fully resolve.
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Name: "${var.a}",
},
Variables: map[string]*variable.Variable{
"a": {
Value: "${var.b}",
},
"b": {
Value: "final",
},
},
},
}

diags := bundle.Apply(t.Context(), b, ResolveVariableReferencesWithoutResources())
require.NoError(t, diags.Error())
assert.Equal(t, "final", b.Config.Bundle.Name)

assert.Contains(t, b.Metrics.BoolValues, protos.BoolMapEntry{Key: "variable_resolution_rounds_gt_1", Value: true})
// 2 rounds should not trigger gt_3.
for _, entry := range b.Metrics.BoolValues {
assert.NotEqual(t, "variable_resolution_rounds_gt_3", entry.Key)
}
}
Loading
Loading