Skip to content

feat(provisionersdk): allow variadic tags in provisionersdk.MutateTags #15518

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

Merged
merged 3 commits into from
Nov 15, 2024
Merged

feat(provisionersdk): allow variadic tags in provisionersdk.MutateTags #15518

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0d5d587
feat(provisionersdk): allow variadic tags in provisionersdk.MutateTags
johnstcn Nov 14, 2024
905e66e
i keep forgetting about util/slice
johnstcn Nov 14, 2024
70de358
address PR comments
johnstcn Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions provisionersdk/provisionertags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ const (
// If the scope is "user", the "owner" is changed to the user ID.
// This is for user-scoped provisioner daemons, where users should
// own their own operations.
// Multiple sets of tags may be passed to this function; they will
// be merged into one single tag set.
// Otherwise, the "owner" tag is always an empty string.
// NOTE: "owner" must NEVER be nil. Otherwise it will end up being
// duplicated in the database, as idx_provisioner_daemons_name_owner_key
// is a partial unique index that includes a JSON field.
func MutateTags(userID uuid.UUID, provided map[string]string) map[string]string {
func MutateTags(userID uuid.UUID, provided ...map[string]string) map[string]string {
tags := map[string]string{}
for k, v := range provided {
tags[k] = v
for _, m := range provided {
tags = mergeTags(tags, m)
}
_, ok := tags[TagScope]
if !ok {
Expand All @@ -39,3 +41,23 @@ func MutateTags(userID uuid.UUID, provided map[string]string) map[string]string
}
return tags
}

// mergeTags merges multiple sets of provisioner tags.
// Given maps a and b, the result will contain all
// keys and values contained in a and in b.
// An exception is made for key-value pairs for which
// a[key] is non-empty but b[key] is empty. In this case
// the non-empty value from a will win.
func mergeTags(a, b map[string]string) map[string]string {
m := make(map[string]string)
for k, v := range a {
m[k] = v
}
for k, v := range b {
if v == "" {
continue
}
m[k] = v
}
return m
}
128 changes: 104 additions & 24 deletions provisionersdk/provisionertags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package provisionersdk_test

import (
"encoding/json"
"testing"

"github.com/coder/coder/v2/coderd/util/slice"
"github.com/coder/coder/v2/provisionersdk"

"github.com/google/uuid"
Expand All @@ -18,13 +18,13 @@ func TestMutateTags(t *testing.T) {
for _, tt := range []struct {
name string
userID uuid.UUID
tags map[string]string
tags []map[string]string
want map[string]string
}{
{
name: "nil tags",
userID: uuid.Nil,
tags: nil,
tags: []map[string]string{nil},
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
Expand All @@ -33,24 +33,28 @@ func TestMutateTags(t *testing.T) {
{
name: "empty tags",
userID: uuid.Nil,
tags: map[string]string{},
tags: slice.New(map[string]string{}),
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
},
},
{
name: "user scope",
tags: map[string]string{provisionersdk.TagScope: provisionersdk.ScopeUser},
name: "user scope",
tags: slice.New(
map[string]string{provisionersdk.TagScope: provisionersdk.ScopeUser},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeUser,
provisionersdk.TagOwner: testUserID.String(),
},
},
{
name: "organization scope",
tags: map[string]string{provisionersdk.TagScope: provisionersdk.ScopeOrganization},
name: "organization scope",
tags: slice.New(
map[string]string{provisionersdk.TagScope: provisionersdk.ScopeOrganization},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
Expand All @@ -59,48 +63,124 @@ func TestMutateTags(t *testing.T) {
},
{
name: "organization scope with owner",
tags: map[string]string{
tags: slice.New(
map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: testUserID.String(),
},
),
userID: uuid.Nil,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: testUserID.String(),
provisionersdk.TagOwner: "",
},
},
{
name: "owner tag with no other context",
tags: slice.New(
map[string]string{
provisionersdk.TagOwner: testUserID.String(),
},
),
userID: uuid.Nil,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
},
},
{
name: "owner tag with no other context",
tags: map[string]string{
provisionersdk.TagOwner: testUserID.String(),
name: "invalid scope",
tags: slice.New(
map[string]string{provisionersdk.TagScope: "360noscope"},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
},
userID: uuid.Nil,
},
{
name: "merge two empty maps",
tags: slice.New(
map[string]string{},
map[string]string{},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
},
},
{
name: "merge empty map with non-empty map",
tags: slice.New(
map[string]string{},
map[string]string{"foo": "bar"},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "bar",
},
},
{
name: "merge non-empty map with empty map",
tags: slice.New(
map[string]string{"foo": "bar"},
map[string]string{},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "bar",
},
},
{
name: "merge map with same map",
tags: slice.New(
map[string]string{"foo": "bar"},
map[string]string{"foo": "bar"},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "bar",
},
},
{
name: "merge map with override",
tags: slice.New(
map[string]string{"foo": "bar"},
map[string]string{"foo": "baz"},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "baz",
},
},
{
name: "invalid scope",
tags: map[string]string{provisionersdk.TagScope: "360noscope"},
name: "do not override empty in second map",
tags: slice.New(
map[string]string{"foo": "bar"},
map[string]string{"foo": ""},
),
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "bar",
},
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
// make a copy of the map because the function under test
// mutates the map
bytes, err := json.Marshal(tt.tags)
require.NoError(t, err)
var tags map[string]string
err = json.Unmarshal(bytes, &tags)
require.NoError(t, err)
got := provisionersdk.MutateTags(tt.userID, tags)
got := provisionersdk.MutateTags(tt.userID, tt.tags...)
require.Equal(t, tt.want, got)
})
}
Expand Down
Loading