Skip to content

Commit f256a23

Browse files
SasSwartSasSwart
authored
feat: validate presets on template import (#18844)
Typos and other errors often result in invalid presets in a template. Coder would import these broken templates and present them to users when they create workspaces. An unsuspecting user who chooses a broken preset would then experience a failed workspace build with no obvious error message. This PR adds additional validation beyond what is possible in the Terraform provider schema. Coder will now present a more helpful error message to template authors when they upload a new template version: <img width="1316" height="286" alt="Screenshot 2025-07-14 at 12 22 49" src="/api/flow.js?q=https%3A%2F%2Fgithub.com%2Fcoder%2Fcoder%2Fcommit%2F%253Ca%2520href%3D"https://github.com/user-attachments/assets/7f5f778f-d9ae-487a-95e2-f6f1ca604a9c">https://github.com/user-attachments/assets/7f5f778f-d9ae-487a-95e2-f6f1ca604a9c" /> The frontend warning is less helpful right now, but I'd like to address that in a follow-up since I need frontend help: <img width="1102" height="616" alt="image" src="/api/flow.js?q=https%3A%2F%2Fgithub.com%2Fcoder%2Fcoder%2Fcommit%2F%253Ca%2520href%3D"https://github.com/user-attachments/assets/e838ffc8-ef4f-428d-9280-74fa0c491666">https://github.com/user-attachments/assets/e838ffc8-ef4f-428d-9280-74fa0c491666" /> closes #17333 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved validation and error reporting for template presets, providing clearer feedback when presets cannot be parsed or reference undefined parameters. * **Bug Fixes** * Enhanced error handling during template version creation to better detect and report issues with presets. * **Tests** * Added new tests to verify validation of both valid and invalid Terraform presets during template version creation. * Improved test reliability by enabling dynamic control over error injection in database-related tests. * **Chores** * Updated a dependency to the latest version for improved stability and features. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 26d232d commit f256a23

File tree

5 files changed

+161
-0
lines changed

5 files changed

+161
-0
lines changed

‎coderd/dynamicparameters/error.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ func tagValidationError(diags hcl.Diagnostics) *DiagnosticError {
2626
}
2727
}
2828

