fix: use dynamic parameter resolution in the cli (#21734)

Uses dynamic parameters EvaluateTemplateVersion vs TemplateVersionRichParameters to determine initial parameter state.

Closes https://github.com/coder/coder/issues/19879
This commit is contained in:
Steven Masley
2026-02-03 14:10:49 -06:00
committed by GitHub
parent 6759b51cd6
commit b1e18f2398
9 changed files with 498 additions and 20 deletions
+5 -1
View File
@@ -69,7 +69,7 @@ func RichParameter(inv *serpent.Invocation, templateVersionParameter codersdk.Te
}
default:
text := "Enter a value"
if !templateVersionParameter.Required {
if defaultValue != "" {
text += fmt.Sprintf(" (default: %q)", defaultValue)
}
text += ":"
@@ -77,6 +77,10 @@ func RichParameter(inv *serpent.Invocation, templateVersionParameter codersdk.Te
value, err = Prompt(inv, PromptOptions{
Text: Bold(text),
Validate: func(value string) error {
// If empty, the default value will be used (if available).
if value == "" && defaultValue != "" {
value = defaultValue
}
return validateRichPrompt(value, templateVersionParameter)
},
})
+50 -3
View File
@@ -323,6 +323,7 @@ func (r *RootCmd) Create(opts CreateOptions) *serpent.Command {
Action: WorkspaceCreate,
TemplateVersionID: templateVersionID,
NewWorkspaceName: workspaceName,
Owner: workspaceOwner,
PresetParameters: presetParameters,
RichParameterFile: parameterFlags.richParameterFile,
@@ -456,6 +457,8 @@ type prepWorkspaceBuildArgs struct {
Action WorkspaceCLIAction
TemplateVersionID uuid.UUID
NewWorkspaceName string
// The owner is required when evaluating dynamic parameters
Owner string
LastBuildParameters []codersdk.WorkspaceBuildParameter
SourceWorkspaceParameters []codersdk.WorkspaceBuildParameter
@@ -550,9 +553,14 @@ func prepWorkspaceBuild(inv *serpent.Invocation, client *codersdk.Client, args p
return nil, xerrors.Errorf("get template version: %w", err)
}
templateVersionParameters, err := client.TemplateVersionRichParameters(inv.Context(), templateVersion.ID)
if err != nil {
return nil, xerrors.Errorf("get template version rich parameters: %w", err)
dynamicParameters := true
if templateVersion.TemplateID != nil {
// TODO: This fetch is often redundant, as the caller often has the template already.
template, err := client.Template(ctx, *templateVersion.TemplateID)
if err != nil {
return nil, xerrors.Errorf("get template: %w", err)
}
dynamicParameters = !template.UseClassicParameterFlow
}
parameterFile := map[string]string{}
@@ -574,6 +582,45 @@ func prepWorkspaceBuild(inv *serpent.Invocation, client *codersdk.Client, args p
WithRichParametersFile(parameterFile).
WithRichParametersDefaults(args.RichParameterDefaults).
WithUseParameterDefaults(args.UseParameterDefaults)
var templateVersionParameters []codersdk.TemplateVersionParameter
if !dynamicParameters {
templateVersionParameters, err = client.TemplateVersionRichParameters(inv.Context(), templateVersion.ID)
if err != nil {
return nil, xerrors.Errorf("get template version rich parameters: %w", err)
}
} else {
var ownerID uuid.UUID
{ // Putting in its own block to limit scope of owningMember, as it might be nil
owningMember, err := client.OrganizationMember(ctx, templateVersion.OrganizationID.String(), args.Owner)
if err != nil {
// This is unfortunate, but if we are an org owner, then we can create workspaces
// for users that are not part of the organization.
owningUser, uerr := client.User(ctx, args.Owner)
if uerr != nil {
return nil, xerrors.Errorf("get owning member: %w", err)
}
ownerID = owningUser.ID
} else {
ownerID = owningMember.UserID
}
}
initial := make(map[string]string)
for _, v := range resolver.InitialValues() {
initial[v.Name] = v.Value
}
eval, err := client.EvaluateTemplateVersion(ctx, templateVersion.ID, ownerID, initial)
if err != nil {
return nil, xerrors.Errorf("evaluate template version dynamic parameters: %w", err)
}
for _, param := range eval.Parameters {
templateVersionParameters = append(templateVersionParameters, param.TemplateVersionParameter())
}
}
buildParameters, err := resolver.Resolve(inv, args.Action, templateVersionParameters)
if err != nil {
return nil, err
+303
View File
@@ -24,6 +24,309 @@ import (
"github.com/coder/coder/v2/testutil"
)
func TestCreateDynamic(t *testing.T) {
t.Parallel()
owner := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
first := coderdtest.CreateFirstUser(t, owner)
member, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID)
// Terraform template with conditional parameters.
// The "region" parameter only appears when "enable_region" is true.
const conditionalParamTF = `
terraform {
required_providers {
coder = {
source = "coder/coder"
}
}
}
data "coder_workspace_owner" "me" {}
data "coder_parameter" "enable_region" {
name = "enable_region"
order = 1
type = "bool"
default = "false"
}
data "coder_parameter" "region" {
name = "region"
count = data.coder_parameter.enable_region.value == "true" ? 1 : 0
order = 2
type = "string"
# No default - this makes it required when it appears
}
`
// Test conditional parameters: a parameter that only appears when another
// parameter has a certain value.
t.Run("ConditionalParam", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
template, _ := coderdtest.DynamicParameterTemplate(t, owner, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{
MainTF: conditionalParamTF,
})
// Test 1: Create without enabling region - region param should not exist
args := []string{
"create", "ws-no-region",
"--template", template.Name,
"--parameter", "enable_region=false",
"-y",
}
inv, root := clitest.New(t, args...)
clitest.SetupConfig(t, member, root)
pty := ptytest.New(t).Attach(inv)
doneChan := make(chan error)
go func() {
doneChan <- inv.Run()
}()
pty.ExpectMatchContext(ctx, "has been created")
err := testutil.RequireReceive(ctx, t, doneChan)
require.NoError(t, err)
// Verify workspace created with only enable_region parameter
ws, err := member.WorkspaceByOwnerAndName(t.Context(), codersdk.Me, "ws-no-region", codersdk.WorkspaceOptions{})
require.NoError(t, err)
buildParams, err := member.WorkspaceBuildParameters(t.Context(), ws.LatestBuild.ID)
require.NoError(t, err)
require.Len(t, buildParams, 1, "expected only enable_region parameter when enable_region=false")
require.Contains(t, buildParams, codersdk.WorkspaceBuildParameter{Name: "enable_region", Value: "false"})
// Test 2: Create with region enabled - region param should exist
args = []string{
"create", "ws-with-region",
"--template", template.Name,
"--parameter", "enable_region=true",
"--parameter", "region=us-east",
"-y",
}
inv, root = clitest.New(t, args...)
clitest.SetupConfig(t, member, root)
pty = ptytest.New(t).Attach(inv)
doneChan = make(chan error)
go func() {
doneChan <- inv.Run()
}()
pty.ExpectMatchContext(ctx, "has been created")
err = testutil.RequireReceive(ctx, t, doneChan)
require.NoError(t, err)
// Verify workspace created with both parameters
ws, err = member.WorkspaceByOwnerAndName(t.Context(), codersdk.Me, "ws-with-region", codersdk.WorkspaceOptions{})
require.NoError(t, err)
buildParams, err = member.WorkspaceBuildParameters(t.Context(), ws.LatestBuild.ID)
require.NoError(t, err)
require.Len(t, buildParams, 2, "expected both enable_region and region parameters when enable_region=true")
require.Contains(t, buildParams, codersdk.WorkspaceBuildParameter{Name: "enable_region", Value: "true"})
require.Contains(t, buildParams, codersdk.WorkspaceBuildParameter{Name: "region", Value: "us-east"})
})
// Test that the CLI prompts for missing conditional parameters.
// When enable_region=true, the region parameter becomes required and CLI should prompt.
t.Run("PromptForConditionalParam", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
template, _ := coderdtest.DynamicParameterTemplate(t, owner, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{
MainTF: conditionalParamTF,
})
// Only provide enable_region=true, don't provide region - CLI should prompt for it
args := []string{
"create", "ws-prompted",
"--template", template.Name,
"--parameter", "enable_region=true",
}
inv, root := clitest.New(t, args...)
clitest.SetupConfig(t, member, root)
pty := ptytest.New(t).Attach(inv)
doneChan := make(chan error)
go func() {
doneChan <- inv.Run()
}()
// CLI should prompt for the region parameter since enable_region=true
pty.ExpectMatchContext(ctx, "region")
pty.WriteLine("eu-west")
// Confirm creation
pty.ExpectMatchContext(ctx, "Confirm create?")
pty.WriteLine("yes")
pty.ExpectMatchContext(ctx, "has been created")
err := <-doneChan
require.NoError(t, err)
// Verify workspace created with both parameters
ws, err := member.WorkspaceByOwnerAndName(t.Context(), codersdk.Me, "ws-prompted", codersdk.WorkspaceOptions{})
require.NoError(t, err)
buildParams, err := member.WorkspaceBuildParameters(t.Context(), ws.LatestBuild.ID)
require.NoError(t, err)
require.Len(t, buildParams, 2, "expected both enable_region and region parameters")
require.Contains(t, buildParams, codersdk.WorkspaceBuildParameter{Name: "enable_region", Value: "true"})
require.Contains(t, buildParams, codersdk.WorkspaceBuildParameter{Name: "region", Value: "eu-west"})
})
// Test that updating a template with a new required parameter causes start to fail
// when the user doesn't provide the new parameter value.
t.Run("UpdateTemplateRequiredParamStartFails", func(t *testing.T) {
t.Parallel()
// Initial template with just enable_region parameter (no default, so required)
const initialTF = `
terraform {
required_providers {
coder = {
source = "coder/coder"
}
}
}
data "coder_workspace_owner" "me" {}
data "coder_parameter" "enable_region" {
name = "enable_region"
type = "bool"
}
`
template, _ := coderdtest.DynamicParameterTemplate(t, owner, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{
MainTF: initialTF,
})
// Create workspace with initial template
inv, root := clitest.New(t, "create", "ws-update-test",
"--template", template.Name,
"--parameter", "enable_region=false",
"-y",
)
clitest.SetupConfig(t, member, root)
err := inv.Run()
require.NoError(t, err)
// Stop the workspace
inv, root = clitest.New(t, "stop", "ws-update-test", "-y")
clitest.SetupConfig(t, member, root)
err = inv.Run()
require.NoError(t, err)
const updatedTF = `
terraform {
required_providers {
coder = {
source = "coder/coder"
}
}
}
data "coder_workspace_owner" "me" {}
data "coder_parameter" "enable_region" {
name = "enable_region"
type = "bool"
}
data "coder_parameter" "region" {
count = data.coder_parameter.enable_region.value == "true" ? 1 : 0
name = "region"
type = "string"
# No default - required when enable_region is true
}
`
coderdtest.DynamicParameterTemplate(t, owner, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{
MainTF: updatedTF,
TemplateID: template.ID,
})
// Try to start the workspace with update - should fail because region is now required
// (enable_region defaults to true, making region appear, but no value provided)
// and we're using -y to skip prompts
inv, root = clitest.New(t, "start", "ws-update-test", "-y", "--parameter", "enable_region=true")
clitest.SetupConfig(t, member, root)
err = inv.Run()
require.Error(t, err, "start should fail because new required parameter 'region' is missing")
require.Contains(t, err.Error(), "region")
})
// Test that dynamic validation allows values that would be invalid with static validation.
// A slider's max value is determined by another parameter, so a value of 8 is invalid
// when max_slider=5, but valid when max_slider=10.
t.Run("DynamicValidation", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
// Template where slider's max is controlled by another parameter
const dynamicValidationTF = `
terraform {
required_providers {
coder = {
source = "coder/coder"
}
}
}
data "coder_workspace_owner" "me" {}
data "coder_parameter" "max_slider" {
name = "max_slider"
type = "number"
default = 5
}
data "coder_parameter" "slider" {
name = "slider"
type = "number"
default = 1
validation {
min = 1
max = data.coder_parameter.max_slider.value
}
}
`
template, _ := coderdtest.DynamicParameterTemplate(t, owner, first.OrganizationID, coderdtest.DynamicParameterTemplateParams{
MainTF: dynamicValidationTF,
})
// Test 1: slider=8 should fail when max_slider=5 (default)
inv, root := clitest.New(t, "create", "ws-validation-fail",
"--template", template.Name,
"--parameter", "slider=8",
"-y",
)
clitest.SetupConfig(t, member, root)
err := inv.Run()
require.Error(t, err, "slider=8 should fail when max_slider=5")
// Test 2: slider=8 should succeed when max_slider=10
inv, root = clitest.New(t, "create", "ws-validation-pass",
"--template", template.Name,
"--parameter", "max_slider=10",
"--parameter", "slider=8",
"-y",
)
clitest.SetupConfig(t, member, root)
pty := ptytest.New(t).Attach(inv)
doneChan := make(chan error)
go func() {
doneChan <- inv.Run()
}()
pty.ExpectMatchContext(ctx, "has been created")
err = <-doneChan
require.NoError(t, err, "slider=8 should succeed when max_slider=10")
// Verify workspace created with correct parameters
ws, err := member.WorkspaceByOwnerAndName(t.Context(), codersdk.Me, "ws-validation-pass", codersdk.WorkspaceOptions{})
require.NoError(t, err)
buildParams, err := member.WorkspaceBuildParameters(t.Context(), ws.LatestBuild.ID)
require.NoError(t, err)
require.Contains(t, buildParams, codersdk.WorkspaceBuildParameter{Name: "max_slider", Value: "10"})
require.Contains(t, buildParams, codersdk.WorkspaceBuildParameter{Name: "slider", Value: "8"})
})
}
func TestCreate(t *testing.T) {
t.Parallel()
t.Run("Create", func(t *testing.T) {
+3
View File
@@ -719,6 +719,7 @@ func (r *RootCmd) scaletestCreateWorkspaces() *serpent.Command {
Action: WorkspaceCreate,
TemplateVersionID: tpl.ActiveVersionID,
NewWorkspaceName: "scaletest-N", // TODO: the scaletest runner will pass in a different name here. Does this matter?
Owner: codersdk.Me,
RichParameterFile: parameterFlags.richParameterFile,
RichParameters: cliRichParameters,
@@ -1065,6 +1066,7 @@ func (r *RootCmd) scaletestWorkspaceUpdates() *serpent.Command {
richParameters, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{
Action: WorkspaceCreate,
TemplateVersionID: tpl.ActiveVersionID,
Owner: codersdk.Me,
RichParameterFile: parameterFlags.richParameterFile,
RichParameters: cliRichParameters,
@@ -1786,6 +1788,7 @@ func (r *RootCmd) scaletestAutostart() *serpent.Command {
richParameters, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{
Action: WorkspaceCreate,
TemplateVersionID: tpl.ActiveVersionID,
Owner: codersdk.Me,
RichParameterFile: parameterFlags.richParameterFile,
RichParameters: cliRichParameters,
+50 -4
View File
@@ -108,8 +108,8 @@ func (pr *ParameterResolver) Resolve(inv *serpent.Invocation, action WorkspaceCL
staged = pr.resolveWithParametersMapFile(staged)
staged = pr.resolveWithCommandLineOrEnv(staged)
staged = pr.resolveWithSourceBuildParameters(staged, templateVersionParameters)
staged = pr.resolveWithLastBuildParameters(staged, templateVersionParameters)
staged = pr.resolveWithSourceBuildParametersInParameters(staged, templateVersionParameters)
staged = pr.resolveWithLastBuildParametersInParameters(staged, templateVersionParameters)
staged = pr.resolveWithPreset(staged) // Preset parameters take precedence from all other parameters
if err = pr.verifyConstraints(staged, action, templateVersionParameters); err != nil {
return nil, err
@@ -120,6 +120,18 @@ func (pr *ParameterResolver) Resolve(inv *serpent.Invocation, action WorkspaceCL
return staged, nil
}
func (pr *ParameterResolver) InitialValues() []codersdk.WorkspaceBuildParameter {
var staged []codersdk.WorkspaceBuildParameter
staged = pr.resolveWithParametersMapFile(staged)
staged = pr.resolveWithCommandLineOrEnv(staged)
staged = pr.resolveWithSourceBuildParameters(staged)
staged = pr.resolveWithLastBuildParameters(staged)
staged = pr.resolveWithPreset(staged) // Preset parameters take precedence from all other parameters
return staged
}
func (pr *ParameterResolver) resolveWithPreset(resolved []codersdk.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter {
next:
for _, presetParameter := range pr.presetParameters {
@@ -180,7 +192,26 @@ nextEphemeralParameter:
return resolved
}
func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter {
func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter {
if pr.promptRichParameters {
return resolved // don't pull parameters from last build
}
next:
for _, buildParameter := range pr.lastBuildParameters {
for i, r := range resolved {
if r.Name == buildParameter.Name {
resolved[i].Value = buildParameter.Value
continue next
}
}
resolved = append(resolved, buildParameter)
}
return resolved
}
func (pr *ParameterResolver) resolveWithLastBuildParametersInParameters(resolved []codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter {
if pr.promptRichParameters {
return resolved // don't pull parameters from last build
}
@@ -216,7 +247,22 @@ next:
return resolved
}
func (pr *ParameterResolver) resolveWithSourceBuildParameters(resolved []codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter {
func (pr *ParameterResolver) resolveWithSourceBuildParameters(resolved []codersdk.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter {
next:
for _, buildParameter := range pr.sourceWorkspaceParameters {
for i, r := range resolved {
if r.Name == buildParameter.Name {
resolved[i].Value = buildParameter.Value
continue next
}
}
resolved = append(resolved, buildParameter)
}
return resolved
}
func (pr *ParameterResolver) resolveWithSourceBuildParametersInParameters(resolved []codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter {
next:
for _, buildParameter := range pr.sourceWorkspaceParameters {
tvp := findTemplateVersionParameter(buildParameter, templateVersionParameters)
+1
View File
@@ -152,6 +152,7 @@ func buildWorkspaceStartRequest(inv *serpent.Invocation, client *codersdk.Client
TemplateVersionID: version,
NewWorkspaceName: workspace.Name,
LastBuildParameters: lastBuildParameters,
Owner: workspace.OwnerID.String(),
PromptEphemeralParameters: parameterFlags.promptEphemeralParameters,
EphemeralParameters: ephemeralParameters,
+12 -12
View File
@@ -413,13 +413,13 @@ func TestUpdateValidateRichParameters(t *testing.T) {
}()
pty.ExpectMatch(stringParameterName)
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("$$")
pty.ExpectMatch("does not match")
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.WriteLine("")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("ABC")
pty.ExpectMatch("does not match")
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("abc")
_ = testutil.TryReceive(ctx, t, doneChan)
})
@@ -459,13 +459,13 @@ func TestUpdateValidateRichParameters(t *testing.T) {
}()
pty.ExpectMatch(numberParameterName)
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("12")
pty.ExpectMatch("is more than the maximum")
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.WriteLine("")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("notanumber")
pty.ExpectMatch("is not a number")
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("8")
_ = testutil.TryReceive(ctx, t, doneChan)
})
@@ -505,13 +505,13 @@ func TestUpdateValidateRichParameters(t *testing.T) {
}()
pty.ExpectMatch(boolParameterName)
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("cat")
pty.ExpectMatch("boolean value can be either \"true\" or \"false\"")
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.WriteLine("")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("dog")
pty.ExpectMatch("boolean value can be either \"true\" or \"false\"")
pty.ExpectMatch("> Enter a value (default: \"\"): ")
pty.ExpectMatch("> Enter a value: ")
pty.WriteLine("false")
_ = testutil.TryReceive(ctx, t, doneChan)
})