Skip to content

Commit 599bb35

Browse files
ssncferreirassncferreira
authored
fix(coderd): list templates returns non-deprecated templates by default (#17747)
## Description Modifies the behaviour of the "list templates" API endpoints to return non-deprecated templates by default. Users can still query for deprecated templates by specifying the `deprecated=true` query parameter. **Note:** The deprecation feature is an enterprise-level feature ## Affected Endpoints * /api/v2/organizations/{organization}/templates * /api/v2/templates Fixes #17565
1 parent 7f056da commit 599bb35

File tree

6 files changed

+329
-1
lines changed

6 files changed

+329
-1
lines changed

‎coderd/apidoc/docs.go

+2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmem/dbmem.go

+17-1
Original file line numberDiff line numberDiff line change
@@ -1380,6 +1380,12 @@ func (q *FakeQuerier) getProvisionerJobsByIDsWithQueuePositionLockedGlobalQueue(
13801380
return jobs, nil
13811381
}
13821382

1383+
// isDeprecated returns true if the template is deprecated.
1384+
// A template is considered deprecated when it has a deprecation message.
1385+
func isDeprecated(template database.Template) bool {
1386+
return template.Deprecated != ""
1387+
}
1388+
13831389
func (*FakeQuerier) AcquireLock(_ context.Context, _ int64) error {
13841390
return xerrors.New("AcquireLock must only be called within a transaction")
13851391
}
@@ -13023,7 +13029,17 @@ func (q *FakeQuerier) GetAuthorizedTemplates(ctx context.Context, arg database.G
1302313029
if arg.ExactName != "" && !strings.EqualFold(template.Name, arg.ExactName) {
1302413030
continue
1302513031
}
13026-
if arg.Deprecated.Valid && arg.Deprecated.Bool == (template.Deprecated != "") {
13032+
// Filters templates based on the search query filter 'Deprecated' status
13033+
// Matching SQL logic:
13034+
// -- Filter by deprecated
13035+
// AND CASE
13036+
// WHEN :deprecated IS NOT NULL THEN
13037+
// CASE
13038+
// WHEN :deprecated THEN deprecated != ''
13039+
// ELSE deprecated = ''
13040+
// END
13041+
// ELSE true
13042+
if arg.Deprecated.Valid && arg.Deprecated.Bool != isDeprecated(template) {
1302713043
continue
1302813044
}
1302913045
if arg.FuzzyName != "" {

‎coderd/templates.go

+14
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,9 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
487487
}
488488

489489
// @Summary Get templates by organization
490+
// @Description Returns a list of templates for the specified organization.
491+
// @Description By default, only non-deprecated templates are returned.
492+
// @Description To include deprecated templates, specify `deprecated:true` in the search query.
490493
// @ID get-templates-by-organization
491494
// @Security CoderSessionToken
492495
// @Produce json
@@ -506,6 +509,9 @@ func (api *API) templatesByOrganization() http.HandlerFunc {
506509
}
507510

508511
// @Summary Get all templates
512+
// @Description Returns a list of templates.
513+
// @Description By default, only non-deprecated templates are returned.
514+
// @Description To include deprecated templates, specify `deprecated:true` in the search query.
509515
// @ID get-all-templates
510516
// @Security CoderSessionToken
511517
// @Produce json
@@ -540,6 +546,14 @@ func (api *API) fetchTemplates(mutate func(r *http.Request, arg *database.GetTem
540546
mutate(r, &args)
541547
}
542548

549+
// By default, deprecated templates are excluded unless explicitly requested
550+
if !args.Deprecated.Valid {
551+
args.Deprecated = sql.NullBool{
552+
Bool: false,
553+
Valid: true,
554+
}
555+
}
556+
543557
// Filter templates based on rbac permissions
544558
templates, err := api.Database.GetAuthorizedTemplates(ctx, args, prepared)
545559
if errors.Is(err, sql.ErrNoRows) {

‎coderd/templates_test.go

+286
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,250 @@ func TestPostTemplateByOrganization(t *testing.T) {
441441
})
442442
}
443443

444+
func TestTemplates(t *testing.T) {
445+
t.Parallel()
446+
447+
t.Run("ListEmpty", func(t *testing.T) {
448+
t.Parallel()
449+
client := coderdtest.New(t, nil)
450+
_ = coderdtest.CreateFirstUser(t, client)
451+
452+
ctx := testutil.Context(t, testutil.WaitLong)
453+
454+
templates, err := client.Templates(ctx, codersdk.TemplateFilter{})
455+
require.NoError(t, err)
456+
require.NotNil(t, templates)
457+
require.Len(t, templates, 0)
458+
})
459+
460+
// Should return only non-deprecated templates by default
461+
t.Run("ListMultiple non-deprecated", func(t *testing.T) {
462+
t.Parallel()
463+
464+
owner, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: false})
465+
user := coderdtest.CreateFirstUser(t, owner)
466+
client, tplAdmin := coderdtest.CreateAnotherUser(t, owner, user.OrganizationID, rbac.RoleTemplateAdmin())
467+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
468+
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
469+
foo := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(request *codersdk.CreateTemplateRequest) {
470+
request.Name = "foo"
471+
})
472+
bar := coderdtest.CreateTemplate(t, client, user.OrganizationID, version2.ID, func(request *codersdk.CreateTemplateRequest) {
473+
request.Name = "bar"
474+
})
475+
476+
ctx := testutil.Context(t, testutil.WaitLong)
477+
478+
// Deprecate bar template
479+
deprecationMessage := "Some deprecated message"
480+
err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{
481+
ID: bar.ID,
482+
RequireActiveVersion: false,
483+
Deprecated: deprecationMessage,
484+
})
485+
require.NoError(t, err)
486+
487+
updatedBar, err := client.Template(ctx, bar.ID)
488+
require.NoError(t, err)
489+
require.True(t, updatedBar.Deprecated)
490+
require.Equal(t, deprecationMessage, updatedBar.DeprecationMessage)
491+
492+
// Should return only the non-deprecated template (foo)
493+
templates, err := client.Templates(ctx, codersdk.TemplateFilter{})
494+
require.NoError(t, err)
495+
require.Len(t, templates, 1)
496+
497+
require.Equal(t, foo.ID, templates[0].ID)
498+
require.False(t, templates[0].Deprecated)
499+
require.Empty(t, templates[0].DeprecationMessage)
500+
})
501+
502+
// Should return only deprecated templates when filtering by deprecated:true
503+
t.Run("ListMultiple deprecated:true", func(t *testing.T) {
504+
t.Parallel()
505+
506+
owner, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: false})
507+
user := coderdtest.CreateFirstUser(t, owner)
508+
client, tplAdmin := coderdtest.CreateAnotherUser(t, owner, user.OrganizationID, rbac.RoleTemplateAdmin())
509+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
510+
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
511+
foo := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(request *codersdk.CreateTemplateRequest) {
512+
request.Name = "foo"
513+
})
514+
bar := coderdtest.CreateTemplate(t, client, user.OrganizationID, version2.ID, func(request *codersdk.CreateTemplateRequest) {
515+
request.Name = "bar"
516+
})
517+
518+
ctx := testutil.Context(t, testutil.WaitLong)
519+
520+
// Deprecate foo and bar templates
521+
deprecationMessage := "Some deprecated message"
522+
err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{
523+
ID: foo.ID,
524+
RequireActiveVersion: false,
525+
Deprecated: deprecationMessage,
526+
})
527+
require.NoError(t, err)
528+
err = db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{
529+
ID: bar.ID,
530+
RequireActiveVersion: false,
531+
Deprecated: deprecationMessage,
532+
})
533+
require.NoError(t, err)
534+
535+
// Should have deprecation message set
536+
updatedFoo, err := client.Template(ctx, foo.ID)
537+
require.NoError(t, err)
538+
require.True(t, updatedFoo.Deprecated)
539+
require.Equal(t, deprecationMessage, updatedFoo.DeprecationMessage)
540+
541+
updatedBar, err := client.Template(ctx, bar.ID)
542+
require.NoError(t, err)
543+
require.True(t, updatedBar.Deprecated)
544+
require.Equal(t, deprecationMessage, updatedBar.DeprecationMessage)
545+
546+
// Should return only the deprecated templates (foo and bar)
547+
templates, err := client.Templates(ctx, codersdk.TemplateFilter{
548+
SearchQuery: "deprecated:true",
549+
})
550+
require.NoError(t, err)
551+
require.Len(t, templates, 2)
552+
553+
// Make sure all the deprecated templates are returned
554+
expectedTemplates := map[uuid.UUID]codersdk.Template{
555+
updatedFoo.ID: updatedFoo,
556+
updatedBar.ID: updatedBar,
557+
}
558+
actualTemplates := map[uuid.UUID]codersdk.Template{}
559+
for _, template := range templates {
560+
actualTemplates[template.ID] = template
561+
}
562+
563+
require.Equal(t, len(expectedTemplates), len(actualTemplates))
564+
for id, expectedTemplate := range expectedTemplates {
565+
actualTemplate, ok := actualTemplates[id]
566+
require.True(t, ok)
567+
require.Equal(t, expectedTemplate.ID, actualTemplate.ID)
568+
require.Equal(t, true, actualTemplate.Deprecated)
569+
require.Equal(t, expectedTemplate.DeprecationMessage, actualTemplate.DeprecationMessage)
570+
}
571+
})
572+
573+
// Should return only non-deprecated templates when filtering by deprecated:false
574+
t.Run("ListMultiple deprecated:false", func(t *testing.T) {
575+
t.Parallel()
576+
577+
client := coderdtest.New(t, nil)
578+
user := coderdtest.CreateFirstUser(t, client)
579+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
580+
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
581+
foo := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(request *codersdk.CreateTemplateRequest) {
582+
request.Name = "foo"
583+
})
584+
bar := coderdtest.CreateTemplate(t, client, user.OrganizationID, version2.ID, func(request *codersdk.CreateTemplateRequest) {
585+
request.Name = "bar"
586+
})
587+
588+
ctx := testutil.Context(t, testutil.WaitLong)
589+
590+
// Should return only the non-deprecated templates
591+
templates, err := client.Templates(ctx, codersdk.TemplateFilter{
592+
SearchQuery: "deprecated:false",
593+
})
594+
require.NoError(t, err)
595+
require.Len(t, templates, 2)
596+
597+
// Make sure all the non-deprecated templates are returned
598+
expectedTemplates := map[uuid.UUID]codersdk.Template{
599+
foo.ID: foo,
600+
bar.ID: bar,
601+
}
602+
actualTemplates := map[uuid.UUID]codersdk.Template{}
603+
for _, template := range templates {
604+
actualTemplates[template.ID] = template
605+
}
606+
607+
require.Equal(t, len(expectedTemplates), len(actualTemplates))
608+
for id, expectedTemplate := range expectedTemplates {
609+
actualTemplate, ok := actualTemplates[id]
610+
require.True(t, ok)
611+
require.Equal(t, expectedTemplate.ID, actualTemplate.ID)
612+
require.Equal(t, false, actualTemplate.Deprecated)
613+
require.Equal(t, expectedTemplate.DeprecationMessage, actualTemplate.DeprecationMessage)
614+
}
615+
})
616+
617+
// Should return a re-enabled template in the default (non-deprecated) list
618+
t.Run("ListMultiple re-enabled template", func(t *testing.T) {
619+
t.Parallel()
620+
621+
owner, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: false})
622+
user := coderdtest.CreateFirstUser(t, owner)
623+
client, tplAdmin := coderdtest.CreateAnotherUser(t, owner, user.OrganizationID, rbac.RoleTemplateAdmin())
624+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
625+
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
626+
foo := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(request *codersdk.CreateTemplateRequest) {
627+
request.Name = "foo"
628+
})
629+
bar := coderdtest.CreateTemplate(t, client, user.OrganizationID, version2.ID, func(request *codersdk.CreateTemplateRequest) {
630+
request.Name = "bar"
631+
})
632+
633+
ctx := testutil.Context(t, testutil.WaitLong)
634+
635+
// Deprecate bar template
636+
deprecationMessage := "Some deprecated message"
637+
err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{
638+
ID: bar.ID,
639+
RequireActiveVersion: false,
640+
Deprecated: deprecationMessage,
641+
})
642+
require.NoError(t, err)
643+
644+
updatedBar, err := client.Template(ctx, bar.ID)
645+
require.NoError(t, err)
646+
require.True(t, updatedBar.Deprecated)
647+
require.Equal(t, deprecationMessage, updatedBar.DeprecationMessage)
648+
649+
// Re-enable bar template
650+
err = db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{
651+
ID: bar.ID,
652+
RequireActiveVersion: false,
653+
Deprecated: "",
654+
})
655+
require.NoError(t, err)
656+
657+
reEnabledBar, err := client.Template(ctx, bar.ID)
658+
require.NoError(t, err)
659+
require.False(t, reEnabledBar.Deprecated)
660+
require.Empty(t, reEnabledBar.DeprecationMessage)
661+
662+
// Should return only the non-deprecated templates (foo and bar)
663+
templates, err := client.Templates(ctx, codersdk.TemplateFilter{})
664+
require.NoError(t, err)
665+
require.Len(t, templates, 2)
666+
667+
// Make sure all the non-deprecated templates are returned
668+
expectedTemplates := map[uuid.UUID]codersdk.Template{
669+
foo.ID: foo,
670+
bar.ID: bar,
671+
}
672+
actualTemplates := map[uuid.UUID]codersdk.Template{}
673+
for _, template := range templates {
674+
actualTemplates[template.ID] = template
675+
}
676+
677+
require.Equal(t, len(expectedTemplates), len(actualTemplates))
678+
for id, expectedTemplate := range expectedTemplates {
679+
actualTemplate, ok := actualTemplates[id]
680+
require.True(t, ok)
681+
require.Equal(t, expectedTemplate.ID, actualTemplate.ID)
682+
require.Equal(t, false, actualTemplate.Deprecated)
683+
require.Equal(t, expectedTemplate.DeprecationMessage, actualTemplate.DeprecationMessage)
684+
}
685+
})
686+
}
687+
444688
func TestTemplatesByOrganization(t *testing.T) {
445689
t.Parallel()
446690
t.Run("ListEmpty", func(t *testing.T) {
@@ -525,6 +769,48 @@ func TestTemplatesByOrganization(t *testing.T) {
525769
require.Len(t, templates, 1)
526770
require.Equal(t, bar.ID, templates[0].ID)
527771
})
772+
773+
// Should return only non-deprecated templates by default
774+
t.Run("ListMultiple non-deprecated", func(t *testing.T) {
775+
t.Parallel()
776+
777+
owner, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: false})
778+
user := coderdtest.CreateFirstUser(t, owner)
779+
client, tplAdmin := coderdtest.CreateAnotherUser(t, owner, user.OrganizationID, rbac.RoleTemplateAdmin())
780+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
781+
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
782+
foo := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(request *codersdk.CreateTemplateRequest) {
783+
request.Name = "foo"
784+
})
785+
bar := coderdtest.CreateTemplate(t, client, user.OrganizationID, version2.ID, func(request *codersdk.CreateTemplateRequest) {
786+
request.Name = "bar"
787+
})
788+
789+
ctx := testutil.Context(t, testutil.WaitLong)
790+
791+
// Deprecate bar template
792+
deprecationMessage := "Some deprecated message"
793+
err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{
794+
ID: bar.ID,
795+
RequireActiveVersion: false,
796+
Deprecated: deprecationMessage,
797+
})
798+
require.NoError(t, err)
799+
800+
updatedBar, err := client.Template(ctx, bar.ID)
801+
require.NoError(t, err)
802+
require.True(t, updatedBar.Deprecated)
803+
require.Equal(t, deprecationMessage, updatedBar.DeprecationMessage)
804+
805+
// Should return only the non-deprecated template (foo)
806+
templates, err := client.TemplatesByOrganization(ctx, user.OrganizationID)
807+
require.NoError(t, err)
808+
require.Len(t, templates, 1)
809+
810+
require.Equal(t, foo.ID, templates[0].ID)
811+
require.False(t, templates[0].Deprecated)
812+
require.Empty(t, templates[0].DeprecationMessage)
813+
})
528814
}
529815

530816
func TestTemplateByOrganizationAndName(t *testing.T) {

0 commit comments

Comments
 (0)