Skip to content

Commit 4b51e02

Browse files
mgyuchtmgyucht
authored
Fix databricks_user and databricks_service_principal integration tests (#3469)
* Fix user and service principal integration tests * Tolerate missing homedir/repos on delete
1 parent 9a3315f commit 4b51e02

File tree

6 files changed

+67
-66
lines changed

6 files changed

+67
-66
lines changed

‎internal/acceptance/service_principal_test.go‎

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package acceptance
33
import (
44
"context"
55
"fmt"
6-
"os"
76
"testing"
87

98
"github.com/databricks/databricks-sdk-go"
@@ -22,36 +21,34 @@ const awsSpn = `resource "databricks_service_principal" "this" {
2221
}`
2322

2423
func TestAccServicePrincipalHomeDeleteSuccess(t *testing.T) {
25-
GetEnvOrSkipTest(t, "ARM_CLIENT_ID")
24+
loadWorkspaceEnv(t)
25+
if !isAzure(t) {
26+
skipf(t)("Test only valid for Azure")
27+
}
28+
uuid := createUuid()
29+
template := `
30+
resource "databricks_service_principal" "a" {
31+
application_id = "` + uuid + `"
32+
force_delete_home_dir = true
33+
}`
34+
var spId string
2635
workspaceLevel(t, step{
27-
Template: `
28-
resource "databricks_service_principal" "a" {
29-
application_id = "{var.RANDOM_UUID}"
30-
force_delete_home_dir = true
31-
}`,
36+
Template: template,
3237
Check: func(s *terraform.State) error {
33-
appId := s.RootModule().Resources["databricks_service_principal.a"].Primary.Attributes["application_id"]
34-
os.Setenv("application_id_a", appId)
38+
spId = s.RootModule().Resources["databricks_service_principal.a"].Primary.Attributes["application_id"]
3539
return nil
3640
},
3741
}, step{
38-
Template: `
39-
resource "databricks_service_principal" "b" {
40-
application_id = "{var.RANDOM_UUID}"
41-
}
42-
`,
42+
Template: template,
43+
Destroy: true,
4344
Check: func(s *terraform.State) error {
4445
w, err := databricks.NewWorkspaceClient()
4546
if err != nil {
4647
return err
4748
}
4849
ctx := context.Background()
49-
_, err = w.Workspace.GetStatusByPath(ctx, fmt.Sprintf("/Users/%v", os.Getenv("application_id_a")))
50-
os.Remove("application_id_a")
51-
if err != nil {
52-
if apierr.IsMissing(err) {
53-
return nil
54-
}
50+
_, err = w.Workspace.GetStatusByPath(ctx, fmt.Sprintf("/Users/%v", spId))
51+
if err != nil && !apierr.IsMissing(err) {
5552
return err
5653
}
5754
return nil
@@ -60,24 +57,26 @@ func TestAccServicePrincipalHomeDeleteSuccess(t *testing.T) {
6057
}
6158

6259
func TestAccServicePrinicpalHomeDeleteNotDeleted(t *testing.T) {
63-
GetEnvOrSkipTest(t, "ARM_CLIENT_ID")
60+
loadWorkspaceEnv(t)
61+
if !isAzure(t) {
62+
skipf(t)("Test only valid for Azure")
63+
}
64+
uuid := createUuid()
65+
template := `
66+
resource "databricks_service_principal" "a" {
67+
application_id = "` + uuid + `"
68+
force_delete_home_dir = false
69+
}`
6470
var appId string
6571
workspaceLevel(t, step{
66-
Template: `
67-
resource "databricks_service_principal" "a" {
68-
application_id = "{var.RANDOM_UUID}"
69-
force_delete_home_dir = false
70-
}`,
72+
Template: template,
7173
Check: func(s *terraform.State) error {
7274
appId = s.RootModule().Resources["databricks_service_principal.a"].Primary.Attributes["application_id"]
7375
return provisionHomeFolder(context.Background(), s, "databricks_service_principal.a", appId)
7476
},
7577
}, step{
76-
Template: `
77-
resource "databricks_service_principal" "b" {
78-
application_id = "{var.RANDOM_UUID}"
79-
}
80-
`,
78+
Template: template,
79+
Destroy: true,
8180
Check: func(s *terraform.State) error {
8281
w, err := databricks.NewWorkspaceClient()
8382
if err != nil {

‎internal/acceptance/user_test.go‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -66,17 +66,16 @@ func TestAccUserHomeDeleteHasNoEffectInAccount(t *testing.T) {
6666

6767
func TestAccUserHomeDelete(t *testing.T) {
6868
username := qa.RandomEmail()
69+
template := `
70+
resource "databricks_user" "first" {
71+
user_name = "` + username + `"
72+
force_delete_home_dir = true
73+
}`
6974
workspaceLevel(t, step{
70-
Template: `
71-
resource "databricks_user" "first" {
72-
user_name = "` + username + `"
73-
force_delete_home_dir = true
74-
}`,
75+
Template: template,
7576
}, step{
76-
Template: `
77-
resource "databricks_user" "second" {
78-
user_name = "{var.RANDOM}@example.com"
79-
}`,
77+
Template: template,
78+
Destroy: true,
8079
Check: func(s *terraform.State) error {
8180
w, err := databricks.NewWorkspaceClient()
8281
if err != nil {
@@ -112,19 +111,18 @@ func provisionHomeFolder(ctx context.Context, s *terraform.State, tfAttribute, u
112111

113112
func TestAccUserHomeDeleteNotDeleted(t *testing.T) {
114113
username := qa.RandomEmail()
114+
template := `
115+
resource "databricks_user" "a" {
116+
user_name = "` + username + `"
117+
}`
115118
workspaceLevel(t, step{
116-
Template: `
117-
resource "databricks_user" "a" {
118-
user_name = "` + username + `"
119-
}`,
119+
Template: template,
120120
Check: func(s *terraform.State) error {
121121
return provisionHomeFolder(context.Background(), s, "databricks_user.a", username)
122122
},
123123
}, step{
124-
Template: `
125-
resource "databricks_user" "b" {
126-
user_name = "{var.RANDOM}@example.com"
127-
}`,
124+
Template: template,
125+
Destroy: true,
128126
Check: func(s *terraform.State) error {
129127
w, err := databricks.NewWorkspaceClient()
130128
if err != nil {

‎scim/resource_service_principal.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/http"
88
"strings"
99

10+
"github.com/databricks/databricks-sdk-go/apierr"
1011
"github.com/databricks/terraform-provider-databricks/common"
1112
"golang.org/x/exp/slices"
1213

@@ -219,22 +220,25 @@ func ResourceServicePrincipal() common.Resource {
219220
} else {
220221
err = spAPI.Delete(d.Id())
221222
}
223+
if err != nil {
224+
return err
225+
}
222226
// Handle force delete flags
223227
if !isAccount && !isDisable && err == nil {
224228
if isForceDeleteRepos {
225229
err = workspace.NewNotebooksAPI(ctx, c).Delete(fmt.Sprintf("/Repos/%v", appId), true)
226-
if err != nil {
230+
if err != nil && !apierr.IsMissing(err) {
227231
return fmt.Errorf("force_delete_repos: %s", err.Error())
228232
}
229233
}
230234
if isForceDeleteHomeDir {
231235
err = workspace.NewNotebooksAPI(ctx, c).Delete(fmt.Sprintf("/Users/%v", appId), true)
232-
if err != nil {
236+
if err != nil && !apierr.IsMissing(err) {
233237
return fmt.Errorf("force_delete_home_dir: %s", err.Error())
234238
}
235239
}
236240
}
237-
return err
241+
return nil
238242
},
239243
}
240244
}

‎scim/resource_service_principal_test.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ func TestResourceServicePrinicpalforce_delete_reposError(t *testing.T) {
451451
}
452452

453453
func TestResourceServicePrincipalDelete_NonExistingRepo(t *testing.T) {
454-
_, err := qa.ResourceFixture{
454+
qa.ResourceFixture{
455455
Fixtures: []qa.HTTPFixture{
456456
{
457457
Method: "DELETE",
@@ -478,8 +478,7 @@ func TestResourceServicePrincipalDelete_NonExistingRepo(t *testing.T) {
478478
application_id = "abc"
479479
force_delete_repos = true
480480
`,
481-
}.Apply(t)
482-
assert.EqualError(t, err, "force_delete_repos: Path (/Repos/abc) doesn't exist.")
481+
}.ApplyNoError(t)
483482
}
484483

485484
func TestResourceServicePrincipalDelete_DirError(t *testing.T) {
@@ -511,7 +510,7 @@ func TestResourceServicePrincipalDelete_DirError(t *testing.T) {
511510
}
512511

513512
func TestResourceServicePrincipalDelete_NonExistingDir(t *testing.T) {
514-
_, err := qa.ResourceFixture{
513+
qa.ResourceFixture{
515514
Fixtures: []qa.HTTPFixture{
516515
{
517516
Method: "DELETE",
@@ -538,8 +537,7 @@ func TestResourceServicePrincipalDelete_NonExistingDir(t *testing.T) {
538537
application_id = "abc"
539538
force_delete_home_dir = true
540539
`,
541-
}.Apply(t)
542-
assert.EqualError(t, err, "force_delete_home_dir: Path (/Users/abc) doesn't exist.")
540+
}.ApplyNoError(t)
543541
}
544542

545543
func TestCreateForceOverridesManuallyAddedServicePrincipalErrorNotMatched(t *testing.T) {

‎scim/resource_user.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"strings"
77

8+
"github.com/databricks/databricks-sdk-go/apierr"
89
"github.com/databricks/terraform-provider-databricks/common"
910
"github.com/databricks/terraform-provider-databricks/workspace"
1011
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -145,22 +146,25 @@ func ResourceUser() common.Resource {
145146
} else {
146147
err = user.Delete(d.Id())
147148
}
149+
if err != nil {
150+
return err
151+
}
148152
// Handle force delete flags
149153
if !isAccount && !isDisable && err == nil {
150154
if isForceDeleteRepos {
151155
err = workspace.NewNotebooksAPI(ctx, c).Delete(fmt.Sprintf("/Repos/%v", userName), true)
152-
if err != nil {
156+
if err != nil && !apierr.IsMissing(err) {
153157
return fmt.Errorf("force_delete_repos: %s", err.Error())
154158
}
155159
}
156160
if isForceDeleteHomeDir {
157161
err = workspace.NewNotebooksAPI(ctx, c).Delete(fmt.Sprintf("/Users/%v", userName), true)
158-
if err != nil {
162+
if err != nil && !apierr.IsMissing(err) {
159163
return fmt.Errorf("force_delete_home_dir: %s", err.Error())
160164
}
161165
}
162166
}
163-
return err
167+
return nil
164168
},
165169
}
166170
}

‎scim/resource_user_test.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ func TestResourceUserforce_delete_reposError(t *testing.T) {
502502
require.Error(t, err, err)
503503
}
504504
func TestResourceUserDelete_NonExistingRepo(t *testing.T) {
505-
_, err := qa.ResourceFixture{
505+
qa.ResourceFixture{
506506
Fixtures: []qa.HTTPFixture{
507507
{
508508
Method: "DELETE",
@@ -529,8 +529,7 @@ func TestResourceUserDelete_NonExistingRepo(t *testing.T) {
529529
user_name = "abc"
530530
force_delete_repos = true
531531
`,
532-
}.Apply(t)
533-
assert.EqualError(t, err, "force_delete_repos: Path (/Repos/abc) doesn't exist.")
532+
}.ApplyNoError(t)
534533
}
535534

536535
func TestResourceUserDelete_DirError(t *testing.T) {
@@ -561,7 +560,7 @@ func TestResourceUserDelete_DirError(t *testing.T) {
561560
require.Error(t, err, err)
562561
}
563562
func TestResourceUserDelete_NonExistingDir(t *testing.T) {
564-
_, err := qa.ResourceFixture{
563+
qa.ResourceFixture{
565564
Fixtures: []qa.HTTPFixture{
566565
{
567566
Method: "DELETE",
@@ -588,8 +587,7 @@ func TestResourceUserDelete_NonExistingDir(t *testing.T) {
588587
user_name = "abc"
589588
force_delete_home_dir = true
590589
`,
591-
}.Apply(t)
592-
assert.EqualError(t, err, "force_delete_home_dir: Path (/Users/abc) doesn't exist.")
590+
}.ApplyNoError(t)
593591
}
594592

595593
func TestResourceUserDelete_ForceDeleteHomeDir(t *testing.T) {

0 commit comments

Comments
 (0)