feat: Validate workspace build parameters (#5807)

This commit is contained in:
Marcin Tojek
2023-01-24 14:22:00 +01:00
committed by GitHub
parent 138887de7e
commit 26c69525d1
23 changed files with 1051 additions and 268 deletions

View File

@ -2650,6 +2650,7 @@ func (q *fakeQuerier) InsertTemplateVersionParameter(_ context.Context, arg data
DefaultValue: arg.DefaultValue,
Icon: arg.Icon,
Options: arg.Options,
ValidationError: arg.ValidationError,
ValidationRegex: arg.ValidationRegex,
ValidationMin: arg.ValidationMin,
ValidationMax: arg.ValidationMax,

View File

@ -350,7 +350,8 @@ CREATE TABLE template_version_parameters (
options jsonb DEFAULT '[]'::jsonb NOT NULL,
validation_regex text NOT NULL,
validation_min integer NOT NULL,
validation_max integer NOT NULL
validation_max integer NOT NULL,
validation_error text DEFAULT ''::text NOT NULL
);
COMMENT ON COLUMN template_version_parameters.name IS 'Parameter name';
@ -373,6 +374,8 @@ COMMENT ON COLUMN template_version_parameters.validation_min IS 'Validation: min
COMMENT ON COLUMN template_version_parameters.validation_max IS 'Validation: maximum length of value';
COMMENT ON COLUMN template_version_parameters.validation_error IS 'Validation: error displayed when the regex does not match.';
CREATE TABLE template_versions (
id uuid NOT NULL,
template_id uuid,

View File

@ -0,0 +1 @@
ALTER TABLE template_version_parameters DROP COLUMN validation_error;

View File

@ -0,0 +1,4 @@
ALTER TABLE template_version_parameters ADD COLUMN validation_error text NOT NULL DEFAULT '';
COMMENT ON COLUMN template_version_parameters.validation_error
IS 'Validation: error displayed when the regex does not match.';

View File

@ -1448,6 +1448,8 @@ type TemplateVersionParameter struct {
ValidationMin int32 `db:"validation_min" json:"validation_min"`
// Validation: maximum length of value
ValidationMax int32 `db:"validation_max" json:"validation_max"`
// Validation: error displayed when the regex does not match.
ValidationError string `db:"validation_error" json:"validation_error"`
}
type User struct {

View File

@ -3543,7 +3543,7 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl
}
const getTemplateVersionParameters = `-- name: GetTemplateVersionParameters :many
SELECT template_version_id, name, description, type, mutable, default_value, icon, options, validation_regex, validation_min, validation_max FROM template_version_parameters WHERE template_version_id = $1
SELECT template_version_id, name, description, type, mutable, default_value, icon, options, validation_regex, validation_min, validation_max, validation_error FROM template_version_parameters WHERE template_version_id = $1
`
func (q *sqlQuerier) GetTemplateVersionParameters(ctx context.Context, templateVersionID uuid.UUID) ([]TemplateVersionParameter, error) {
@ -3567,6 +3567,7 @@ func (q *sqlQuerier) GetTemplateVersionParameters(ctx context.Context, templateV
&i.ValidationRegex,
&i.ValidationMin,
&i.ValidationMax,
&i.ValidationError,
); err != nil {
return nil, err
}
@ -3594,7 +3595,8 @@ INSERT INTO
options,
validation_regex,
validation_min,
validation_max
validation_max,
validation_error
)
VALUES
(
@ -3608,8 +3610,9 @@ VALUES
$8,
$9,
$10,
$11
) RETURNING template_version_id, name, description, type, mutable, default_value, icon, options, validation_regex, validation_min, validation_max
$11,
$12
) RETURNING template_version_id, name, description, type, mutable, default_value, icon, options, validation_regex, validation_min, validation_max, validation_error
`
type InsertTemplateVersionParameterParams struct {
@ -3624,6 +3627,7 @@ type InsertTemplateVersionParameterParams struct {
ValidationRegex string `db:"validation_regex" json:"validation_regex"`
ValidationMin int32 `db:"validation_min" json:"validation_min"`
ValidationMax int32 `db:"validation_max" json:"validation_max"`
ValidationError string `db:"validation_error" json:"validation_error"`
}
func (q *sqlQuerier) InsertTemplateVersionParameter(ctx context.Context, arg InsertTemplateVersionParameterParams) (TemplateVersionParameter, error) {
@ -3639,6 +3643,7 @@ func (q *sqlQuerier) InsertTemplateVersionParameter(ctx context.Context, arg Ins
arg.ValidationRegex,
arg.ValidationMin,
arg.ValidationMax,
arg.ValidationError,
)
var i TemplateVersionParameter
err := row.Scan(
@ -3653,6 +3658,7 @@ func (q *sqlQuerier) InsertTemplateVersionParameter(ctx context.Context, arg Ins
&i.ValidationRegex,
&i.ValidationMin,
&i.ValidationMax,
&i.ValidationError,
)
return i, err
}

View File

@ -11,7 +11,8 @@ INSERT INTO
options,
validation_regex,
validation_min,
validation_max
validation_max,
validation_error
)
VALUES
(
@ -25,7 +26,8 @@ VALUES
$8,
$9,
$10,
$11
$11,
$12
) RETURNING *;
-- name: GetTemplateVersionParameters :many

View File

@ -648,6 +648,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
Icon: richParameter.Icon,
Options: options,
ValidationRegex: richParameter.ValidationRegex,
ValidationError: richParameter.ValidationError,
ValidationMin: richParameter.ValidationMin,
ValidationMax: richParameter.ValidationMax,
})

View File

@ -223,7 +223,7 @@ func (api *API) templateVersionRichParameters(rw http.ResponseWriter, r *http.Re
})
return
}
dbParameters, err := api.Database.GetTemplateVersionParameters(ctx, templateVersion.ID)
dbTemplateVersionParameters, err := api.Database.GetTemplateVersionParameters(ctx, templateVersion.ID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template version parameters.",
@ -231,19 +231,16 @@ func (api *API) templateVersionRichParameters(rw http.ResponseWriter, r *http.Re
})
return
}
params := make([]codersdk.TemplateVersionParameter, 0)
for _, dbParameter := range dbParameters {
param, err := convertTemplateVersionParameter(dbParameter)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting template version parameter.",
Detail: err.Error(),
})
return
}
params = append(params, param)
templateVersionParameters, err := convertTemplateVersionParameters(dbTemplateVersionParameters)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting template version parameter.",
Detail: err.Error(),
})
return
}
httpapi.Write(ctx, rw, http.StatusOK, params)
httpapi.Write(ctx, rw, http.StatusOK, templateVersionParameters)
}
// @Summary Get parameters by template version
@ -1379,6 +1376,18 @@ func convertTemplateVersion(version database.TemplateVersion, job codersdk.Provi
}
}
func convertTemplateVersionParameters(dbParams []database.TemplateVersionParameter) ([]codersdk.TemplateVersionParameter, error) {
params := make([]codersdk.TemplateVersionParameter, 0)
for _, dbParameter := range dbParams {
param, err := convertTemplateVersionParameter(dbParameter)
if err != nil {
return nil, err
}
params = append(params, param)
}
return params, nil
}
func convertTemplateVersionParameter(param database.TemplateVersionParameter) (codersdk.TemplateVersionParameter, error) {
var protoOptions []*sdkproto.RichParameterOption
err := json.Unmarshal(param.Options, &protoOptions)
@ -1405,6 +1414,7 @@ func convertTemplateVersionParameter(param database.TemplateVersionParameter) (c
ValidationRegex: param.ValidationRegex,
ValidationMin: param.ValidationMin,
ValidationMax: param.ValidationMax,
ValidationError: param.ValidationError,
}, nil
}

View File

@ -449,7 +449,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
state = priorHistory.ProvisionerState
}
templateVersionParameters, err := api.Database.GetTemplateVersionParameters(ctx, createBuild.TemplateVersionID)
dbTemplateVersionParameters, err := api.Database.GetTemplateVersionParameters(ctx, createBuild.TemplateVersionID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template version parameters.",
@ -457,6 +457,23 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
})
return
}
templateVersionParameters, err := convertTemplateVersionParameters(dbTemplateVersionParameters)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting template version parameters.",
Detail: err.Error(),
})
return
}
err = codersdk.ValidateWorkspaceBuildParameters(templateVersionParameters, createBuild.RichParameterValues)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Error validating workspace build parameters.",
Detail: err.Error(),
})
return
}
lastBuildParameters, err := api.Database.GetWorkspaceBuildParameters(ctx, priorHistory.ID)
if err != nil {

View File

@ -780,3 +780,172 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
require.Error(t, err)
})
}
func TestWorkspaceBuildValidateRichParameters(t *testing.T) {
t.Parallel()
const (
stringParameterName = "string_parameter"
stringParameterValue = "abc"
numberParameterName = "number_parameter"
numberParameterValue = "7"
boolParameterName = "bool_parameter"
boolParameterValue = "true"
)
initialBuildParameters := []codersdk.WorkspaceBuildParameter{
{Name: stringParameterName, Value: stringParameterValue},
{Name: numberParameterName, Value: numberParameterValue},
{Name: boolParameterName, Value: boolParameterValue},
}
prepareEchoResponses := func(richParameters []*proto.RichParameter) *echo.Responses {
return &echo.Responses{
Parse: echo.ParseComplete,
ProvisionPlan: []*proto.Provision_Response{
{
Type: &proto.Provision_Response_Complete{
Complete: &proto.Provision_Complete{
Parameters: richParameters,
},
},
}},
ProvisionApply: []*proto.Provision_Response{
{
Type: &proto.Provision_Response_Complete{
Complete: &proto.Provision_Complete{},
},
},
},
}
}
t.Run("NoValidation", func(t *testing.T) {
t.Parallel()
richParameters := []*proto.RichParameter{
{Name: stringParameterName, Type: "string", Mutable: true},
{Name: numberParameterName, Type: "number", Mutable: true},
{Name: boolParameterName, Type: "bool", Mutable: true},
}
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, prepareEchoResponses(richParameters))
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.RichParameterValues = initialBuildParameters
})
workspaceBuild := coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
require.Equal(t, codersdk.WorkspaceStatusRunning, workspaceBuild.Status)
// Update build parameters
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
nextBuildParameters := []codersdk.WorkspaceBuildParameter{
{Name: numberParameterName, Value: "42"},
}
nextWorkspaceBuild, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
Transition: codersdk.WorkspaceTransitionStart,
RichParameterValues: nextBuildParameters,
})
require.NoError(t, err)
coderdtest.AwaitWorkspaceBuildJob(t, client, nextWorkspaceBuild.ID)
_, err = client.WorkspaceBuildParameters(ctx, nextWorkspaceBuild.ID)
require.NoError(t, err)
})
t.Run("Validation", func(t *testing.T) {
t.Parallel()
numberRichParameters := []*proto.RichParameter{
{Name: stringParameterName, Type: "string", Mutable: true},
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: 3, ValidationMax: 10},
{Name: boolParameterName, Type: "bool", Mutable: true},
}
stringRichParameters := []*proto.RichParameter{
{Name: stringParameterName, Type: "string", Mutable: true},
{Name: numberParameterName, Type: "number", Mutable: true},
{Name: boolParameterName, Type: "bool", Mutable: true},
}
boolRichParameters := []*proto.RichParameter{
{Name: stringParameterName, Type: "string", Mutable: true},
{Name: numberParameterName, Type: "number", Mutable: true},
{Name: boolParameterName, Type: "bool", Mutable: true},
}
regexRichParameters := []*proto.RichParameter{
{Name: stringParameterName, Type: "string", Mutable: true, ValidationRegex: "^[a-z]+$", ValidationError: "this is error"},
{Name: numberParameterName, Type: "number", Mutable: true},
{Name: boolParameterName, Type: "bool", Mutable: true},
}
tests := []struct {
parameterName string
value string
valid bool
richParameters []*proto.RichParameter
}{
{numberParameterName, "2", false, numberRichParameters},
{numberParameterName, "3", true, numberRichParameters},
{numberParameterName, "10", true, numberRichParameters},
{numberParameterName, "11", false, numberRichParameters},
{stringParameterName, "", false, stringRichParameters},
{stringParameterName, "foobar", true, stringRichParameters},
{stringParameterName, "abcd", true, regexRichParameters},
{stringParameterName, "abcd1", false, regexRichParameters},
{boolParameterName, "true", true, boolRichParameters},
{boolParameterName, "false", true, boolRichParameters},
{boolParameterName, "cat", false, boolRichParameters},
}
for _, tc := range tests {
t.Run(tc.parameterName+"-"+tc.value, func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, prepareEchoResponses(tc.richParameters))
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.RichParameterValues = initialBuildParameters
})
workspaceBuild := coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
require.Equal(t, codersdk.WorkspaceStatusRunning, workspaceBuild.Status)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
nextBuildParameters := []codersdk.WorkspaceBuildParameter{
{Name: tc.parameterName, Value: tc.value},
}
nextWorkspaceBuild, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
Transition: codersdk.WorkspaceTransitionStart,
RichParameterValues: nextBuildParameters,
})
if tc.valid {
require.NoError(t, err)
coderdtest.AwaitWorkspaceBuildJob(t, client, nextWorkspaceBuild.ID)
} else {
require.Error(t, err)
}
})
}
})
}

View File

@ -390,6 +390,34 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
})
return
}
dbTemplateVersionParameters, err := api.Database.GetTemplateVersionParameters(ctx, templateVersion.ID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template version parameters.",
Detail: err.Error(),
})
return
}
templateVersionParameters, err := convertTemplateVersionParameters(dbTemplateVersionParameters)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting template version parameters.",
Detail: err.Error(),
})
return
}
err = codersdk.ValidateWorkspaceBuildParameters(templateVersionParameters, createWorkspace.RichParameterValues)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Error validating workspace build parameters.",
Detail: err.Error(),
})
return
}
templateVersionJob, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{