Skip to content

Commit 35fa21e

Browse files
johnstcnpull[bot]
johnstcn
authored and
pull[bot]
committed
feat(provisionersdk): allow variadic tags in provisionersdk.MutateTags (#15518)
Relates to #15087 and #15427 Allows specifying multiple sets of provisioner tags into `MutateTags`. These tags get additively merged. This will simplify handling tags from multiple sources when sniffing tags from the template.
1 parent aba2241 commit 35fa21e

File tree

2 files changed

+133
-26
lines changed

2 files changed

+133
-26
lines changed

‎provisionersdk/provisionertags.go

Lines changed: 22 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,20 @@ func MutateTags(userID uuid.UUID, provided map[string]string) map[string]string
3941
}
4042
return tags
4143
}
44+
45+
// mergeTags merges two sets of provisioner tags.
46+
// If b[key] is an empty string, the value from a[key] is retained.
47+
// This function handles nil maps gracefully.
48+
func mergeTags(a, b map[string]string) map[string]string {
49+
m := make(map[string]string)
50+
for k, v := range a {
51+
m[k] = v
52+
}
53+
for k, v := range b {
54+
if v == "" {
55+
continue
56+
}
57+
m[k] = v
58+
}
59+
return m
60+
}

‎provisionersdk/provisionertags_test.go

Lines changed: 111 additions & 23 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: []map[string]string{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: []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: []map[string]string{
44+
{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: []map[string]string{
55+
{provisionersdk.TagScope: provisionersdk.ScopeOrganization},
56+
},
5457
userID: testUserID,
5558
want: map[string]string{
5659
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
@@ -59,9 +62,11 @@ func TestMutateTags(t *testing.T) {
5962
},
6063
{
6164
name: "organization scope with owner",
62-
tags: map[string]string{
63-
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
64-
provisionersdk.TagOwner: testUserID.String(),
65+
tags: []map[string]string{
66+
{
67+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
68+
provisionersdk.TagOwner: testUserID.String(),
69+
},
6570
},
6671
userID: uuid.Nil,
6772
want: map[string]string{
@@ -71,8 +76,10 @@ func TestMutateTags(t *testing.T) {
7176
},
7277
{
7378
name: "owner tag with no other context",
74-
tags: map[string]string{
75-
provisionersdk.TagOwner: testUserID.String(),
79+
tags: []map[string]string{
80+
{
81+
provisionersdk.TagOwner: testUserID.String(),
82+
},
7683
},
7784
userID: uuid.Nil,
7885
want: map[string]string{
@@ -81,8 +88,96 @@ func TestMutateTags(t *testing.T) {
8188
},
8289
},
8390
{
84-
name: "invalid scope",
85-
tags: map[string]string{provisionersdk.TagScope: "360noscope"},
91+
name: "invalid scope",
92+
tags: []map[string]string{
93+
{provisionersdk.TagScope: "360noscope"},
94+
},
95+
userID: testUserID,
96+
want: map[string]string{
97+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
98+
provisionersdk.TagOwner: "",
99+
},
100+
},
101+
{
102+
name: "merge two empty maps",
103+
tags: []map[string]string{
104+
{},
105+
{},
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: []map[string]string{
116+
{},
117+
{"foo": "bar"},
118+
},
119+
userID: testUserID,
120+
want: map[string]string{
121+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
122+
provisionersdk.TagOwner: "",
123+
"foo": "bar",
124+
},
125+
},
126+
{
127+
name: "merge non-empty map with empty map",
128+
tags: []map[string]string{
129+
{"foo": "bar"},
130+
{},
131+
},
132+
userID: testUserID,
133+
want: map[string]string{
134+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
135+
provisionersdk.TagOwner: "",
136+
"foo": "bar",
137+
},
138+
},
139+
{
140+
name: "merge map with same map",
141+
tags: []map[string]string{
142+
{"foo": "bar"},
143+
{"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: []map[string]string{
155+
{"foo": "bar"},
156+
{"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: []map[string]string{
168+
{"foo": "bar"},
169+
{"foo": ""},
170+
},
171+
userID: testUserID,
172+
want: map[string]string{
173+
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
174+
provisionersdk.TagOwner: "",
175+
"foo": "bar",
176+
},
177+
},
178+
{
179+
name: "merge nil map with nil map",
180+
tags: []map[string]string{nil, nil},
86181
userID: testUserID,
87182
want: map[string]string{
88183
provisionersdk.TagScope: provisionersdk.ScopeOrganization,
@@ -93,14 +188,7 @@ func TestMutateTags(t *testing.T) {
93188
tt := tt
94189
t.Run(tt.name, func(t *testing.T) {
95190
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)
191+
got := provisionersdk.MutateTags(tt.userID, tt.tags...)
104192
require.Equal(t, tt.want, got)
105193
})
106194
}

0 commit comments

Comments
 (0)