Prepare 0.3.0 release (#464)

* Simplified instance pool tests
* Fixed concurrent testing for instance profiles
* Added 0.3.0 migration instructions
* Fix minor inconsistencies
* Verified state upgraders for notebooks and files
* Add global rate limit to HTTP calls
* Fixed redundant multiple mounting clusters (#445)
* Sort outputs in group data source (#467)
* Use correct Azure environment for management endpoint (#470)
* Increased code coverage
* Added databricks_current_user data
* Send empty JSON body while deleting IP ACL (#426)
* Locking single EC2 Instance profile for tests
* Use Terraform 0.14+ as primary testing binary

Co-authored-by: Serge Smertin <[email protected]>

Author: nfx

.gitignore

@@ -333,4 +333,6 @@ website/public/**
 
 .vscode/private.env
 tf.log
-*.env
+*.env
+
+scripts/tt

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## 0.3.0
 
+* Added global timeout for resource provisioning, which defaults to 15 minutes.
+* Added [databricks_current_user] to simplify applying the same Terraform configuration by different users in the shared workspace for testing purposes. 
+* Added client-side rate limiting to release the pressure on backend APIs and prevent client blocking ([#465](https://github.com/databrickslabs/terraform-provider-databricks/pull/465))
 * Member usernames, group names and instance profile names in `databricks_group` data source are now sorted and providing consistent behavior between runs ([#449](https://github.com/databrickslabs/terraform-provider-databricks/issues/449))
 * Fixed redundant multiple mounting clusters ([#445](https://github.com/databrickslabs/terraform-provider-databricks/issues/445))
 * Added optional parameter azure_environment to provider config which defaults to public ([#437](https://github.com/databrickslabs/terraform-provider-databricks/pull/437)).
@@ -14,11 +17,10 @@
 * Added validation for secret scope name in `databricks_secret`, `databricks_secret_scope` and `databricks_secret_acl`. Non-compliant names may cause errors.
 * Added [databricks_spark_version](https://github.com/databrickslabs/terraform-provider-databricks/issues/347) data source.
 * Fixed support for [single node clusters](https://docs.databricks.com/clusters/single-node.html) support by allowing [`num_workers` to be `0`](https://github.com/databrickslabs/terraform-provider-databricks/pull/454).
-* All resource imports are now making call to corresponding Databricks API by default ([#471](https://github.com/databrickslabs/terraform-provider-databricks/issues/471))
+* Fixed bug in destruction of IP access lists ([#426](https://github.com/databrickslabs/terraform-provider-databricks/issues/426)).
+* All resource imports are now making call to corresponding Databricks API by default ([#471](https://github.com/databrickslabs/terraform-provider-databricks/issues/471)).
 
 **Behavior changes**
-
-* Added optional parameter `azure_environment` to provider config which defaults to `public`.
 * Removed deprecated `library_jar`, `library_egg`, `library_whl`, `library_pypi`, `library_cran`, and `library_maven` from `databricks_cluster` and `databricks_job` in favor of more API-transparent [library](https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs/resources/cluster#library-configuration-block) configuration block.
 * Removed deprecated `notebook_path` and `notebook_base_parameters` from `databricks_job` in favor of [notebook_task](https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs/resources/job#notebook_task-configuration-block) configuration block.
 * Removed deprecated `jar_uri`, `jar_main_class_name`, and `jar_parameters` from `databricks_job` in favor of [spark_jar_task](https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs/resources/job#spark_jar_task-configuration-block) configuration block.
@@ -28,7 +30,7 @@
 * Removed deprecated `databricks_scim_group` resource in favor of [databricks_group](https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs/resources/group).
 * Removed deprecated `databricks_default_user_roles` data source in favor of [databricks_group](https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs/data-sources/group#attribute-reference) data source.
 * Removed deprecated `basic_auth` and `azure_auth` provider configuration blocks in favor of [documented authentication methods](https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs).
-* `format`, `overwrite`, and `mkdirs` were removed from `databricks_notebook`. TODO: handle RESOURCE_ALREADY_EXISTS for mkdirs.
+* `format`, `overwrite`, and `mkdirs` were removed from `databricks_notebook`. To follow expected behavior of Terraform, notebooks are always overwritten.
 * `skip_validation` from `databricks_instance_profile` was removed and is always set to `true` for subsequent requests.
 * `databricks_mws_workspace` got `verify_workspace_runnning` removed and now validates all every deployment. In case deployment failed, it removes workspace that failed and returns error message with explanation.
 * `default_tags` were removed from `databricks_instance_pool`. `disk_spec` got new attribute `disk_type`, that contains `azure_disk_volume_type` and `ebs_volume_type`. This change is made to closer reflect API structure.

CONTRIBUTING.md

@@ -109,6 +109,7 @@ $ docker run -it -v $(pwd):/workpace -w /workpace databricks-terraform apply
 * Consider test functions as scenarios, that you are debugging from IDE when specific issues arise. Test tables are discouraged. Single-use functions in tests are discouraged, unless resource definitions they make are longer than 80 lines.
 * All tests should be capable of repeatedly running on "dirty" environment, which means not requiring a new clean environment every time the test runs.
 * All tests should re-use compute resources whenever possible.
+* Prefer `require.NoError` (stops the test on error) to `assert.NoError` (continues the test on error) when checking the results.
 
 ## Code conventions
 
@@ -129,7 +130,7 @@ Eventually, all of resources would be automatically checked for a unit test pres
 
 ```go
 for name, resource := range p.ResourcesMap {
-	if name != "databricks_scim_user" {
+	if name != "databricks_user" {
 		continue
 	}
 	//...

Makefile

@@ -24,10 +24,13 @@ build:
 	@echo "✓ Building source code with go build ..."
 	@go build -mod vendor -v -o terraform-provider-databricks
 
-install: build
-	@echo "✓ Installing provider into ~/.terraform.d/plugins ..."
+install12: build
+	@echo "✓ Installing provider for Terraform 0.12 into ~/.terraform.d/plugins ..."
 	@test -d $(HOME)/.terraform.d/plugins && rm $(HOME)/.terraform.d/plugins/terraform-provider-databricks* || mkdir -p $(HOME)/.terraform.d/plugins
 	@cp terraform-provider-databricks $(HOME)/.terraform.d/plugins
+
+install: build
+	@echo "✓ Installing provider for Terraform 0.13+ into ~/.terraform.d/plugins ..."
 	@mkdir -p '$(HOME)/.terraform.d/plugins/registry.terraform.io/databrickslabs/databricks/$(shell ./terraform-provider-databricks version)/$(shell go version | awk '{print $$4}' | sed 's#/#_#')'
 	@cp terraform-provider-databricks '$(HOME)/.terraform.d/plugins/registry.terraform.io/databrickslabs/databricks/$(shell ./terraform-provider-databricks version)/$(shell go version | awk '{print $$4}' | sed 's#/#_#')/terraform-provider-databricks'
 	@echo "✓ Use the following configuration to enable the version you've built"

README.md

@@ -2,7 +2,11 @@
 
 ![Resources](docs/resources.png)
 
-End-to-end workspace creation on [AWS](scripts/awsmt-integration) or [Azure](scripts/azvnet-integration/databricks.tf)
+[AWS](docs/guides/aws-workspace.md) tutorial
+| [Azure](docs/guides/azure-workspace.md) tutorial
+| [End-to-end](docs/guides/workspace-management.md) tutorial
+| Migration from [0.2.x to 0.3.x](docs/guides/migration-0.3.x.md)
+| [Changelog](CHANGELOG.md)
 | [Authentication](docs/index.md)
 | [databricks_aws_s3_mount](docs/resources/aws_s3_mount.md)
 | [databricks_aws_assume_role_policy](docs/data-sources/aws_assume_role_policy.md) data
@@ -13,6 +17,7 @@ End-to-end workspace creation on [AWS](scripts/awsmt-integration) or [Azure](scr
 | [databricks_azure_blob_mount](docs/resources/azure_blob_mount.md)
 | [databricks_cluster](docs/resources/cluster.md)
 | [databricks_cluster_policy](docs/resources/cluster_policy.md)
+| [databricks_current_user](docs/data-sources/current_user.md)
 | [databricks_dbfs_file](docs/resources/dbfs_file.md)
 | [databricks_dbfs_file_paths](docs/data-sources/dbfs_file_paths.md) data
 | [databricks_dbfs_file](docs/data-sources/dbfs_file.md) data
@@ -44,7 +49,6 @@ End-to-end workspace creation on [AWS](scripts/awsmt-integration) or [Azure](scr
 | [databricks_user_instance_profile](docs/resources/user_instance_profile.md)
 | [databricks_workspace_conf](docs/resources/workspace_conf.md)
 | [Contributing and Development Guidelines](CONTRIBUTING.md)
-| [Changelog](CHANGELOG.md)
 
 [![build](https://github.com/databrickslabs/terraform-provider-databricks/workflows/build/badge.svg?branch=master)](https://github.com/databrickslabs/terraform-provider-databricks/actions?query=workflow%3Abuild+branch%3Amaster) [![codecov](https://codecov.io/gh/databrickslabs/terraform-provider-databricks/branch/master/graph/badge.svg)](https://codecov.io/gh/databrickslabs/terraform-provider-databricks)
 
@@ -75,24 +79,44 @@ provider "databricks" {
   token = "<your PAT token>"
 }
 
+data "databricks_current_user" "me" {}
+data "databricks_spark_version" "latest" {}
 data "databricks_node_type" "smallest" {
   local_disk = true
 }
 
-data "databricks_spark_version" "latest_lts" {
-  long_term_support = true
+resource "databricks_notebook" "this" {
+  path     = "${data.databricks_current_user.me.home}/Terraform"
+  language = "PYTHON"
+  content_base64 = base64encode(<<-EOT
+    # created from ${abspath(path.module)}
+    display(spark.range(10))
+    EOT
+  )
 }
 
-resource "databricks_cluster" "shared_autoscaling" {
-  cluster_name            = "Shared Autoscaling"
-  spark_version           = data.databricks_spark_version.latest_lts.id
-  node_type_id            = data.databricks_node_type.smallest.id
-  autotermination_minutes = 20
+resource "databricks_job" "this" {
+  name = "Terraform Demo (${data.databricks_current_user.me.alphanumeric})"
 
-  autoscale {
-    min_workers = 1
-    max_workers = 50
+  new_cluster {
+    num_workers   = 1
+    spark_version = data.databricks_spark_version.latest.id
+    node_type_id  = data.databricks_node_type.smallest.id
   }
+
+  notebook_task {
+    notebook_path = databricks_notebook.this.path
+  }
+
+  email_notifications {}
+}
+
+output "notebook_url" {
+  value = databricks_notebook.this.url
+}
+
+output "job_url" {
+  value = databricks_job.this.url
 }
 ```
 

access/resource_ipaccesslist.go

@@ -75,7 +75,7 @@ func (a ipAccessListsAPI) Update(objectID string, ur ipAccessListUpdateRequest)
 }
 
 func (a ipAccessListsAPI) Delete(objectID string) (err error) {
-	err = a.client.Delete(a.context, "/ip-access-lists/"+objectID, nil)
+	err = a.client.Delete(a.context, "/ip-access-lists/"+objectID, map[string]interface{}{})
 	return
 }
 

access/resource_ipaccesslist_test.go

@@ -3,12 +3,15 @@ package access
 // REST API: https://docs.databricks.com/dev-tools/api/latest/ip-access-list.html#operation/create-list
 
 import (
+	"context"
 	"net/http"
+	"os"
 	"testing"
 
 	"github.com/databrickslabs/databricks-terraform/common"
 	"github.com/databrickslabs/databricks-terraform/internal/qa"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var (
@@ -21,6 +24,36 @@ var (
 	TestingIPAddressesState = []interface{}{"1.2.3.4", "1.2.4.0/24"}
 )
 
+func TestAccIPACL(t *testing.T) {
+	cloud := os.Getenv("CLOUD_ENV")
+	if cloud == "" {
+		t.Skip("Acceptance tests skipped unless env 'CLOUD_ENV' is set")
+	}
+	client := common.NewClientFromEnvironment()
+	ctx := context.Background()
+	ipAccessListsAPI := NewIPAccessListsAPI(ctx, client)
+	res, err := ipAccessListsAPI.Create(createIPAccessListRequest{
+		Label:       qa.RandomName("ipacl-"),
+		ListType:    "BLOCK",
+		IPAddresses: TestingIPAddresses,
+	})
+	require.NoError(t, err)
+	defer func() {
+		err = ipAccessListsAPI.Delete(res.ListID)
+		require.NoError(t, err)
+	}()
+	err = ipAccessListsAPI.Update(res.ListID, ipAccessListUpdateRequest{
+		Label:       res.Label,
+		ListType:    res.ListType,
+		Enabled:     true,
+		IPAddresses: []string{"4.3.2.1"},
+	})
+	require.NoError(t, err)
+	updated, err := ipAccessListsAPI.Read(res.ListID)
+	require.NoError(t, err)
+	assert.Equal(t, "4.3.2.1", updated.IPAddresses[0])
+}
+
 func TestIPACLCreate(t *testing.T) {
 	d, err := qa.ResourceFixture{
 		Fixtures: []qa.HTTPFixture{

common/azure_auth.go

@@ -69,18 +69,19 @@ type TokenInfo struct {
 var authorizerMutex sync.Mutex
 
 func (aa *AzureAuth) getAzureEnvironment() (azure.Environment, error) {
+	// Used for unit testing purposes
+	if aa.azureManagementEndpoint != "" {
+		return azure.Environment{
+			ResourceManagerEndpoint: aa.azureManagementEndpoint,
+		}, nil
+	}
+
 	if aa.Environment == "" {
 		return azure.PublicCloud, nil
 	}
 
 	envName := fmt.Sprintf("AZURE%sCLOUD", strings.ToUpper(aa.Environment))
-	env, err := azure.EnvironmentFromName(envName)
-
-	if err != nil {
-		return env, err
-	}
-
-	return env, nil
+	return azure.EnvironmentFromName(envName)
 }
 
 func (aa *AzureAuth) resourceID() string {
@@ -276,14 +277,15 @@ func (aa *AzureAuth) ensureWorkspaceURL(ctx context.Context,
 		return fmt.Errorf("Somehow resource id is not set")
 	}
 	log.Println("[DEBUG] Getting Workspace ID via management token.")
-	endpoint := "https://management.azure.com"
-	if aa.azureManagementEndpoint != "" {
-		// sets endpoint specified in unit test
-		endpoint = aa.azureManagementEndpoint
+	env, err := aa.getAzureEnvironment()
+	if err != nil {
+		return err
 	}
+	// All azure endpoints typically end with a trailing slash removing it because resourceID starts with slash
+	managementResourceUrl := strings.TrimSuffix(env.ResourceManagerEndpoint, "/") + resourceID
 	var workspace azureDatabricksWorkspace
 	resp, err := aa.databricksClient.genericQuery(ctx, http.MethodGet,
-		endpoint+resourceID,
+		managementResourceUrl,
 		map[string]string{
 			"api-version": "2018-04-01",
 		}, func(r *http.Request) error {

common/azure_auth_test.go

@@ -107,6 +107,16 @@ func TestEnsureWorkspaceURL_CornerCases(t *testing.T) {
 	aa.databricksClient = &DatabricksClient{}
 	err = aa.ensureWorkspaceURL(context.Background(), nil)
 	assert.EqualError(t, err, "Somehow resource id is not set")
+
+	aa = AzureAuth{
+		Environment:      "xyz",
+		SubscriptionID:   "a",
+		ResourceGroup:    "b",
+		WorkspaceName:    "c",
+		databricksClient: &DatabricksClient{},
+	}
+	err = aa.ensureWorkspaceURL(context.Background(), nil)
+	assert.EqualError(t, err, "autorest/azure: There is no cloud environment matching the name \"AZUREXYZCLOUD\"")
 }
 
 func TestAcquirePAT_CornerCases(t *testing.T) {
@@ -132,18 +142,6 @@ func TestAcquirePAT_CornerCases(t *testing.T) {
 	assert.Equal(t, "...", auth.TokenValue)
 }
 
-func TestAzureAuth_isClientSecretSet(t *testing.T) {
-	aa := AzureAuth{}
-	assert.False(t, aa.IsClientSecretSet())
-
-	aa.ClientID = "a"
-	assert.False(t, aa.IsClientSecretSet())
-	aa.ClientSecret = "b"
-	assert.False(t, aa.IsClientSecretSet())
-	aa.TenantID = "c"
-	assert.True(t, aa.IsClientSecretSet())
-}
-
 func TestAzureAuth_ensureWorkspaceURL(t *testing.T) {
 	aa := AzureAuth{}
 
@@ -167,7 +165,8 @@ func TestAzureAuth_ensureWorkspaceURL(t *testing.T) {
 	defer server.Close()
 
 	aa.ResourceID = "/subscriptions/a/resourceGroups/b/providers/Microsoft.Databricks/workspaces/c"
-	aa.azureManagementEndpoint = server.URL
+	// resource management endpoints end with a trailing slash in url
+	aa.azureManagementEndpoint = fmt.Sprintf("%s/", server.URL)
 
 	client := DatabricksClient{InsecureSkipVerify: true}
 	client.configureHTTPCLient()
@@ -249,7 +248,8 @@ func TestAzureAuth_configureWithClientSecret(t *testing.T) {
 	aa.ClientID = "a"
 	aa.ClientSecret = "b"
 	aa.TenantID = "c"
-	aa.azureManagementEndpoint = server.URL
+	// resource management endpoints end with a trailing slash in url
+	aa.azureManagementEndpoint = fmt.Sprintf("%s/", server.URL)
 	auth, err = aa.configureWithClientSecret()
 	assert.NotNil(t, auth)
 	assert.NoError(t, err)
@@ -272,6 +272,23 @@ func TestAzureAuth_configureWithClientSecret(t *testing.T) {
 	assert.Len(t, zi.Zones, 3)
 }
 
+func TestAzureEnvironment_WithAzureManagementEndpoint(t *testing.T) {
+	fakeEndpoint := "http://google.com"
+	aa := AzureAuth{azureManagementEndpoint: fakeEndpoint}
+	env, err := aa.getAzureEnvironment()
+	assert.Nil(t, err)
+	// This value should be populated with azureManagementEndpoint for testing
+	assert.Equal(t, env.ResourceManagerEndpoint, fakeEndpoint)
+	// The rest should be nill
+	assert.Equal(t, env.ActiveDirectoryEndpoint, "")
+
+	// Making the azureManagementEndpoint empty should yield PublicCloud
+	aa.azureManagementEndpoint = ""
+	env, err = aa.getAzureEnvironment()
+	assert.Nil(t, err)
+	assert.Equal(t, azure.PublicCloud, env)
+}
+
 func TestAzureEnvironment(t *testing.T) {
 	aa := AzureAuth{}
 	env, err := aa.getAzureEnvironment()