Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow referencing bundle resources by name #872

Merged
merged 12 commits into from
Jan 4, 2024
3 changes: 2 additions & 1 deletion .codegen.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
},
"batch": {
".codegen/cmds-workspace.go.tmpl": "cmd/workspace/cmd.go",
".codegen/cmds-account.go.tmpl": "cmd/account/cmd.go"
".codegen/cmds-account.go.tmpl": "cmd/account/cmd.go",
".codegen/resolvers.go.tmpl": "bundle/resolvers/resolvers.go"
},
"toolchain": {
"required": ["go"],
Expand Down
54 changes: 54 additions & 0 deletions .codegen/resolvers.go.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Code generated from OpenAPI specs by Databricks SDK Generator. DO NOT EDIT.

package resolvers

{{ $allowlist :=
list
"alerts"
"clusters"
"cluster-policies"
"clusters"
"dashboards"
"instance-pools"
"jobs"
"metastores"
"pipelines"
"queries"
"warehouses"
}}

import (
"context"
"fmt"
"github.com/databricks/cli/bundle"
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
)

type ResolverFunc func(ctx context.Context, b *bundle.Bundle, name string) (string, error)

func Resolvers() map[string](ResolverFunc) {
resolvers := make(map[string](ResolverFunc), 0)
{{range .Services -}}
{{- if in $allowlist .KebabName -}}
{{- if not .IsAccounts -}}
{{- if and .List .List.GetByName }}
resolvers["{{.KebabName}}"] = func(ctx context.Context, b *bundle.Bundle, name string) (string, error) {
w := b.WorkspaceClient()
entity, err := w.{{.PascalName}}.GetBy{{range .List.NamedIdMap.NamePath}}{{.PascalName}}{{end}}(ctx, name)
if err != nil {
return "", err
}

return fmt.Sprint(entity{{ template "field-path" .List.NamedIdMap.IdPath }}), nil
}
{{- end -}}
{{- end -}}
{{- end -}}
{{- end}}

return resolvers
}


{{- define "field-path" -}}
{{- range .}}.{{.PascalName}}{{end}}
{{- end -}}
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
bundle/resolvers/resolvers.go linguist-generated=true
cmd/account/access-control/access-control.go linguist-generated=true
cmd/account/billable-usage/billable-usage.go linguist-generated=true
cmd/account/budgets/budgets.go linguist-generated=true
Expand Down
67 changes: 67 additions & 0 deletions bundle/config/mutator/resolve_resource_references.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package mutator

import (
"context"
"fmt"
"strings"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/resolvers"
"github.com/databricks/cli/libs/log"
"golang.org/x/sync/errgroup"
)

const separator string = ":"

type resolveResourceReferences struct {
resolvers map[string]resolvers.ResolverFunc
}

func ResolveResourceReferences() bundle.Mutator {
return &resolveResourceReferences{
resolvers: resolvers.Resolvers(),
}
}

func (m *resolveResourceReferences) Apply(ctx context.Context, b *bundle.Bundle) error {
errs, errCtx := errgroup.WithContext(ctx)

for k := range b.Config.Variables {
v := b.Config.Variables[k]
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
if v.Lookup == "" {
continue
}

if v.HasValue() {
log.Debugf(ctx, "Ignoring '%s' lookup for the variable '%s' because the value is set", v.Lookup, k)
continue
}

lookup := v.Lookup
resource, name, ok := strings.Cut(lookup, separator)
if !ok {
return fmt.Errorf("unexpected format for lookup: %s. Expected lookup string to be of the form <resource_type>:<name>", lookup)
}

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

errs.Go(func() error {
id, err := resolver(errCtx, b, name)
if err != nil {
return fmt.Errorf("failed to resolve %s reference %s, err: %w", resource, lookup, err)
}

v.Set(id)
return nil
})
}

return errs.Wait()
}

func (*resolveResourceReferences) Name() string {
return "ResolveResourceReferences"
}
207 changes: 207 additions & 0 deletions bundle/config/mutator/resolve_resource_references_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
package mutator

import (
"context"
"testing"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/variable"
"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/stretchr/testify/require"
)

type MockClusterService struct{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pietern just with quick fix suggestion in IDE.


// ChangeOwner implements compute.ClustersService.
func (MockClusterService) ChangeOwner(ctx context.Context, request compute.ChangeClusterOwner) error {
panic("unimplemented")
}

// Create implements compute.ClustersService.
func (MockClusterService) Create(ctx context.Context, request compute.CreateCluster) (*compute.CreateClusterResponse, error) {
panic("unimplemented")
}

// Delete implements compute.ClustersService.
func (MockClusterService) Delete(ctx context.Context, request compute.DeleteCluster) error {
panic("unimplemented")
}

// Edit implements compute.ClustersService.
func (MockClusterService) Edit(ctx context.Context, request compute.EditCluster) error {
panic("unimplemented")
}

// Events implements compute.ClustersService.
func (MockClusterService) Events(ctx context.Context, request compute.GetEvents) (*compute.GetEventsResponse, error) {
panic("unimplemented")
}

// Get implements compute.ClustersService.
func (MockClusterService) Get(ctx context.Context, request compute.GetClusterRequest) (*compute.ClusterDetails, error) {
panic("unimplemented")
}

// GetPermissionLevels implements compute.ClustersService.
func (MockClusterService) GetPermissionLevels(ctx context.Context, request compute.GetClusterPermissionLevelsRequest) (*compute.GetClusterPermissionLevelsResponse, error) {
panic("unimplemented")
}

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

// List implements compute.ClustersService.
func (MockClusterService) List(ctx context.Context, request compute.ListClustersRequest) (*compute.ListClustersResponse, error) {
return &compute.ListClustersResponse{
Clusters: []compute.ClusterDetails{
{ClusterId: "1234-5678-abcd", ClusterName: "Some Custom Cluster"},
{ClusterId: "9876-5432-xywz", ClusterName: "Some Other Name"},
},
}, nil
}

// ListNodeTypes implements compute.ClustersService.
func (MockClusterService) ListNodeTypes(ctx context.Context) (*compute.ListNodeTypesResponse, error) {
panic("unimplemented")
}

// ListZones implements compute.ClustersService.
func (MockClusterService) ListZones(ctx context.Context) (*compute.ListAvailableZonesResponse, error) {
panic("unimplemented")
}

// PermanentDelete implements compute.ClustersService.
func (MockClusterService) PermanentDelete(ctx context.Context, request compute.PermanentDeleteCluster) error {
panic("unimplemented")
}

// Pin implements compute.ClustersService.
func (MockClusterService) Pin(ctx context.Context, request compute.PinCluster) error {
panic("unimplemented")
}

// Resize implements compute.ClustersService.
func (MockClusterService) Resize(ctx context.Context, request compute.ResizeCluster) error {
panic("unimplemented")
}

// Restart implements compute.ClustersService.
func (MockClusterService) Restart(ctx context.Context, request compute.RestartCluster) error {
panic("unimplemented")
}

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

// SparkVersions implements compute.ClustersService.
func (MockClusterService) SparkVersions(ctx context.Context) (*compute.GetSparkVersionsResponse, error) {
panic("unimplemented")
}

// Start implements compute.ClustersService.
func (MockClusterService) Start(ctx context.Context, request compute.StartCluster) error {
panic("unimplemented")
}

// Unpin implements compute.ClustersService.
func (MockClusterService) Unpin(ctx context.Context, request compute.UnpinCluster) error {
panic("unimplemented")
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pietern added to do list :)


func TestResolveClusterReference(t *testing.T) {
clusterRef1 := "clusters:Some Custom Cluster"
clusterRef2 := "clusters:Some Other Name"
justString := "random string"
b := &bundle.Bundle{
Config: config.Root{
Variables: map[string]*variable.Variable{
"my-cluster-id-1": {
Lookup: clusterRef1,
},
"my-cluster-id-2": {
Lookup: clusterRef2,
},
"some-variable": {
Value: &justString,
},
},
},
}

b.WorkspaceClient().Clusters.WithImpl(MockClusterService{})

err := bundle.Apply(context.Background(), b, ResolveResourceReferences())
require.NoError(t, err)
require.Equal(t, "1234-5678-abcd", *b.Config.Variables["my-cluster-id-1"].Value)
require.Equal(t, "9876-5432-xywz", *b.Config.Variables["my-cluster-id-2"].Value)
}

func TestResolveNonExistentClusterReference(t *testing.T) {
clusterRef := "clusters:Random"
justString := "random string"
b := &bundle.Bundle{
Config: config.Root{
Variables: map[string]*variable.Variable{
"my-cluster-id": {
Lookup: clusterRef,
},
"some-variable": {
Value: &justString,
},
},
},
}

b.WorkspaceClient().Clusters.WithImpl(MockClusterService{})

err := bundle.Apply(context.Background(), b, ResolveResourceReferences())
require.ErrorContains(t, err, "failed to resolve clusters reference clusters:Random, err: ClusterDetails named 'Random' does not exist")
}

func TestResolveNonExistentResourceType(t *testing.T) {
clusterRef := "donotexist:Random"
b := &bundle.Bundle{
Config: config.Root{
Variables: map[string]*variable.Variable{
"my-cluster-id": {
Lookup: clusterRef,
},
},
},
}

b.WorkspaceClient().Clusters.WithImpl(MockClusterService{})

err := bundle.Apply(context.Background(), b, ResolveResourceReferences())
require.ErrorContains(t, err, "unable to resolve resource reference donotexist:Random, no resolvers for donotexist")
}

func TestNoLookupIfVariableIsSet(t *testing.T) {
clusterRef := "donotexist:Random"
b := &bundle.Bundle{
Config: config.Root{
Variables: map[string]*variable.Variable{
"my-cluster-id": {
Lookup: clusterRef,
},
},
},
}

b.WorkspaceClient().Clusters.WithImpl(MockClusterService{})
b.Config.Variables["my-cluster-id"].Set("random value")

err := bundle.Apply(context.Background(), b, ResolveResourceReferences())
require.NoError(t, err)
require.Equal(t, "random value", *b.Config.Variables["my-cluster-id"].Value)
}
6 changes: 6 additions & 0 deletions bundle/config/mutator/set_variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ func setVariable(ctx context.Context, v *variable.Variable, name string) error {
return nil
}

// case: Defined a variable for named lookup for a resource
// It will be resolved later in ResolveResourceReferences mutator
if v.Lookup != "" {
return nil
}

// We should have had a value to set for the variable at this point.
// TODO: use cmdio to request values for unassigned variables if current
// terminal is a tty. Tracked in https://github.com/databricks/cli/issues/379
Expand Down
20 changes: 16 additions & 4 deletions bundle/config/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,25 @@ func (r *Root) MergeTargetOverrides(target *Target) error {

if target.Variables != nil {
for k, v := range target.Variables {
variable, ok := r.Variables[k]
rootVariable, ok := r.Variables[k]
if !ok {
return fmt.Errorf("variable %s is not defined but is assigned a value", k)
}
// we only allow overrides of the default value for a variable
defaultVal := v
variable.Default = &defaultVal

if sv, ok := v.(string); ok {
// we allow overrides of the default value for a variable
defaultVal := sv
rootVariable.Default = &defaultVal
} else if vv, ok := v.(map[string]interface{}); ok {
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
// we also allow overrides of the lookup value for a variable
lookup, ok := vv["lookup"]
if !ok {
return fmt.Errorf("variable %s is incorrectly defined lookup override, no 'lookup' key defined", k)
}
rootVariable.Lookup = lookup.(string)
} else {
return fmt.Errorf("variable %s is incorrectly defined in target override", k)
}
}
}

Expand Down
Loading
Loading