-
Notifications
You must be signed in to change notification settings - Fork 949
feat: include template variables in dynamic parameter rendering #18819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e86224c
40ee3fd
520fff7
a8bdd04
a2f2de6
b45cbf2
c0b54b1
d0c594a
3cdcb38
e58c5e1
ac9fccb
432021d
2347119
7b2213c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package dynamicparameters | ||
|
||
import ( | ||
"strconv" | ||
|
||
"github.com/zclconf/go-cty/cty" | ||
"github.com/zclconf/go-cty/cty/json" | ||
"golang.org/x/xerrors" | ||
|
||
"github.com/coder/coder/v2/coderd/database" | ||
) | ||
|
||
// VariableValues is a helper function that converts a slice of TemplateVersionVariable | ||
// into a map of cty.Value for use in coder/preview. | ||
func VariableValues(vals []database.TemplateVersionVariable) (map[string]cty.Value, error) { | ||
ctyVals := make(map[string]cty.Value, len(vals)) | ||
for _, v := range vals { | ||
value := v.Value | ||
if value == "" && v.DefaultValue != "" { | ||
value = v.DefaultValue | ||
} | ||
|
||
if value == "" { | ||
// Empty strings are unsupported I guess? | ||
continue // omit non-set vals | ||
} | ||
|
||
var err error | ||
switch v.Type { | ||
// Defaulting the empty type to "string" | ||
// TODO: This does not match the terraform behavior, however it is too late | ||
Emyrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// at this point in the code to determine this, as the database type stores all values | ||
// as strings. The code needs to be fixed in the `Parse` step of the provisioner. | ||
// That step should determine the type of the variable correctly and store it in the database. | ||
case "string", "": | ||
ctyVals[v.Name] = cty.StringVal(value) | ||
Comment on lines
+30
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not ideal, will resolve in another PR |
||
case "number": | ||
ctyVals[v.Name], err = cty.ParseNumberVal(value) | ||
if err != nil { | ||
return nil, xerrors.Errorf("parse variable %q: %w", v.Name, err) | ||
} | ||
case "bool": | ||
parsed, err := strconv.ParseBool(value) | ||
if err != nil { | ||
return nil, xerrors.Errorf("parse variable %q: %w", v.Name, err) | ||
} | ||
ctyVals[v.Name] = cty.BoolVal(parsed) | ||
default: | ||
// If it is a complex type, let the cty json code give it a try. | ||
// TODO: Ideally we parse `list` & `map` and build the type ourselves. | ||
ty, err := json.ImpliedType([]byte(value)) | ||
if err != nil { | ||
return nil, xerrors.Errorf("implied type for variable %q: %w", v.Name, err) | ||
} | ||
|
||
jv, err := json.Unmarshal([]byte(value), ty) | ||
if err != nil { | ||
return nil, xerrors.Errorf("unmarshal variable %q: %w", v.Name, err) | ||
} | ||
ctyVals[v.Name] = jv | ||
} | ||
} | ||
|
||
return ctyVals, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ import ( | |
"github.com/google/uuid" | ||
"github.com/moby/moby/pkg/namesgenerator" | ||
"github.com/sqlc-dev/pqtype" | ||
"github.com/zclconf/go-cty/cty" | ||
"golang.org/x/xerrors" | ||
|
||
"cdr.dev/slog" | ||
|
@@ -1585,7 +1586,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht | |
var parsedTags map[string]string | ||
var ok bool | ||
if dynamicTemplate { | ||
parsedTags, ok = api.dynamicTemplateVersionTags(ctx, rw, organization.ID, apiKey.UserID, file) | ||
parsedTags, ok = api.dynamicTemplateVersionTags(ctx, rw, organization.ID, apiKey.UserID, file, req.UserVariableValues) | ||
if !ok { | ||
return | ||
} | ||
|
@@ -1762,7 +1763,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht | |
warnings)) | ||
} | ||
|
||
func (api *API) dynamicTemplateVersionTags(ctx context.Context, rw http.ResponseWriter, orgID uuid.UUID, owner uuid.UUID, file database.File) (map[string]string, bool) { | ||
func (api *API) dynamicTemplateVersionTags(ctx context.Context, rw http.ResponseWriter, orgID uuid.UUID, owner uuid.UUID, file database.File, templateVariables []codersdk.VariableValue) (map[string]string, bool) { | ||
ownerData, err := dynamicparameters.WorkspaceOwner(ctx, api.Database, orgID, owner) | ||
if err != nil { | ||
if httpapi.Is404Error(err) { | ||
|
@@ -1800,11 +1801,19 @@ func (api *API) dynamicTemplateVersionTags(ctx context.Context, rw http.Response | |
return nil, false | ||
} | ||
|
||
// Pass in any manually specified template variables as TFVars. | ||
// TODO: Does this break if the type is not a string? | ||
tfVarValues := make(map[string]cty.Value) | ||
for _, variable := range templateVariables { | ||
tfVarValues[variable.Name] = cty.StringVal(variable.Value) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAICT There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is, which I just found strange. This is working for now, I will test complex behavior as I edit the source of this (the parse function) |
||
} | ||
|
||
output, diags := preview.Preview(ctx, preview.Input{ | ||
PlanJSON: nil, // Template versions are before `terraform plan` | ||
ParameterValues: nil, // No user-specified parameters | ||
Owner: *ownerData, | ||
Logger: stdslog.New(stdslog.DiscardHandler), | ||
TFVars: tfVarValues, | ||
}, files) | ||
tagErr := dynamicparameters.CheckTags(output, diags) | ||
if tagErr != nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// Base case for workspace tags + parameters. | ||
terraform { | ||
required_providers { | ||
coder = { | ||
source = "coder/coder" | ||
} | ||
docker = { | ||
source = "kreuzwerker/docker" | ||
version = "3.0.2" | ||
} | ||
} | ||
} | ||
|
||
variable "one" { | ||
default = "alice" | ||
type = string | ||
} | ||
|
||
|
||
data "coder_parameter" "variable_values" { | ||
name = "variable_values" | ||
description = "Just to show the variable values" | ||
type = "string" | ||
default = var.one | ||
|
||
option { | ||
name = "one" | ||
value = var.one | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.