mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
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="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="https://github.com/user-attachments/assets/e838ffc8-ef4f-428d-9280-74fa0c491666" /> closes https://github.com/coder/coder/issues/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 -->
This commit is contained in:
@@ -26,6 +26,14 @@ func tagValidationError(diags hcl.Diagnostics) *DiagnosticError {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func presetValidationError(diags hcl.Diagnostics) *DiagnosticError {
|
||||||
|
return &DiagnosticError{
|
||||||
|
Message: "Unable to validate presets",
|
||||||
|
Diagnostics: diags,
|
||||||
|
KeyedDiagnostics: make(map[string]hcl.Diagnostics),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
type DiagnosticError struct {
|
type DiagnosticError struct {
|
||||||
// Message is the human-readable message that will be returned to the user.
|
// Message is the human-readable message that will be returned to the user.
|
||||||
Message string
|
Message string
|
||||||
|
|||||||
@@ -0,0 +1,28 @@
|
|||||||
|
package dynamicparameters
|
||||||
|
|
||||||
|
import (
|
||||||
|
"github.com/hashicorp/hcl/v2"
|
||||||
|
|
||||||
|
"github.com/coder/preview"
|
||||||
|
)
|
||||||
|
|
||||||
|
// CheckPresets extracts the preset related diagnostics from a template version preset
|
||||||
|
func CheckPresets(output *preview.Output, diags hcl.Diagnostics) *DiagnosticError {
|
||||||
|
de := presetValidationError(diags)
|
||||||
|
if output == nil {
|
||||||
|
return de
|
||||||
|
}
|
||||||
|
|
||||||
|
presets := output.Presets
|
||||||
|
for _, preset := range presets {
|
||||||
|
if hcl.Diagnostics(preset.Diagnostics).HasErrors() {
|
||||||
|
de.Extend(preset.Name, hcl.Diagnostics(preset.Diagnostics))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if de.HasError() {
|
||||||
|
return de
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
@@ -11,6 +11,10 @@ import (
|
|||||||
|
|
||||||
func CheckTags(output *preview.Output, diags hcl.Diagnostics) *DiagnosticError {
|
func CheckTags(output *preview.Output, diags hcl.Diagnostics) *DiagnosticError {
|
||||||
de := tagValidationError(diags)
|
de := tagValidationError(diags)
|
||||||
|
if output == nil {
|
||||||
|
return de
|
||||||
|
}
|
||||||
|
|
||||||
failedTags := output.WorkspaceTags.UnusableTags()
|
failedTags := output.WorkspaceTags.UnusableTags()
|
||||||
if len(failedTags) == 0 && !de.HasError() {
|
if len(failedTags) == 0 && !de.HasError() {
|
||||||
return nil // No errors, all is good!
|
return nil // No errors, all is good!
|
||||||
|
|||||||
@@ -1822,6 +1822,14 @@ func (api *API) dynamicTemplateVersionTags(ctx context.Context, rw http.Response
|
|||||||
return nil, false
|
return nil, false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Fails early if presets are invalid to prevent downstream workspace creation errors
|
||||||
|
presetErr := dynamicparameters.CheckPresets(output, nil)
|
||||||
|
if presetErr != nil {
|
||||||
|
code, resp := presetErr.Response()
|
||||||
|
httpapi.Write(ctx, rw, code, resp)
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
|
|
||||||
return output.WorkspaceTags.Tags(), true
|
return output.WorkspaceTags.Tags(), true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -620,6 +620,119 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("Presets", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
store, ps := dbtestutil.NewDB(t)
|
||||||
|
client := coderdtest.New(t, &coderdtest.Options{
|
||||||
|
Database: store,
|
||||||
|
Pubsub: ps,
|
||||||
|
})
|
||||||
|
owner := coderdtest.CreateFirstUser(t, client)
|
||||||
|
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
|
||||||
|
|
||||||
|
for _, tt := range []struct {
|
||||||
|
name string
|
||||||
|
files map[string]string
|
||||||
|
expectError string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "valid preset",
|
||||||
|
files: map[string]string{
|
||||||
|
`main.tf`: `
|
||||||
|
terraform {
|
||||||
|
required_providers {
|
||||||
|
coder = {
|
||||||
|
source = "coder/coder"
|
||||||
|
version = "2.8.0"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
data "coder_parameter" "valid_parameter" {
|
||||||
|
name = "valid_parameter_name"
|
||||||
|
default = "valid_option_value"
|
||||||
|
option {
|
||||||
|
name = "valid_option_name"
|
||||||
|
value = "valid_option_value"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
data "coder_workspace_preset" "valid_preset" {
|
||||||
|
name = "valid_preset"
|
||||||
|
parameters = {
|
||||||
|
"valid_parameter_name" = "valid_option_value"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "invalid preset",
|
||||||
|
files: map[string]string{
|
||||||
|
`main.tf`: `
|
||||||
|
terraform {
|
||||||
|
required_providers {
|
||||||
|
coder = {
|
||||||
|
source = "coder/coder"
|
||||||
|
version = "2.8.0"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
data "coder_parameter" "valid_parameter" {
|
||||||
|
name = "valid_parameter_name"
|
||||||
|
default = "valid_option_value"
|
||||||
|
option {
|
||||||
|
name = "valid_option_name"
|
||||||
|
value = "valid_option_value"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
data "coder_workspace_preset" "invalid_parameter_name" {
|
||||||
|
name = "invalid_parameter_name"
|
||||||
|
parameters = {
|
||||||
|
"invalid_parameter_name" = "irrelevant_value"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
},
|
||||||
|
expectError: "Undefined Parameter",
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
ctx := testutil.Context(t, testutil.WaitShort)
|
||||||
|
|
||||||
|
// Create an archive from the files provided in the test case.
|
||||||
|
tarFile := testutil.CreateTar(t, tt.files)
|
||||||
|
|
||||||
|
// Post the archive file
|
||||||
|
fi, err := templateAdmin.Upload(ctx, "application/x-tar", bytes.NewReader(tarFile))
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Create a template version from the archive
|
||||||
|
tvName := testutil.GetRandomNameHyphenated(t)
|
||||||
|
tv, err := templateAdmin.CreateTemplateVersion(ctx, owner.OrganizationID, codersdk.CreateTemplateVersionRequest{
|
||||||
|
Name: tvName,
|
||||||
|
StorageMethod: codersdk.ProvisionerStorageMethodFile,
|
||||||
|
Provisioner: codersdk.ProvisionerTypeTerraform,
|
||||||
|
FileID: fi.ID,
|
||||||
|
})
|
||||||
|
|
||||||
|
if tt.expectError == "" {
|
||||||
|
require.NoError(t, err)
|
||||||
|
// Assert the expected provisioner job is created from the template version import
|
||||||
|
pj, err := store.GetProvisionerJobByID(ctx, tv.Job.ID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, pj)
|
||||||
|
// Also assert that we get the expected information back from the API endpoint
|
||||||
|
require.Zero(t, tv.MatchedProvisioners.Count)
|
||||||
|
require.Zero(t, tv.MatchedProvisioners.Available)
|
||||||
|
require.Zero(t, tv.MatchedProvisioners.MostRecentlySeen.Time)
|
||||||
|
} else {
|
||||||
|
require.ErrorContains(t, err, tt.expectError)
|
||||||
|
require.Equal(t, tv.Job.ID, uuid.Nil)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestPatchCancelTemplateVersion(t *testing.T) {
|
func TestPatchCancelTemplateVersion(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user