Allow referencing bundle resources by name #872

andrewnester · 2023-10-16T12:56:00Z

Changes

Now we can define variables with values which reference different Databricks resources by name.
When references like this, DABs automatically looks up the resource by this name and replaces the reference with ID of the resource referenced. Thus when the variable is used in the configuration it will contain the correct resolved ID of resource.

The resolvers are code generated and thus DABs support referencing all resources which has GetByName-like methods in Go SDK.

Example

variables:
  my_cluster_id:
    description: An existing cluster.
    lookup: 
      cluster: "12.2 shared"

resources:
  jobs:
    my_job:
      name: "My Job"
      tasks:
        - task_key: TestTask
          existing_cluster_id: ${var.my_cluster_id}

targets:
  dev:
    variables:
      my_cluster_id:
        lookup: 
           cluster: "dev-cluster"

Tests

Added unit test + manual testing

pietern

Nice work!

Is it possible to write:

variables:
  foo:
    lookup: cluster_policy:${var.cluster_policy_name}

And it still works? It might, if we allow interpolation on variables before running the lookup mutator.

pietern · 2023-10-20T13:13:05Z

.codegen/resolvers.go.tmpl

+func Resolvers() map[string](ResolverFunc) {
+    resolvers := make(map[string](ResolverFunc), 0)
+    {{range .Services -}}
+    {{- if not .IsAccounts -}}


Conversely, is there an IsWorkspace that we can use?

@pietern unfortunately not

pietern · 2023-10-20T13:21:44Z

bundle/config/mutator/resolve_resource_references_test.go

+	"github.com/stretchr/testify/require"
+)
+
+type MockClusterService struct{}


How did you generate these mocks? I think having a structured way to (auto-) generate them would be good to have, because as-is, any time a method is added to the upstream service, it would need to be manually reflected here.

That, and it would be great to use client mocking in more tests.

@pietern just with quick fix suggestion in IDE.

bundle/config/mutator/resolve_resource_references.go

pietern · 2023-10-20T13:31:28Z

bundle/config/mutator/resolve_resource_references.go

+		resource, name := parts[0], parts[1]
+		resolver, ok := m.resolvers[resource]
+		if !ok {
+			return fmt.Errorf("unable to resolve resource reference %s, no resovler for %s", lookup, resource)


For future reference, it would be super if we can use maps.Keys(m.resolvers) into some kind of "did you mean XYZ" error message in case of typos.

bundle/config/mutator/resolve_resource_references.go

bundle/resolvers/resolvers.go

andrewnester · 2023-10-23T08:03:40Z

And it still works? It might, if we allow interpolation on variables before running the lookup mutator.

@pietern At the moment it doesn't work. The reason is that lookups are also variables and to make variables inside lookup work we will either need to do interpolation twice (before and after lookup) or incorporate the lookup into interpolation.
Overall it makes the logic more complex and we should better allow override lookups per targets

shreyas-goenka

This is pretty cool!

bundle/config/mutator/resolve_resource_references.go

Co-authored-by: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>

.codegen/resolvers.go.tmpl

lennartkats-db · 2023-11-13T14:41:28Z

.codegen/resolvers.go.tmpl

+        "metastores"
+        "pipelines"
+        "queries"
+        "warehouses"


These are great! But could you also support service principals and webhooks as seen in https://github.com/databricks/universe/blob/master/serverless-smoke-tests/databricks.yml?

@andrewnester could you still take a look at this? Are those resource types supported or not?

@lennartkats-db added service principals but it seems like webhook notifications does not have an API (at least the one defined in OpenAPI) hence no support yet.

I see. It's disappointing we can't support webhooks yet, but we should indeed do that with a proper public API. Let's push on getting that API support. I tagged you on a related thread.

codecov-commenter · 2024-01-02T13:49:56Z

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (9cb0985) 48.18% compared to head (d655b67) 48.50%.
Report is 3 commits behind head on main.

Files	Patch %	Lines
bundle/config/root.go	0.00%	11 Missing ⚠️
bundle/config/mutator/set_variables.go	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #872      +/-   ##
==========================================
+ Coverage   48.18%   48.50%   +0.31%     
==========================================
  Files         268      269       +1     
  Lines       10509    10523      +14     
==========================================
+ Hits         5064     5104      +40     
+ Misses       4894     4865      -29     
- Partials      551      554       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pietern

Tried this PR out with a sample bundle and ran into the following:

Singular vs plural; I tried out instance-pool first and got no resolvers for instance-pool, but had to specify the plural name. This is doing a lookup of a single instance so I think we should use singular in the prefix.
I prefixed the name of the pool with a space, like this: instance-pools: pieter's test pool and got an error saying it doesn't exist (with a leading space). This can (should?) be trimmed. If we break it out into a separate field, this ambiguity would be gone (default YAML would trim leading spaces, but you could force it to use leading spaces by quoting a string).
Using a parameterized name doesn't work yet, and it looks like it should work:

variables:
  instance_pool_name:
    default: pieter's test pool

  instance_pool_id:
    lookup: "instance-pools:${var.instance_pool_name}"

bundle/config/target.go

bundle/tests/variables/env_overrides/databricks.yml

andrewnester · 2024-01-04T13:51:24Z

@pietern addressed points 1 and 2 by using typed structure for Lookup field. As to using variables in lookups it is currently not supported, it would require multiple interpolation passes. Instead we emit an error when variable reference is used in lookup field

bundle/config/mutator/resolve_resource_references.go

pietern · 2024-01-04T14:39:27Z

bundle/config/mutator/resolve_resource_references_test.go

+// UpdatePermissions implements compute.ClustersService.
+func (MockClusterService) UpdatePermissions(ctx context.Context, request compute.ClusterPermissionsRequest) (*compute.ClusterPermissions, error) {
+	panic("unimplemented")
+}


TBD: migrate this to the new SDK mocks when available in this repo

@pietern added to do list :)