29+
func presetValidationError(diags hcl.Diagnostics) *DiagnosticError {
30+
return &DiagnosticError{
31+
Message: "Unable to validate presets",
32+
Diagnostics: diags,
33+
KeyedDiagnostics: make(map[string]hcl.Diagnostics),
34+
}
35+
}
36+
2937
type DiagnosticError struct {
3038
// Message is the human-readable message that will be returned to the user.
3139
Message string

‎coderd/dynamicparameters/presets.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package dynamicparameters
2+
3+
import (
4+
"github.com/hashicorp/hcl/v2"
5+
6+
"github.com/coder/preview"
7+
)
8+
9+
// CheckPresets extracts the preset related diagnostics from a template version preset
10+
func CheckPresets(output *preview.Output, diags hcl.Diagnostics) *DiagnosticError {
11+
de := presetValidationError(diags)
12+
if output == nil {
13+
return de
14+
}
15+
16+
presets := output.Presets
17+
for _, preset := range presets {
18+
if hcl.Diagnostics(preset.Diagnostics).HasErrors() {
19+
de.Extend(preset.Name, hcl.Diagnostics(preset.Diagnostics))
20+
}
21+
}
22+
23+
if de.HasError() {
24+
return de
25+
}
26+
27+
return nil
28+
}

‎coderd/dynamicparameters/tags.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ import (
1111

1212
func CheckTags(output *preview.Output, diags hcl.Diagnostics) *DiagnosticError {
1313
de := tagValidationError(diags)
14+
if output == nil {
15+
return de
16+
}
17+
1418
failedTags := output.WorkspaceTags.UnusableTags()
1519
if len(failedTags) == 0 && !de.HasError() {
1620
return nil // No errors, all is good!

‎coderd/templateversions.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,6 +1822,14 @@ func (api *API) dynamicTemplateVersionTags(ctx context.Context, rw http.Response
18221822
return nil, false
18231823
}
18241824

1825+
// Fails early if presets are invalid to prevent downstream workspace creation errors
1826+
presetErr := dynamicparameters.CheckPresets(output, nil)
1827+
if presetErr != nil {
1828+
code, resp := presetErr.Response()
1829+
httpapi.Write(ctx, rw, code, resp)
1830+
return nil, false
1831+
}
1832+
18251833
return output.WorkspaceTags.Tags(), true
18261834
}
18271835

‎coderd/templateversions_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,119 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
620620
})
621621
}
622622
})
623+
624+
t.Run("Presets", func(t *testing.T) {
625+
t.Parallel()
626+
store, ps := dbtestutil.NewDB(t)
627+
client := coderdtest.New(t, &coderdtest.Options{
628+
Database: store,
629+
Pubsub: ps,
630+
})
631+
owner := coderdtest.CreateFirstUser(t, client)
632+
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
633+
634+
for _, tt := range []struct {
635+
name string
636+
files map[string]string
637+
expectError string
638+
}{
639+
{
640+
name: "valid preset",
641+
files: map[string]string{
642+
`main.tf`: `
643+
terraform {
644+
required_providers {
645+
coder = {
646+
source = "coder/coder"
647+
version = "2.8.0"
648+
}
649+
}
650+
}
651+
data "coder_parameter" "valid_parameter" {
652+
name = "valid_parameter_name"
653+
default = "valid_option_value"
654+
option {
655+
name = "valid_option_name"
656+
value = "valid_option_value"
657+
}
658+
}
659+
data "coder_workspace_preset" "valid_preset" {
660+
name = "valid_preset"
661+
parameters = {
662+
"valid_parameter_name" = "valid_option_value"
663+
}
664+
}
665+
`,
666+
},
667+
},
668+
{
669+
name: "invalid preset",
670+
files: map[string]string{
671+
`main.tf`: `
672+
terraform {
673+
required_providers {
674+
coder = {
675+
source = "coder/coder"
676+
version = "2.8.0"
677+
}
678+
}
679+
}
680+
data "coder_parameter" "valid_parameter" {
681+
name = "valid_parameter_name"
682+
default = "valid_option_value"
683+
option {
684+
name = "valid_option_name"
685+
value = "valid_option_value"
686+
}
687+
}
688+
data "coder_workspace_preset" "invalid_parameter_name" {
689+
name = "invalid_parameter_name"
690+
parameters = {
691+
"invalid_parameter_name" = "irrelevant_value"
692+
}
693+
}
694+
`,
695+
},
696+
expectError: "Undefined Parameter",
697+
},
698+
} {
699+
t.Run(tt.name, func(t *testing.T) {
700+
t.Parallel()
701+
ctx := testutil.Context(t, testutil.WaitShort)
702+
703+
// Create an archive from the files provided in the test case.
704+
tarFile := testutil.CreateTar(t, tt.files)
705+
706+
// Post the archive file
707+
fi, err := templateAdmin.Upload(ctx, "application/x-tar", bytes.NewReader(tarFile))
708+
require.NoError(t, err)
709+
710+
// Create a template version from the archive
711+
tvName := testutil.GetRandomNameHyphenated(t)
712+
tv, err := templateAdmin.CreateTemplateVersion(ctx, owner.OrganizationID, codersdk.CreateTemplateVersionRequest{
713+
Name: tvName,
714+
StorageMethod: codersdk.ProvisionerStorageMethodFile,
715+
Provisioner: codersdk.ProvisionerTypeTerraform,
716+
FileID: fi.ID,
717+
})
718+
719+
if tt.expectError == "" {
720+
require.NoError(t, err)
721+
// Assert the expected provisioner job is created from the template version import
722+
pj, err := store.GetProvisionerJobByID(ctx, tv.Job.ID)
723+
require.NoError(t, err)
724+
require.NotNil(t, pj)
725+
// Also assert that we get the expected information back from the API endpoint
726+
require.Zero(t, tv.MatchedProvisioners.Count)
727+
require.Zero(t, tv.MatchedProvisioners.Available)
728+
require.Zero(t, tv.MatchedProvisioners.MostRecentlySeen.Time)
729+
} else {
730+
require.ErrorContains(t, err, tt.expectError)
731+
require.Equal(t, tv.Job.ID, uuid.Nil)
732+
}
733+
})
734+
}
735+
})
623736
}
624737

625738
func TestPatchCancelTemplateVersion(t *testing.T) {

0 commit comments

Comments
 (0)