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 all 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
25 changes: 22 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,20 @@ func MutateTags(userID uuid.UUID, provided map[string]string) map[string]string
}
return tags
}

// mergeTags merges two sets of provisioner tags.
// If b[key] is an empty string, the value from a[key] is retained.
// This function handles nil maps gracefully.
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
}
134 changes: 111 additions & 23 deletions provisionersdk/provisionertags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package provisionersdk_test

import (
"encoding/json"
"testing"

"github.com/coder/coder/v2/provisionersdk"
Expand All @@ -18,13 +17,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 +32,28 @@ func TestMutateTags(t *testing.T) {
{
name: "empty tags",
userID: uuid.Nil,
tags: map[string]string{},
tags: []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: []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: []map[string]string{
{provisionersdk.TagScope: provisionersdk.ScopeOrganization},
},
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
Expand All @@ -59,9 +62,11 @@ func TestMutateTags(t *testing.T) {
},
{
name: "organization scope with owner",
tags: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: testUserID.String(),
tags: []map[string]string{
{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: testUserID.String(),
},
},
userID: uuid.Nil,
want: map[string]string{
Expand All @@ -71,8 +76,10 @@ func TestMutateTags(t *testing.T) {
},
{
name: "owner tag with no other context",
tags: map[string]string{
provisionersdk.TagOwner: testUserID.String(),
tags: []map[string]string{
{
provisionersdk.TagOwner: testUserID.String(),
},
},
userID: uuid.Nil,
want: map[string]string{
Expand All @@ -81,8 +88,96 @@ func TestMutateTags(t *testing.T) {
},
},
{
name: "invalid scope",
tags: map[string]string{provisionersdk.TagScope: "360noscope"},
name: "invalid scope",
tags: []map[string]string{
{provisionersdk.TagScope: "360noscope"},
},
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
},
},
{
name: "merge two empty maps",
tags: []map[string]string{
{},
{},
},
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
},
},
{
name: "merge empty map with non-empty map",
tags: []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: []map[string]string{
{"foo": "bar"},
{},
},
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "bar",
},
},
{
name: "merge map with same map",
tags: []map[string]string{
{"foo": "bar"},
{"foo": "bar"},
},
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "bar",
},
},
{
name: "merge map with override",
tags: []map[string]string{
{"foo": "bar"},
{"foo": "baz"},
},
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "baz",
},
},
{
name: "do not override empty in second map",
tags: []map[string]string{
{"foo": "bar"},
{"foo": ""},
},
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
provisionersdk.TagOwner: "",
"foo": "bar",
},
},
{
name: "merge nil map with nil map",
tags: []map[string]string{nil, nil},
userID: testUserID,
want: map[string]string{
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
Expand All @@ -93,14 +188,7 @@ func TestMutateTags(t *testing.T) {
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