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

Updated the resourceTFEPolicySetUpdate function #185

Merged

Conversation

hcrhall
Copy link
Contributor

@hcrhall hcrhall commented Jun 14, 2020

Description

This PR contains a change to the resourceTFEPolicySetUpdate function within the tfe_policy_set resource that detects changes to the vcs_repo attributes. The issue is covered in more detail in the following:

Policy vcs_repo can't be updated #140

Testing plan

  1. Create a main.tf:
provider "tfe" {
  version  = "0.18.0"
}

resource "tfe_policy_set" "test" {
  name          = "example_tfe_policy_set"
  description   = "example policy set"
  organization  = "my_org_name"

  vcs_repo {
    identifier         = "github_account/repo_name"
    branch             = "master"
    ingress_submodules = false
    oauth_token_id     = "token_id"
  }
}
  1. Monitor a run and let provisioning occur
  2. Update main.tf:
provider "tfe" {
  version  = "0.18.0"
}

resource "tfe_policy_set" "test" {
  name          = "example_tfe_policy_set"
  description   = "example policy set"
  organization  = "my_org_name"

  vcs_repo {
    identifier         = "github_account/repo_name"
    branch             = "my_other_branch_name" # <= updated property 
    ingress_submodules = false
    oauth_token_id     = "token_id"
  }
}
  1. Monitor the run and check Policy Set in Terraform UI and confirm that branch name did change in alignment with the configuration change.

Output from acceptance tests

PASS tfe.TestAccTFEPolicySetParameter_basic (2.26s)
PASS tfe.TestAccTFEPolicySetParameter_update (3.42s)
PASS tfe.TestAccTFEPolicySetParameter_import (2.28s)
PASS tfe.TestAccTFEPolicySet_basic (2.38s)
PASS tfe.TestAccTFEPolicySet_update (4.08s)
PASS tfe.TestAccTFEPolicySet_updateWorkspaceIDs (5.31s)
PASS tfe.TestAccTFEPolicySet_updateEmpty (3.18s)
PASS tfe.TestAccTFEPolicySet_updatePopulated (4.40s)
PASS tfe.TestAccTFEPolicySet_updatePopulatedWorkspaceIDs (4.51s)
PASS tfe.TestAccTFEPolicySet_updateToGlobal (3.69s)
PASS tfe.TestAccTFEPolicySet_updateWorkspaceIDsToGlobal (3.75s)
PASS tfe.TestAccTFEPolicySet_updateToWorkspace (3.86s)
PASS tfe.TestAccTFEPolicySet_updateGlobalToWorkspaceIDs (3.72s)
PASS tfe.TestAccTFEPolicySet_vcs (4.02s)
PASS tfe.TestAccTFEPolicySet_updateToVcs (6.17s)
PASS tfe.TestAccTFEPolicySetImport (2.40s)
...

@hcrhall hcrhall added the bug label Jun 14, 2020
@hcrhall hcrhall self-assigned this Jun 14, 2020
@ghost ghost added the size/M label Jun 14, 2020
@hcrhall hcrhall marked this pull request as ready for review June 15, 2020 01:10
@@ -103,6 +103,7 @@ var GITHUB_WORKSPACE_IDENTIFIER = os.Getenv("GITHUB_WORKSPACE_IDENTIFIER")
var GITHUB_WORKSPACE_BRANCH = os.Getenv("GITHUB_WORKSPACE_BRANCH")
var GITHUB_POLICY_SET_IDENTIFIER = os.Getenv("GITHUB_POLICY_SET_IDENTIFIER")
var GITHUB_POLICY_SET_BRANCH = os.Getenv("GITHUB_POLICY_SET_BRANCH")
var GITHUB_POLICY_SET_ALT_BRANCH = os.Getenv("GITHUB_POLICY_SET_ALT_BRANCH")
Copy link
Contributor

@lafentres lafentres Jun 15, 2020

Choose a reason for hiding this comment

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

would it be possible to avoid introducing another env var here? there are getting unwieldy. i'd love to find an alternative, if there is one, but it is pretty low priority.

the tfe_workspace tests also have a vcs repo and test going from master to GITHUB_WORKSPACE_BRANCH. is that something that would be possible to do here as well, to avoid the need for another env var?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lafentres I've made a few changes to the vcs related tests so as to move away from our reliance on env vars. Take a look and let me know what you think.

Copy link
Contributor

@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.

This looks good to me! I tested migrating from the old version to this new version, updating various VCS repo fields, and removing the VCS repo. Everything worked as expected.

@hcrhall hcrhall merged commit da73b3e into master Jun 16, 2020
@hcrhall hcrhall deleted the hcrhall/update_tfe_policy_set_resource_to_update_vcs_branch branch June 16, 2020 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants