Fixed cleaning up artifact path .internal folder (#2572)

## Changes
Fixed cleaning up artifact path .internal folder

## Why
Due to the bug in the code (we can't delete the root folder using
filers) we never cleaned up .internal folder where artifacts are
uploaded to.

## Tests
Added regression test

Author: andrewnester

NEXT_CHANGELOG.md

@@ -2,8 +2,12 @@
 
 ## Release v0.246.0
 
+### Notable Changes
+Previously ".internal" folder under artifact_path was not cleaned up as expected. In this release this behaviour is fixed and now DABs cleans up this folder before uploading artifacts to it.
+
 ### CLI
 
 ### Bundles
+* Fixed cleaning up artifact path .internal folder ([#2572](https://github.com/databricks/cli/pull/2572))
 
 ### API Changes

acceptance/bundle/artifacts/whl_explicit/output.txt

@@ -32,3 +32,24 @@ my_test_code/dist/my_test_code-0.0.1-py3-none-any.whl
 >>> jq .path
 "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal/my_test_code-0.0.1-py3-none-any.whl"
 "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/files/my_test_code/dist/my_test_code-0.0.1-py3-none-any.whl"
+
+=== Expecting delete request to artifact_path/.internal folder
+>>> jq -s .[] | select(.path=="/api/2.0/workspace/delete") | select(.body.path | test(".*/artifacts/.internal")) out.requests.txt
+{
+  "method": "POST",
+  "path": "/api/2.0/workspace/delete",
+  "body": {
+    "path": "/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal",
+    "recursive": true
+  }
+}
+
+=== Expecting mkdirs request to artifact_path/.internal folder
+>>> jq -s .[] | select(.path=="/api/2.0/workspace/mkdirs") | select(.body.path | test(".*/artifacts/.internal")) out.requests.txt
+{
+  "method": "POST",
+  "path": "/api/2.0/workspace/mkdirs",
+  "body": {
+    "path": "/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal"
+  }
+}

acceptance/bundle/artifacts/whl_explicit/script

@@ -8,4 +8,11 @@ trace jq -s '.[] | select(.path=="/api/2.1/jobs/create") | .body.tasks' out.requ
 title "Expecting 1 wheel to be uploaded"
 trace jq .path < out.requests.txt | grep import | grep whl | sort
 
+title "Expecting delete request to artifact_path/.internal folder"
+trace jq -s '.[] | select(.path=="/api/2.0/workspace/delete") | select(.body.path | test(".*/artifacts/.internal"))' out.requests.txt
+
+title "Expecting mkdirs request to artifact_path/.internal folder"
+trace jq -s '.[] | select(.path=="/api/2.0/workspace/mkdirs") | select(.body.path | test(".*/artifacts/.internal"))' out.requests.txt
+
+
 rm out.requests.txt

bundle/artifacts/upload.go

@@ -21,18 +21,18 @@ func (m *cleanUp) Name() string {
 }
 
 func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-	client, uploadPath, diags := libraries.GetFilerForLibraries(ctx, b)
+	client, uploadPath, diags := libraries.GetFilerForLibrariesCleanup(ctx, b)
 	if diags.HasError() {
 		return diags
 	}
 
 	// We intentionally ignore the error because it is not critical to the deployment
-	err := client.Delete(ctx, ".", filer.DeleteRecursively)
+	err := client.Delete(ctx, libraries.InternalDirName, filer.DeleteRecursively)
 	if err != nil {
 		log.Debugf(ctx, "failed to delete %s: %v", uploadPath, err)
 	}
 
-	err = client.Mkdir(ctx, ".")
+	err = client.Mkdir(ctx, libraries.InternalDirName)
 	if err != nil {
 		return diag.Errorf("unable to create directory for %s: %v", uploadPath, err)
 	}

bundle/libraries/filer.go

@@ -2,6 +2,7 @@ package libraries
 
 import (
 	"context"
+	"path"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
@@ -22,11 +23,28 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, s
 		return nil, "", diag.Errorf("remote artifact path not configured")
 	}
 
+	uploadPath := path.Join(b.Config.Workspace.ArtifactPath, InternalDirName)
+
+	switch {
+	case IsVolumesPath(artifactPath):
+		return filerForVolume(b, uploadPath)
+
+	default:
+		return filerForWorkspace(b, uploadPath)
+	}
+}
+
+func GetFilerForLibrariesCleanup(ctx context.Context, b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
+	artifactPath := b.Config.Workspace.ArtifactPath
+	if artifactPath == "" {
+		return nil, "", diag.Errorf("remote artifact path not configured")
+	}
+
 	switch {
 	case IsVolumesPath(artifactPath):
-		return filerForVolume(b)
+		return filerForVolume(b, b.Config.Workspace.ArtifactPath)
 
 	default:
-		return filerForWorkspace(b)
+		return filerForWorkspace(b, b.Config.Workspace.ArtifactPath)
 	}
 }

bundle/libraries/filer_test.go

@@ -7,6 +7,8 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/libs/filer"
+	databrickscfg "github.com/databricks/databricks-sdk-go/config"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -20,13 +22,37 @@ func TestGetFilerForLibrariesValidWsfs(t *testing.T) {
 		},
 	}
 
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &databrickscfg.Config{}
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
 	client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
 	require.NoError(t, diags.Error())
 	assert.Equal(t, "/foo/bar/artifacts/.internal", uploadPath)
 
 	assert.IsType(t, &filer.WorkspaceFilesClient{}, client)
 }
 
+func TestGetFilerForLibrariesCleanupValidWsfs(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/foo/bar/artifacts",
+			},
+		},
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &databrickscfg.Config{}
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
+	client, uploadPath, diags := GetFilerForLibrariesCleanup(context.Background(), b)
+	require.NoError(t, diags.Error())
+	assert.Equal(t, "/foo/bar/artifacts", uploadPath)
+
+	assert.IsType(t, &filer.WorkspaceFilesClient{}, client)
+}
+
 func TestGetFilerForLibrariesValidUcVolume(t *testing.T) {
 	b := &bundle.Bundle{
 		Config: config.Root{
@@ -36,20 +62,48 @@ func TestGetFilerForLibrariesValidUcVolume(t *testing.T) {
 		},
 	}
 
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &databrickscfg.Config{}
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
 	client, uploadPath, diags := GetFilerForLibraries(context.Background(), b)
 	require.NoError(t, diags.Error())
 	assert.Equal(t, "/Volumes/main/my_schema/my_volume/.internal", uploadPath)
 
 	assert.IsType(t, &filer.FilesClient{}, client)
 }
 
+func TestGetFilerForLibrariesCleanupValidUcVolume(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				ArtifactPath: "/Volumes/main/my_schema/my_volume",
+			},
+		},
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &databrickscfg.Config{}
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
+	client, uploadPath, diags := GetFilerForLibrariesCleanup(context.Background(), b)
+	require.NoError(t, diags.Error())
+	assert.Equal(t, "/Volumes/main/my_schema/my_volume", uploadPath)
+
+	assert.IsType(t, &filer.FilesClient{}, client)
+}
+
 func TestGetFilerForLibrariesRemotePathNotSet(t *testing.T) {
 	b := &bundle.Bundle{
 		Config: config.Root{
 			Workspace: config.Workspace{},
 		},
 	}
 
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &databrickscfg.Config{}
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
 	_, _, diags := GetFilerForLibraries(context.Background(), b)
 	require.EqualError(t, diags.Error(), "remote artifact path not configured")
 }

bundle/libraries/filer_volume.go

@@ -1,16 +1,13 @@
 package libraries
 
 import (
-	"path"
-
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/filer"
 )
 
-func filerForVolume(b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
+func filerForVolume(b *bundle.Bundle, uploadPath string) (filer.Filer, string, diag.Diagnostics) {
 	w := b.WorkspaceClient()
-	uploadPath := path.Join(b.Config.Workspace.ArtifactPath, InternalDirName)
 	f, err := filer.NewFilesClient(w, uploadPath)
 	return f, uploadPath, diag.FromErr(err)
 }

bundle/libraries/filer_volume_test.go

@@ -1,15 +1,12 @@
 package libraries
 
 import (
-	"path"
-
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/filer"
 )
 
-func filerForWorkspace(b *bundle.Bundle) (filer.Filer, string, diag.Diagnostics) {
-	uploadPath := path.Join(b.Config.Workspace.ArtifactPath, InternalDirName)
+func filerForWorkspace(b *bundle.Bundle, uploadPath string) (filer.Filer, string, diag.Diagnostics) {
 	f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), uploadPath)
 	return f, uploadPath, diag.FromErr(err)
 }