Skip to content

Commit

Permalink
Prefer kv tags when tags defined as set
Browse files Browse the repository at this point in the history 
This will prevent duplicate workspace tags from being created when set tags are matched using kv bindings, or vice-versa.
  • Loading branch information
@brandonc
brandonc committed Oct 31, 2024
1 parent 042a4d9 commit c1069f9
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 64 deletions.
151 changes: 100 additions & 51 deletions internal/cloud/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"net/url"
"os"
"slices"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -46,7 +47,7 @@ const (
genericHostname = "localterraform.com"
)

var ErrCloudDoesNotSupportKVTags = errors.New("your version of Terraform Enterprise does not support key-value tags. Please upgrade to a version that supports this feature.")
var ErrCloudDoesNotSupportKVTags = errors.New("your version of Terraform Enterprise does not support key-value tags. Please upgrade Terraform Enterprise to a version that supports this feature or use set type tags instead.")

// Cloud is an implementation of EnhancedBackend in service of the HCP Terraform or Terraform Enterprise
// integration for Terraform CLI. This backend is not intended to be surfaced at the user level and
Expand Down Expand Up @@ -632,7 +633,18 @@ func (b *Cloud) Workspaces() ([]string, error) {
if b.WorkspaceMapping.Strategy() == WorkspaceTagsStrategy {
options.Tags = strings.Join(b.WorkspaceMapping.TagsAsSet, ",")
} else if b.WorkspaceMapping.Strategy() == WorkspaceKVTagsStrategy {
options.TagBindings = b.WorkspaceMapping.tfeTagBindings()
options.TagBindings = b.WorkspaceMapping.asTFETagBindings()

// Populate keys, too, just in case backend does not support key/value tags.
// The backend will end up applying both filters but that should always
// be the same result set anyway.
for _, tag := range options.TagBindings {
if options.Tags != "" {
options.Tags = options.Tags + ","
}
options.Tags = options.Tags + tag.Key
}

}
log.Printf("[TRACE] cloud: Listing workspaces with tag bindings %q", b.WorkspaceMapping.DescribeTags())

Expand Down Expand Up @@ -760,7 +772,7 @@ func (b *Cloud) StateMgr(name string) (statemgr.Full, error) {
if b.WorkspaceMapping.Strategy() == WorkspaceTagsStrategy {
workspaceCreateOptions.Tags = b.WorkspaceMapping.tfeTags()
} else if b.WorkspaceMapping.Strategy() == WorkspaceKVTagsStrategy {
workspaceCreateOptions.TagBindings = b.WorkspaceMapping.tfeTagBindings()
workspaceCreateOptions.TagBindings = b.WorkspaceMapping.asTFETagBindings()
}

// Create project if not exists, otherwise use it
Expand Down Expand Up @@ -813,26 +825,24 @@ func (b *Cloud) StateMgr(name string) (statemgr.Full, error) {
}
}

updateRequired, err := b.workspaceTagsRequireUpdate(context.Background(), workspace, b.WorkspaceMapping)
if updateRequired {
if err != nil {
if errors.Is(err, tfe.ErrResourceNotFound) {
return nil, fmt.Errorf("workspace %s does not exist or access to it is unauthorized", name)
} else if errors.Is(err, ErrCloudDoesNotSupportKVTags) {
return nil, err
tagCheck, errFromTagCheck := b.workspaceTagsRequireUpdate(context.Background(), workspace, b.WorkspaceMapping)
if tagCheck.requiresUpdate {
if errFromTagCheck != nil {
if errors.Is(errFromTagCheck, ErrCloudDoesNotSupportKVTags) {
return nil, fmt.Errorf("backend does not support key/value tags. Try using key-only tags: %w", errFromTagCheck)
}
return nil, fmt.Errorf("error checking if workspace %s requires update: %w", name, err)
}

log.Printf("[TRACE] cloud: Updating tags for %s workspace %s/%s to %s", b.appName, b.Organization, name, b.WorkspaceMapping.DescribeTags())
if b.WorkspaceMapping.Strategy() == WorkspaceTagsStrategy {
log.Printf("[TRACE] cloud: Updating tags for %s workspace %s/%s to %q", b.appName, b.Organization, name, b.WorkspaceMapping.DescribeTags())
// Always update using KV tags if possible
if !tagCheck.supportsKVTags {
options := tfe.WorkspaceAddTagsOptions{
Tags: b.WorkspaceMapping.tfeTags(),
}
err = b.client.Workspaces.AddTags(context.Background(), workspace.ID, options)
} else if b.WorkspaceMapping.Strategy() == WorkspaceKVTagsStrategy {
} else {
options := tfe.WorkspaceAddTagBindingsOptions{
TagBindings: b.WorkspaceMapping.tfeTagBindings(),
TagBindings: b.WorkspaceMapping.asTFETagBindings(),
}
_, err = b.client.Workspaces.AddTagBindings(context.Background(), workspace.ID, options)
}
Expand Down Expand Up @@ -1160,39 +1170,75 @@ func (b *Cloud) cliColorize() *colorstring.Colorize {
}
}

func (b *Cloud) workspaceTagsRequireUpdate(ctx context.Context, workspace *tfe.Workspace, workspaceMapping WorkspaceMapping) (bool, error) {
if workspaceMapping.Strategy() == WorkspaceTagsStrategy {
existingTags := map[string]struct{}{}
for _, t := range workspace.TagNames {
existingTags[t] = struct{}{}
}
type tagRequiresUpdateResult struct {
requiresUpdate bool
supportsKVTags bool
}

for _, tag := range workspaceMapping.TagsAsSet {
if _, ok := existingTags[tag]; !ok {
return true, nil
}
}
} else if workspaceMapping.Strategy() == WorkspaceKVTagsStrategy {
existingTags := make(map[string]string)
bindings, err := b.client.Workspaces.ListTagBindings(ctx, workspace.ID)
if err != nil && err == tfe.ErrResourceNotFound {
return true, ErrCloudDoesNotSupportKVTags
} else if err != nil {
return true, err
}
func (b *Cloud) workspaceTagsRequireUpdate(ctx context.Context, workspace *tfe.Workspace, workspaceMapping WorkspaceMapping) (result tagRequiresUpdateResult, err error) {
result = tagRequiresUpdateResult{
supportsKVTags: true,
}

for _, binding := range bindings {
existingTags[binding.Key] = binding.Value
// First, depending on the strategy, build a map of the tags defined in config
// so we can compare them to the actual tags on the workspace
normalizedTagMap := make(map[string]string)
if workspaceMapping.IsTagsStrategy() {
for _, b := range workspaceMapping.asTFETagBindings() {
normalizedTagMap[b.Key] = b.Value
}

for tag, val := range workspaceMapping.TagsAsMap {
if existingVal, ok := existingTags[tag]; !ok || existingVal != val {
return true, nil
} else {
// Not a tag strategy
return
}

// Fetch tag bindings and determine if they should be checked
bindings, err := b.client.Workspaces.ListTagBindings(ctx, workspace.ID)
if err != nil && errors.Is(err, tfe.ErrResourceNotFound) {
// By this time, the workspace should have been fetched, proving that the
// authenticated user has access to it. If the tag bindings are not found,
// it would mean that the backened does not support tag bindings.
result.supportsKVTags = false
} else if err != nil {
return
}

err = nil
check:
// Check desired workspace tags against existing tags
for k, v := range normalizedTagMap {
log.Printf("[TRACE] cloud: Checking tag %q=%q", k, v)
if v == "" {
// Tag can exist in legacy tags or tag bindings
if !slices.Contains(workspace.TagNames, k) || (result.supportsKVTags && !slices.ContainsFunc(bindings, func(b *tfe.TagBinding) bool {
return b.Key == k
})) {
result.requiresUpdate = true
break check
}
} else if !result.supportsKVTags {
// There is a value defined, but the backend does not support tag bindings
result.requiresUpdate = true
err = ErrCloudDoesNotSupportKVTags
break check
} else {
// There is a value, so it must match a tag binding
if !slices.ContainsFunc(bindings, func(b *tfe.TagBinding) bool {
return b.Key == k && b.Value == v
}) {
result.requiresUpdate = true
break check
}
}
}

return false, nil
doesOrDoesnot := "does "
if !result.requiresUpdate {
doesOrDoesnot = "does not "
}
log.Printf("[TRACE] cloud: Workspace %s %srequire tag update", workspace.Name, doesOrDoesnot)

return
}

type WorkspaceMapping struct {
Expand Down Expand Up @@ -1390,21 +1436,24 @@ func (wm WorkspaceMapping) tfeTags() []*tfe.Tag {
return tags
}

func (wm WorkspaceMapping) tfeTagBindings() []*tfe.TagBinding {
func (wm WorkspaceMapping) asTFETagBindings() []*tfe.TagBinding {
var tagBindings []*tfe.TagBinding

if wm.Strategy() != WorkspaceKVTagsStrategy {
return tagBindings
}
if wm.Strategy() == WorkspaceKVTagsStrategy {
tagBindings = make([]*tfe.TagBinding, len(wm.TagsAsMap))

tagBindings = make([]*tfe.TagBinding, len(wm.TagsAsMap))
index := 0
for key, val := range wm.TagsAsMap {
tagBindings[index] = &tfe.TagBinding{Key: key, Value: val}
index += 1
}
} else if wm.Strategy() == WorkspaceTagsStrategy {
tagBindings = make([]*tfe.TagBinding, len(wm.TagsAsSet))

index := 0
for key, val := range wm.TagsAsMap {
tagBindings[index] = &tfe.TagBinding{Key: key, Value: val}
index += 1
for i, tag := range wm.TagsAsSet {
tagBindings[i] = &tfe.TagBinding{Key: tag}
}
}

return tagBindings
}

Expand Down
32 changes: 31 additions & 1 deletion internal/cloud/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,36 @@ func TestCloud_backendWithTags(t *testing.T) {
}
}

func TestCloud_backendWithKVTags(t *testing.T) {
b, bCleanup := testBackendWithKVTags(t)
defer bCleanup()

_, err := b.client.Workspaces.Create(context.Background(), "hashicorp", tfe.WorkspaceCreateOptions{
Name: tfe.String("ws-billing-101"),
TagBindings: []*tfe.TagBinding{
{Key: "dept", Value: "billing"},
{Key: "costcenter", Value: "101"},
},
})
if err != nil {
t.Fatalf("error creating workspace: %s", err)
}

workspaces, err := b.Workspaces()
if err != nil {
t.Fatalf("error: %s", err)
}

actual := len(workspaces)
if actual != 1 {
t.Fatalf("expected 1 workspaces, got %d", actual)
}

if workspaces[0] != "ws-billing-101" {
t.Fatalf("expected workspace name to be 'ws-billing-101', got %s", workspaces[0])
}
}

func TestCloud_DescribeTags(t *testing.T) {
cases := map[string]struct {
expected string
Expand Down Expand Up @@ -1223,7 +1253,7 @@ func TestCloud_resolveCloudConfig(t *testing.T) {
t.Fatalf("%s: expected final config of %#v but instead got %#v", name, tc.expectedResult, result)
}

if !reflect.DeepEqual(tc.expectedResult.workspaceMapping.tfeTagBindings(), result.workspaceMapping.tfeTagBindings()) {
if !reflect.DeepEqual(tc.expectedResult.workspaceMapping.asTFETagBindings(), result.workspaceMapping.asTFETagBindings()) {
t.Fatalf("%s: expected final config of %#v but instead got %#v", name, tc.expectedResult, result)
}
}
Expand Down
18 changes: 18 additions & 0 deletions internal/cloud/testing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,24 @@ func testBackendWithTags(t *testing.T) (*Cloud, func()) {
return b, c
}

func testBackendWithKVTags(t *testing.T) (*Cloud, func()) {
obj := cty.ObjectVal(map[string]cty.Value{
"hostname": cty.NullVal(cty.String),
"organization": cty.StringVal("hashicorp"),
"token": cty.NullVal(cty.String),
"workspaces": cty.ObjectVal(map[string]cty.Value{
"name": cty.NullVal(cty.String),
"tags": cty.MapVal(map[string]cty.Value{
"dept": cty.StringVal("billing"),
"costcenter": cty.StringVal("101"),
}),
"project": cty.NullVal(cty.String),
}),
})
b, _, c := testBackend(t, obj, nil)
return b, c
}

func testBackendNoOperations(t *testing.T) (*Cloud, func()) {
obj := cty.ObjectVal(map[string]cty.Value{
"hostname": cty.NullVal(cty.String),
Expand Down
40 changes: 28 additions & 12 deletions internal/cloud/tfe_client_mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1942,35 +1942,51 @@ func newMockWorkspaces(client *MockClient) *MockWorkspaces {
func (m *MockWorkspaces) List(ctx context.Context, organization string, options *tfe.WorkspaceListOptions) (*tfe.WorkspaceList, error) {
wl := &tfe.WorkspaceList{}
// Get all the workspaces that match the Search value
searchValue := ""
var ws []*tfe.Workspace
var tags []string
searchValue := ""
searchTags := make(map[string]string)

if options != nil {
if len(options.Search) > 0 {
searchValue = options.Search
}
if len(options.Tags) > 0 {
tags = strings.Split(options.Tags, ",")
for _, tag := range strings.Split(options.Tags, ",") {
searchTags[tag] = ""
}
}
if len(options.TagBindings) > 0 {
for _, kvTag := range options.TagBindings {
searchTags[kvTag.Key] = kvTag.Value
}
}
}

for _, w := range m.workspaceIDs {
wTags := make(map[string]struct{})
wTags := make(map[string]string)
for _, wTag := range w.Tags {
wTags[wTag.Name] = struct{}{}
wTags[wTag.Name] = ""
}

if strings.Contains(w.Name, searchValue) {
tagsSatisfied := true
for _, tag := range tags {
if _, ok := wTags[tag]; !ok {
for _, kvTag := range w.TagBindings {
wTags[kvTag.Key] = kvTag.Value
}

tagsSatisfied := true
for k, v := range searchTags {
if value, ok := wTags[k]; ok {
if value != v {
tagsSatisfied = false
break
}
} else {
tagsSatisfied = false
break
}
if tagsSatisfied {
ws = append(ws, w)
}
}

if strings.Contains(w.Name, searchValue) && tagsSatisfied {
ws = append(ws, w)
}
}

Expand Down

0 comments on commit c1069f9

Please sign in to comment.