From 57736b3ca24e151ddba7f564b4f727d5ab8e592d Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 4 Nov 2026 22:27:57 +0000 Subject: [PATCH 1/2] yamlsaver: keep alphabetical order when no explicit order is given ConvertToMapValue sorts keys alphabetically, but without an Order the entries were stored with zero-valued locations and the encoder, which orders by Location.Line, fell back to Go map iteration order. Number the entries so the sorted order survives. Existing callers all pass an Order and are unaffected. Co-authored-by: Isaac --- libs/dyn/yamlsaver/utils.go | 7 ++++++- libs/dyn/yamlsaver/utils_test.go | 12 ++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/libs/dyn/yamlsaver/utils.go b/libs/dyn/yamlsaver/utils.go index 61f41790029..b4b846cc63a 100644 --- a/libs/dyn/yamlsaver/utils.go +++ b/libs/dyn/yamlsaver/utils.go @@ -65,6 +65,10 @@ func sortMapAlphabetically(mv dyn.Value) (dyn.Value, error) { } func skipAndOrder(mv dyn.Value, order *Order, skipFields []string, dst map[string]dyn.Value) (dyn.Value, error) { + // The YAML encoder orders map entries by Location.Line. Without an explicit + // order, number the entries so the alphabetical order from + // sortMapAlphabetically survives instead of Go map iteration order. + line := 0 for _, pair := range mv.MustMap().Pairs() { k := pair.Key.MustString() v := pair.Value @@ -84,7 +88,8 @@ func skipAndOrder(mv dyn.Value, order *Order, skipFields []string, dst map[strin } if order == nil { - dst[k] = v + line++ + dst[k] = dyn.NewValue(v.Value(), []diag.Location{{Line: line}}) } else { dst[k] = dyn.NewValue(v.Value(), []diag.Location{{Line: order.Get(k)}}) } diff --git a/libs/dyn/yamlsaver/utils_test.go b/libs/dyn/yamlsaver/utils_test.go index 8b056e0076f..9188edc69de 100644 --- a/libs/dyn/yamlsaver/utils_test.go +++ b/libs/dyn/yamlsaver/utils_test.go @@ -34,23 +34,23 @@ func TestConvertToMap(t *testing.T) { result, err := ConvertToMapValue(v, nil, []string{"format"}, map[string]dyn.Value{}) assert.NoError(t, err) dynassert.Equal(t, dyn.V(map[string]dyn.Value{ - "list": dyn.V( + "list": dyn.NewValue( []dyn.Value{ dyn.V("a"), dyn.V("b"), dyn.V("c"), }, + []diag.Location{{Line: 1}}, ), - "long_name_field": dyn.V("long name goes here"), - "map": dyn.V( + "long_name_field": dyn.NewValue("long name goes here", []diag.Location{{Line: 2}}), + "map": dyn.NewValue( map[string]dyn.Value{ "key1": dyn.V("value1"), "key2": dyn.V("value2"), }, + []diag.Location{{Line: 3}}, ), - "name": dyn.V( - "test", - ), + "name": dyn.NewValue("test", []diag.Location{{Line: 4}}), }), result) } From 50ad83b9d60d22a92eafe6da3bce6aeb469a42d0 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 4 Nov 2026 22:28:11 +0000 Subject: [PATCH 2/2] bundle generate: add machinery for single-call resource generators Adds a table-driven generate subcommand for resource types that are read with one API call and have no files to download. A resource[R, C] entry names the lookup flag and a typed fetch; the command narrows the response to the bundle resource type via a JSON round-trip, drops CLI-managed and skipped fields, derives the key from the resource name, writes /..yml under the bundle root, warns if the file is not included, and supports the hidden --bind flag like the existing generators. The table starts empty; resource families are added in follow-up PRs. A parity test in cmd/bundle fails when a group from config.SupportedResources is neither generated nor listed as unsupported, so new resource types get an explicit decision. Co-authored-by: Isaac --- bundle/generate/resource.go | 33 +++++ bundle/generate/resource_test.go | 82 +++++++++++ cmd/bundle/generate.go | 1 + cmd/bundle/generate/generic.go | 204 ++++++++++++++++++++++++++++ cmd/bundle/generate/generic_test.go | 122 +++++++++++++++++ cmd/bundle/generate_test.go | 81 +++++++++++ 6 files changed, 523 insertions(+) create mode 100644 bundle/generate/resource.go create mode 100644 bundle/generate/resource_test.go create mode 100644 cmd/bundle/generate/generic.go create mode 100644 cmd/bundle/generate/generic_test.go create mode 100644 cmd/bundle/generate_test.go diff --git a/bundle/generate/resource.go b/bundle/generate/resource.go new file mode 100644 index 00000000000..9181b9e6641 --- /dev/null +++ b/bundle/generate/resource.go @@ -0,0 +1,33 @@ +package generate + +import ( + "encoding/json" + "slices" + + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/yamlsaver" +) + +// cliManagedFields are populated by the CLI on every bundle resource and never +// belong in configuration a user maintains. +var cliManagedFields = []string{"id", "modified_status", "url"} + +// FromResponse narrows an API response to the bundle resource type C by +// round-tripping it through JSON. Only fields present in C's schema survive. +// The response must share C's field layout: wrapper responses (for example a +// struct with a single `experiment` field) have to be unwrapped by the caller. +func FromResponse[C any](response any) (C, error) { + var resource C + data, err := json.Marshal(response) + if err != nil { + return resource, err + } + err = json.Unmarshal(data, &resource) + return resource, err +} + +// ConvertResourceToValue serializes a bundle resource for a generated YAML file, +// dropping nil fields, CLI-managed fields, and skipFields. +func ConvertResourceToValue(resource any, skipFields ...string) (dyn.Value, error) { + return yamlsaver.ConvertToMapValue(resource, nil, slices.Concat(cliManagedFields, skipFields), make(map[string]dyn.Value)) +} diff --git a/bundle/generate/resource_test.go b/bundle/generate/resource_test.go new file mode 100644 index 00000000000..91fddee7f98 --- /dev/null +++ b/bundle/generate/resource_test.go @@ -0,0 +1,82 @@ +package generate + +import ( + "testing" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/catalog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFromResponseKeepsOnlyConfigFields(t *testing.T) { + info := catalog.CatalogInfo{ + Name: "main", + Comment: "primary", + Owner: "someone@example.com", + CatalogType: catalog.CatalogTypeManagedCatalog, + CreatedAt: 1234, + Properties: map[string]string{"k": "v"}, + } + + c, err := FromResponse[resources.Catalog](info) + require.NoError(t, err) + + assert.Equal(t, "main", c.Name) + assert.Equal(t, "primary", c.Comment) + assert.Equal(t, map[string]string{"k": "v"}, c.Properties) + // Owner is not part of CreateCatalog; CatalogType and CreatedAt are read-only. + assert.Empty(t, c.ID) +} + +func TestConvertResourceToValueDropsManagedAndSkippedFields(t *testing.T) { + c := resources.Catalog{ + ID: "abc", + CreateCatalog: catalog.CreateCatalog{ + Name: "main", + Comment: "primary", + StorageRoot: "s3://bucket", + }, + } + c.URL = "https://example.com/catalogs/abc" + + v, err := ConvertResourceToValue(c, "storage_root") + require.NoError(t, err) + + m, ok := v.AsMap() + require.True(t, ok) + assert.ElementsMatch(t, []string{"name", "comment"}, keys(m)) +} + +func TestConvertResourceToValueOrdersKeysAlphabetically(t *testing.T) { + c := resources.Catalog{ + CreateCatalog: catalog.CreateCatalog{ + StorageRoot: "s3://bucket", + Name: "main", + Comment: "primary", + }, + } + + v, err := ConvertResourceToValue(c) + require.NoError(t, err) + + m, ok := v.AsMap() + require.True(t, ok) + var got []string + var lines []int + for _, p := range m.Pairs() { + got = append(got, p.Key.MustString()) + lines = append(lines, p.Value.Location().Line) + } + assert.Equal(t, []string{"comment", "name", "storage_root"}, got) + assert.Equal(t, []int{1, 2, 3}, lines) +} + +func keys(m dyn.Mapping) []string { + var out []string + for _, k := range m.Keys() { + out = append(out, k.MustString()) + } + return out +} diff --git a/cmd/bundle/generate.go b/cmd/bundle/generate.go index 12452293bc6..8e526bc484e 100644 --- a/cmd/bundle/generate.go +++ b/cmd/bundle/generate.go @@ -41,6 +41,7 @@ Use --bind to automatically bind the generated resource to the existing workspac cmd.AddCommand(generate.NewGenerateAlertCommand()) cmd.AddCommand(generate.NewGenerateAppCommand()) cmd.AddCommand(generate.NewGenerateGenieSpaceCommand()) + cmd.AddCommand(generate.NewResourceCommands()...) cmd.PersistentFlags().StringVar(&key, "key", "", `resource key to use for the generated configuration`) return cmd } diff --git a/cmd/bundle/generate/generic.go b/cmd/bundle/generate/generic.go new file mode 100644 index 00000000000..5d9e6d731d7 --- /dev/null +++ b/cmd/bundle/generate/generic.go @@ -0,0 +1,204 @@ +package generate + +import ( + "context" + "fmt" + "path/filepath" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/generate" + "github.com/databricks/cli/cmd/bundle/deployment" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/yamlsaver" + "github.com/databricks/cli/libs/logdiag" + "github.com/databricks/cli/libs/textutil" + "github.com/databricks/databricks-sdk-go" + "github.com/spf13/cobra" +) + +// resourceSpecs lists the resource types served by the generic generate command, +// one per resource family. The parity test in cmd/bundle checks it against +// config.SupportedResources. +var resourceSpecs []resourceSpec + +// NewResourceCommands returns a generate subcommand for every entry in resourceSpecs. +func NewResourceCommands() []*cobra.Command { + cmds := make([]*cobra.Command, 0, len(resourceSpecs)) + for _, spec := range resourceSpecs { + cmds = append(cmds, newResourceCommand(spec)) + } + return cmds +} + +// resource describes a resource type that is read with a single API call and has +// no files to download. R is the API response type, C the bundle resource type in +// bundle/config/resources that R is narrowed to. Build a resourceSpec from it with +// newResourceSpec. +type resource[R, C any] struct { + // LookupFlag names the flag that identifies the existing resource, for + // example "existing-catalog-name". Its value is also what --bind binds to. + LookupFlag string + LookupUsage string + // Fetch reads the resource identified by the lookup value. + Fetch func(context.Context, *databricks.WorkspaceClient, string) (R, error) + // SkipFields are dropped from the generated YAML in addition to the + // CLI-managed fields. Use it for server-populated fields the create API rejects. + SkipFields []string +} + +// bundleResource is the part of config.ConfigResource the generic command needs. +type bundleResource interface { + ResourceDescription() resources.ResourceDescription + GetName() string +} + +// resourceSpec is the type-erased form of resource that resourceSpecs stores. +type resourceSpec struct { + description resources.ResourceDescription + lookupFlag string + lookupUsage string + fetch func(context.Context, *databricks.WorkspaceClient, string) (any, error) + // convert narrows the response to the bundle resource and returns its + // serialized form together with its in-product name. + convert func(any) (dyn.Value, string, error) +} + +func newResourceSpec[R, C any, PC interface { + *C + bundleResource +}](r resource[R, C]) resourceSpec { + var zero C + return resourceSpec{ + description: PC(&zero).ResourceDescription(), + lookupFlag: r.LookupFlag, + lookupUsage: r.LookupUsage, + fetch: func(ctx context.Context, w *databricks.WorkspaceClient, lookup string) (any, error) { + return r.Fetch(ctx, w, lookup) + }, + convert: func(response any) (dyn.Value, string, error) { + c, err := generate.FromResponse[C](response) + if err != nil { + return dyn.InvalidValue, "", err + } + v, err := generate.ConvertResourceToValue(c, r.SkipFields...) + if err != nil { + return dyn.InvalidValue, "", err + } + return v, PC(&c).GetName(), nil + }, + } +} + +// resourceOptions carries the flag values of a generic generate subcommand. +type resourceOptions struct { + lookup string + key string + configDir string + force bool +} + +func newResourceCommand(spec resourceSpec) *cobra.Command { + var opts resourceOptions + var bind bool + + name := spec.description.SingularName + title := spec.description.SingularTitle + cmd := &cobra.Command{ + Use: name, + Short: "Generate bundle configuration for " + article(title) + " " + title, + Long: fmt.Sprintf(`Generate bundle configuration for an existing %s. + +This command reads the %s from the workspace and writes it as a bundle resource +to /.%s.yml so that you can deploy it to other targets or +manage it as code. + +Examples: + databricks bundle generate %s --%s NAME --key my_%s`, + title, title, name, name, spec.lookupFlag, name), + } + + cmd.Flags().StringVar(&opts.lookup, spec.lookupFlag, "", spec.lookupUsage) + cmd.MarkFlagRequired(spec.lookupFlag) + cmd.Flags().StringVarP(&opts.configDir, "config-dir", "d", "resources", `directory to write the configuration to`) + cmd.Flags().BoolVarP(&opts.force, "force", "f", false, `force overwrite existing files in the output directory`) + cmd.Flags().BoolVarP(&bind, "bind", "b", false, `automatically bind the generated resource to the existing resource`) + cmd.Flags().MarkHidden("bind") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + ctx := logdiag.InitContext(cmd.Context()) + cmd.SetContext(ctx) + + b := root.MustConfigureBundle(cmd) + if b == nil || logdiag.HasError(ctx) { + return root.ErrAlreadyPrinted + } + + opts.key = cmd.Flag("key").Value.String() + key, err := generateResource(ctx, b, spec, opts) + if err != nil { + return err + } + + if bind { + return deployment.BindResource(cmd, key, opts.lookup, true, false, true) + } + return nil + } + + return cmd +} + +// generateResource fetches the resource, writes it as /..yml +// under the bundle root, and returns the resource key it used. +func generateResource(ctx context.Context, b *bundle.Bundle, spec resourceSpec, opts resourceOptions) (string, error) { + response, err := spec.fetch(ctx, b.WorkspaceClient(ctx), opts.lookup) + if err != nil { + return "", err + } + v, name, err := spec.convert(response) + if err != nil { + return "", err + } + + key := opts.key + if key == "" { + key = textutil.NormalizeString(name) + } + if key == "" { + return "", fmt.Errorf("cannot derive a resource key for %s %q, pass --key", spec.description.SingularName, opts.lookup) + } + + configDir := opts.configDir + if !filepath.IsAbs(configDir) { + configDir = filepath.Join(b.BundleRootPath, configDir) + } + filename := filepath.Join(configDir, key+"."+spec.description.SingularName+".yml") + + result := map[string]dyn.Value{ + "resources": dyn.V(map[string]dyn.Value{ + spec.description.PluralName: dyn.V(map[string]dyn.Value{ + key: v, + }), + }), + } + err = yamlsaver.NewSaver().SaveAsYAML(result, filename, opts.force) + if err != nil { + return "", err + } + + cmdio.LogString(ctx, spec.description.SingularTitle+" configuration successfully saved to "+filepath.ToSlash(filename)) + warnIfNotIncluded(ctx, b, filename) + return key, nil +} + +// article picks the indefinite article for a title such as "Experiment". +func article(title string) string { + switch title[0] { + case 'A', 'E', 'I', 'O', 'U': + return "an" + } + return "a" +} diff --git a/cmd/bundle/generate/generic_test.go b/cmd/bundle/generate/generic_test.go new file mode 100644 index 00000000000..faaef1004e0 --- /dev/null +++ b/cmd/bundle/generate/generic_test.go @@ -0,0 +1,122 @@ +package generate + +import ( + "context" + "errors" + "io" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/logdiag" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/catalog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var testCatalogSpec = newResourceSpec(resource[*catalog.CatalogInfo, resources.Catalog]{ + LookupFlag: "existing-catalog-name", + LookupUsage: "name of the catalog", + Fetch: func(ctx context.Context, w *databricks.WorkspaceClient, name string) (*catalog.CatalogInfo, error) { + if name == "missing" { + return nil, errors.New("catalog not found") + } + return &catalog.CatalogInfo{ + Name: name, + Comment: "primary", + Owner: "someone@example.com", + CatalogType: catalog.CatalogTypeManagedCatalog, + StorageRoot: "s3://bucket", + }, nil + }, + SkipFields: []string{"storage_root"}, +}) + +func testBundle(t *testing.T) (context.Context, *bundle.Bundle) { + t.Helper() + b := &bundle.Bundle{BundleRootPath: t.TempDir()} + b.SetWorkpaceClient(mocks.NewMockWorkspaceClient(t).WorkspaceClient) + ctx := logdiag.InitContext(cmdio.MockDiscard(t.Context())) + logdiag.SetCollect(ctx, true) + return ctx, b +} + +func TestGenerateResourceWritesConfig(t *testing.T) { + ctx, b := testBundle(t) + + key, err := generateResource(ctx, b, testCatalogSpec, resourceOptions{lookup: "My Catalog", configDir: "resources"}) + require.NoError(t, err) + assert.Equal(t, "my_catalog", key) + + data, err := os.ReadFile(filepath.Join(b.BundleRootPath, "resources", "my_catalog.catalog.yml")) + require.NoError(t, err) + assert.Equal(t, `resources: + catalogs: + my_catalog: + comment: primary + name: My Catalog +`, string(data)) + + // The bundle has no include patterns, so the file is reported as not included. + diags := logdiag.FlushCollected(ctx) + require.Len(t, diags, 1) + assert.Equal(t, "Generated configuration is not included in the bundle", diags[0].Summary) +} + +func TestGenerateResourceHonorsKeyAndConfigDir(t *testing.T) { + ctx, b := testBundle(t) + + key, err := generateResource(ctx, b, testCatalogSpec, resourceOptions{lookup: "main", key: "k", configDir: "conf"}) + require.NoError(t, err) + assert.Equal(t, "k", key) + assert.FileExists(t, filepath.Join(b.BundleRootPath, "conf", "k.catalog.yml")) +} + +func TestGenerateResourceReturnsFetchError(t *testing.T) { + ctx, b := testBundle(t) + + _, err := generateResource(ctx, b, testCatalogSpec, resourceOptions{lookup: "missing", configDir: "resources"}) + assert.EqualError(t, err, "catalog not found") +} + +func TestGenerateResourceRefusesToOverwriteWithoutForce(t *testing.T) { + ctx, b := testBundle(t) + existing := filepath.Join(b.BundleRootPath, "resources", "main.catalog.yml") + require.NoError(t, os.MkdirAll(filepath.Dir(existing), 0o755)) + require.NoError(t, os.WriteFile(existing, []byte("keep"), 0o644)) + + opts := resourceOptions{lookup: "main", configDir: "resources"} + _, err := generateResource(ctx, b, testCatalogSpec, opts) + assert.ErrorContains(t, err, "already exists") + + opts.force = true + _, err = generateResource(ctx, b, testCatalogSpec, opts) + require.NoError(t, err) + data, err := os.ReadFile(existing) + require.NoError(t, err) + assert.Contains(t, string(data), "name: main") +} + +func TestResourceCommandFlagsAndHelp(t *testing.T) { + cmd := newResourceCommand(testCatalogSpec) + assert.Equal(t, "catalog", cmd.Use) + assert.Equal(t, "Generate bundle configuration for a Catalog", cmd.Short) + assert.Contains(t, cmd.Long, "--existing-catalog-name NAME --key my_catalog") + assert.True(t, cmd.Flags().Lookup("bind").Hidden) + + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs(nil) + err := cmd.Execute() + assert.ErrorContains(t, err, `required flag(s) "existing-catalog-name" not set`) +} + +func TestArticle(t *testing.T) { + assert.Equal(t, "an", article("Experiment")) + assert.Equal(t, "a", article("Catalog")) +} diff --git a/cmd/bundle/generate_test.go b/cmd/bundle/generate_test.go new file mode 100644 index 00000000000..f408745e4eb --- /dev/null +++ b/cmd/bundle/generate_test.go @@ -0,0 +1,81 @@ +package bundle + +import ( + "slices" + "strings" + "testing" + + "github.com/databricks/cli/bundle/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// generateUnsupported lists the resource groups without a generate subcommand. +// Remove a group once its command lands. A group that is in neither this list +// nor the command tree fails the test, so every new resource type gets a decision. +var generateUnsupported = []string{ + "catalogs", + "cluster_policies", + "clusters", + "data_quality_monitors", + "database_catalogs", + "database_instances", + "experiments", + "external_locations", + "features", + "instance_pools", + "internal_immutable_snapshots", + "job_runs", + "materialization_sets", + "mcp_services", + "model_provider_services", + "model_services", + "model_serving_endpoints", + "models", + "postgres_branches", + "postgres_catalogs", + "postgres_databases", + "postgres_endpoints", + "postgres_projects", + "postgres_roles", + "postgres_snapshot_schedules", + "postgres_synced_tables", + "quality_monitors", + "registered_models", + "schemas", + "secret_scopes", + "secrets", + "sql_warehouses", + "synced_database_tables", + "vector_search_endpoints", + "vector_search_indexes", + "volumes", +} + +func TestGenerateCommandCoversAllResourceGroups(t *testing.T) { + groups := config.SupportedResources() + + // Subcommands are named after the resource type's singular name with dashes. + byCommand := make(map[string]string, len(groups)) + for group, d := range groups { + byCommand[strings.ReplaceAll(d.SingularName, "_", "-")] = group + } + + covered := make(map[string]bool) + for _, sub := range newGenerateCommand().Commands() { + group, ok := byCommand[sub.Name()] + require.True(t, ok, "generate subcommand %q does not match the singular name of any resource type", sub.Name()) + covered[group] = true + } + + for _, group := range generateUnsupported { + assert.Contains(t, groups, group, "%q is not a resource group; drop it from generateUnsupported", group) + assert.False(t, covered[group], "%q has a generate subcommand; drop it from generateUnsupported", group) + } + for group := range groups { + if slices.Contains(generateUnsupported, group) { + continue + } + assert.True(t, covered[group], "%q has no generate subcommand; add one or list it in generateUnsupported", group) + } +}