Skip to content

Commit 0d5d587

Browse files
johnstcnjohnstcn
committed
feat(provisionersdk): allow variadic tags in provisionersdk.MutateTags
1 parent 6b1fafb commit 0d5d587

File tree

2 files changed

+132
-27
lines changed

2 files changed

+132
-27
lines changed

‎provisionersdk/provisionertags.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,16 @@ const (
1414
// If the scope is "user", the "owner" is changed to the user ID.
1515
// This is for user-scoped provisioner daemons, where users should
1616
// own their own operations.
17+
// Multiple sets of tags may be passed to this function; they will
18+
// be merged into one single tag set.
1719
// Otherwise, the "owner" tag is always an empty string.
1820
// NOTE: "owner" must NEVER be nil. Otherwise it will end up being
1921
// duplicated in the database, as idx_provisioner_daemons_name_owner_key
2022
// is a partial unique index that includes a JSON field.
21-
func MutateTags(userID uuid.UUID, provided map[string]string) map[string]string {
23+
func MutateTags(userID uuid.UUID, provided ...map[string]string) map[string]string {
2224
tags := map[string]string{}
23-
for k, v := range provided {
24-
tags[k] = v
25+
for _, m := range provided {
26+
tags = mergeTags(tags, m)
2527
}
2628
_, ok := tags[TagScope]
2729
if !ok {
@@ -39,3 +41,23 @@ func MutateTags(userID uuid.UUID, provided map[string]string) map[string]string
3941
}
4042
return tags
4143
}
44+
45+
// mergeTags merges multiple sets of provisioner tags.
46+
// Given maps a and b, the result will contain all
47+
// keys and values contained in a and in b.
48+
// An exception is made for key-value pairs for which
49+
// a[key] is non-empty but b[key] is empty. In this case
50+
// the non-empty value from a will win.
51+
func mergeTags(a, b map[string]string) map[string]string {
52+
m := make(map[string]string)
53+
for k, v := range a {
54+
m[k] = v
55+
}
56+
for k, v := range b {
57+
if v == "" {
58+
continue
59+
}
60+
m[k] = v
61+
}
62+
return m
63+
}

‎provisionersdk/provisionertags_test.go

Lines changed: 107 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package provisionersdk_test
22

33
import (
4-
"encoding/json"
54
"testing"
65

76
"github.com/coder/coder/v2/provisionersdk"
@@ -18,13 +17,13 @@ func TestMutateTags(t *testing.T) {
1817
for _, tt := range []struct {
1918
name string
2019
userID uuid.UUID
21-
tags map[string]string
20+
tags []map[string]string
2221
want map[string]string
2322
}{
2423
{
2524
name: "nil tags",
2625
userID: uuid.Nil,
27-
tags: nil,
26+
tags: mapslice(nil),
2827
want: map[string]string{
2928
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
3029
provisionersdk.TagOwner: "",
@@ -33,24 +32,28 @@ func TestMutateTags(t *testing.T) {
3332
{
3433
name: "empty tags",
3534
userID: uuid.Nil,
36-
tags: map[string]string{},
35+
tags: mapslice(map[string]string{}),
3736
want: map[string]string{
3837
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
3938
provisionersdk.TagOwner: "",
4039
},
4140
},
4241
{
43-
name: "user scope",
44-
tags: map[string]string{provisionersdk.TagScope: provisionersdk.ScopeUser},
42+
name: "user scope",
43+
tags: mapslice(
44+
map[string]string{provisionersdk.TagScope: provisionersdk.ScopeUser},
45+
),
4546
userID: testUserID,
4647
want: map[string]string{
4748
provisionersdk.TagScope: provisionersdk.ScopeUser,
4849
provisionersdk.TagOwner: testUserID.String(),
4950
},
5051
},
5152
{
52-
name: "organization scope",
53-
tags: map[string]string{provisionersdk.TagScope: provisionersdk.ScopeOrganization},
53+
name: "organization scope",
54+
tags: mapslice(
55+
map[string]string{provisionersdk.TagScope: provisionersdk.ScopeOrganization},
56+
),
5457
userID: testUserID,
5558
want: map[string]string{
5659
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
@@ -59,49 +62,129 @@ func TestMutateTags(t *testing.T) {
5962
},
6063
{
6164
name: "organization scope with owner",
62-
tags: map[string]string{
65+
tags: mapslice(
66+
map[string]string{
67+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
68+
provisionersdk.TagOwner: testUserID.String(),
69+
},
70+
),
71+
userID: uuid.Nil,
72+
want: map[string]string{
6373
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
64-
provisionersdk.TagOwner: testUserID.String(),
74+
provisionersdk.TagOwner: "",
6575
},
76+
},
77+
{
78+
name: "owner tag with no other context",
79+
tags: mapslice(
80+
map[string]string{
81+
provisionersdk.TagOwner: testUserID.String(),
82+
},
83+
),
6684
userID: uuid.Nil,
6785
want: map[string]string{
6886
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
6987
provisionersdk.TagOwner: "",
7088
},
7189
},
7290
{
73-
name: "owner tag with no other context",
74-
tags: map[string]string{
75-
provisionersdk.TagOwner: testUserID.String(),
91+
name: "invalid scope",
92+
tags: mapslice(
93+
map[string]string{provisionersdk.TagScope: "360noscope"},
94+
),
95+
userID: testUserID,
96+
want: map[string]string{
97+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
98+
provisionersdk.TagOwner: "",
7699
},
77-
userID: uuid.Nil,
100+
},
101+
{
102+
name: "merge two empty maps",
103+
tags: mapslice(
104+
map[string]string{},
105+
map[string]string{},
106+
),
107+
userID: testUserID,
108+
want: map[string]string{
109+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
110+
provisionersdk.TagOwner: "",
111+
},
112+
},
113+
{
114+
name: "merge empty map with non-empty map",
115+
tags: mapslice(
116+
map[string]string{},
117+
map[string]string{"foo": "bar"},
118+
),
119+
userID: testUserID,
78120
want: map[string]string{
79121
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
80122
provisionersdk.TagOwner: "",
123+
"foo": "bar",
81124
},
82125
},
83126
{
84-
name: "invalid scope",
85-
tags: map[string]string{provisionersdk.TagScope: "360noscope"},
127+
name: "merge non-empty map with empty map",
128+
tags: mapslice(
129+
map[string]string{"foo": "bar"},
130+
map[string]string{},
131+
),
86132
userID: testUserID,
87133
want: map[string]string{
88134
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
89135
provisionersdk.TagOwner: "",
136+
"foo": "bar",
137+
},
138+
},
139+
{
140+
name: "merge map with same map",
141+
tags: mapslice(
142+
map[string]string{"foo": "bar"},
143+
map[string]string{"foo": "bar"},
144+
),
145+
userID: testUserID,
146+
want: map[string]string{
147+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
148+
provisionersdk.TagOwner: "",
149+
"foo": "bar",
150+
},
151+
},
152+
{
153+
name: "merge map with override",
154+
tags: mapslice(
155+
map[string]string{"foo": "bar"},
156+
map[string]string{"foo": "baz"},
157+
),
158+
userID: testUserID,
159+
want: map[string]string{
160+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
161+
provisionersdk.TagOwner: "",
162+
"foo": "baz",
163+
},
164+
},
165+
{
166+
name: "do not override empty in second map",
167+
tags: mapslice(
168+
map[string]string{"foo": "bar"},
169+
map[string]string{"foo": ""},
170+
),
171+
userID: testUserID,
172+
want: map[string]string{
173+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
174+
provisionersdk.TagOwner: "",
175+
"foo": "bar",
90176
},
91177
},
92178
} {
93179
tt := tt
94180
t.Run(tt.name, func(t *testing.T) {
95181
t.Parallel()
96-
// make a copy of the map because the function under test
97-
// mutates the map
98-
bytes, err := json.Marshal(tt.tags)
99-
require.NoError(t, err)
100-
var tags map[string]string
101-
err = json.Unmarshal(bytes, &tags)
102-
require.NoError(t, err)
103-
got := provisionersdk.MutateTags(tt.userID, tags)
182+
got := provisionersdk.MutateTags(tt.userID, tt.tags...)
104183
require.Equal(t, tt.want, got)
105184
})
106185
}
107186
}
187+
188+
func mapslice(m ...map[string]string) []map[string]string {
189+
return m
190+
}

0 commit comments

Comments
 (0)