Skip to content

Commit 9681b7f

Browse files
authored
Resolves $resources as regular variables if set by user (#3370)
## Changes - Resolve references like `${resources.jobs.foo.name}` like regular variables if they are present in the config. - Update hasUpdates only if lookupFn returned a valid value (no test coverage for this, but makes more sense here). ## Why - It's consistent with how resolutions work in general. This makes resolved value visible in bundle validate -o json. - It simplifies direct deployment backend. Now we only need to implement state/output fields there. - It minimizes direct & TF implementation differences when we roll out direct deployment. ## Tests New test for these references.
1 parent b906545 commit 9681b7f

File tree

6 files changed

+87
-2
lines changed

6 files changed

+87
-2
lines changed

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
### CLI
1212

1313
### Bundles
14+
* Changed logic for resolving `${resources...}` references. Previously this would be done by terraform at deploy time. Now if it references a field that is present in the config, it will be done by DABs during bundle loading ([#3370](https://github.com/databricks/cli/pull/3370))
1415
* Add support for tagging pipelines ([#3086](https://github.com/databricks/cli/pull/3086))
1516

1617
### API Changes
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
variables:
2+
a:
3+
default: ${resources.schemas.foo.name}
4+
b:
5+
default: this is var.b ${var.a}
6+
7+
resources:
8+
schemas:
9+
foo:
10+
catalog_name: main
11+
name: ${resources.schemas.bar.name}
12+
# non_existing and "id" stay unresolved
13+
comment: unresolved? schemas.bar.non_existing:"${resources.schemas.bar.non_existing}" schemas.bar.id:"${resources.schemas.bar.id}"
14+
bar:
15+
catalog_name: main
16+
# "id" and "url" are output only and not present in the config -- should be left unresolved
17+
name: unresolved? apps.baz.id="${resources.apps.baz.id}" apps.baz.url="${resources.apps.baz.url}"
18+
# includes direct and indirect references to $resources
19+
comment: var.b=${var.b} schemas.bar.name=${resources.schemas.bar.name} schemas.foo.name=${resources.schemas.foo.name}
20+
apps:
21+
baz:
22+
name: myapp
23+
source_code_path: .
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Local = true
2+
Cloud = false
3+
4+
[EnvMatrix]
5+
DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct-exp"]
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
{
2+
"a": {
3+
"default": "${resources.schemas.foo.name}",
4+
"value": "${resources.schemas.foo.name}"
5+
},
6+
"b": {
7+
"default": "this is var.b ${resources.schemas.foo.name}",
8+
"value": "this is var.b ${resources.schemas.foo.name}"
9+
}
10+
}
11+
{
12+
"apps": {
13+
"baz": {
14+
"name": "myapp",
15+
"permissions": [],
16+
"source_code_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files"
17+
}
18+
},
19+
"schemas": {
20+
"bar": {
21+
"catalog_name": "main",
22+
"comment": "var.b=this is var.b unresolved? apps.baz.id=\"${resources.apps.baz.id}\" apps.baz.url=\"${resources.apps.baz.url}\" schemas.bar.name=unresolved? apps.baz.id=\"${resources.apps.baz.id}\" apps.baz.url=\"${resources.apps.baz.url}\" schemas.foo.name=unresolved? apps.baz.id=\"${resources.apps.baz.id}\" apps.baz.url=\"${resources.apps.baz.url}\"",
23+
"name": "unresolved? apps.baz.id=\"${resources.apps.baz.id}\" apps.baz.url=\"${resources.apps.baz.url}\""
24+
},
25+
"foo": {
26+
"catalog_name": "main",
27+
"comment": "unresolved? schemas.bar.non_existing:\"${resources.schemas.bar.non_existing}\" schemas.bar.id:\"${resources.schemas.bar.id}\"",
28+
"name": "unresolved? apps.baz.id=\"${resources.apps.baz.id}\" apps.baz.url=\"${resources.apps.baz.url}\""
29+
}
30+
}
31+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
$CLI bundle validate -o json | jq '.variables, .resources'

bundle/config/mutator/resolve_variable_references.go

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ var defaultPrefixes = []string{
4141
"bundle",
4242
"workspace",
4343
"variables",
44+
"resources",
45+
}
46+
47+
var optionalResolution = map[string]bool{
48+
// Enables different mode for resolution:
49+
// - normalization is not done, if field is not set by user it's missing
50+
// - if field is missing (either it's a valid field but not set or invalid field), it remains unresolved, no error
51+
"resources": true,
4452
}
4553

4654
var artifactPath = dyn.MustPathFromString("artifacts")
@@ -229,8 +237,24 @@ func (m *resolveVariableReferences) resolveOnce(b *bundle.Bundle, prefixes []dyn
229237
// Perform resolution only if the path starts with one of the specified prefixes.
230238
for _, prefix := range prefixes {
231239
if path.HasPrefix(prefix) {
232-
hasUpdates = true
233-
return m.lookupFn(normalized, path, b)
240+
isOpt := optionalResolution[prefix[0].Key()]
241+
var value dyn.Value
242+
var err error
243+
if isOpt {
244+
// We don't want injected zero value when resolving $resources.
245+
// We only want entries that are explicitly provided by users, so we're using root not normalized here.
246+
value, err = m.lookupFn(root, path, b)
247+
if !value.IsValid() {
248+
// Not having a value is not an error in this case, it might be resolved at deploy time.
249+
// TODO: we still could check whether it's part of the schema or not. If latter, we can reject it right away.
250+
// TODO: This might be better done after we got rid of TF.
251+
return dyn.InvalidValue, dynvar.ErrSkipResolution
252+
}
253+
} else {
254+
value, err = m.lookupFn(normalized, path, b)
255+
}
256+
hasUpdates = hasUpdates || (err == nil && value.IsValid())
257+
return value, err
234258
}
235259
}
236260

0 commit comments

Comments
 (0)