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.

Sign up for GitHub

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

Jump to bottom

Allow FileTriggersEnabled to be set to false when Git tags are present #468

Merged
merged 4 commits into from Aug 10, 2022
Merged

Allow FileTriggersEnabled to be set to false when Git tags are present #468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cfdc0ea
Allow FileTriggersEnabled to be set to `false` when Git tags are present
hashimoon Jul 16, 2022
09fe6a7
Bring tags-regex feature out of beta
hashimoon Jul 19, 2022
9f796df
add commit author to changelog
mjyocca Aug 9, 2022
6e9d327
add TagsRegex defined helper
mjyocca Aug 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@
* Adds additional Task Stage and Run Statuses for Pre-plan run tasks by @glennsarti [#469](https://github.com/hashicorp/go-tfe/pull/469)
* Adds `stage` field to the create and update methods for Workspace Run Tasks by @glennsarti [#469](https://github.com/hashicorp/go-tfe/pull/469)
* Adds `ResourcesProcessed`, `StateVersion`, `TerraformVersion`, `Modules`, `Providers`, and `Resources` fields to the State Version struct by @laurenolivia [#484](https://github.com/hashicorp/go-tfe/pull/484)
* Add support for disabled file trigger runs when setting Git tags by @hashimoon [#468](https://github.com/hashicorp/go-tfe/pull/468)

# v1.6.0

Expand Down
22 changes: 16 additions & 6 deletions workspace.go
View file
Open in desktop
Expand Up @@ -1072,15 +1072,15 @@ func (o WorkspaceCreateOptions) valid() error {
o.TriggerPatterns != nil && len(o.TriggerPatterns) > 0 {
return ErrUnsupportedBothTriggerPatternsAndPrefixes
}
if o.VCSRepo != nil && o.VCSRepo.TagsRegex != nil &&
if tagRegexDefined(o.VCSRepo) &&
o.TriggerPatterns != nil && len(o.TriggerPatterns) > 0 {
return ErrUnsupportedBothTagsRegexAndTriggerPatterns
}
if o.VCSRepo != nil && o.VCSRepo.TagsRegex != nil &&
if tagRegexDefined(o.VCSRepo) &&
o.TriggerPrefixes != nil && len(o.TriggerPrefixes) > 0 {
return ErrUnsupportedBothTagsRegexAndTriggerPrefixes
}
if o.VCSRepo != nil && o.VCSRepo.TagsRegex != nil &&
if tagRegexDefined(o.VCSRepo) &&
o.FileTriggersEnabled != nil && *o.FileTriggersEnabled {
return ErrUnsupportedBothTagsRegexAndFileTriggersEnabled
}
Expand All @@ -1103,15 +1103,15 @@ func (o WorkspaceUpdateOptions) valid() error {
return ErrUnsupportedBothTriggerPatternsAndPrefixes
}

if o.VCSRepo != nil && o.VCSRepo.TagsRegex != nil &&
if tagRegexDefined(o.VCSRepo) &&
o.TriggerPatterns != nil && len(o.TriggerPatterns) > 0 {
return ErrUnsupportedBothTagsRegexAndTriggerPatterns
}
if o.VCSRepo != nil && o.VCSRepo.TagsRegex != nil &&
if tagRegexDefined(o.VCSRepo) &&
o.TriggerPrefixes != nil && len(o.TriggerPrefixes) > 0 {
return ErrUnsupportedBothTagsRegexAndTriggerPrefixes
}
if o.VCSRepo != nil && o.VCSRepo.TagsRegex != nil &&
if tagRegexDefined(o.VCSRepo) &&
o.FileTriggersEnabled != nil && *o.FileTriggersEnabled {
return ErrUnsupportedBothTagsRegexAndFileTriggersEnabled
}
Expand Down Expand Up @@ -1221,3 +1221,13 @@ func validateWorkspaceIncludeParams(params []WSIncludeOpt) error {

return nil
}

func tagRegexDefined(options *VCSRepoOptions) bool {
if options == nil {
return false
}
if options.TagsRegex != nil && *options.TagsRegex != "" {
return true
}
return false
}
33 changes: 28 additions & 5 deletions workspace_integration_test.go
View file
Open in desktop
Expand Up @@ -344,10 +344,10 @@ func TestWorkspacesCreate(t *testing.T) {
assert.EqualError(t, err, ErrUnsupportedBothTriggerPatternsAndPrefixes.Error())
})

t.Run("when options include tags-regex(behind a feature flag)", func(t *testing.T) {
t.Run("when options include tags-regex", func(t *testing.T) {
// Remove the below organization creation and use the one from the outer scope once the feature flag is removed
orgTest, orgTestCleanup := createOrganizationWithOptions(t, client, OrganizationCreateOptions{
Name: String("tst-" + randomString(t)[0:20] + "-git-tag-ff-on"),
Name: String("tst-" + randomString(t)[0:20]),
Email: String(fmt.Sprintf("%s@tfe.local", randomString(t))),
})
defer orgTestCleanup()
Expand Down Expand Up @@ -413,6 +413,18 @@ func TestWorkspacesCreate(t *testing.T) {
assert.EqualError(t, err, ErrUnsupportedBothTagsRegexAndFileTriggersEnabled.Error())
})

t.Run("when options include both non-empty tags-regex and file-triggers-enabled as false an error is not returned", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: We could use table driven tests and merge together this and the above 3 tests

Copy link
Contributor

Choose a reason for hiding this comment

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

@hashimoon We're good with adding table driven tests here - agree that it would help readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given the rest of the file is not utilizing table driven tests and the style is consistent with what is already present, can we add them in a future refactor? @annawinkler

options := WorkspaceCreateOptions{
Name: String("foobar"),
FileTriggersEnabled: Bool(false),
VCSRepo: &VCSRepoOptions{TagsRegex: String("foobar")},
}
w, err := client.Workspaces.Create(ctx, orgTest.Name, options)

require.NotNil(t, w)
require.NoError(t, err)
})

t.Run("when options include trigger-patterns populated and empty trigger-paths workspace is created", func(t *testing.T) {
// Remove the below organization creation and use the one from the outer scope once the feature flag is removed
orgTest, orgTestCleanup := createOrganizationWithOptions(t, client, OrganizationCreateOptions{
Expand Down Expand Up @@ -832,11 +844,10 @@ func TestWorkspacesUpdate(t *testing.T) {
}
})

t.Run("when options include VCSRepo tags-regex (behind a feature flag)", func(t *testing.T) {
skipIfBeta(t)
t.Run("when options include VCSRepo tags-regex", func(t *testing.T) {
// Remove the below organization and workspace creation and use the one from the outer scope once the feature flag is removed
orgTest, orgTestCleanup := createOrganizationWithOptions(t, client, OrganizationCreateOptions{
Name: String("tst-" + randomString(t)[0:20] + "-git-tag-ff-on"),
Name: String("tst-" + randomString(t)[0:20]),
Email: String(fmt.Sprintf("%s@tfe.local", randomString(t))),
})
defer orgTestCleanup()
Expand Down Expand Up @@ -889,6 +900,18 @@ func TestWorkspacesUpdate(t *testing.T) {
assert.EqualError(t, err, ErrUnsupportedBothTagsRegexAndFileTriggersEnabled.Error())
})

t.Run("when options include both non-empty tags-regex and file-triggers-enabled an error is returned", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

And this one could be a table driven test

options := WorkspaceUpdateOptions{
Name: String("foobar"),
FileTriggersEnabled: Bool(true),
VCSRepo: &VCSRepoOptions{TagsRegex: String("foobar")},
}
w, err := client.Workspaces.Update(ctx, orgTest.Name, wTest.Name, options)

assert.Nil(t, w)
assert.EqualError(t, err, ErrUnsupportedBothTagsRegexAndFileTriggersEnabled.Error())
})

t.Run("when options include both tags-regex and trigger-prefixes an error is returned", func(t *testing.T) {
options := WorkspaceUpdateOptions{
Name: String("foobar"),
Expand Down