Add Git tags support #549

hashimoon · 2022-07-12T19:24:27Z

Description

Adding support to Git Tags

Testing plan

Use resource like the following:

locals {
  organization = "..."
  oauth_token_id = "..."
  identifier = "user/repo
}

resource "tfe_workspace" "my-workspace" {
  name                  = "my-workspace"
  organization          = local.organization
  allow_destroy_plan    = true
  auto_apply            = true
  execution_mode        = "remote"
  speculative_enabled   = true
  file_triggers_enabled = false

  vcs_repo {
    identifier         = local.identifier
    ingress_submodules = false
    oauth_token_id = local.oauth_token_id
    tags_regex = "^\\d+.\\d+.\\d+$" # semantic versioning
  }
}

Verify the correct settings were set in TF Cloud/TFE VCS settings for the workspace that git tags semantic versioning is selected
- Release a new tagged release in the repo specified in local.identifier
- See a new run has been trigged
Change configuration to the following

locals {
  organization = "..." # set
  oauth_token_id = "..." # set
  identifier = "user/repo
  working_directory = "..." # set
}

resource "tfe_workspace" "my-workspace" {
  name                  = "my-workspace"
  organization          = local.organization
  allow_destroy_plan    = true
  auto_apply            = true
  execution_mode        = "remote"
  speculative_enabled   = true
  file_triggers_enabled = true
  working_directory = local.working_directory

  vcs_repo {
    identifier         = local.identifier
    ingress_submodules = false
    oauth_token_id = local.oauth_token_id
  }
}

Terraform plan/apply succeeds
Verify in TF Cloud/TFE in VCS settings git tags is not selected.

External links

Include any links here that might be helpful for people reviewing your PR. If there are none, feel free to delete this section.

Output from acceptance tests

Please run applicable acceptance tests locally and include the output here. See TESTS.md to learn how to run acceptance tests.

If you are an external contributor, your contribution(s) will first be reviewed before running them against the project's CI pipeline.

$ TESTARGS="-run TestAccTFEWorkspace" make testacc

...

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1202370402785697

tfe/data_source_workspace.go

tfe/resource_tfe_policy_set_test.go

tfe/resource_tfe_workspace_test.go

website/docs/r/policy_set.html.markdown

omarismail

Overall looks good, but we should add a number of tests for this

tfe/resource_tfe_workspace.go

tfe/resource_tfe_workspace_migrate.go

omarismail

great addition of tests, just one more change needed to use ConflictsWith to keep consistent with the API

omarismail

lgtm! I think there are some test failures tho

hashimoon · 2022-07-15T15:00:06Z

great addition of tests, just one more change needed to use ConflictsWith to keep consistent with the API

Is ConflictsWith going to be applicable if we need to set the other values as nil/[]? @omarismail

brandonc · 2022-07-18T19:58:51Z

@omarismail @hashimoon Do you think it would be best to land this after the beta flag is removed and the feature is GA?

matejrisek · 2022-07-19T07:35:56Z

tfe/resource_tfe_workspace_migrate.go

@@ -95,6 +95,12 @@ func resourceTfeWorkspaceResourceV0() *schema.Resource {
 							Type:     schema.TypeString,
 							Required: true,
 						},
+
+						"tags_regex": {


learning: What do we actually migrate here? I would argue that v0 of workspace didn't have tags_regex. I might be wrong about the purpose of migrations in the tfe provider.

Yeah I don't think we need this. This is called from here and its about state migration. We don't have tags_regex for in state, so no need to migrate anything.

omarismail · 2022-07-27T14:52:58Z

tfe/resource_tfe_workspace_test.go

+			},
+		},
+	})
+}


We should add another test where we start a new config without tags_regex, then add it, then delete it

omarismail · 2022-07-27T14:53:25Z

website/docs/r/workspace.html.markdown

@@ -124,6 +124,7 @@ The `vcs_repo` block supports:
  cloning the VCS repository. Defaults to `false`.
 * `oauth_token_id` - (Required) The VCS Connection (OAuth Connection + Token) to use.
  This ID can be obtained from a `tfe_oauth_client` resource.
+* `tags_regex` - (Optional) A regular expression used to trigger a Workspace run for matching Git tags.


We should add that this conflicts with trigger_patterns and trigger_prefixes

omarismail

A couple comments regarding:

Adding another test
removing the migration
updating docs

matejrisek · 2022-08-01T09:18:46Z

tfe/resource_tfe_workspace_test.go

