-
Notifications
You must be signed in to change notification settings - Fork 57
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
Changes from 4 commits
5a0cc80
33578a6
6969f84
0a9ac46
e844f89
0248e66
60953f3
766ba5b
485bd05
af687ad
8288b7a
d655b67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 -}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Conversely, is there an There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 -}} | ||
{{- end}} | ||
|
||
return resolvers | ||
} | ||
|
||
|
||
{{- define "field-path" -}} | ||
{{- range .}}.{{.PascalName}}{{end}} | ||
{{- end -}} |
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" | ||
} |
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{} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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") | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewnester could you still take a look at this? Are those resource types supported or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.