Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix workspace panic #162

Merged
merged 4 commits into from
Apr 28, 2020
Merged

Fix workspace panic #162

merged 4 commits into from
Apr 28, 2020

Conversation

leetrout
Copy link
Contributor

@leetrout leetrout commented Apr 21, 2020

Description

As reported in #149 the provider will panic if a workspace that is managed by Terraform is deleted from the web app. This PR pulls in the fix from from @sean-nixon in #150.

I added a failing test case to confirm the behavior before cherry picking the fix so we can be sure it was addressed.

Testing plan

CircleCI ran both commits to demonstrate the failing and passing results:

  1. Failing TestAccTFEWorkspace_panic in build 99
  2. Passing in build 104

Manual testing

  1. Create and org and a workspace via Terraform with a build of this branch
  2. Delete the org in TFC/E
  3. Run apply again 🌮

dont-panic

Running tests

  1. Checkout first commit on this branch. I am piping the test command to panic parse
  2. Run the panic test TESTARGS="-run TestAccTFEWorkspace_panic" make testacc |& pp
  3. Clean up the missing org (See below)
  4. Checkout the tip of this branch
  5. Run again 🎉

Failing Output

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEWorkspace_panic -timeout 15m
?   	github.com/terraform-providers/terraform-provider-tfe	[no test files]
=== RUN   TestAccTFEWorkspace_panic
--- FAIL: TestAccTFEWorkspace_panic (4.02s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x60 pc=0x1be968d]

FAIL	github.com/terraform-providers/terraform-provider-tfe/tfe	4.574s
?   	github.com/terraform-providers/terraform-provider-tfe/version	[no test files]
make: *** [testacc] Error 1

To see all goroutines, visit https://github.com/maruel/panicparse#gotraceback

