diff --git a/.github/workflows/message.yml b/.github/workflows/message.yml new file mode 100644 index 0000000000..ab54041af3 --- /dev/null +++ b/.github/workflows/message.yml @@ -0,0 +1,30 @@ +name: Validate Commit Message + +on: + pull_request: + types: [opened, synchronize, edited] + merge_group: + types: [checks_requested] + +jobs: + validate: + runs-on: ubuntu-latest + # GitHub required checks are shared between PRs and the Merge Queue. + # Since there is no PR title on Merge Queue, we need to trigger and + # skip this test for Merge Queue to succeed. + if: github.event_name == 'pull_request' + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - name: Validate Tag + run: | + TAG=$(echo ${{ github.event.pull_request.title }} | sed -ne 's/\[\(.*\)\].*/\1/p') + if grep -q "tag: \"\[$TAG\]\"" .codegen/changelog_config.yml; then + echo "Valid tag found: [$TAG]" + else + echo "Invalid or missing tag in commit message: [$TAG]" + exit 1 + fi \ No newline at end of file diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 0936a2807d..4a6f8892d4 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -54,22 +54,3 @@ jobs: run: | # Exit with status code 1 if there are differences (i.e. unformatted files) git diff --exit-code - - commit-message: - runs-on: ubuntu-latest - if: ${{ github.event_name == 'pull_request' }} - steps: - - name: Checkout - uses: actions/checkout@v3 - with: - fetch-depth: 0 - - - name: Validate Tag - run: | - TAG=$(echo ${{ github.event.pull_request.title }} | sed -ne 's/\[\(.*\)\].*/\1/p') - if grep -q "tag: \"\[$TAG\]\"" .codegen/changelog_config.yml; then - echo "Valid tag found: [$TAG]" - else - echo "Invalid or missing tag in commit message: [$TAG]" - exit 1 - fi \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 7af788afce..dd5b9e77fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,8 +27,6 @@ ### Internal Changes * Add Release tag ([#3748](https://github.com/databricks/terraform-provider-databricks/pull/3748)). * Improve Changelog by grouping changes ([#3747](https://github.com/databricks/terraform-provider-databricks/pull/3747)). - * Upgrade Go SDK to v0.43.2 ([#3750](https://github.com/databricks/terraform-provider-databricks/pull/3750)). - * Add new APIErrorBody struct and update deps ([#3745](https://github.com/databricks/terraform-provider-databricks/pull/3745)). * Change TF registry ownership ([#3736](https://github.com/databricks/terraform-provider-databricks/pull/3736)). * Refactored `databricks_cluster(s)` data sources to Go SDK ([#3685](https://github.com/databricks/terraform-provider-databricks/pull/3685)). * Upgrade databricks-sdk-go ([#3743](https://github.com/databricks/terraform-provider-databricks/pull/3743)). @@ -36,6 +34,12 @@ * Make `dashboard_name` random in integration tests for `databricks_dashboard` resource ([#3763](https://github.com/databricks/terraform-provider-databricks/pull/3763)). +## 1.48.3 + +### Internal Changes + * Bump Go SDK to `0.43.2` ([#3750](https://github.com/databricks/terraform-provider-databricks/pull/3750)) + * Added new `APIErrorBody` struct and update deps ([#3745](https://github.com/databricks/terraform-provider-databricks/pull/3745)) + ## 1.48.2 ### New Features and Improvements diff --git a/docs/resources.png b/docs/resources.png index 907b3c491a..925a0fa1dc 100644 Binary files a/docs/resources.png and b/docs/resources.png differ diff --git a/docs/resources/dashboard.md b/docs/resources/dashboard.md index c0199add23..b18ce755d3 100644 --- a/docs/resources/dashboard.md +++ b/docs/resources/dashboard.md @@ -57,6 +57,18 @@ In addition to all arguments above, the following attributes are exported: * `id` - The unique ID of the dashboard. +## Access Control + +[databricks_permissions](permissions.md#dashboard-usage) can control which groups or individual users can *Manage*, *Edit*, *Read* or *Run* individual dashboards. + +## Import + +You can import a `databricks_dashboard` resource with ID like the following: + +```bash +terraform import databricks_dashboard.this +``` + ## Notes * Only one of `serialized_dashboard` or `file_path` can be used throughout the lifecycle of the dashboard. If you want to switch from one to the other, you must first destroy the dashboard resource and then recreate it with the new attribute. -* Dashboards managed by Terraform will be published automatically. \ No newline at end of file +* Dashboards managed by Terraform will be published automatically. diff --git a/docs/resources/permissions.md b/docs/resources/permissions.md index e447b84319..5629d64051 100644 --- a/docs/resources/permissions.md +++ b/docs/resources/permissions.md @@ -662,9 +662,9 @@ resource "databricks_permissions" "endpoint_usage" { } ``` -## SQL Dashboard usage +## Dashboard usage -[SQL dashboards](https://docs.databricks.com/sql/user/security/access-control/dashboard-acl.html) have three possible permissions: `CAN_VIEW`, `CAN_RUN` and `CAN_MANAGE`: +[Dashboards](https://docs.databricks.com/en/dashboards/tutorials/manage-permissions.html) have four possible permissions: `CAN_READ`, `CAN_RUN`, `CAN_EDIT` and `CAN_MANAGE`: ```hcl resource "databricks_group" "auto" { @@ -675,7 +675,41 @@ resource "databricks_group" "eng" { display_name = "Engineering" } -resource "databricks_permissions" "endpoint_usage" { +resource "databricks_dashboard" "dashboard" { + display_name = "TF New Dashboard" + # ... +} + + +resource "databricks_permissions" "dashboard_usage" { + dashboard_id = databricks_dashboard.dashboard.id + + access_control { + group_name = databricks_group.auto.display_name + permission_level = "CAN_RUN" + } + + access_control { + group_name = databricks_group.eng.display_name + permission_level = "CAN_MANAGE" + } +} +``` + +## Legacy SQL Dashboard usage + +[Legacy SQL dashboards](https://docs.databricks.com/sql/user/security/access-control/dashboard-acl.html) have three possible permissions: `CAN_VIEW`, `CAN_RUN` and `CAN_MANAGE`: + +```hcl +resource "databricks_group" "auto" { + display_name = "Automation" +} + +resource "databricks_group" "eng" { + display_name = "Engineering" +} + +resource "databricks_permissions" "sql_dashboard_usage" { sql_dashboard_id = "3244325" access_control { @@ -819,7 +853,7 @@ Exactly one of the below arguments is required: In addition to all arguments above, the following attributes are exported: -- `id` - Canonical unique identifier for the permissions in form of `/object_type/object_id`. +- `id` - Canonical unique identifier for the permissions in form of `//`. - `object_type` - type of permissions. ## Import diff --git a/exporter/context.go b/exporter/context.go index ee3f2a753b..441ed12078 100644 --- a/exporter/context.go +++ b/exporter/context.go @@ -1651,7 +1651,7 @@ func (ic *importContext) dataToHcl(i importable, path []string, // In case when have zero value, but there is non-zero default, we also need to produce it shouldSkip = false } - if shouldSkip { + if shouldSkip && (i.ShouldGenerateField == nil || !i.ShouldGenerateField(ic, pathString, as, d)) { continue } switch as.Type { diff --git a/exporter/exporter_test.go b/exporter/exporter_test.go index 30c92591d6..2739286e66 100644 --- a/exporter/exporter_test.go +++ b/exporter/exporter_test.go @@ -1917,6 +1917,13 @@ func TestImportingSqlObjects(t *testing.T) { err := ic.Run() assert.NoError(t, err) + + content, err := os.ReadFile(tmpDir + "/sql-endpoints.tf") + assert.NoError(t, err) + contentStr := string(content) + assert.True(t, strings.Contains(contentStr, `enable_serverless_compute = false`)) + assert.True(t, strings.Contains(contentStr, `resource "databricks_sql_endpoint" "test" {`)) + assert.False(t, strings.Contains(contentStr, `tags {`)) }) } diff --git a/exporter/importables.go b/exporter/importables.go index 72e8f71d4b..e607951f04 100644 --- a/exporter/importables.go +++ b/exporter/importables.go @@ -1713,6 +1713,25 @@ var resourcesMap map[string]importable = map[string]importable{ return nil }, Ignore: generateIgnoreObjectWithoutName("databricks_sql_endpoint"), + ShouldOmitField: func(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool { + switch pathString { + case "enable_serverless_compute": + return false + case "tags": + return d.Get("tags.0.custom_tags.#").(int) == 0 + case "channel.0.name": + channelName := d.Get(pathString).(string) + return channelName == "" || channelName == "CHANNEL_NAME_CURRENT" + case "channel": + channelName := d.Get(pathString + ".0.name").(string) + return channelName == "" || channelName == "CHANNEL_NAME_CURRENT" + } + return defaultShouldOmitFieldFunc(ic, pathString, as, d) + }, + ShouldGenerateField: func(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool { + // We need to generate it even if it's false... + return pathString == "enable_serverless_compute" + }, }, "databricks_sql_global_config": { WorkspaceLevel: true, @@ -1739,6 +1758,12 @@ var resourcesMap map[string]importable = map[string]importable{ } return nil }, + ShouldOmitField: func(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool { + if pathString == "enable_serverless_compute" { + return false + } + return defaultShouldOmitFieldFunc(ic, pathString, as, d) + }, Depends: []reference{ {Path: "instance_profile_arn", Resource: "databricks_instance_profile"}, }, diff --git a/exporter/model.go b/exporter/model.go index c03709207d..63b56a7fe0 100644 --- a/exporter/model.go +++ b/exporter/model.go @@ -153,6 +153,8 @@ type importable struct { Ignore func(ic *importContext, r *resource) bool // Function to check if the field in the given resource should be omitted or not ShouldOmitField func(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool + // Function to check if the field in the given resource should be generated or not independently of the value + ShouldGenerateField func(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool // Defines which API version should be used for this specific resource ApiVersion common.ApiVersion // Defines if specific service is account level resource diff --git a/go.sum b/go.sum index 63534b76fb..6571a14507 100644 --- a/go.sum +++ b/go.sum @@ -26,8 +26,6 @@ github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBS github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= -github.com/databricks/databricks-sdk-go v0.43.1 h1:JJJ0S5yiDLQF8dzo6V1O2jKsOAkULtNqrnmFcvHstLg= -github.com/databricks/databricks-sdk-go v0.43.1/go.mod h1:nlzeOEgJ1Tmb5HyknBJ3GEorCZKWqEBoHprvPmTSNq8= github.com/databricks/databricks-sdk-go v0.43.2 h1:4B+sHAYO5kFqwZNQRmsF70eecqsFX6i/0KfXoDFQT/E= github.com/databricks/databricks-sdk-go v0.43.2/go.mod h1:nlzeOEgJ1Tmb5HyknBJ3GEorCZKWqEBoHprvPmTSNq8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/internal/acceptance/cluster_test.go b/internal/acceptance/cluster_test.go index 6a1a2e0c87..892ab3104c 100644 --- a/internal/acceptance/cluster_test.go +++ b/internal/acceptance/cluster_test.go @@ -88,7 +88,10 @@ func awsClusterTemplate(availability string) string { num_workers = 1 autotermination_minutes = 10 aws_attributes { - availability = "%s" + availability = "%s" + } + custom_tags = { + "Owner" = "eng-dev-ecosystem-team@databricks.com" } node_type_id = "i3.xlarge" } diff --git a/internal/acceptance/dashboard_test.go b/internal/acceptance/dashboard_test.go index 6d9d52f56e..bf67339315 100644 --- a/internal/acceptance/dashboard_test.go +++ b/internal/acceptance/dashboard_test.go @@ -44,7 +44,17 @@ func makeTemplate(template templateStruct) string { `, template.EmbedCredentials) } templateString += `} - ` + +resource "databricks_permissions" "dashboard_usage" { + dashboard_id = databricks_dashboard.d1.id + + access_control { + group_name = "users" + permission_level = "CAN_READ" + } +} +` + return templateString } diff --git a/internal/acceptance/restrict_workspace_admins_test.go b/internal/acceptance/restrict_workspace_admins_test.go index d168a4946b..b6c4efb9ae 100644 --- a/internal/acceptance/restrict_workspace_admins_test.go +++ b/internal/acceptance/restrict_workspace_admins_test.go @@ -21,7 +21,7 @@ func TestAccRestrictWorkspaceAdminsSetting(t *testing.T) { } } ` - workspaceLevel(t, step{ + unityWorkspaceLevel(t, step{ Template: template, Check: resourceCheckWithState("databricks_restrict_workspace_admins_setting.this", func(ctx context.Context, client *common.DatabricksClient, state *terraform.InstanceState) error { diff --git a/internal/acceptance/sql_endpoint_test.go b/internal/acceptance/sql_endpoint_test.go index 2a318d1542..66bdbea5cb 100644 --- a/internal/acceptance/sql_endpoint_test.go +++ b/internal/acceptance/sql_endpoint_test.go @@ -12,18 +12,34 @@ import ( func TestAccSQLEndpoint(t *testing.T) { workspaceLevel(t, step{ - Template: `resource "databricks_sql_endpoint" "this" { - name = "tf-{var.RANDOM}" - cluster_size = "2X-Small" - max_num_clusters = 1 - }`, + Template: ` + resource "databricks_sql_endpoint" "this" { + name = "tf-{var.RANDOM}" + cluster_size = "2X-Small" + max_num_clusters = 1 + + tags { + custom_tags { + key = "Owner" + value = "eng-dev-ecosystem-team@databricks.com" + } + } + }`, }, step{ Template: ` resource "databricks_sql_endpoint" "that" { name = "tf-{var.RANDOM}" cluster_size = "2X-Small" max_num_clusters = 1 + enable_serverless_compute = false + + tags { + custom_tags { + key = "Owner" + value = "eng-dev-ecosystem-team@databricks.com" + } + } }`, Check: func(s *terraform.State) error { w, err := databricks.NewWorkspaceClient() diff --git a/permissions/resource_permissions.go b/permissions/resource_permissions.go index 2240ce7256..c86055e00f 100644 --- a/permissions/resource_permissions.go +++ b/permissions/resource_permissions.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log" "path" "strconv" "strings" @@ -163,6 +164,7 @@ func (a PermissionsAPI) put(objectID string, objectACL AccessControlChangeList) // SQLA entities use POST for permission updates. return a.client.Post(a.context, urlPathForObjectID(objectID), objectACL, nil) } + log.Printf("[DEBUG] PUT %s %v", objectID, objectACL) return a.client.Put(a.context, urlPathForObjectID(objectID), objectACL) } @@ -262,6 +264,10 @@ func (a PermissionsAPI) Read(objectID string) (objectACL ObjectACL, err error) { err = apiErr return } + if strings.HasPrefix(objectID, "/dashboards/") { + // workaround for inconsistent API response returning object ID of file in the workspace + objectACL.ObjectID = objectID + } return } @@ -306,6 +312,7 @@ func permissionsResourceIDFields() []permissionsIDFieldMapping { {"sql_dashboard_id", "dashboard", "sql/dashboards", []string{"CAN_EDIT", "CAN_RUN", "CAN_MANAGE", "CAN_VIEW"}, SIMPLE}, {"sql_alert_id", "alert", "sql/alerts", []string{"CAN_EDIT", "CAN_RUN", "CAN_MANAGE", "CAN_VIEW"}, SIMPLE}, {"sql_query_id", "query", "sql/queries", []string{"CAN_EDIT", "CAN_RUN", "CAN_MANAGE", "CAN_VIEW"}, SIMPLE}, + {"dashboard_id", "dashboard", "dashboards", []string{"CAN_EDIT", "CAN_RUN", "CAN_MANAGE", "CAN_READ"}, SIMPLE}, {"experiment_id", "mlflowExperiment", "experiments", []string{"CAN_READ", "CAN_EDIT", "CAN_MANAGE"}, SIMPLE}, {"registered_model_id", "registered-model", "registered-models", []string{ "CAN_READ", "CAN_EDIT", "CAN_MANAGE_STAGING_VERSIONS", "CAN_MANAGE_PRODUCTION_VERSIONS", "CAN_MANAGE"}, SIMPLE}, @@ -335,9 +342,10 @@ func (oa *ObjectACL) ToPermissionsEntity(d *schema.ResourceData, me string) (Per } } for _, mapping := range permissionsResourceIDFields() { - if mapping.objectType != oa.ObjectType { + if mapping.objectType != oa.ObjectType || !strings.HasPrefix(oa.ObjectID[1:], mapping.resourceType) { continue } + log.Printf("[DEBUG] mapping %v for object %v", mapping, oa) entity.ObjectType = mapping.objectType var pathVariant any if mapping.objectType == "file" { diff --git a/permissions/resource_permissions_test.go b/permissions/resource_permissions_test.go index 7f856612b5..b741885114 100644 --- a/permissions/resource_permissions_test.go +++ b/permissions/resource_permissions_test.go @@ -365,6 +365,44 @@ func TestResourcePermissionsRead_SQLA_Asset(t *testing.T) { assert.Equal(t, "CAN_READ", firstElem["permission_level"]) } +func TestResourcePermissionsRead_Dashboard(t *testing.T) { + d, err := qa.ResourceFixture{ + Fixtures: []qa.HTTPFixture{ + me, + { + Method: http.MethodGet, + Resource: "/api/2.0/permissions/dashboards/abc", + Response: ObjectACL{ + ObjectID: "dashboards/abc", + ObjectType: "dashboard", + AccessControlList: []AccessControl{ + { + UserName: TestingUser, + PermissionLevel: "CAN_READ", + }, + { + UserName: TestingAdminUser, + PermissionLevel: "CAN_MANAGE", + }, + }, + }, + }, + }, + Resource: ResourcePermissions(), + Read: true, + New: true, + ID: "/dashboards/abc", + }.Apply(t) + assert.NoError(t, err) + assert.Equal(t, "/dashboards/abc", d.Id()) + ac := d.Get("access_control").(*schema.Set) + assert.Equal(t, "abc", d.Get("dashboard_id").(string)) + require.Equal(t, 1, len(ac.List())) + firstElem := ac.List()[0].(map[string]any) + assert.Equal(t, TestingUser, firstElem["user_name"]) + assert.Equal(t, "CAN_READ", firstElem["permission_level"]) +} + func TestResourcePermissionsRead_NotFound(t *testing.T) { qa.ResourceFixture{ Fixtures: []qa.HTTPFixture{