From 7e7faed32908585af12288f0c7c11ac390f9f895 Mon Sep 17 00:00:00 2001 From: Mario Minardi Date: Wed, 21 Sep 2022 11:29:12 -0600 Subject: [PATCH 01/10] Add safe-delete workspace API, and org setting to control if workspace admins can force delete --- organization.go | 7 ++ organization_integration_test.go | 34 +++++++++ workspace.go | 66 ++++++++++++++--- workspace_integration_test.go | 120 +++++++++++++++++++++++++++++++ 4 files changed, 216 insertions(+), 11 deletions(-) diff --git a/organization.go b/organization.go index d7b0a2b0a..6f91a7c29 100644 --- a/organization.go +++ b/organization.go @@ -78,6 +78,7 @@ type Organization struct { TrialExpiresAt time.Time `jsonapi:"attr,trial-expires-at,iso8601"` TwoFactorConformant bool `jsonapi:"attr,two-factor-conformant"` SendPassingStatusesForUntriggeredSpeculativePlans bool `jsonapi:"attr,send-passing-statuses-for-untriggered-speculative-plans"` + AllowForceDeleteWorkspaces bool `jsonapi:"attr,allow-force-delete-workspaces"` } // Capacity represents the current run capacity of an organization. @@ -166,6 +167,9 @@ type OrganizationCreateOptions struct { // Optional: SendPassingStatusesForUntriggeredSpeculativePlans toggles behavior of untriggered speculative plans to send status updates to version control systems like GitHub. SendPassingStatusesForUntriggeredSpeculativePlans *bool `jsonapi:"attr,send-passing-statuses-for-untriggered-speculative-plans,omitempty"` + + // Optional: AllowForceDeleteWorkspaces toggles behavior of allowing workspace admins to delete workspaces with resources under management. + AllowForceDeleteWorkspaces *bool `jsonapi:"attr,allow-force-delete-workspaces,omitempty"` } // OrganizationUpdateOptions represents the options for updating an organization. @@ -202,6 +206,9 @@ type OrganizationUpdateOptions struct { // SendPassingStatusesForUntriggeredSpeculativePlans toggles behavior of untriggered speculative plans to send status updates to version control systems like GitHub. SendPassingStatusesForUntriggeredSpeculativePlans *bool `jsonapi:"attr,send-passing-statuses-for-untriggered-speculative-plans,omitempty"` + + // Optional: AllowForceDeleteWorkspaces toggles behavior of allowing workspace admins to delete workspaces with resources under management. + AllowForceDeleteWorkspaces *bool `jsonapi:"attr,allow-force-delete-workspaces,omitempty"` } // ReadRunQueueOptions represents the options for showing the queue. diff --git a/organization_integration_test.go b/organization_integration_test.go index 41c24f84f..da1c74a6b 100644 --- a/organization_integration_test.go +++ b/organization_integration_test.go @@ -565,7 +565,41 @@ func TestOrganizationsReadRunTasksEntitlement(t *testing.T) { assert.NotEmpty(t, entitlements.ID) assert.True(t, entitlements.RunTasks) }) +} + +func TestOrganizationsAllowForceDeleteSetting(t *testing.T) { + client := testClient(t) + ctx := context.Background() + t.Run("creates and updates allow force delete", func(t *testing.T) { + options := OrganizationCreateOptions{ + Name: String(randomString(t)), + Email: String(randomString(t) + "@tfe.local"), + AllowForceDeleteWorkspaces: Bool(true), + } + + org, err := client.Organizations.Create(ctx, options) + assert.Nil(t, err) + + t.Cleanup(func() { + err := client.Organizations.Delete(ctx, org.Name) + if err != nil { + t.Logf("error deleting organization (%s): %s", org.Name, err) + } + }) + + assert.Equal(t, *options.Name, org.Name) + assert.Equal(t, *options.Email, org.Email) + assert.True(t, org.AllowForceDeleteWorkspaces) + + org, err = client.Organizations.Update(ctx, org.Name, OrganizationUpdateOptions{AllowForceDeleteWorkspaces: Bool(false)}) + assert.Nil(t, err) + assert.False(t, org.AllowForceDeleteWorkspaces) + + org, err = client.Organizations.Read(ctx, org.Name) + assert.Nil(t, err) + assert.False(t, org.AllowForceDeleteWorkspaces) + }) } func orgItemsContainsName(items []*Organization, name string) bool { diff --git a/workspace.go b/workspace.go index 462048e3b..89554b1b4 100644 --- a/workspace.go +++ b/workspace.go @@ -50,6 +50,12 @@ type Workspaces interface { // DeleteByID deletes a workspace by its ID. DeleteByID(ctx context.Context, workspaceID string) error + // SafeDelete a workspace by its name. + SafeDelete(ctx context.Context, organization string, workspace string) error + + // SafeDeleteByID deletes a workspace by its ID. + SafeDeleteByID(ctx context.Context, workspaceID string) error + // RemoveVCSConnection from a workspace. RemoveVCSConnection(ctx context.Context, organization, workspace string) (*Workspace, error) @@ -194,17 +200,18 @@ type WorkspaceActions struct { // WorkspacePermissions represents the workspace permissions. type WorkspacePermissions struct { - CanDestroy bool `jsonapi:"attr,can-destroy"` - CanForceUnlock bool `jsonapi:"attr,can-force-unlock"` - CanLock bool `jsonapi:"attr,can-lock"` - CanManageRunTasks bool `jsonapi:"attr,can-manage-run-tasks"` - CanQueueApply bool `jsonapi:"attr,can-queue-apply"` - CanQueueDestroy bool `jsonapi:"attr,can-queue-destroy"` - CanQueueRun bool `jsonapi:"attr,can-queue-run"` - CanReadSettings bool `jsonapi:"attr,can-read-settings"` - CanUnlock bool `jsonapi:"attr,can-unlock"` - CanUpdate bool `jsonapi:"attr,can-update"` - CanUpdateVariable bool `jsonapi:"attr,can-update-variable"` + CanDestroy bool `jsonapi:"attr,can-destroy"` + CanForceUnlock bool `jsonapi:"attr,can-force-unlock"` + CanLock bool `jsonapi:"attr,can-lock"` + CanManageRunTasks bool `jsonapi:"attr,can-manage-run-tasks"` + CanQueueApply bool `jsonapi:"attr,can-queue-apply"` + CanQueueDestroy bool `jsonapi:"attr,can-queue-destroy"` + CanQueueRun bool `jsonapi:"attr,can-queue-run"` + CanReadSettings bool `jsonapi:"attr,can-read-settings"` + CanUnlock bool `jsonapi:"attr,can-unlock"` + CanUpdate bool `jsonapi:"attr,can-update"` + CanUpdateVariable bool `jsonapi:"attr,can-update-variable"` + CanForceDelete *bool `jsonapi:"attr,can-force-delete"` // pointer b/c it will be useful to check if this property exists, as opposed to having it default to false } // WSIncludeOpt represents the available options for include query params. @@ -770,6 +777,43 @@ func (s *workspaces) DeleteByID(ctx context.Context, workspaceID string) error { return req.Do(ctx, nil) } +// SafeDelete a workspace by its name. +func (s *workspaces) SafeDelete(ctx context.Context, organization, workspace string) error { + if !validStringID(&organization) { + return ErrInvalidOrg + } + if !validStringID(&workspace) { + return ErrInvalidWorkspaceValue + } + + u := fmt.Sprintf( + "organizations/%s/workspaces/%s/actions/safe-delete", + url.QueryEscape(organization), + url.QueryEscape(workspace), + ) + req, err := s.client.NewRequest("POST", u, nil) + if err != nil { + return err + } + + return req.Do(ctx, nil) +} + +// SafeDeleteByID safely deletes a workspace by its ID. +func (s *workspaces) SafeDeleteByID(ctx context.Context, workspaceID string) error { + if !validStringID(&workspaceID) { + return ErrInvalidWorkspaceID + } + + u := fmt.Sprintf("workspaces/%s/actions/safe-delete", url.QueryEscape(workspaceID)) + req, err := s.client.NewRequest("POST", u, nil) + if err != nil { + return err + } + + return req.Do(ctx, nil) +} + // RemoveVCSConnection from a workspace. func (s *workspaces) RemoveVCSConnection(ctx context.Context, organization, workspace string) (*Workspace, error) { if !validStringID(&organization) { diff --git a/workspace_integration_test.go b/workspace_integration_test.go index cb915b788..461c2f764 100644 --- a/workspace_integration_test.go +++ b/workspace_integration_test.go @@ -1211,6 +1211,126 @@ func TestWorkspacesDeleteByID(t *testing.T) { }) } +func TestCanForceDeletePermission(t *testing.T) { + client := testClient(t) + ctx := context.Background() + + orgTest, orgTestCleanup := createOrganization(t, client) + defer orgTestCleanup() + + wTest, _ := createWorkspace(t, client, orgTest) + + t.Run("workspace permission set includes can-force-delete", func(t *testing.T) { + w, err := client.Workspaces.ReadByID(ctx, wTest.ID) + require.NoError(t, err) + assert.Equal(t, wTest, w) + assert.Equal(t, wTest, w) + assert.NotNil(t, w.Permissions.CanForceDelete) + assert.True(t, *w.Permissions.CanForceDelete) + }) +} + +func TestWorkspacesSafeDelete(t *testing.T) { + client := testClient(t) + ctx := context.Background() + + orgTest, orgTestCleanup := createOrganization(t, client) + defer orgTestCleanup() + + wTest, _ := createWorkspace(t, client, orgTest) + + t.Run("with valid options", func(t *testing.T) { + err := client.Workspaces.SafeDelete(ctx, orgTest.Name, wTest.Name) + require.NoError(t, err) + + // Try loading the workspace - it should fail. + _, err = client.Workspaces.Read(ctx, orgTest.Name, wTest.Name) + assert.Equal(t, ErrResourceNotFound, err) + }) + + t.Run("when organization is invalid", func(t *testing.T) { + err := client.Workspaces.SafeDelete(ctx, badIdentifier, wTest.Name) + assert.EqualError(t, err, ErrInvalidOrg.Error()) + }) + + t.Run("when workspace is invalid", func(t *testing.T) { + err := client.Workspaces.SafeDelete(ctx, orgTest.Name, badIdentifier) + assert.EqualError(t, err, ErrInvalidWorkspaceValue.Error()) + }) + + t.Run("when workspace is locked", func(t *testing.T) { + wTest, workspaceCleanup := createWorkspace(t, client, orgTest) + defer workspaceCleanup() + w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) + assert.NoError(t, err) + assert.True(t, w.Locked) + + err = client.Workspaces.SafeDelete(ctx, orgTest.Name, wTest.Name) + assert.Contains(t, err.Error(), "conflict") + assert.Contains(t, err.Error(), "currently locked") + }) + + t.Run("when workspace has resources under management", func(t *testing.T) { + wTest, workspaceCleanup := createWorkspace(t, client, orgTest) + defer workspaceCleanup() + _, svTestCleanup := createStateVersion(t, client, 0, wTest) + t.Cleanup(svTestCleanup) + + err := client.Workspaces.SafeDelete(ctx, orgTest.Name, wTest.Name) + // cant verify the exact error here because it is timing dependent on the backend + // based on whether the state version has been processed yet + assert.Contains(t, err.Error(), "conflict") + }) +} + +func TestWorkspacesSafeDeleteByID(t *testing.T) { + client := testClient(t) + ctx := context.Background() + + orgTest, orgTestCleanup := createOrganization(t, client) + defer orgTestCleanup() + + wTest, _ := createWorkspace(t, client, orgTest) + + t.Run("with valid options", func(t *testing.T) { + err := client.Workspaces.SafeDeleteByID(ctx, wTest.ID) + require.NoError(t, err) + + // Try loading the workspace - it should fail. + _, err = client.Workspaces.ReadByID(ctx, wTest.ID) + assert.Equal(t, ErrResourceNotFound, err) + }) + + t.Run("without a valid workspace ID", func(t *testing.T) { + err := client.Workspaces.SafeDeleteByID(ctx, badIdentifier) + assert.EqualError(t, err, ErrInvalidWorkspaceID.Error()) + }) + + t.Run("when workspace is locked", func(t *testing.T) { + wTest, workspaceCleanup := createWorkspace(t, client, orgTest) + defer workspaceCleanup() + w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) + assert.NoError(t, err) + assert.True(t, w.Locked) + + err = client.Workspaces.SafeDeleteByID(ctx, wTest.ID) + assert.Contains(t, err.Error(), "conflict") + assert.Contains(t, err.Error(), "currently locked") + }) + + t.Run("when workspace has resources under management", func(t *testing.T) { + wTest, workspaceCleanup := createWorkspace(t, client, orgTest) + defer workspaceCleanup() + _, svTestCleanup := createStateVersion(t, client, 0, wTest) + t.Cleanup(svTestCleanup) + + err := client.Workspaces.SafeDeleteByID(ctx, wTest.ID) + // cant verify the exact error here because it is timing dependent on the backend + // based on whether the state version has been processed yet + assert.Contains(t, err.Error(), "conflict") + }) +} + func TestWorkspacesRemoveVCSConnection(t *testing.T) { skipIfNotCINode(t) From 8730b72ffa23e6ba35df7c9e1e99276a22890144 Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Tue, 11 Oct 2022 11:43:49 -0400 Subject: [PATCH 02/10] Add a comment to clarify that AllowForceDelete will be false for old TFE versions --- organization.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/organization.go b/organization.go index 6f91a7c29..e4b9086ee 100644 --- a/organization.go +++ b/organization.go @@ -78,7 +78,9 @@ type Organization struct { TrialExpiresAt time.Time `jsonapi:"attr,trial-expires-at,iso8601"` TwoFactorConformant bool `jsonapi:"attr,two-factor-conformant"` SendPassingStatusesForUntriggeredSpeculativePlans bool `jsonapi:"attr,send-passing-statuses-for-untriggered-speculative-plans"` - AllowForceDeleteWorkspaces bool `jsonapi:"attr,allow-force-delete-workspaces"` + // Note: This will be false for TFE versions older than v202211, where the setting was introduced. + // On those TFE versions, safe delete does not exist, so ALL deletes will be force deletes. + AllowForceDeleteWorkspaces bool `jsonapi:"attr,allow-force-delete-workspaces"` } // Capacity represents the current run capacity of an organization. From 51341b4fde47328074304a323269ad1ebb4bc90e Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Tue, 11 Oct 2022 11:48:10 -0400 Subject: [PATCH 03/10] Generate mocks for SafeDelete --- mocks/workspace_mocks.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/mocks/workspace_mocks.go b/mocks/workspace_mocks.go index f5fa01ce3..adde2eb7a 100644 --- a/mocks/workspace_mocks.go +++ b/mocks/workspace_mocks.go @@ -330,6 +330,34 @@ func (mr *MockWorkspacesMockRecorder) RemoveVCSConnectionByID(ctx, workspaceID i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveVCSConnectionByID", reflect.TypeOf((*MockWorkspaces)(nil).RemoveVCSConnectionByID), ctx, workspaceID) } +// SafeDelete mocks base method. +func (m *MockWorkspaces) SafeDelete(ctx context.Context, organization, workspace string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SafeDelete", ctx, organization, workspace) + ret0, _ := ret[0].(error) + return ret0 +} + +// SafeDelete indicates an expected call of SafeDelete. +func (mr *MockWorkspacesMockRecorder) SafeDelete(ctx, organization, workspace interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SafeDelete", reflect.TypeOf((*MockWorkspaces)(nil).SafeDelete), ctx, organization, workspace) +} + +// SafeDeleteByID mocks base method. +func (m *MockWorkspaces) SafeDeleteByID(ctx context.Context, workspaceID string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SafeDeleteByID", ctx, workspaceID) + ret0, _ := ret[0].(error) + return ret0 +} + +// SafeDeleteByID indicates an expected call of SafeDeleteByID. +func (mr *MockWorkspacesMockRecorder) SafeDeleteByID(ctx, workspaceID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SafeDeleteByID", reflect.TypeOf((*MockWorkspaces)(nil).SafeDeleteByID), ctx, workspaceID) +} + // UnassignSSHKey mocks base method. func (m *MockWorkspaces) UnassignSSHKey(ctx context.Context, workspaceID string) (*tfe.Workspace, error) { m.ctrl.T.Helper() From 16d153bd1325c6ddb41c7aa2e44ddd0b06ead402 Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Fri, 14 Oct 2022 15:51:03 -0400 Subject: [PATCH 04/10] Remove duplicate assertion --- workspace_integration_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/workspace_integration_test.go b/workspace_integration_test.go index 461c2f764..270a1268a 100644 --- a/workspace_integration_test.go +++ b/workspace_integration_test.go @@ -1224,7 +1224,6 @@ func TestCanForceDeletePermission(t *testing.T) { w, err := client.Workspaces.ReadByID(ctx, wTest.ID) require.NoError(t, err) assert.Equal(t, wTest, w) - assert.Equal(t, wTest, w) assert.NotNil(t, w.Permissions.CanForceDelete) assert.True(t, *w.Permissions.CanForceDelete) }) From 2213c1e199bef88e0096b93a8242e9d6e3e67fff Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Fri, 14 Oct 2022 16:17:07 -0400 Subject: [PATCH 05/10] Add changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa5747068..04848596b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ * Add `Query` and `Status` fields to `OrganizationMembershipListOptions` to allow filtering memberships by status or username by @sebasslash [#550](https://github.com/hashicorp/go-tfe/pull/550) * Add `ListForWorkspace` method to `VariableSets` interface to enable fetching variable sets associated with a workspace by @tstapler [#552](https://github.com/hashicorp/go-tfe/pull/552) * Add `NotificationTriggerAssessmentDrifted` and `NotificationTriggerAssessmentFailed` notification trigger types by @lawliet89 [#542](https://github.com/hashicorp/go-tfe/pull/542) +* Add `AllowForceDeleteWorkspaces` setting to `Organizations` by @JarrettSpiker [#539](https://github.com/hashicorp/go-tfe/pull/539) +* Add `SafeDelete` and `SafeDeleteID` APIs to `Workspaces` by @JarrettSpiker [#539](https://github.com/hashicorp/go-tfe/pull/539) + ## Bug Fixes * Fix marshalling of run variables in `RunCreateOptions`. The `Variables` field type in `Run` struct has changed from `[]*RunVariable` to `[]*RunVariableAttr` by @Uk1288 [#531](https://github.com/hashicorp/go-tfe/pull/531) From e8b91eff06841e5530f4264f89650f156687736e Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Fri, 21 Oct 2022 16:25:37 -0400 Subject: [PATCH 06/10] Update tests to exit early if there is a setup error --- organization_integration_test.go | 10 ++++++---- workspace_integration_test.go | 6 ++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/organization_integration_test.go b/organization_integration_test.go index da1c74a6b..22952721b 100644 --- a/organization_integration_test.go +++ b/organization_integration_test.go @@ -568,6 +568,8 @@ func TestOrganizationsReadRunTasksEntitlement(t *testing.T) { } func TestOrganizationsAllowForceDeleteSetting(t *testing.T) { + skipIfNotCINode(t) + client := testClient(t) ctx := context.Background() @@ -579,12 +581,12 @@ func TestOrganizationsAllowForceDeleteSetting(t *testing.T) { } org, err := client.Organizations.Create(ctx, options) - assert.Nil(t, err) + require.NoError(t, err) t.Cleanup(func() { err := client.Organizations.Delete(ctx, org.Name) if err != nil { - t.Logf("error deleting organization (%s): %s", org.Name, err) + t.Errorf("error deleting organization (%s): %s", org.Name, err) } }) @@ -593,11 +595,11 @@ func TestOrganizationsAllowForceDeleteSetting(t *testing.T) { assert.True(t, org.AllowForceDeleteWorkspaces) org, err = client.Organizations.Update(ctx, org.Name, OrganizationUpdateOptions{AllowForceDeleteWorkspaces: Bool(false)}) - assert.Nil(t, err) + require.NoError(t, err) assert.False(t, org.AllowForceDeleteWorkspaces) org, err = client.Organizations.Read(ctx, org.Name) - assert.Nil(t, err) + require.NoError(t, err) assert.False(t, org.AllowForceDeleteWorkspaces) }) } diff --git a/workspace_integration_test.go b/workspace_integration_test.go index 270a1268a..d8a7455e2 100644 --- a/workspace_integration_test.go +++ b/workspace_integration_test.go @@ -1212,6 +1212,8 @@ func TestWorkspacesDeleteByID(t *testing.T) { } func TestCanForceDeletePermission(t *testing.T) { + skipIfNotCINode(t) + client := testClient(t) ctx := context.Background() @@ -1230,6 +1232,8 @@ func TestCanForceDeletePermission(t *testing.T) { } func TestWorkspacesSafeDelete(t *testing.T) { + skipIfNotCINode(t) + client := testClient(t) ctx := context.Background() @@ -1283,6 +1287,8 @@ func TestWorkspacesSafeDelete(t *testing.T) { } func TestWorkspacesSafeDeleteByID(t *testing.T) { + skipIfNotCINode(t) + client := testClient(t) ctx := context.Background() From 01e526771af2a8f732b94aabec9952cd467948f5 Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Mon, 24 Oct 2022 10:29:11 -0400 Subject: [PATCH 07/10] Attempt cleanup on workspaces which should be deleted --- helper_test.go | 6 +++++ workspace_integration_test.go | 44 +++++++++++++++++++++-------------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/helper_test.go b/helper_test.go index 1a3f6b194..53067c52a 100644 --- a/helper_test.go +++ b/helper_test.go @@ -1599,6 +1599,12 @@ func createWorkspaceWithOptions(t *testing.T, client *Client, org *Organization, return w, func() { if err := client.Workspaces.Delete(ctx, org.Name, w.Name); err != nil { + + // if the workspace has already been cleaned up, or destroyed in the testing process, no need to raise the alarm + if err == ErrResourceNotFound { + return + } + t.Errorf("Error destroying workspace! WARNING: Dangling resources\n"+ "may exist! The full error is shown below.\n\n"+ "Workspace: %s\nError: %s", w.Name, err) diff --git a/workspace_integration_test.go b/workspace_integration_test.go index d8a7455e2..45434b12f 100644 --- a/workspace_integration_test.go +++ b/workspace_integration_test.go @@ -737,7 +737,8 @@ func TestWorkspacesUpdate(t *testing.T) { upgradeOrganizationSubscription(t, client, orgTest) - wTest, _ := createWorkspace(t, client, orgTest) + wTest, wCleanup := createWorkspace(t, client, orgTest) + t.Cleanup(wCleanup) t.Run("when updating a subset of values", func(t *testing.T) { options := WorkspaceUpdateOptions{ @@ -853,10 +854,11 @@ func TestWorkspacesUpdate(t *testing.T) { }) defer orgTestCleanup() - wTest, _ := createWorkspaceWithOptions(t, client, orgTest, WorkspaceCreateOptions{ + wTest, wCleanup := createWorkspaceWithOptions(t, client, orgTest, WorkspaceCreateOptions{ Name: String(randomString(t)), TriggerPrefixes: []string{"/prefix-1/", "/prefix-2/"}, }) + t.Cleanup(wCleanup) assert.Equal(t, wTest.TriggerPrefixes, []string{"/prefix-1/", "/prefix-2/"}) // Sanity test options := WorkspaceUpdateOptions{ @@ -901,10 +903,11 @@ func TestWorkspacesUpdate(t *testing.T) { }) defer orgTestCleanup() - wTest, _ := createWorkspaceWithOptions(t, client, orgTest, WorkspaceCreateOptions{ + wTest, wCleanup := createWorkspaceWithOptions(t, client, orgTest, WorkspaceCreateOptions{ Name: String(randomString(t)), TriggerPatterns: []string{"/pattern-1/**/*", "/pattern-2/**/*"}, }) + t.Cleanup(wCleanup) assert.Equal(t, wTest.TriggerPatterns, []string{"/pattern-1/**/*", "/pattern-2/**/*"}) // Sanity test options := WorkspaceUpdateOptions{ @@ -939,7 +942,8 @@ func TestWorkspacesUpdateTableDriven(t *testing.T) { orgTest, orgTestCleanup := createOrganization(t, client) defer orgTestCleanup() - wTest, _ := createWorkspace(t, client, orgTest) + wTest, wCleanup := createWorkspace(t, client, orgTest) + t.Cleanup(wCleanup) workspaceTableTests := []WorkspaceTableTest{ { @@ -1076,7 +1080,8 @@ func TestWorkspacesUpdateByID(t *testing.T) { orgTest, orgTestCleanup := createOrganization(t, client) defer orgTestCleanup() - wTest, _ := createWorkspace(t, client, orgTest) + wTest, wCleanup := createWorkspace(t, client, orgTest) + t.Cleanup(wCleanup) t.Run("when updating a subset of values", func(t *testing.T) { options := WorkspaceUpdateOptions{ @@ -1163,7 +1168,8 @@ func TestWorkspacesDelete(t *testing.T) { orgTest, orgTestCleanup := createOrganization(t, client) defer orgTestCleanup() - wTest, _ := createWorkspace(t, client, orgTest) + wTest, wCleanup := createWorkspace(t, client, orgTest) + t.Cleanup(wCleanup) t.Run("with valid options", func(t *testing.T) { err := client.Workspaces.Delete(ctx, orgTest.Name, wTest.Name) @@ -1194,7 +1200,8 @@ func TestWorkspacesDeleteByID(t *testing.T) { orgTest, orgTestCleanup := createOrganization(t, client) defer orgTestCleanup() - wTest, _ := createWorkspace(t, client, orgTest) + wTest, wCleanup := createWorkspace(t, client, orgTest) + t.Cleanup(wCleanup) t.Run("with valid options", func(t *testing.T) { err := client.Workspaces.DeleteByID(ctx, wTest.ID) @@ -1218,9 +1225,10 @@ func TestCanForceDeletePermission(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) - wTest, _ := createWorkspace(t, client, orgTest) + wTest, wCleanup := createWorkspace(t, client, orgTest) + t.Cleanup(wCleanup) t.Run("workspace permission set includes can-force-delete", func(t *testing.T) { w, err := client.Workspaces.ReadByID(ctx, wTest.ID) @@ -1238,9 +1246,10 @@ func TestWorkspacesSafeDelete(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) - wTest, _ := createWorkspace(t, client, orgTest) + wTest, wCleanup := createWorkspace(t, client, orgTest) + t.Cleanup(wCleanup) t.Run("with valid options", func(t *testing.T) { err := client.Workspaces.SafeDelete(ctx, orgTest.Name, wTest.Name) @@ -1263,7 +1272,7 @@ func TestWorkspacesSafeDelete(t *testing.T) { t.Run("when workspace is locked", func(t *testing.T) { wTest, workspaceCleanup := createWorkspace(t, client, orgTest) - defer workspaceCleanup() + t.Cleanup(workspaceCleanup) w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) assert.NoError(t, err) assert.True(t, w.Locked) @@ -1275,7 +1284,7 @@ func TestWorkspacesSafeDelete(t *testing.T) { t.Run("when workspace has resources under management", func(t *testing.T) { wTest, workspaceCleanup := createWorkspace(t, client, orgTest) - defer workspaceCleanup() + t.Cleanup(workspaceCleanup) _, svTestCleanup := createStateVersion(t, client, 0, wTest) t.Cleanup(svTestCleanup) @@ -1293,9 +1302,10 @@ func TestWorkspacesSafeDeleteByID(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) - wTest, _ := createWorkspace(t, client, orgTest) + wTest, wCleanup := createWorkspace(t, client, orgTest) + t.Cleanup(wCleanup) t.Run("with valid options", func(t *testing.T) { err := client.Workspaces.SafeDeleteByID(ctx, wTest.ID) @@ -1313,7 +1323,7 @@ func TestWorkspacesSafeDeleteByID(t *testing.T) { t.Run("when workspace is locked", func(t *testing.T) { wTest, workspaceCleanup := createWorkspace(t, client, orgTest) - defer workspaceCleanup() + t.Cleanup(workspaceCleanup) w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) assert.NoError(t, err) assert.True(t, w.Locked) @@ -1325,7 +1335,7 @@ func TestWorkspacesSafeDeleteByID(t *testing.T) { t.Run("when workspace has resources under management", func(t *testing.T) { wTest, workspaceCleanup := createWorkspace(t, client, orgTest) - defer workspaceCleanup() + t.Cleanup(workspaceCleanup) _, svTestCleanup := createStateVersion(t, client, 0, wTest) t.Cleanup(svTestCleanup) From dc48caef949db4f974d5378889000c7957e73590 Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Tue, 25 Oct 2022 15:31:14 -0400 Subject: [PATCH 08/10] Cleanup workspaces by ID not name --- helper_test.go | 10 +-- workspace_integration_test.go | 162 +++++++++++++++++----------------- 2 files changed, 83 insertions(+), 89 deletions(-) diff --git a/helper_test.go b/helper_test.go index 53067c52a..e587d19b9 100644 --- a/helper_test.go +++ b/helper_test.go @@ -1598,16 +1598,10 @@ func createWorkspaceWithOptions(t *testing.T, client *Client, org *Organization, } return w, func() { - if err := client.Workspaces.Delete(ctx, org.Name, w.Name); err != nil { - - // if the workspace has already been cleaned up, or destroyed in the testing process, no need to raise the alarm - if err == ErrResourceNotFound { - return - } - + if err := client.Workspaces.DeleteByID(ctx, w.ID); err != nil { t.Errorf("Error destroying workspace! WARNING: Dangling resources\n"+ "may exist! The full error is shown below.\n\n"+ - "Workspace: %s\nError: %s", w.Name, err) + "Workspace: %s\nError: %s", w.ID, err) } if orgCleanup != nil { diff --git a/workspace_integration_test.go b/workspace_integration_test.go index 45434b12f..e16b6aa77 100644 --- a/workspace_integration_test.go +++ b/workspace_integration_test.go @@ -39,12 +39,12 @@ func TestWorkspacesList(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest1, wTest1Cleanup := createWorkspace(t, client, orgTest) - defer wTest1Cleanup() + t.Cleanup(wTest1Cleanup) wTest2, wTest2Cleanup := createWorkspace(t, client, orgTest) - defer wTest2Cleanup() + t.Cleanup(wTest2Cleanup) t.Run("without list options", func(t *testing.T) { wl, err := client.Workspaces.List(ctx, orgTest.Name, nil) @@ -193,7 +193,7 @@ func TestWorkspacesCreateTableDriven(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) workspaceTableTests := []WorkspaceTableTest{ { @@ -216,8 +216,8 @@ func TestWorkspacesCreateTableDriven(t *testing.T) { w, wTestCleanup := createWorkspaceWithVCS(t, client, orgTest, *options.createOptions) return w, func() { - defer orgTestCleanup() - defer wTestCleanup() + t.Cleanup(orgTestCleanup) + t.Cleanup(wTestCleanup) } }, assertion: func(w *Workspace, options *WorkspaceTableOptions, err error) { @@ -292,7 +292,7 @@ func TestWorkspacesCreateTableDriven(t *testing.T) { w, wTestCleanup := createWorkspaceWithVCS(t, client, orgTest, *options.createOptions) return w, func() { - defer wTestCleanup() + t.Cleanup(wTestCleanup) } }, assertion: func(w *Workspace, options *WorkspaceTableOptions, err error) { @@ -325,7 +325,7 @@ func TestWorkspacesCreate(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) t.Run("with valid options", func(t *testing.T) { options := WorkspaceCreateOptions{ @@ -457,7 +457,7 @@ func TestWorkspacesCreate(t *testing.T) { Name: String("tst-" + randomString(t)[0:20] + "-ff-on"), Email: String(fmt.Sprintf("%s@tfe.local", randomString(t))), }) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) options := WorkspaceCreateOptions{ Name: String("foobar"), @@ -500,7 +500,7 @@ func TestWorkspacesCreate(t *testing.T) { Name: String("tst-" + randomString(t)[0:20] + "-ff-on"), Email: String(fmt.Sprintf("%s@tfe.local", randomString(t))), }) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) options := WorkspaceCreateOptions{ Name: String(fmt.Sprintf("foobar-%s", randomString(t))), @@ -522,10 +522,10 @@ func TestWorkspacesRead(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) t.Run("when the workspace exists", func(t *testing.T) { w, err := client.Workspaces.Read(ctx, orgTest.Name, wTest.Name) @@ -570,13 +570,13 @@ func TestWorkspacesReadWithOptions(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) svTest, svTestCleanup := createStateVersion(t, client, 0, wTest) - defer svTestCleanup() + t.Cleanup(svTestCleanup) // give TFC some time to process the statefile and extract the outputs. waitForSVOutputs(t, client, svTest.ID) @@ -620,13 +620,13 @@ func TestWorkspacesReadWithHistory(t *testing.T) { client := testClient(t) orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) _, rCleanup := createRunApply(t, client, wTest) - defer rCleanup() + t.Cleanup(rCleanup) _, err := retry(func() (interface{}, error) { w, err := client.Workspaces.Read(context.Background(), orgTest.Name, wTest.Name) @@ -655,13 +655,13 @@ func TestWorkspacesReadReadme(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspaceWithVCS(t, client, orgTest, WorkspaceCreateOptions{}) - defer wTestCleanup() + t.Cleanup(wTestCleanup) _, rCleanup := createRunApply(t, client, wTest) - defer rCleanup() + t.Cleanup(rCleanup) t.Run("when the readme exists", func(t *testing.T) { w, err := client.Workspaces.Readme(ctx, wTest.ID) @@ -697,10 +697,10 @@ func TestWorkspacesReadByID(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) t.Run("when the workspace exists", func(t *testing.T) { w, err := client.Workspaces.ReadByID(ctx, wTest.ID) @@ -733,7 +733,7 @@ func TestWorkspacesUpdate(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) upgradeOrganizationSubscription(t, client, orgTest) @@ -852,7 +852,7 @@ func TestWorkspacesUpdate(t *testing.T) { Name: String("tst-" + randomString(t)[0:20] + "-ff-on"), Email: String(fmt.Sprintf("%s@tfe.local", randomString(t))), }) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wCleanup := createWorkspaceWithOptions(t, client, orgTest, WorkspaceCreateOptions{ Name: String(randomString(t)), @@ -901,7 +901,7 @@ func TestWorkspacesUpdate(t *testing.T) { Name: String("tst-" + randomString(t)[0:20] + "-ff-on"), Email: String(fmt.Sprintf("%s@tfe.local", randomString(t))), }) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wCleanup := createWorkspaceWithOptions(t, client, orgTest, WorkspaceCreateOptions{ Name: String(randomString(t)), @@ -940,7 +940,7 @@ func TestWorkspacesUpdateTableDriven(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wCleanup := createWorkspace(t, client, orgTest) t.Cleanup(wCleanup) @@ -969,8 +969,8 @@ func TestWorkspacesUpdateTableDriven(t *testing.T) { wTest, wTestCleanup := createWorkspaceWithVCS(t, client, orgTest, *options.createOptions) return wTest, func() { - defer orgTestCleanup() - defer wTestCleanup() + t.Cleanup(orgTestCleanup) + t.Cleanup(wTestCleanup) } }, assertion: func(workspace *Workspace, options *WorkspaceTableOptions, _ error) { @@ -1078,7 +1078,7 @@ func TestWorkspacesUpdateByID(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wCleanup := createWorkspace(t, client, orgTest) t.Cleanup(wCleanup) @@ -1166,10 +1166,10 @@ func TestWorkspacesDelete(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) - wTest, wCleanup := createWorkspace(t, client, orgTest) - t.Cleanup(wCleanup) + // ignore workspace cleanup b/c it will be destroyed during tests + wTest, _ := createWorkspace(t, client, orgTest) t.Run("with valid options", func(t *testing.T) { err := client.Workspaces.Delete(ctx, orgTest.Name, wTest.Name) @@ -1198,10 +1198,10 @@ func TestWorkspacesDeleteByID(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) - wTest, wCleanup := createWorkspace(t, client, orgTest) - t.Cleanup(wCleanup) + // ignore workspace cleanup b/c it will be destroyed during tests + wTest, _ := createWorkspace(t, client, orgTest) t.Run("with valid options", func(t *testing.T) { err := client.Workspaces.DeleteByID(ctx, wTest.ID) @@ -1248,8 +1248,8 @@ func TestWorkspacesSafeDelete(t *testing.T) { orgTest, orgTestCleanup := createOrganization(t, client) t.Cleanup(orgTestCleanup) - wTest, wCleanup := createWorkspace(t, client, orgTest) - t.Cleanup(wCleanup) + // ignore workspace cleanup b/c it will be destroyed during tests + wTest, _ := createWorkspace(t, client, orgTest) t.Run("with valid options", func(t *testing.T) { err := client.Workspaces.SafeDelete(ctx, orgTest.Name, wTest.Name) @@ -1274,8 +1274,8 @@ func TestWorkspacesSafeDelete(t *testing.T) { wTest, workspaceCleanup := createWorkspace(t, client, orgTest) t.Cleanup(workspaceCleanup) w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) - assert.NoError(t, err) - assert.True(t, w.Locked) + require.NoError(t, err) + require.True(t, w.Locked) err = client.Workspaces.SafeDelete(ctx, orgTest.Name, wTest.Name) assert.Contains(t, err.Error(), "conflict") @@ -1304,8 +1304,8 @@ func TestWorkspacesSafeDeleteByID(t *testing.T) { orgTest, orgTestCleanup := createOrganization(t, client) t.Cleanup(orgTestCleanup) - wTest, wCleanup := createWorkspace(t, client, orgTest) - t.Cleanup(wCleanup) + // ignore workspace cleanup b/c it will be destroyed during tests + wTest, _ := createWorkspace(t, client, orgTest) t.Run("with valid options", func(t *testing.T) { err := client.Workspaces.SafeDeleteByID(ctx, wTest.ID) @@ -1325,8 +1325,8 @@ func TestWorkspacesSafeDeleteByID(t *testing.T) { wTest, workspaceCleanup := createWorkspace(t, client, orgTest) t.Cleanup(workspaceCleanup) w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) - assert.NoError(t, err) - assert.True(t, w.Locked) + require.NoError(t, err) + require.True(t, w.Locked) err = client.Workspaces.SafeDeleteByID(ctx, wTest.ID) assert.Contains(t, err.Error(), "conflict") @@ -1353,10 +1353,10 @@ func TestWorkspacesRemoveVCSConnection(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspaceWithVCS(t, client, orgTest, WorkspaceCreateOptions{}) - defer wTestCleanup() + t.Cleanup(wTestCleanup) t.Run("remove vcs integration", func(t *testing.T) { w, err := client.Workspaces.RemoveVCSConnection(ctx, orgTest.Name, wTest.Name) @@ -1372,10 +1372,10 @@ func TestWorkspacesRemoveVCSConnectionByID(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspaceWithVCS(t, client, orgTest, WorkspaceCreateOptions{}) - defer wTestCleanup() + t.Cleanup(wTestCleanup) t.Run("remove vcs integration", func(t *testing.T) { w, err := client.Workspaces.RemoveVCSConnectionByID(ctx, wTest.ID) @@ -1391,10 +1391,10 @@ func TestWorkspacesLock(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) t.Run("with valid options", func(t *testing.T) { w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) @@ -1421,10 +1421,10 @@ func TestWorkspacesUnlock(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) if err != nil { @@ -1446,10 +1446,10 @@ func TestWorkspacesUnlock(t *testing.T) { t.Run("when a workspace is locked by a run", func(t *testing.T) { wTest2, wTest2Cleanup := createWorkspace(t, client, orgTest) - defer wTest2Cleanup() + t.Cleanup(wTest2Cleanup) _, rTestCleanup := createRun(t, client, wTest2) - defer rTestCleanup() + t.Cleanup(rTestCleanup) // Wait for wTest2 to be locked by a run waitForRunLock(t, client, wTest2.ID) @@ -1472,10 +1472,10 @@ func TestWorkspacesForceUnlock(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) w, err := client.Workspaces.Lock(ctx, wTest.ID, WorkspaceLockOptions{}) if err != nil { @@ -1509,13 +1509,13 @@ func TestWorkspacesAssignSSHKey(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) sshKeyTest, sshKeyTestCleanup := createSSHKey(t, client, orgTest) - defer sshKeyTestCleanup() + t.Cleanup(sshKeyTestCleanup) t.Run("with valid options", func(t *testing.T) { w, err := client.Workspaces.AssignSSHKey(ctx, wTest.ID, WorkspaceAssignSSHKeyOptions{ @@ -1556,13 +1556,13 @@ func TestWorkspacesUnassignSSHKey(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) sshKeyTest, sshKeyTestCleanup := createSSHKey(t, client, orgTest) - defer sshKeyTestCleanup() + t.Cleanup(sshKeyTestCleanup) w, err := client.Workspaces.AssignSSHKey(ctx, wTest.ID, WorkspaceAssignSSHKeyOptions{ SSHKeyID: String(sshKeyTest.ID), @@ -1594,10 +1594,10 @@ func TestWorkspaces_AddRemoteStateConsumers(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) // Update workspace to not allow global remote state options := WorkspaceUpdateOptions{ @@ -1608,9 +1608,9 @@ func TestWorkspaces_AddRemoteStateConsumers(t *testing.T) { t.Run("successfully adds a remote state consumer", func(t *testing.T) { wTestConsumer1, wTestCleanupConsumer1 := createWorkspace(t, client, orgTest) - defer wTestCleanupConsumer1() + t.Cleanup(wTestCleanupConsumer1) wTestConsumer2, wTestCleanupConsumer2 := createWorkspace(t, client, orgTest) - defer wTestCleanupConsumer2() + t.Cleanup(wTestCleanupConsumer2) err := client.Workspaces.AddRemoteStateConsumers(ctx, wTest.ID, WorkspaceAddRemoteStateConsumersOptions{ Workspaces: []*Workspace{wTestConsumer1, wTestConsumer2}, @@ -1653,10 +1653,10 @@ func TestWorkspaces_RemoveRemoteStateConsumers(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) // Update workspace to not allow global remote state options := WorkspaceUpdateOptions{ @@ -1667,9 +1667,9 @@ func TestWorkspaces_RemoveRemoteStateConsumers(t *testing.T) { t.Run("successfully removes a remote state consumer", func(t *testing.T) { wTestConsumer1, wTestCleanupConsumer1 := createWorkspace(t, client, orgTest) - defer wTestCleanupConsumer1() + t.Cleanup(wTestCleanupConsumer1) wTestConsumer2, wTestCleanupConsumer2 := createWorkspace(t, client, orgTest) - defer wTestCleanupConsumer2() + t.Cleanup(wTestCleanupConsumer2) err := client.Workspaces.AddRemoteStateConsumers(ctx, wTest.ID, WorkspaceAddRemoteStateConsumersOptions{ Workspaces: []*Workspace{wTestConsumer1, wTestConsumer2}, @@ -1731,10 +1731,10 @@ func TestWorkspaces_UpdateRemoteStateConsumers(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) // Update workspace to not allow global remote state options := WorkspaceUpdateOptions{ @@ -1745,9 +1745,9 @@ func TestWorkspaces_UpdateRemoteStateConsumers(t *testing.T) { t.Run("successfully updates a remote state consumer", func(t *testing.T) { wTestConsumer1, wTestCleanupConsumer1 := createWorkspace(t, client, orgTest) - defer wTestCleanupConsumer1() + t.Cleanup(wTestCleanupConsumer1) wTestConsumer2, wTestCleanupConsumer2 := createWorkspace(t, client, orgTest) - defer wTestCleanupConsumer2() + t.Cleanup(wTestCleanupConsumer2) err := client.Workspaces.AddRemoteStateConsumers(ctx, wTest.ID, WorkspaceAddRemoteStateConsumersOptions{ Workspaces: []*Workspace{wTestConsumer1}, @@ -1797,10 +1797,10 @@ func TestWorkspaces_AddTags(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) options := WorkspaceAddTagsOptions{ Tags: []*Tag{ @@ -1848,7 +1848,7 @@ func TestWorkspaces_AddTags(t *testing.T) { t.Run("successfully adds tags by id and name", func(t *testing.T) { wTest2, wTest2Cleanup := createWorkspace(t, client, orgTest) - defer wTest2Cleanup() + t.Cleanup(wTest2Cleanup) // add a tag to another workspace err := client.Workspaces.AddTags(ctx, wTest2.ID, WorkspaceAddTagsOptions{ @@ -1911,10 +1911,10 @@ func TestWorkspaces_RemoveTags(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) tags := []*Tag{ { @@ -2084,10 +2084,10 @@ func TestWorkspacesRunTasksPermission(t *testing.T) { ctx := context.Background() orgTest, orgTestCleanup := createOrganization(t, client) - defer orgTestCleanup() + t.Cleanup(orgTestCleanup) wTest, wTestCleanup := createWorkspace(t, client, orgTest) - defer wTestCleanup() + t.Cleanup(wTestCleanup) t.Run("when the workspace exists", func(t *testing.T) { w, err := client.Workspaces.Read(ctx, orgTest.Name, wTest.Name) From d86cb66d5384ae523b798fa27a2c20df2afc9358 Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Tue, 25 Oct 2022 16:01:06 -0400 Subject: [PATCH 09/10] Exit test early if workspace permissions nil to avoid panic --- workspace_integration_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/workspace_integration_test.go b/workspace_integration_test.go index e16b6aa77..1c2eee58e 100644 --- a/workspace_integration_test.go +++ b/workspace_integration_test.go @@ -1234,6 +1234,7 @@ func TestCanForceDeletePermission(t *testing.T) { w, err := client.Workspaces.ReadByID(ctx, wTest.ID) require.NoError(t, err) assert.Equal(t, wTest, w) + require.NotNil(t, w.Permissions) assert.NotNil(t, w.Permissions.CanForceDelete) assert.True(t, *w.Permissions.CanForceDelete) }) From f3a30d5dbe699d433ba1446d87cb1982d1d8d7b2 Mon Sep 17 00:00:00 2001 From: Jarrett Spiker Date: Wed, 26 Oct 2022 16:37:19 -0400 Subject: [PATCH 10/10] Prevent test panic --- workspace_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workspace_integration_test.go b/workspace_integration_test.go index 1c2eee58e..797db98d5 100644 --- a/workspace_integration_test.go +++ b/workspace_integration_test.go @@ -1235,7 +1235,7 @@ func TestCanForceDeletePermission(t *testing.T) { require.NoError(t, err) assert.Equal(t, wTest, w) require.NotNil(t, w.Permissions) - assert.NotNil(t, w.Permissions.CanForceDelete) + require.NotNil(t, w.Permissions.CanForceDelete) assert.True(t, *w.Permissions.CanForceDelete) }) }