1: running [Created by testing.(*T).Run @ testing.go:916]
    testing  testing.go:830                     tRunner.func1(*T(#2))
             panic.go:522                       panic(uintptr(0x1d6dae0))
    tfe      resource_tfe_workspace.go:208      resourceTFEWorkspaceRead(*ResourceData(0xc00014a000), interface{}(0x1df2ec0), 0xc00050a5c0, 0x13)
    tfe      resource_tfe_workspace_test.go:445 testAccCheckTFEWorkspacePanic.func1(string(#1, len=0), 0x0)
    resource testing.go:883                     ComposeTestCheckFunc.func1(TestCheckFunc(#1), TestCheckFunc(0x0))
    resource testing_config.go:109              testStep(ContextOpts(0x0), *State(0x0), TestStep(0x0), 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    resource testing_config.go:27               testStepConfig(...)
    resource testing.go:543                     Test(TestT(0x21d6280), TestCase(0x0), 0xc00048be90, 0x0, 0x0, 0x1f4c6a8, 0xc000855ed0, 0x1, ...)
    tfe      resource_tfe_workspace_test.go:50  TestAccTFEWorkspace_panic(*T(#2))
    testing  testing.go:865                     tRunner(#2, 0x1f4c2f0)

Passing Output:

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEWorkspace_panic -timeout 15m
?   	github.com/terraform-providers/terraform-provider-tfe	[no test files]
=== RUN   TestAccTFEWorkspace_panic
--- PASS: TestAccTFEWorkspace_panic (7.98s)
PASS
ok  	github.com/terraform-providers/terraform-provider-tfe/tfe	(cached)
?   	github.com/terraform-providers/terraform-provider-tfe/version	[no test files]

Cleaning up the missing org

Send a DELETE for the tst-terraform org which will not be cleaned up due to the panic.

curl -H "Authorization: Bearer $TFE_TOKEN" \
-X DELETE \
https://${TFE_HOSTNAME}/api/v2/organizations/tst-terraform

Output from acceptance tests

$ make testacc

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -timeout 15m
?   	github.com/terraform-providers/terraform-provider-tfe	[no test files]
=== RUN   TestAccTFESSHKeyDataSource_basic
--- PASS: TestAccTFESSHKeyDataSource_basic (8.99s)
=== RUN   TestAccTFETeamAccessDataSource_basic
--- PASS: TestAccTFETeamAccessDataSource_basic (11.91s)
=== RUN   TestAccTFETeamDataSource_basic
--- PASS: TestAccTFETeamDataSource_basic (8.49s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_basic
--- PASS: TestAccTFEWorkspaceIDsDataSource_basic (9.88s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_wildcard
--- PASS: TestAccTFEWorkspaceIDsDataSource_wildcard (10.37s)
=== RUN   TestAccTFEWorkspaceDataSource_basic
--- PASS: TestAccTFEWorkspaceDataSource_basic (8.62s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestProvider_versionConstraints
--- PASS: TestProvider_versionConstraints (0.00s)
=== RUN   TestAccTFENotificationConfiguration_basic
--- PASS: TestAccTFENotificationConfiguration_basic (8.40s)
=== RUN   TestAccTFENotificationConfiguration_update
--- PASS: TestAccTFENotificationConfiguration_update (13.75s)
=== RUN   TestAccTFENotificationConfiguration_slackWithToken
--- PASS: TestAccTFENotificationConfiguration_slackWithToken (5.00s)
=== RUN   TestAccTFENotificationConfiguration_duplicateTriggers
--- PASS: TestAccTFENotificationConfiguration_duplicateTriggers (8.36s)
=== RUN   TestAccTFENotificationConfigurationImport
--- PASS: TestAccTFENotificationConfigurationImport (9.22s)
=== RUN   TestAccTFEOAuthClient_basic
--- PASS: TestAccTFEOAuthClient_basic (7.65s)
=== RUN   TestAccTFEOrganizationMembership_basic
--- PASS: TestAccTFEOrganizationMembership_basic (7.04s)
=== RUN   TestAccTFEOrganization_basic
--- PASS: TestAccTFEOrganization_basic (6.53s)
=== RUN   TestAccTFEOrganization_update
--- PASS: TestAccTFEOrganization_update (10.39s)
=== RUN   TestAccTFEOrganization_import
--- PASS: TestAccTFEOrganization_import (6.66s)
=== RUN   TestAccTFEOrganizationToken_basic
--- PASS: TestAccTFEOrganizationToken_basic (8.23s)
=== RUN   TestAccTFEOrganizationToken_existsWithoutForce
--- PASS: TestAccTFEOrganizationToken_existsWithoutForce (9.08s)
=== RUN   TestAccTFEOrganizationToken_existsWithForce
--- PASS: TestAccTFEOrganizationToken_existsWithForce (13.98s)
=== RUN   TestAccTFEOrganizationToken_import
--- PASS: TestAccTFEOrganizationToken_import (8.23s)
=== RUN   TestAccTFEPolicySetParameter_basic
--- PASS: TestAccTFEPolicySetParameter_basic (9.85s)
=== RUN   TestAccTFEPolicySetParameter_update
--- PASS: TestAccTFEPolicySetParameter_update (15.30s)
=== RUN   TestAccTFEPolicySetParameter_import
--- PASS: TestAccTFEPolicySetParameter_import (11.40s)
=== RUN   TestAccTFEPolicySet_basic
--- PASS: TestAccTFEPolicySet_basic (10.99s)
=== RUN   TestAccTFEPolicySet_update
--- PASS: TestAccTFEPolicySet_update (20.22s)
=== RUN   TestAccTFEPolicySet_updateEmpty
--- PASS: TestAccTFEPolicySet_updateEmpty (16.17s)
=== RUN   TestAccTFEPolicySet_updatePopulated
--- PASS: TestAccTFEPolicySet_updatePopulated (24.13s)
=== RUN   TestAccTFEPolicySet_updateToGlobal
--- PASS: TestAccTFEPolicySet_updateToGlobal (19.98s)
=== RUN   TestAccTFEPolicySet_updateToWorkspace
--- PASS: TestAccTFEPolicySet_updateToWorkspace (19.22s)
=== RUN   TestAccTFEPolicySet_vcs
--- SKIP: TestAccTFEPolicySet_vcs (0.42s)
    resource_tfe_policy_set_test.go:258: Please set GITHUB_POLICY_SET_IDENTIFIER to run this test
=== RUN   TestAccTFEPolicySetImport
--- FAIL: TestAccTFEPolicySetImport (31.01s)
    testing.go:569: Step 0 error: errors during apply:

        Error: Get <ngrok>/api/v2/ping: dial tcp <ip>:443: i/o timeout

          on <empty> line 0:
          (source code not available)


=== RUN   TestAccTFERunTrigger_basic
--- PASS: TestAccTFERunTrigger_basic (9.80s)
=== RUN   TestAccTFERunTriggerImport
--- PASS: TestAccTFERunTriggerImport (9.68s)
=== RUN   TestAccTFESentinelPolicy_basic
--- PASS: TestAccTFESentinelPolicy_basic (10.14s)
=== RUN   TestAccTFESentinelPolicy_update
--- PASS: TestAccTFESentinelPolicy_update (18.15s)
=== RUN   TestAccTFESentinelPolicy_import
--- PASS: TestAccTFESentinelPolicy_import (11.97s)
=== RUN   TestAccTFESSHKey_basic
--- PASS: TestAccTFESSHKey_basic (7.24s)
=== RUN   TestAccTFESSHKey_update
--- PASS: TestAccTFESSHKey_update (12.10s)
=== RUN   TestResourceTfeTeamAccessStateUpgradeV0
--- PASS: TestResourceTfeTeamAccessStateUpgradeV0 (0.22s)
=== RUN   TestAccTFETeamAccess_basic
--- PASS: TestAccTFETeamAccess_basic (10.06s)
=== RUN   TestAccTFETeamAccess_import
--- PASS: TestAccTFETeamAccess_import (10.50s)
=== RUN   TestPackTeamMemberID
--- PASS: TestPackTeamMemberID (0.00s)
=== RUN   TestUnpackTeamMemberID
--- PASS: TestUnpackTeamMemberID (0.00s)
=== RUN   TestAccTFETeamMember_basic
--- FAIL: TestAccTFETeamMember_basic (6.79s)
    testing.go:569: Step 0 error: errors during apply:

        Error: Error adding user "admin" to team team-o2265BiPdF6ELSfQ: bad request

        admin is not a member of the organization

          on /var/folders/m2/d8r3zfbn03lg3qr061dnsdvr0000gp/T/tf-test575165050/main.tf line 12:
          (source code not available)


=== RUN   TestAccTFETeamMember_import
--- FAIL: TestAccTFETeamMember_import (5.01s)
    testing.go:569: Step 0 error: errors during apply:

        Error: Error adding user "admin" to team team-vkF1sAknpjfTVhou: bad request

        admin is not a member of the organization

          on /var/folders/m2/d8r3zfbn03lg3qr061dnsdvr0000gp/T/tf-test988882876/main.tf line 12:
          (source code not available)


=== RUN   TestAccTFETeamMembers_basic
--- SKIP: TestAccTFETeamMembers_basic (0.42s)
    resource_tfe_team_members_test.go:22: Please set TFE_USER1 to run this test
=== RUN   TestAccTFETeamMembers_update
--- SKIP: TestAccTFETeamMembers_update (0.42s)
    resource_tfe_team_members_test.go:55: Please set TFE_USER1 to run this test
=== RUN   TestAccTFETeamMembers_import
--- SKIP: TestAccTFETeamMembers_import (0.43s)
    resource_tfe_team_members_test.go:102: Please set TFE_USER1 to run this test
=== RUN   TestPackTeamOrganizationMemberID
--- PASS: TestPackTeamOrganizationMemberID (0.00s)
=== RUN   TestUnpackTeamOrganizationMemberID
--- PASS: TestUnpackTeamOrganizationMemberID (0.00s)
=== RUN   TestAccTFETeamOrganizationMember_basic
--- PASS: TestAccTFETeamOrganizationMember_basic (9.52s)
=== RUN   TestAccTFETeamOrganizationMember_import
--- PASS: TestAccTFETeamOrganizationMember_import (9.30s)
=== RUN   TestAccTFETeam_basic
--- PASS: TestAccTFETeam_basic (8.17s)
=== RUN   TestAccTFETeam_import
--- PASS: TestAccTFETeam_import (10.95s)
=== RUN   TestAccTFETeamToken_basic
--- PASS: TestAccTFETeamToken_basic (12.58s)
=== RUN   TestAccTFETeamToken_existsWithoutForce
--- PASS: TestAccTFETeamToken_existsWithoutForce (15.89s)
=== RUN   TestAccTFETeamToken_existsWithForce
--- PASS: TestAccTFETeamToken_existsWithForce (22.20s)
=== RUN   TestAccTFETeamToken_import
--- PASS: TestAccTFETeamToken_import (13.07s)
=== RUN   TestResourceTfeVariableStateUpgradeV0
--- PASS: TestResourceTfeVariableStateUpgradeV0 (0.21s)
=== RUN   TestAccTFEVariable_basic
--- PASS: TestAccTFEVariable_basic (15.17s)
=== RUN   TestAccTFEVariable_update
--- PASS: TestAccTFEVariable_update (24.29s)
=== RUN   TestAccTFEVariable_import
--- PASS: TestAccTFEVariable_import (15.49s)
=== RUN   TestResourceTfeWorkspaceStateUpgradeV0
--- PASS: TestResourceTfeWorkspaceStateUpgradeV0 (0.00s)
=== RUN   TestAccTFEWorkspace_basic
--- PASS: TestAccTFEWorkspace_basic (10.90s)
=== RUN   TestAccTFEWorkspace_panic
--- PASS: TestAccTFEWorkspace_panic (9.65s)
=== RUN   TestAccTFEWorkspace_monorepo
--- PASS: TestAccTFEWorkspace_monorepo (10.34s)
=== RUN   TestAccTFEWorkspace_renamed
--- PASS: TestAccTFEWorkspace_renamed (14.44s)
=== RUN   TestAccTFEWorkspace_update
--- PASS: TestAccTFEWorkspace_update (17.31s)
=== RUN   TestAccTFEWorkspace_updateWorkingDirectory
--- PASS: TestAccTFEWorkspace_updateWorkingDirectory (23.32s)
=== RUN   TestAccTFEWorkspace_updateFileTriggers
--- PASS: TestAccTFEWorkspace_updateFileTriggers (16.79s)
=== RUN   TestAccTFEWorkspace_updateTriggerPrefixes
--- PASS: TestAccTFEWorkspace_updateTriggerPrefixes (16.62s)
=== RUN   TestAccTFEWorkspace_sshKey
--- PASS: TestAccTFEWorkspace_sshKey (25.42s)
=== RUN   TestAccTFEWorkspace_import
--- PASS: TestAccTFEWorkspace_import (10.95s)
=== RUN   TestFetchWorkspaceExternalID
=== RUN   TestFetchWorkspaceExternalID/non_exisiting_organization
=== RUN   TestFetchWorkspaceExternalID/non_exisiting_workspace
=== RUN   TestFetchWorkspaceExternalID/found_workspace
--- PASS: TestFetchWorkspaceExternalID (0.21s)
    --- PASS: TestFetchWorkspaceExternalID/non_exisiting_organization (0.00s)
    --- PASS: TestFetchWorkspaceExternalID/non_exisiting_workspace (0.00s)
    --- PASS: TestFetchWorkspaceExternalID/found_workspace (0.00s)
=== RUN   TestFetchWorkspaceHumanID
=== RUN   TestFetchWorkspaceHumanID/non_exisiting_workspace
=== RUN   TestFetchWorkspaceHumanID/found_workspace
--- PASS: TestFetchWorkspaceHumanID (0.00s)
    --- PASS: TestFetchWorkspaceHumanID/non_exisiting_workspace (0.00s)
    --- PASS: TestFetchWorkspaceHumanID/found_workspace (0.00s)
=== RUN   TestPackWorkspaceID
--- PASS: TestPackWorkspaceID (0.00s)
=== RUN   TestUnpackWorkspaceID
--- PASS: TestUnpackWorkspaceID (0.00s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-tfe/tfe	785.709s
?   	github.com/terraform-providers/terraform-provider-tfe/version	[no test files]
make: *** [testacc] Error 1

@ghost ghost added the size/M label Apr 21, 2020
@leetrout leetrout requested review from caseylang and lafentres April 21, 2020 03:08
Copy link

@caseylang caseylang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally with success!

Copy link
Collaborator

@lafentres lafentres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can't reproduce the panic but this code looks good, the other steps work, and it looks like how we handle this scenario for other resource reads so 🎉

@leetrout leetrout force-pushed the leetrout/fix-nil-panic branch from 5b9609b to b2a4975 Compare April 26, 2020 19:50
Example Output:
2020/04/26 15:48:27 [DEBUG] Workspace ws-CJygBQQU156fwXGF no longer exists
@leetrout leetrout force-pushed the leetrout/fix-nil-panic branch from b2a4975 to 9aedfa7 Compare April 26, 2020 19:52
@leetrout leetrout force-pushed the leetrout/fix-nil-panic branch from 57ad44f to 77addab Compare April 27, 2020 16:28
@leetrout leetrout merged commit 357645d into master Apr 28, 2020
@leetrout leetrout deleted the leetrout/fix-nil-panic branch April 28, 2020 15:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants