Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Workspace working directory updates #137

Merged

Conversation

etopeter
Copy link
Contributor

@etopeter etopeter commented Feb 20, 2020

Description

Added support for changing working directory for existing workspaces.

Testing plan

If you create workspace with defined working directory and after apply you try to change it, provider will show there are no changes to be done. (working directory is computed value)
Issue reference #136

Output from acceptance tests

Changes were made to test TestAccTFEWorkspace_update

$ 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 (11.54s)
=== RUN   TestAccTFETeamAccessDataSource_basic
--- FAIL: TestAccTFETeamAccessDataSource_basic (9.51s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test-1002912244890962745 for organization tst-terraform-1002912244890962745: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test080338603/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeamDataSource_basic
--- FAIL: TestAccTFETeamDataSource_basic (3.82s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test-4515755384739009815 for organization tst-terraform-4515755384739009815: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test354555413/main.tf line 7:
          (source code not available)
        
        
    testing.go:630: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.
        
        Error: errors during refresh: Could not find team tst-terraform-4515755384739009815/team-test-4515755384739009815
        
        State: <nil>
=== RUN   TestAccTFEWorkspaceIDsDataSource_basic
--- PASS: TestAccTFEWorkspaceIDsDataSource_basic (13.18s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_wildcard
--- PASS: TestAccTFEWorkspaceIDsDataSource_wildcard (12.89s)
=== RUN   TestAccTFEWorkspaceDataSource_basic
--- PASS: TestAccTFEWorkspaceDataSource_basic (11.96s)
=== 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 (10.07s)
=== RUN   TestAccTFENotificationConfiguration_update
--- PASS: TestAccTFENotificationConfiguration_update (18.30s)
=== RUN   TestAccTFENotificationConfiguration_slackWithToken
--- PASS: TestAccTFENotificationConfiguration_slackWithToken (6.84s)
=== RUN   TestAccTFENotificationConfiguration_duplicateTriggers
--- PASS: TestAccTFENotificationConfiguration_duplicateTriggers (11.22s)
=== RUN   TestAccTFENotificationConfigurationImport
--- PASS: TestAccTFENotificationConfigurationImport (9.99s)
=== RUN   TestAccTFEOAuthClient_basic
--- SKIP: TestAccTFEOAuthClient_basic (0.76s)
    resource_tfe_oauth_client_test.go:19: Please set GITHUB_TOKEN to run this test
=== RUN   TestAccTFEOrganization_basic
--- PASS: TestAccTFEOrganization_basic (6.79s)
=== RUN   TestAccTFEOrganization_update
--- PASS: TestAccTFEOrganization_update (12.73s)
=== RUN   TestAccTFEOrganization_import
--- PASS: TestAccTFEOrganization_import (9.41s)
=== RUN   TestAccTFEOrganizationToken_basic
--- PASS: TestAccTFEOrganizationToken_basic (9.36s)
=== RUN   TestAccTFEOrganizationToken_existsWithoutForce
--- PASS: TestAccTFEOrganizationToken_existsWithoutForce (12.67s)
=== RUN   TestAccTFEOrganizationToken_existsWithForce
--- PASS: TestAccTFEOrganizationToken_existsWithForce (17.21s)
=== RUN   TestAccTFEOrganizationToken_import
--- PASS: TestAccTFEOrganizationToken_import (9.45s)
=== RUN   TestAccTFEPolicySetParameter_basic
--- FAIL: TestAccTFEPolicySetParameter_basic (4.88s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating policy set policy-set-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test986548502/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySetParameter_update
--- FAIL: TestAccTFEPolicySetParameter_update (6.70s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating policy set policy-set-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test383445688/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySetParameter_import
--- FAIL: TestAccTFEPolicySetParameter_import (3.78s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating policy set policy-set-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test933158570/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySet_basic
--- FAIL: TestAccTFEPolicySet_basic (3.90s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-foo for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test411482476/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySet_update
--- FAIL: TestAccTFEPolicySet_update (5.28s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-foo for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test048005502/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySet_updateEmpty
--- FAIL: TestAccTFEPolicySet_updateEmpty (6.13s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-foo for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test653186912/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySet_updatePopulated
--- FAIL: TestAccTFEPolicySet_updatePopulated (6.18s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-foo for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test072995730/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySet_updateToGlobal
--- FAIL: TestAccTFEPolicySet_updateToGlobal (7.83s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-foo for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test374378644/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySet_updateToWorkspace
--- FAIL: TestAccTFEPolicySet_updateToWorkspace (8.11s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-foo for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test211559654/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEPolicySet_vcs
--- SKIP: TestAccTFEPolicySet_vcs (0.38s)
    resource_tfe_policy_set_test.go:255: Please set GITHUB_TOKEN to run this test
=== RUN   TestAccTFEPolicySetImport
--- FAIL: TestAccTFEPolicySetImport (9.63s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-foo for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test945578760/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFERunTrigger_basic
--- PASS: TestAccTFERunTrigger_basic (11.39s)
=== RUN   TestAccTFERunTriggerImport
--- PASS: TestAccTFERunTriggerImport (10.88s)
=== RUN   TestAccTFESentinelPolicy_basic
--- FAIL: TestAccTFESentinelPolicy_basic (7.99s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test505657429/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFESentinelPolicy_update
--- FAIL: TestAccTFESentinelPolicy_update (4.33s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test173030735/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFESentinelPolicy_import
--- FAIL: TestAccTFESentinelPolicy_import (6.46s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating sentinel policy policy-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test364277337/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFESSHKey_basic
--- PASS: TestAccTFESSHKey_basic (9.17s)
=== RUN   TestAccTFESSHKey_update
--- PASS: TestAccTFESSHKey_update (15.64s)
=== RUN   TestAccTFETeamAccess_basic
--- FAIL: TestAccTFETeamAccess_basic (7.75s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test621783626/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeamAccess_import
--- FAIL: TestAccTFETeamAccess_import (9.10s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test576932876/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestPackTeamMemberID
--- PASS: TestPackTeamMemberID (0.00s)
=== RUN   TestUnpackTeamMemberID
--- PASS: TestUnpackTeamMemberID (0.00s)
=== RUN   TestAccTFETeamMember_basic
--- FAIL: TestAccTFETeamMember_basic (5.23s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test026215198/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeamMember_import
--- FAIL: TestAccTFETeamMember_import (3.73s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test999731200/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeamMembers_basic
--- SKIP: TestAccTFETeamMembers_basic (0.38s)
    resource_tfe_team_members_test.go:22: Please set TFE_USER1 to run this test
=== RUN   TestAccTFETeamMembers_update
--- SKIP: TestAccTFETeamMembers_update (0.38s)
    resource_tfe_team_members_test.go:55: Please set TFE_USER1 to run this test
=== RUN   TestAccTFETeamMembers_import
--- SKIP: TestAccTFETeamMembers_import (1.39s)
    resource_tfe_team_members_test.go:102: Please set TFE_USER1 to run this test
=== RUN   TestAccTFETeam_basic
--- FAIL: TestAccTFETeam_basic (4.16s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test261097778/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeam_import
--- FAIL: TestAccTFETeam_import (4.55s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test325023540/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeamToken_basic
--- FAIL: TestAccTFETeamToken_basic (4.97s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test230234758/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeamToken_existsWithoutForce
--- FAIL: TestAccTFETeamToken_existsWithoutForce (6.57s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test710004648/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeamToken_existsWithForce
--- FAIL: TestAccTFETeamToken_existsWithForce (7.37s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test684923162/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFETeamToken_import
--- FAIL: TestAccTFETeamToken_import (4.94s)
    testing.go:569: Step 0 error: errors during apply:
        
        Error: Error creating team team-test for organization tst-terraform: resource not found
        
          on /var/folders/cd/hq9glwbs4b72hkwl8kv2k2_jjp2x5y/T/tf-test055901532/main.tf line 7:
          (source code not available)
        
        
=== RUN   TestAccTFEVariable_basic
--- PASS: TestAccTFEVariable_basic (12.90s)
=== RUN   TestAccTFEVariable_update
--- PASS: TestAccTFEVariable_update (17.74s)
=== RUN   TestAccTFEVariable_import
--- PASS: TestAccTFEVariable_import (12.13s)
=== RUN   TestPackWorkspaceID
--- PASS: TestPackWorkspaceID (0.00s)
=== RUN   TestUnpackWorkspaceID
--- PASS: TestUnpackWorkspaceID (0.00s)
=== RUN   TestAccTFEWorkspace_basic
--- PASS: TestAccTFEWorkspace_basic (8.61s)
=== RUN   TestAccTFEWorkspace_monorepo
--- PASS: TestAccTFEWorkspace_monorepo (9.07s)
=== RUN   TestAccTFEWorkspace_renamed
--- PASS: TestAccTFEWorkspace_renamed (14.18s)
=== RUN   TestAccTFEWorkspace_update
--- PASS: TestAccTFEWorkspace_update (17.63s)
=== RUN   TestAccTFEWorkspace_updateFileTriggers
--- PASS: TestAccTFEWorkspace_updateFileTriggers (13.18s)
=== RUN   TestAccTFEWorkspace_sshKey
--- PASS: TestAccTFEWorkspace_sshKey (21.53s)
=== RUN   TestAccTFEWorkspace_import
--- PASS: TestAccTFEWorkspace_import (8.97s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-tfe/tfe	523.026s
?   	github.com/terraform-providers/terraform-provider-tfe/version	[no test files]
FAIL
make: *** [testacc] Error 1
...

@ghost ghost added the size/M label Feb 20, 2020
Copy link
Contributor

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

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

Looks good, but it does need a small change before we can merge it.

tfe/resource_tfe_workspace_test.go Outdated Show resolved Hide resolved
tfe/resource_tfe_workspace_test.go Outdated Show resolved Hide resolved
tfe/resource_tfe_workspace.go Show resolved Hide resolved
@etopeter
Copy link
Contributor Author

Output of new added dedicated test for working directory update:

TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEWorkspace_updateWorkingDirectory -timeout 15m
?       github.com/terraform-providers/terraform-provider-tfe   [no test files]
=== RUN   TestAccTFEWorkspace_updateWorkingDirectory
--- PASS: TestAccTFEWorkspace_updateWorkingDirectory (21.29s)
PASS
ok      github.com/terraform-providers/terraform-provider-tfe/tfe       21.501s
?       github.com/terraform-providers/terraform-provider-tfe/version   [no test files]

Reverted my changes for fix of TestAccTFEWorkspace_update. Now its failing like before:

TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEWorkspace_update -timeout 15m
?       github.com/terraform-providers/terraform-provider-tfe   [no test files]
=== RUN   TestAccTFEWorkspace_update
--- FAIL: TestAccTFEWorkspace_update (20.03s)
    testing.go:569: Step 1 error: Check failed: Check 2/12 error: Bad operations: true
=== RUN   TestAccTFEWorkspace_updateWorkingDirectory
--- PASS: TestAccTFEWorkspace_updateWorkingDirectory (22.04s)
=== RUN   TestAccTFEWorkspace_updateFileTriggers
--- PASS: TestAccTFEWorkspace_updateFileTriggers (13.70s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-tfe/tfe       57.188s
?       github.com/terraform-providers/terraform-provider-tfe/version   [no test files]
FAIL
make: *** [testacc] Error 1```

svanharmelen
svanharmelen previously approved these changes Feb 26, 2020
Copy link
Contributor

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

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

Nice @etopeter! But it seems this introduces some backwards incompatible behavior...

@svanharmelen svanharmelen dismissed their stale review February 26, 2020 18:50

Thought about backwards compatibility

@svanharmelen
Copy link
Contributor

I just thought about the possible impact for existing users of this provider and I think this change introduces a backwards incompatibility and/or risk.

If you created workspaces previously without setting a workspace in your TF config, and over time you manually (or by any other means, but not using TF) updated your working directories they will now all be reset to the root directory.

Not sure what would be a good mitigation, but good to think about this before merging this PR.

@etopeter
Copy link
Contributor Author

@svanharmelen The only way I can think of to have this backward compatible is to change attribute name and support both. Use of one would exclude another. We could name it working_directory and terraform_working_directory. I'm not a big fan of this approach as it's confusing for users and complicates more code.

In my opinion current behavior of plugin is buggy. You can set working directory but you can't change it to root once it was set to something. You always have to have non null value for changes to be detected.

Why I think we should use proposed changes:
Arguably if this feature was pushed out as is in this PR, users who have custom working directory set manually will see changes in plan. It only affect users who created workspaces without specifying working_directory attribute and then set the value in GUI after creating resources. I think the number of users with this configuration is low. I think most users who customize working directory is using Terraform to control that.

What could happen?
Worst case scenario is If someone have automation around managing workspaces and applies plan blindly, it would change working directory to root and thus break those workspaces. Its possible that someone have different Terraform files in root folder than inside subdirectory and if that auto applies it could have catastrophic consequences.
On the other hand best practice for anyone who auto apply plan is to lock all modules as well as all provider versions. In such event it would be fault of person who made automation without implementing safeguards. Same concerns are around not only this provider but any other peace of code that is being auto applied without version lock.

@svanharmelen
Copy link
Contributor

@etopeter I fully agree and also don't think it will have a big impact, yet we need to be careful with backwards incompatible changes of course.

@radeksimko @beekus could you maybe step in here? Not sure who currently has ownership/responsibility of this provider within HashiCorp, but I guess they should give their input on this one.

@nicsnet nicsnet requested a review from a team March 3, 2020 18:14
@caseylang
Copy link

Hey @svanharmelen and @etopeter

Thanks for your work on this! @nicsnet happens to be working on a very similar issue related to trigger prefixes for workspaces. These two PRs together fill in some gaps we've had in the provider nicely.

As these two PRs are merged, we'll be sure to call out in the changelog the important note that "if you have workspaces which are configured through the TFE provider, but have set the working directory or trigger prefixes manually, through the UI, you'll need to update your configuration". In the case where the user does not update their configuration after pulling in this provider change, they'll still be informed of the drift when they run the Plan step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants