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
37 changes: 37 additions & 0 deletions .codegen/resolvers.go.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Code generated from OpenAPI specs by Databricks SDK Generator. DO NOT EDIT.

package resolvers

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 not .IsAccounts -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Conversely, is there an IsWorkspace that we can use?

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 unfortunately not

{{- 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}}

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
61 changes: 61 additions & 0 deletions bundle/config/mutator/resolve_resource_references.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package mutator

import (
"context"
"fmt"
"strings"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/resolvers"
"github.com/databricks/cli/libs/log"
)

var separator string = ":"
andrewnester marked this conversation as resolved.
Show resolved Hide resolved

type resolveResourceReferences struct {
resolvers map[string](resolvers.ResolverFunc)
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
}

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

func (m *resolveResourceReferences) Apply(ctx context.Context, b *bundle.Bundle) error {
for k, v := range b.Config.Variables {
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
parts := strings.Split(lookup, separator)
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
if len(parts) != 2 {
return fmt.Errorf("incorrect lookup specified %s", lookup)
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
}

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

}

id, err := resolver(ctx, b, name)
if err != nil {
return fmt.Errorf("failed to resolve resource reference %s, err: %w", lookup, err)
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
}

v.Set(id)
}

return nil
}

func (*resolveResourceReferences) Name() string {
return "ResolveResourceReferences"
}
202 changes: 202 additions & 0 deletions bundle/config/mutator/resolve_resource_references_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
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) {
clusterRef := "clusters:Some Custom Cluster"
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.NoError(t, err)
require.Equal(t, "1234-5678-abcd", *b.Config.Variables["my-cluster-id"].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 resource 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 resovler 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
6 changes: 6 additions & 0 deletions bundle/config/variable/variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ type Variable struct {
// 5. Throw error, since if no default value is defined, then the variable
// is required
Value *string `json:"value,omitempty" bundle:"readonly"`

// A string value that represents a reference to the remote resource by name
// Format: "<resource>:<name>"
// The value of this field will be used to lookup the resource by name
// And assign the value of the variable to ID of the resource found.
Lookup string `json:"lookup,omitempty"`
}

// True if the variable has been assigned a default value. Variables without a
Expand Down
1 change: 1 addition & 0 deletions bundle/phases/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func Initialize() bundle.Mutator {
mutator.ExpandWorkspaceRoot(),
mutator.DefineDefaultWorkspacePaths(),
mutator.SetVariables(),
mutator.ResolveResourceReferences(),
interpolation.Interpolate(
interpolation.IncludeLookupsInPath("bundle"),
interpolation.IncludeLookupsInPath("workspace"),
Expand Down
Loading