bundle/config/variable/lookup.go

bundle/config/root.go

.codegen/lookup.go.tmpl

pietern

Unblock

CLI: * Tweak command groups in CLI help ([#1092](#1092)). * Fixed storage-credentials list command in text output ([#1094](#1094)). * Allow account client auth with environment variables when no `.databrickscfg` file present ([#1097](#1097)). * Always log with text format by default ([#1096](#1096)). Bundles: * Added exec.NewCommandExecutor to execute commands with correct interpreter ([#1075](#1075)). * Use resource key as name in permissions code ([#1087](#1087)). * Add support for reprompts if user input does not match template schema ([#946](#946)). * Improve experience for multiple builtin templates ([#1052](#1052)). * Improve error when bundle root is not writable ([#1093](#1093)). * Change recommended production deployment path from `/Shared` to `/Users` ([#1091](#1091)). * Allow referencing bundle resources by name ([#872](#872)). * Make libs/exec fallback to `sh` if `bash` cannot be found ([#1114](#1114)). Internal: * Functionality to walk a `config.Value` tree ([#1081](#1081)). * Rename libs/config -> libs/dyn ([#1086](#1086)). * Added codecov integration on code push ([#1095](#1095)). * Consolidate functions to convert `dyn.Value` to native types ([#1100](#1100)). * Define constant for the invalid `dyn.Value` ([#1101](#1101)). API Changes: * Changed `databricks serving-endpoints update-config` command with new required argument order. * Changed `databricks account ip-access-lists create` command with new required argument order. * Changed `databricks account ip-access-lists replace` command with new required argument order. * Changed `databricks account ip-access-lists update` command with new required argument order. * Changed `databricks ip-access-lists create` command with new required argument order. * Changed `databricks ip-access-lists replace` command with new required argument order. * Changed `databricks ip-access-lists update` command with new required argument order. OpenAPI commit a7a9dc025bb80303e676bf3708942c6aa06689f1 (2024-01-04) Dependency updates: * Bump github.com/hashicorp/terraform-exec from 0.19.0 to 0.20.0 ([#1088](#1088)). * Bump go.uber.org/mock from 0.3.0 to 0.4.0 ([#1089](#1089)). * Bump github.com/hashicorp/terraform-json from 0.18.0 to 0.20.0 ([#1090](#1090)). * Bump golang.org/x/term from 0.15.0 to 0.16.0 ([#1103](#1103)). * Bump github.com/cloudflare/circl from 1.3.3 to 1.3.7 ([#1105](#1105)). * Bump golang.org/x/sync from 0.5.0 to 0.6.0 ([#1104](#1104)). * Upgrade golang.org/x/crypto to v0.17.0 in internal module ([#1110](#1110)). * Upgrade Go SDK to v0.29.0 ([#1111](#1111)).

Allow referencing bundle resources by name

5a0cc80

andrewnester requested review from pietern and shreyas-goenka October 16, 2023 12:56

use lookup syntax

33578a6

pietern reviewed Oct 20, 2023

View reviewed changes

parallelised + fixes

6969f84

andrewnester requested a review from pietern October 20, 2023 15:47

shreyas-goenka reviewed Oct 25, 2023

View reviewed changes

bundle/config/mutator/resolve_resource_references.go Outdated Show resolved Hide resolved

Update bundle/config/mutator/resolve_resource_references.go

0a9ac46

Co-authored-by: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>

lennartkats-db reviewed Nov 13, 2023

View reviewed changes

Merge branch 'main' into named-reference

e844f89

target override

0248e66

andrewnester requested a review from shreyas-goenka January 2, 2024 14:53

andrewnester added 2 commits January 2, 2024 15:55

fixed formatting

60953f3

fixed typo

766ba5b

pietern reviewed Jan 4, 2024

View reviewed changes

bundle/config/target.go Outdated Show resolved Hide resolved

bundle/config/target.go Outdated Show resolved Hide resolved

bundle/tests/variables/env_overrides/databricks.yml Outdated Show resolved Hide resolved

andrewnester added 2 commits January 4, 2024 14:44

structured lookup field

485bd05

updated comment

af687ad

andrewnester requested review from pietern and lennartkats-db January 4, 2024 13:49

andrewnester enabled auto-merge January 4, 2024 13:51

pietern reviewed Jan 4, 2024

View reviewed changes

fixes

8288b7a

andrewnester requested a review from pietern January 4, 2024 15:06

pietern reviewed Jan 4, 2024

View reviewed changes

.codegen/lookup.go.tmpl Outdated Show resolved Hide resolved

pietern approved these changes Jan 4, 2024

View reviewed changes

added service-principals

d655b67

andrewnester added this pull request to the merge queue Jan 4, 2024

Merged via the queue into main with commit 5fb40f9 Jan 4, 2024
4 checks passed

andrewnester deleted the named-reference branch January 4, 2024 21:10

pietern mentioned this pull request Jan 11, 2024

Release v0.212.0 #1116

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow referencing bundle resources by name #872

Allow referencing bundle resources by name #872

andrewnester commented Oct 16, 2023 •

edited by pietern

Loading

pietern left a comment

pietern Oct 20, 2023

andrewnester Oct 20, 2023

pietern Oct 20, 2023

andrewnester Oct 20, 2023

pietern Oct 20, 2023

pietern Oct 20, 2023

andrewnester commented Oct 23, 2023

shreyas-goenka left a comment

lennartkats-db Nov 13, 2023

lennartkats-db Jan 4, 2024

andrewnester Jan 4, 2024

lennartkats-db Jan 5, 2024

codecov-commenter commented Jan 2, 2024 •

edited

Loading

pietern left a comment

andrewnester commented Jan 4, 2024

pietern Jan 4, 2024

andrewnester Jan 4, 2024

pietern left a comment

Allow referencing bundle resources by name #872

Allow referencing bundle resources by name #872

Conversation

andrewnester commented Oct 16, 2023 • edited by pietern Loading

Changes

Example

Tests

pietern left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewnester commented Oct 23, 2023

shreyas-goenka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jan 2, 2024 • edited Loading

Codecov Report

pietern left a comment

Choose a reason for hiding this comment

andrewnester commented Jan 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietern left a comment

Choose a reason for hiding this comment

andrewnester commented Oct 16, 2023 •

edited by pietern

Loading

codecov-commenter commented Jan 2, 2024 •

edited

Loading