diff --git a/acceptance/bundle/invariant/configs/catalog_optional_fields.yml.tmpl b/acceptance/bundle/invariant/configs/catalog_optional_fields.yml.tmpl new file mode 100644 index 00000000000..9950f35211c --- /dev/null +++ b/acceptance/bundle/invariant/configs/catalog_optional_fields.yml.tmpl @@ -0,0 +1,11 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + catalogs: + foo: + name: test-catalog-$UNIQUE_NAME + comment: This is a test catalog + custom_max_retention_hours: 48 + managed_encryption_settings: + customer_managed_key_id: 00000000-0000-0000-0000-000000000000 diff --git a/acceptance/bundle/invariant/continue_293/out.test.toml b/acceptance/bundle/invariant/continue_293/out.test.toml index d652e02679c..c294b244621 100644 --- a/acceptance/bundle/invariant/continue_293/out.test.toml +++ b/acceptance/bundle/invariant/continue_293/out.test.toml @@ -6,6 +6,7 @@ EnvMatrix.INPUT_CONFIG = [ "alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", + "catalog_optional_fields.yml.tmpl", "cluster.yml.tmpl", "cluster_apply_policy_default_values.yml.tmpl", "dashboard.yml.tmpl", diff --git a/acceptance/bundle/invariant/delete_idempotent/out.test.toml b/acceptance/bundle/invariant/delete_idempotent/out.test.toml index 45cdef9cc7c..f65b1680aa1 100644 --- a/acceptance/bundle/invariant/delete_idempotent/out.test.toml +++ b/acceptance/bundle/invariant/delete_idempotent/out.test.toml @@ -6,6 +6,7 @@ EnvMatrix.INPUT_CONFIG = [ "alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", + "catalog_optional_fields.yml.tmpl", "cluster.yml.tmpl", "cluster_apply_policy_default_values.yml.tmpl", "dashboard.yml.tmpl", diff --git a/acceptance/bundle/invariant/destroy_idempotent/out.test.toml b/acceptance/bundle/invariant/destroy_idempotent/out.test.toml index 45cdef9cc7c..f65b1680aa1 100644 --- a/acceptance/bundle/invariant/destroy_idempotent/out.test.toml +++ b/acceptance/bundle/invariant/destroy_idempotent/out.test.toml @@ -6,6 +6,7 @@ EnvMatrix.INPUT_CONFIG = [ "alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", + "catalog_optional_fields.yml.tmpl", "cluster.yml.tmpl", "cluster_apply_policy_default_values.yml.tmpl", "dashboard.yml.tmpl", diff --git a/acceptance/bundle/invariant/migrate/test.toml b/acceptance/bundle/invariant/migrate/test.toml index 37dc2557922..5c7694bc985 100644 --- a/acceptance/bundle/invariant/migrate/test.toml +++ b/acceptance/bundle/invariant/migrate/test.toml @@ -8,6 +8,7 @@ EnvMatrixExclude.no_job_run = ["INPUT_CONFIG=job_run.yml.tmpl"] # Error: Catalog resources are only supported with direct deployment mode EnvMatrixExclude.no_catalog = ["INPUT_CONFIG=catalog.yml.tmpl"] +EnvMatrixExclude.no_catalog_optional_fields = ["INPUT_CONFIG=catalog_optional_fields.yml.tmpl"] EnvMatrixExclude.no_external_location = ["INPUT_CONFIG=external_location.yml.tmpl"] # Genie spaces are direct-only too; the terraform deploy that seeds the migration fails for them. EnvMatrixExclude.no_genie_space = ["INPUT_CONFIG=genie_space.yml.tmpl"] diff --git a/acceptance/bundle/invariant/no_drift/out.test.toml b/acceptance/bundle/invariant/no_drift/out.test.toml index 45cdef9cc7c..f65b1680aa1 100644 --- a/acceptance/bundle/invariant/no_drift/out.test.toml +++ b/acceptance/bundle/invariant/no_drift/out.test.toml @@ -6,6 +6,7 @@ EnvMatrix.INPUT_CONFIG = [ "alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", + "catalog_optional_fields.yml.tmpl", "cluster.yml.tmpl", "cluster_apply_policy_default_values.yml.tmpl", "dashboard.yml.tmpl", diff --git a/acceptance/bundle/invariant/test.toml b/acceptance/bundle/invariant/test.toml index a53a93b84a6..1d0d883f6d5 100644 --- a/acceptance/bundle/invariant/test.toml +++ b/acceptance/bundle/invariant/test.toml @@ -24,6 +24,7 @@ EnvMatrix.INPUT_CONFIG = [ "alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", + "catalog_optional_fields.yml.tmpl", "cluster.yml.tmpl", "cluster_apply_policy_default_values.yml.tmpl", "dashboard.yml.tmpl", @@ -99,6 +100,10 @@ no_database_instance_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=database_ins no_database_catalog_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=database_catalog.yml.tmpl"] no_synced_database_table_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=synced_database_table.yml.tmpl"] +# managed_encryption_settings needs a CMK registered with the workspace, so this +# config only runs against the mock server +no_catalog_optional_fields_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=catalog_optional_fields.yml.tmpl"] + # External locations require actual storage credentials with cloud IAM setup # which are environment-specific, so we only test locally with the mock server no_external_location_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=external_location.yml.tmpl"] diff --git a/libs/testserver/catalogs.go b/libs/testserver/catalogs.go index 351c5a76499..4d80152373d 100644 --- a/libs/testserver/catalogs.go +++ b/libs/testserver/catalogs.go @@ -24,21 +24,26 @@ func (s *FakeWorkspace) CatalogsCreate(req Request) Response { } } + // Echo back every field create accepts: a dropped one makes the next plan see a + // phantom change. catalogInfo := catalog.CatalogInfo{ - Name: createRequest.Name, - Comment: createRequest.Comment, - StorageRoot: createRequest.StorageRoot, - ProviderName: createRequest.ProviderName, - ShareName: createRequest.ShareName, - Options: createRequest.Options, - Properties: createRequest.Properties, - FullName: createRequest.Name, - CreatedAt: nowMilli(), - CreatedBy: s.CurrentUser().UserName, - UpdatedBy: s.CurrentUser().UserName, - MetastoreId: nextUUID(), - Owner: s.CurrentUser().UserName, - CatalogType: catalog.CatalogTypeManagedCatalog, + Name: createRequest.Name, + Comment: createRequest.Comment, + ConnectionName: createRequest.ConnectionName, + CustomMaxRetentionHours: createRequest.CustomMaxRetentionHours, + ManagedEncryptionSettings: createRequest.ManagedEncryptionSettings, + StorageRoot: createRequest.StorageRoot, + ProviderName: createRequest.ProviderName, + ShareName: createRequest.ShareName, + Options: createRequest.Options, + Properties: createRequest.Properties, + FullName: createRequest.Name, + CreatedAt: nowMilli(), + CreatedBy: s.CurrentUser().UserName, + UpdatedBy: s.CurrentUser().UserName, + MetastoreId: nextUUID(), + Owner: s.CurrentUser().UserName, + CatalogType: catalog.CatalogTypeManagedCatalog, } catalogInfo.UpdatedAt = catalogInfo.CreatedAt if catalogInfo.Properties == nil && createRequest.Name == catalogNameManagedDefaults { @@ -83,6 +88,18 @@ func (s *FakeWorkspace) CatalogsUpdate(req Request, name string) Response { if updateRequest.Comment != "" { existing.Comment = updateRequest.Comment } + if updateRequest.CustomMaxRetentionHours != 0 { + existing.CustomMaxRetentionHours = updateRequest.CustomMaxRetentionHours + } + if updateRequest.ManagedEncryptionSettings != nil { + existing.ManagedEncryptionSettings = updateRequest.ManagedEncryptionSettings + } + if updateRequest.Options != nil { + existing.Options = updateRequest.Options + } + if updateRequest.Properties != nil { + existing.Properties = updateRequest.Properties + } if updateRequest.Owner != "" { existing.Owner = updateRequest.Owner } diff --git a/libs/testserver/catalogs_test.go b/libs/testserver/catalogs_test.go new file mode 100644 index 00000000000..4606fdf28b5 --- /dev/null +++ b/libs/testserver/catalogs_test.go @@ -0,0 +1,102 @@ +package testserver + +import ( + "encoding/json" + "reflect" + "strings" + "testing" + + "github.com/databricks/databricks-sdk-go/service/catalog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// createCatalogRequest sets every field CreateCatalog accepts. Tests below assert +// the fake echoes all of them back and that the request stays exhaustive. +const createCatalogRequest = `{ + "name": "my_catalog", + "comment": "c", + "connection_name": "my_connection", + "custom_max_retention_hours": 48, + "managed_encryption_settings": {"customer_managed_key_id": "key-1"}, + "options": {"opt": "1"}, + "properties": {"prop": "2"}, + "provider_name": "my_provider", + "share_name": "my_share", + "storage_root": "s3://bucket/root" +}` + +// jsonFieldNames returns the wire names of the fields typ serializes. +func jsonFieldNames(typ reflect.Type) []string { + var names []string + for field := range typ.Fields() { + name, _, _ := strings.Cut(field.Tag.Get("json"), ",") + if name == "" || name == "-" { + continue + } + names = append(names, name) + } + return names +} + +func TestCatalogsCreate_EchoesEveryAcceptedField(t *testing.T) { + workspace := NewFakeWorkspace("http://test", "dbapi123") + + response := workspace.CatalogsCreate(Request{Body: []byte(createCatalogRequest)}) + assert.Equal(t, 0, response.StatusCode) + + info, ok := response.Body.(catalog.CatalogInfo) + require.True(t, ok) + + assert.Equal(t, "my_catalog", info.Name) + assert.Equal(t, "c", info.Comment) + assert.Equal(t, "my_connection", info.ConnectionName) + assert.Equal(t, int64(48), info.CustomMaxRetentionHours) + require.NotNil(t, info.ManagedEncryptionSettings) + assert.Equal(t, "key-1", info.ManagedEncryptionSettings.CustomerManagedKeyId) + assert.Equal(t, map[string]string{"opt": "1"}, info.Options) + assert.Equal(t, map[string]string{"prop": "2"}, info.Properties) + assert.Equal(t, "my_provider", info.ProviderName) + assert.Equal(t, "my_share", info.ShareName) + assert.Equal(t, "s3://bucket/root", info.StorageRoot) +} + +// CatalogsCreate builds its response field by field, so a field the SDK adds to +// CreateCatalog is silently dropped until someone extends the literal. Fail here +// instead, where the message points at the field, rather than in a bundle test +// that reports drift on a catalog it just created. +func TestCatalogsCreate_RequestCoversEveryAcceptedField(t *testing.T) { + var sent map[string]any + require.NoError(t, json.Unmarshal([]byte(createCatalogRequest), &sent)) + + for _, name := range jsonFieldNames(reflect.TypeFor[catalog.CreateCatalog]()) { + assert.Contains(t, sent, name) + } +} + +func TestCatalogsUpdate_AppliesUpdatableFields(t *testing.T) { + workspace := NewFakeWorkspace("http://test", "dbapi123") + require.Equal(t, 0, workspace.CatalogsCreate(Request{Body: []byte(createCatalogRequest)}).StatusCode) + + // UpdateCatalog also accepts enable_predictive_optimization and isolation_mode, + // which DABs deliberately leaves unset (see dresources.ResourceCatalog.DoUpdate), + // so the fake does not model them. + response := workspace.CatalogsUpdate(Request{Body: []byte(`{ + "comment": "updated", + "custom_max_retention_hours": 72, + "managed_encryption_settings": {"customer_managed_key_id": "key-2"}, + "options": {"opt": "3"}, + "properties": {"prop": "4"} + }`)}, "my_catalog") + assert.Equal(t, 0, response.StatusCode) + + info, ok := response.Body.(catalog.CatalogInfo) + require.True(t, ok) + + assert.Equal(t, "updated", info.Comment) + assert.Equal(t, int64(72), info.CustomMaxRetentionHours) + require.NotNil(t, info.ManagedEncryptionSettings) + assert.Equal(t, "key-2", info.ManagedEncryptionSettings.CustomerManagedKeyId) + assert.Equal(t, map[string]string{"opt": "3"}, info.Options) + assert.Equal(t, map[string]string{"prop": "4"}, info.Properties) +}