chore: Return copied templates to prevent reference issues (#6679)

Emyrk · web-flow · commit 65945aef160b · 2023-03-20T13:13:21.000-05:00
diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go
@@ -1761,7 +1761,7 @@ func (q *fakeQuerier) GetTemplateByID(ctx context.Context, id uuid.UUID) (databa
 func (q *fakeQuerier) getTemplateByIDNoLock(_ context.Context, id uuid.UUID) (database.Template, error) {
 	for _, template := range q.templates {
 		if template.ID == id {
-			return template, nil
+			return template.DeepCopy(), nil
 		}
 	}
 	return database.Template{}, sql.ErrNoRows
@@ -1785,7 +1785,7 @@ func (q *fakeQuerier) GetTemplateByOrganizationAndName(_ context.Context, arg da
 		if template.Deleted != arg.Deleted {
 			continue
 		}
-		return template, nil
+		return template.DeepCopy(), nil
 	}
 	return database.Template{}, sql.ErrNoRows
 }
@@ -1808,7 +1808,7 @@ func (q *fakeQuerier) UpdateTemplateMetaByID(_ context.Context, arg database.Upd
 		tpl.Description = arg.Description
 		tpl.Icon = arg.Icon
 		q.templates[idx] = tpl
-		return tpl, nil
+		return tpl.DeepCopy(), nil
 	}
 
 	return database.Template{}, sql.ErrNoRows
@@ -1830,7 +1830,7 @@ func (q *fakeQuerier) UpdateTemplateScheduleByID(_ context.Context, arg database
 		tpl.DefaultTTL = arg.DefaultTTL
 		tpl.MaxTTL = arg.MaxTTL
 		q.templates[idx] = tpl
-		return tpl, nil
+		return tpl.DeepCopy(), nil
 	}
 
 	return database.Template{}, sql.ErrNoRows
@@ -1889,7 +1889,7 @@ func (q *fakeQuerier) GetAuthorizedTemplates(ctx context.Context, arg database.G
 				continue
 			}
 		}
-		templates = append(templates, template)
+		templates = append(templates, template.DeepCopy())
 	}
 	if len(templates) > 0 {
 		slices.SortFunc(templates, func(i, j database.Template) bool {
@@ -2190,6 +2190,9 @@ func (q *fakeQuerier) GetTemplates(_ context.Context) ([]database.Template, erro
 	defer q.mutex.RUnlock()
 
 	templates := slices.Clone(q.templates)
+	for i := range templates {
+		templates[i] = templates[i].DeepCopy()
+	}
 	slices.SortFunc(templates, func(i, j database.Template) bool {
 		if i.Name != j.Name {
 			return i.Name < j.Name
@@ -2775,7 +2778,7 @@ func (q *fakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl
 		AllowUserCancelWorkspaceJobs: arg.AllowUserCancelWorkspaceJobs,
 	}
 	q.templates = append(q.templates, template)
-	return template, nil
+	return template.DeepCopy(), nil
 }
 
 func (q *fakeQuerier) InsertTemplateVersion(_ context.Context, arg database.InsertTemplateVersionParams) (database.TemplateVersion, error) {
@@ -3403,7 +3406,7 @@ func (q *fakeQuerier) UpdateTemplateACLByID(_ context.Context, arg database.Upda
 			template.UserACL = arg.UserACL
 
 			q.templates[i] = template
-			return template, nil
+			return template.DeepCopy(), nil
 		}
 	}
 
diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go
@@ -4,6 +4,8 @@ import (
 	"sort"
 	"strconv"
 
+	"golang.org/x/exp/maps"
+
 	"github.com/coder/coder/coderd/rbac"
 )
 
@@ -86,6 +88,13 @@ func (t Template) RBACObject() rbac.Object {
 		WithGroupACL(t.GroupACL)
 }
 
+func (t Template) DeepCopy() Template {
+	cpy := t
+	cpy.UserACL = maps.Clone(t.UserACL)
+	cpy.GroupACL = maps.Clone(t.GroupACL)
+	return cpy
+}
+
 func (TemplateVersion) RBACObject(template Template) rbac.Object {
 	// Just use the parent template resource for controlling versions
 	return template.RBACObject()

Original file line number	Diff line number	Diff line change
`@@ -1761,7 +1761,7 @@ func (q *fakeQuerier) GetTemplateByID(ctx context.Context, id uuid.UUID) (databa`
`1761`	`1761`	`func (q *fakeQuerier) getTemplateByIDNoLock(_ context.Context, id uuid.UUID) (database.Template, error) {`
`1762`	`1762`	`for _, template := range q.templates {`
`1763`	`1763`	`if template.ID == id {`
`1764`		`- return template, nil`
	`1764`	`+ return template.DeepCopy(), nil`
`1765`	`1765`	`}`
`1766`	`1766`	`}`
`1767`	`1767`	`return database.Template{}, sql.ErrNoRows`
`@@ -1785,7 +1785,7 @@ func (q *fakeQuerier) GetTemplateByOrganizationAndName(_ context.Context, arg da`
`1785`	`1785`	`if template.Deleted != arg.Deleted {`
`1786`	`1786`	`continue`
`1787`	`1787`	`}`
`1788`		`- return template, nil`
	`1788`	`+ return template.DeepCopy(), nil`
`1789`	`1789`	`}`
`1790`	`1790`	`return database.Template{}, sql.ErrNoRows`
`1791`	`1791`	`}`
`@@ -1808,7 +1808,7 @@ func (q *fakeQuerier) UpdateTemplateMetaByID(_ context.Context, arg database.Upd`
`1808`	`1808`	`tpl.Description = arg.Description`
`1809`	`1809`	`tpl.Icon = arg.Icon`
`1810`	`1810`	`q.templates[idx] = tpl`
`1811`		`- return tpl, nil`
	`1811`	`+ return tpl.DeepCopy(), nil`
`1812`	`1812`	`}`
`1813`	`1813`
`1814`	`1814`	`return database.Template{}, sql.ErrNoRows`
`@@ -1830,7 +1830,7 @@ func (q *fakeQuerier) UpdateTemplateScheduleByID(_ context.Context, arg database`
`1830`	`1830`	`tpl.DefaultTTL = arg.DefaultTTL`
`1831`	`1831`	`tpl.MaxTTL = arg.MaxTTL`
`1832`	`1832`	`q.templates[idx] = tpl`
`1833`		`- return tpl, nil`
	`1833`	`+ return tpl.DeepCopy(), nil`
`1834`	`1834`	`}`
`1835`	`1835`
`1836`	`1836`	`return database.Template{}, sql.ErrNoRows`
`@@ -1889,7 +1889,7 @@ func (q *fakeQuerier) GetAuthorizedTemplates(ctx context.Context, arg database.G`
`1889`	`1889`	`continue`
`1890`	`1890`	`}`
`1891`	`1891`	`}`
`1892`		`- templates = append(templates, template)`
	`1892`	`+ templates = append(templates, template.DeepCopy())`
`1893`	`1893`	`}`
`1894`	`1894`	`if len(templates) > 0 {`
`1895`	`1895`	`slices.SortFunc(templates, func(i, j database.Template) bool {`
`@@ -2190,6 +2190,9 @@ func (q *fakeQuerier) GetTemplates(_ context.Context) ([]database.Template, erro`
`2190`	`2190`	`defer q.mutex.RUnlock()`
`2191`	`2191`
`2192`	`2192`	`templates := slices.Clone(q.templates)`
	`2193`	`+ for i := range templates {`
	`2194`	`+ templates[i] = templates[i].DeepCopy()`
	`2195`	`+ }`
`2193`	`2196`	`slices.SortFunc(templates, func(i, j database.Template) bool {`
`2194`	`2197`	`if i.Name != j.Name {`
`2195`	`2198`	`return i.Name < j.Name`
`@@ -2775,7 +2778,7 @@ func (q *fakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl`
`2775`	`2778`	`AllowUserCancelWorkspaceJobs: arg.AllowUserCancelWorkspaceJobs,`
`2776`	`2779`	`}`
`2777`	`2780`	`q.templates = append(q.templates, template)`
`2778`		`- return template, nil`
	`2781`	`+ return template.DeepCopy(), nil`
`2779`	`2782`	`}`
`2780`	`2783`
`2781`	`2784`	`func (q *fakeQuerier) InsertTemplateVersion(_ context.Context, arg database.InsertTemplateVersionParams) (database.TemplateVersion, error) {`
`@@ -3403,7 +3406,7 @@ func (q *fakeQuerier) UpdateTemplateACLByID(_ context.Context, arg database.Upda`
`3403`	`3406`	`template.UserACL = arg.UserACL`
`3404`	`3407`
`3405`	`3408`	`q.templates[i] = template`
`3406`		`- return template, nil`
	`3409`	`+ return template.DeepCopy(), nil`
`3407`	`3410`	`}`
`3408`	`3411`	`}`
`3409`	`3412`