+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheck(t)
+			if GITHUB_TOKEN == "" {


This is third time these checks are repeated in this file, IMHO it warrants method extraction

…it tags

mjyocca · 2022-08-10T21:32:31Z

Updating go-tfe package with pr fixes the current broken test

omarismail

lgtm!

brandonc · 2022-08-11T20:34:19Z

I re-ran the tests in SSH so I could extract the GITHUB_WORKSPACE config and cancelling it caused the CI failure. I will straighten it out!

brandonc

Works well!

I had some trouble removing the entire vcs_repo config. This did not seem to delete the ingress trigger from the workspace, so tags_regex continues to trigger conflicts with trigger_prefixes etc. I think this problem is not specific to git_tags and probably is a separate provider bug.

hashimoon force-pushed the hashimoon/git-tags branch 3 times, most recently from c238c0b to e108cd9 Compare July 12, 2022 19:39

hashimoon marked this pull request as ready for review July 12, 2022 19:45

hashimoon force-pushed the hashimoon/git-tags branch from e108cd9 to 074c706 Compare July 13, 2022 05:17

omarismail reviewed Jul 13, 2022

View reviewed changes

tfe/data_source_workspace.go Show resolved Hide resolved

omarismail reviewed Jul 13, 2022

View reviewed changes

tfe/resource_tfe_policy_set_test.go Outdated Show resolved Hide resolved

omarismail reviewed Jul 13, 2022

View reviewed changes

tfe/resource_tfe_workspace_test.go Outdated Show resolved Hide resolved

omarismail reviewed Jul 13, 2022

View reviewed changes

website/docs/r/policy_set.html.markdown Outdated Show resolved Hide resolved

omarismail requested changes Jul 13, 2022

View reviewed changes

hashimoon force-pushed the hashimoon/git-tags branch 3 times, most recently from e75a1fb to ac02684 Compare July 14, 2022 00:11

omarismail reviewed Jul 14, 2022

View reviewed changes

tfe/resource_tfe_workspace.go Outdated Show resolved Hide resolved

omarismail reviewed Jul 14, 2022

View reviewed changes

tfe/resource_tfe_workspace_migrate.go Outdated Show resolved Hide resolved

omarismail requested changes Jul 14, 2022

View reviewed changes

hashimoon force-pushed the hashimoon/git-tags branch 2 times, most recently from 0a4a33a to e209606 Compare July 14, 2022 18:49

omarismail approved these changes Jul 14, 2022

View reviewed changes

hashimoon force-pushed the hashimoon/git-tags branch 7 times, most recently from 3c74d85 to c9c5f88 Compare July 16, 2022 02:29

hashimoon requested a review from a team as a code owner July 18, 2022 18:42

hashimoon force-pushed the hashimoon/git-tags branch from 73363f0 to 5903494 Compare July 19, 2022 01:31

hashimoon force-pushed the hashimoon/git-tags branch 2 times, most recently from 75a1f96 to d55c76c Compare July 19, 2022 02:08

matejrisek reviewed Jul 19, 2022

View reviewed changes

hashimoon marked this pull request as draft July 20, 2022 18:21

omarismail reviewed Jul 27, 2022

View reviewed changes

omarismail requested changes Jul 27, 2022

View reviewed changes

matejrisek reviewed Aug 1, 2022

View reviewed changes

mjyocca and others added 5 commits August 6, 2022 01:25

Add support to allow for workspace runs to be triggered by matching G…

8588bb0

…it tags

Revert changes for setting TagsRegex using a PolicySet

8b346fa

Remove tags_regex from schema migration map

8870ada

update tags_regex description for workspace docs

b613ebd

add test for switching from tagsregex to trigger pattern

93b745a

mjyocca force-pushed the hashimoon/git-tags branch from d55c76c to 93b745a Compare August 10, 2022 19:21

mjyocca marked this pull request as ready for review August 10, 2022 19:22

mjyocca requested a review from omarismail August 10, 2022 19:22

mjyocca added 2 commits August 10, 2022 15:33

update go-tfe package version to v1.7.0

fadf866

extract github env variable check into individual method

2cef6da

omarismail approved these changes Aug 11, 2022

View reviewed changes

mjyocca requested a review from brandonc August 11, 2022 18:15

update changelog with git tags entry

0dfb813

brandonc approved these changes Aug 11, 2022

View reviewed changes

mjyocca merged commit 60ae826 into main Aug 11, 2022

mjyocca deleted the hashimoon/git-tags branch August 11, 2022 21:19

mjyocca mentioned this pull request Aug 15, 2022

Fix issue with removing vcs_repo config from workspace with tags_regex #594

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Git tags support #549

Add Git tags support #549

hashimoon commented Jul 12, 2022 •

edited by mjyocca

omarismail left a comment

omarismail left a comment

omarismail left a comment

hashimoon commented Jul 15, 2022

brandonc commented Jul 18, 2022

matejrisek Jul 19, 2022

omarismail Jul 27, 2022

omarismail Jul 27, 2022

omarismail Jul 27, 2022

omarismail left a comment

matejrisek Aug 1, 2022

mjyocca commented Aug 10, 2022

omarismail left a comment

brandonc commented Aug 11, 2022

brandonc left a comment

Add Git tags support #549

Add Git tags support #549

Conversation

hashimoon commented Jul 12, 2022 • edited by mjyocca

Description

Testing plan

External links

Output from acceptance tests

omarismail left a comment

Choose a reason for hiding this comment

omarismail left a comment

Choose a reason for hiding this comment

omarismail left a comment

Choose a reason for hiding this comment

hashimoon commented Jul 15, 2022

brandonc commented Jul 18, 2022

matejrisek Jul 19, 2022

Choose a reason for hiding this comment

omarismail Jul 27, 2022

Choose a reason for hiding this comment

omarismail Jul 27, 2022

Choose a reason for hiding this comment

omarismail Jul 27, 2022

Choose a reason for hiding this comment

omarismail left a comment

Choose a reason for hiding this comment

matejrisek Aug 1, 2022

Choose a reason for hiding this comment

mjyocca commented Aug 10, 2022

omarismail left a comment

Choose a reason for hiding this comment

brandonc commented Aug 11, 2022

brandonc left a comment

Choose a reason for hiding this comment

hashimoon commented Jul 12, 2022 •

edited by mjyocca