Updated docs and tests for databricks_volume resource (#2355)

* comments * comments * comments * update * upd
databricks · May 31, 2023 · 9bbb300 · 9bbb300
1 parent ad25de3
commit 9bbb300
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 53 deletions.
diff --git a/catalog/resource_volume.go b/catalog/resource_volume.go
@@ -48,17 +48,18 @@ func ResourceVolume() *schema.Resource {
 			}
 			d.SetId(v.FullName)
 
-			// Update owner if it is provided
-			if d.Get("owner") != "" {
-				var updateVolumeRequestContent catalog.UpdateVolumeRequestContent
-				common.DataToStructPointer(d, s, &updateVolumeRequestContent)
-				updateVolumeRequestContent.FullNameArg = d.Id()
-				_, err = w.Volumes.Update(ctx, updateVolumeRequestContent)
-				if err != nil {
-					return err
-				}
+			// Don't update owner if it is not provided
+			if d.Get("owner") == "" {
+				return nil
 			}
 
+			var updateVolumeRequestContent catalog.UpdateVolumeRequestContent
+			common.DataToStructPointer(d, s, &updateVolumeRequestContent)
+			updateVolumeRequestContent.FullNameArg = d.Id()
+			_, err = w.Volumes.Update(ctx, updateVolumeRequestContent)
+			if err != nil {
+				return err
+			}
 			return nil
 		},
 		Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {

diff --git a/catalog/resource_volume_test.go b/catalog/resource_volume_test.go
@@ -41,7 +41,7 @@ func TestVolumesCreateWithoutInitialOwner(t *testing.T) {
 			},
 			{
 				Method:   http.MethodGet,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName" + "?",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName?",
 				Response: catalog.VolumeInfo{
 					Name:        "testName",
 					VolumeType:  catalog.VolumeType("testVolumeType"),
@@ -97,7 +97,7 @@ func TestVolumesCreateWithInitialOwner(t *testing.T) {
 			},
 			{
 				Method:   http.MethodGet,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName" + "?",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName?",
 				Response: catalog.VolumeInfo{
 					Name:        "testName",
 					VolumeType:  catalog.VolumeType("testVolumeType"),
@@ -110,7 +110,7 @@ func TestVolumesCreateWithInitialOwner(t *testing.T) {
 			},
 			{
 				Method:   http.MethodPatch,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName",
 				ExpectedRequest: catalog.UpdateVolumeRequestContent{
 					Name:    "testName",
 					Comment: "This is a test comment.",
@@ -201,7 +201,7 @@ func TestVolumesCreateWithInitialOwner_Error(t *testing.T) {
 			},
 			{
 				Method:   http.MethodGet,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName" + "?",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName?",
 				Response: catalog.VolumeInfo{
 					Name:        "testName",
 					VolumeType:  catalog.VolumeType("testVolumeType"),
@@ -214,7 +214,7 @@ func TestVolumesCreateWithInitialOwner_Error(t *testing.T) {
 			},
 			{
 				Method:   http.MethodPatch,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName",
 				Response: apierr.APIErrorBody{
 					ErrorCode: "SERVER_ERROR",
 					Message:   "Something unexpected happened",
@@ -241,7 +241,7 @@ func TestVolumesRead(t *testing.T) {
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   http.MethodGet,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName" + "?",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName?",
 				Response: catalog.VolumeInfo{
 					Name:        "testName",
 					VolumeType:  catalog.VolumeType("testVolumeType"),
@@ -276,7 +276,7 @@ func TestResourceVolumeRead_Error(t *testing.T) {
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   "GET",
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName" + "?",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName?",
 				Response: apierr.APIErrorBody{
 					ErrorCode: "INVALID_REQUEST",
 					Message:   "Internal error happened",
@@ -297,7 +297,7 @@ func TestVolumesUpdate(t *testing.T) {
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   http.MethodGet,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName" + "?",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName?",
 				Response: catalog.VolumeInfo{
 					Name:        "testNameNew",
 					VolumeType:  catalog.VolumeType("testVolumeType"),
@@ -310,7 +310,7 @@ func TestVolumesUpdate(t *testing.T) {
 			},
 			{
 				Method:   http.MethodPatch,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName",
 				ExpectedRequest: catalog.UpdateVolumeRequestContent{
 					Name:    "testNameNew",
 					Comment: "This is a new test comment.",
@@ -352,7 +352,7 @@ func TestVolumeUpdate_Error(t *testing.T) {
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   http.MethodPatch,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName",
 				ExpectedRequest: catalog.UpdateVolumeRequestContent{
 					Name:    "testNameNew",
 					Comment: "This is a new test comment.",
@@ -385,7 +385,7 @@ func TestVolumeDelete(t *testing.T) {
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   http.MethodDelete,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName" + "?",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName?",
 			},
 		},
 		Resource: ResourceVolume(),
@@ -401,7 +401,7 @@ func TestVolumeDelete_Error(t *testing.T) {
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   http.MethodDelete,
-				Resource: "/api/2.1/unity-catalog/volumes/" + "testCatalogName.testSchemaName.testName" + "?",
+				Resource: "/api/2.1/unity-catalog/volumes/testCatalogName.testSchemaName.testName?",
 				Response: apierr.APIErrorBody{
 					ErrorCode: "INVALID_STATE",
 					Message:   "Something went wrong",

diff --git a/docs/resources/volume.md b/docs/resources/volume.md
@@ -29,13 +29,26 @@ resource "databricks_schema" "things" {
   }
 }
 
+resource "databricks_storage_credential" "external" {
+  name = "creds"
+  aws_iam_role {
+    role_arn = aws_iam_role.external_data_access.arn
+  }
+}
+
+resource "databricks_external_location" "some" {
+  name            = "external-location"
+  url             = "s3://${aws_s3_bucket.external.id}/some"
+  credential_name = databricks_storage_credential.external.id
+}
+
 resource "databricks_volume" "this" {
   name = "quickstart_volume"
   catalog_name = databricks_catalog.sandbox.name
   schema_name = databricks_schema.things.name 
   owner = "volume_owner"
   volume_type = "EXTERNAL"
-  storage_location   = ""
+  storage_location   = databricks_external_location.some.url 
   comment = "this volume is managed by terraform"
 }
 ```

diff --git a/internal/acceptance/volume_test.go b/internal/acceptance/volume_test.go
@@ -1,33 +1,28 @@
 package acceptance
 
 import (
-	"fmt"
 	"testing"
-
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 )
 
 func TestUcAccVolumesResourceWithoutInitialOwnerAWSFullLifecycle(t *testing.T) {
-	GetEnvOrSkipTest(t, "TEST_BUCKET")
-	randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 	unityWorkspaceLevel(t, step{
-		Template: fmt.Sprintf(`
+		Template: `
 		resource "databricks_schema" "this" {
-			name 		 = "schema-%[1]s"
+			name 		 = "schema-{var.STICKY_RANDOM}"
 			catalog_name = "main"
 		}
 
 		resource "databricks_storage_credential" "external" {
-			name = "cred-%[1]s"
+			name = "cred-{var.STICKY_RANDOM}"
 			aws_iam_role {
 				role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}"
 			}
 			comment = "Managed by TF"
 		}
 
 		resource "databricks_external_location" "some" {
-			name            = "external-%[1]s"
-			url             = "s3://{env.TEST_BUCKET}/somepath-%[1]s"
+			name            = "external-{var.STICKY_RANDOM}"
+			url             = "s3://{env.TEST_BUCKET}/somepath-{var.STICKY_RANDOM}"
 			credential_name = databricks_storage_credential.external.id
 			comment         = "Managed by TF"
 		}
@@ -39,25 +34,25 @@ func TestUcAccVolumesResourceWithoutInitialOwnerAWSFullLifecycle(t *testing.T) {
 			schema_name = databricks_schema.this.name 
 			volume_type = "EXTERNAL"
 			storage_location   = databricks_external_location.some.url
-		}`, randomName),
+		}`,
 	}, step{
-		Template: fmt.Sprintf(`
+		Template: `
 		resource "databricks_schema" "this" {
-			name 		 = "schema-%[1]s"
+			name 		 = "schema-{var.STICKY_RANDOM}"
 			catalog_name = "main"
 		}
 
 		resource "databricks_storage_credential" "external" {
-			name = "cred-%[1]s"
+			name = "cred-{var.STICKY_RANDOM}"
 			aws_iam_role {
 				role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}"
 			}
 			comment = "Managed by TF"
 		}
 
 		resource "databricks_external_location" "some" {
-			name            = "external-%[1]s"
-			url             = "s3://{env.TEST_BUCKET}/somepath-%[1]s"
+			name            = "external-{var.STICKY_RANDOM}"
+			url             = "s3://{env.TEST_BUCKET}/somepath-{var.STICKY_RANDOM}"
 			credential_name = databricks_storage_credential.external.id
 			comment         = "Managed by TF"
 		}
@@ -70,31 +65,29 @@ func TestUcAccVolumesResourceWithoutInitialOwnerAWSFullLifecycle(t *testing.T) {
 			schema_name = databricks_schema.this.name 
 			volume_type = "EXTERNAL"
 			storage_location   = databricks_external_location.some.url
-		}`, randomName),
+		}`,
 	})
 }
 
 func TestUcAccVolumesResourceWithInitialOnwerAWSFullLifecycle(t *testing.T) {
-	GetEnvOrSkipTest(t, "TEST_BUCKET")
-	randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 	unityWorkspaceLevel(t, step{
-		Template: fmt.Sprintf(`
+		Template: `
 		resource "databricks_schema" "this" {
-			name 		 = "schema-%[1]s"
+			name 		 = "schema-{var.STICKY_RANDOM}"
 			catalog_name = "main"
 		}
 
 		resource "databricks_storage_credential" "external" {
-			name = "cred-%[1]s"
+			name = "cred-{var.STICKY_RANDOM}"
 			aws_iam_role {
 				role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}"
 			}
 			comment = "Managed by TF"
 		}
 
 		resource "databricks_external_location" "some" {
-			name            = "external-%[1]s"
-			url             = "s3://{env.TEST_BUCKET}/somepath-%[1]s"
+			name            = "external-{var.STICKY_RANDOM}"
+			url             = "s3://{env.TEST_BUCKET}/somepath-{var.STICKY_RANDOM}"
 			credential_name = databricks_storage_credential.external.id
 			comment         = "Managed by TF"
 		}
@@ -107,25 +100,25 @@ func TestUcAccVolumesResourceWithInitialOnwerAWSFullLifecycle(t *testing.T) {
 			schema_name = databricks_schema.this.name 
 			volume_type = "EXTERNAL"
 			storage_location   = databricks_external_location.some.url
-		}`, randomName),
+		}`,
 	}, step{
-		Template: fmt.Sprintf(`
+		Template: `
 		resource "databricks_schema" "this" {
-			name 		 = "schema-%[1]s"
+			name 		 = "schema-{var.STICKY_RANDOM}"
 			catalog_name = "main"
 		}
 
 		resource "databricks_storage_credential" "external" {
-			name = "cred-%[1]s"
+			name = "cred-{var.STICKY_RANDOM}"
 			aws_iam_role {
 				role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}"
 			}
 			comment = "Managed by TF"
 		}
 
 		resource "databricks_external_location" "some" {
-			name            = "external-%[1]s"
-			url             = "s3://{env.TEST_BUCKET}/somepath-%[1]s"
+			name            = "external-{var.STICKY_RANDOM}"
+			url             = "s3://{env.TEST_BUCKET}/somepath-{var.STICKY_RANDOM}"
 			credential_name = databricks_storage_credential.external.id
 			comment         = "Managed by TF"
 		}
@@ -138,6 +131,6 @@ func TestUcAccVolumesResourceWithInitialOnwerAWSFullLifecycle(t *testing.T) {
 			schema_name = databricks_schema.this.name 
 			volume_type = "EXTERNAL"
 			storage_location   = databricks_external_location.some.url
-		}`, randomName),
+		}`,
 	})
 }