Skip to content

Commit 643c5ee

Browse files
committed
integrate preset validation into preview.Preview
add tests
1 parent 625c327 commit 643c5ee

File tree

8 files changed

+213
-100
lines changed

8 files changed

+213
-100
lines changed

extract/parameter.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,15 +274,18 @@ func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic
274274
}
275275

276276
diag := &hcl.Diagnostic{
277-
Severity: hcl.DiagError,
278-
Summary: fmt.Sprintf("Invalid %q attribute for block %s", key, block.Label()),
279-
Detail: fmt.Sprintf("Expected a string, got %q", typeName),
280-
Subject: &(tyAttr.HCLAttribute().Range),
281-
//Context: &(block.HCLBlock().DefRange),
282-
Expression: tyAttr.HCLAttribute().Expr,
277+
Severity: hcl.DiagError,
278+
Summary: fmt.Sprintf("Invalid %q attribute for block %s", key, block.Label()),
279+
Detail: fmt.Sprintf("Expected a string, got %q", typeName),
283280
EvalContext: block.Context().Inner(),
284281
}
285282

283+
if tyAttr.IsNotNil() {
284+
diag.Subject = &(tyAttr.HCLAttribute().Range)
285+
// diag.Context = &(block.HCLBlock().DefRange)
286+
diag.Expression = tyAttr.HCLAttribute().Expr
287+
}
288+
286289
if !tyVal.IsWhollyKnown() {
287290
refs := hclext.ReferenceNames(tyAttr.HCLAttribute().Expr)
288291
if len(refs) > 0 {

extract/preset.go

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,26 +6,40 @@ import (
66
"github.com/hashicorp/hcl/v2"
77
)
88

9-
func PresetFromBlock(block *terraform.Block) (*types.Preset, hcl.Diagnostics) {
10-
var diags hcl.Diagnostics
11-
12-
pName, nameDiag := requiredString(block, "name")
13-
if nameDiag != nil {
14-
diags = append(diags, nameDiag)
15-
}
16-
9+
func PresetFromBlock(block *terraform.Block) types.Preset {
1710
p := types.Preset{
1811
PresetData: types.PresetData{
19-
Name: pName,
2012
Parameters: make(map[string]string),
2113
},
2214
Diagnostics: types.Diagnostics{},
2315
}
2416

17+
if !block.IsResourceType(types.BlockTypePreset) {
18+
p.Diagnostics = append(p.Diagnostics, &hcl.Diagnostic{
19+
Severity: hcl.DiagError,
20+
Summary: "Invalid Preset",
21+
Detail: "Block is not a preset",
22+
})
23+
return p
24+
}
25+
26+
pName, nameDiag := requiredString(block, "name")
27+
if nameDiag != nil {
28+
p.Diagnostics = append(p.Diagnostics, nameDiag)
29+
}
30+
p.Name = pName
31+
32+
// GetAttribute and AsMapValue both gracefully handle `nil`, `null` and `unknown` values.
33+
// All of these return an empty map, which then makes the loop below a no-op.
2534
params := block.GetAttribute("parameters").AsMapValue()
2635
for presetParamName, presetParamValue := range params.Value() {
2736
p.Parameters[presetParamName] = presetParamValue
2837
}
2938

30-
return &p, diags
39+
defaultAttr := block.GetAttribute("default")
40+
if defaultAttr != nil {
41+
p.Default = defaultAttr.Value().True()
42+
}
43+
44+
return p
3145
}

preset.go

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,25 @@ import (
1111
"github.com/coder/preview/types"
1212
)
1313

14-
func presets(modules terraform.Modules, parameters []types.Parameter) ([]types.Preset, hcl.Diagnostics) {
15-
diags := make(hcl.Diagnostics, 0)
16-
presets := make([]types.Preset, 0)
14+
// presets extracts all presets from the given modules. It then validates the name,
15+
// parameters and default preset.
16+
func presets(modules terraform.Modules, parameters []types.Parameter) []types.Preset {
17+
foundPresets := make([]types.Preset, 0)
18+
var defaultPreset *types.Preset
1719

1820
for _, mod := range modules {
1921
blocks := mod.GetDatasByType(types.BlockTypePreset)
2022
for _, block := range blocks {
21-
preset, pDiags := extract.PresetFromBlock(block)
22-
if len(pDiags) > 0 {
23-
diags = diags.Extend(pDiags)
24-
}
25-
26-
if preset == nil {
27-
continue
23+
preset := extract.PresetFromBlock(block)
24+
switch true {
25+
case defaultPreset != nil && preset.Default:
26+
preset.Diagnostics = append(preset.Diagnostics, &hcl.Diagnostic{
27+
Severity: hcl.DiagError,
28+
Summary: "Multiple default presets",
29+
Detail: fmt.Sprintf("Only one preset can be marked as default. %q is already marked as default", defaultPreset.Name),
30+
})
31+
case defaultPreset == nil && preset.Default:
32+
defaultPreset = &preset
2833
}
2934

3035
for paramName, paramValue := range preset.Parameters {
@@ -35,7 +40,7 @@ func presets(modules terraform.Modules, parameters []types.Parameter) ([]types.P
3540
preset.Diagnostics = append(preset.Diagnostics, &hcl.Diagnostic{
3641
Severity: hcl.DiagError,
3742
Summary: "Undefined Parameter",
38-
Detail: fmt.Sprintf("Preset %q requires parameter %q, but it is not defined by the template.", preset.Name, paramName),
43+
Detail: fmt.Sprintf("Preset parameter %q is not defined by the template.", paramName),
3944
})
4045
continue
4146
}
@@ -45,9 +50,9 @@ func presets(modules terraform.Modules, parameters []types.Parameter) ([]types.P
4550
}
4651
}
4752

48-
presets = append(presets, *preset)
53+
foundPresets = append(foundPresets, preset)
4954
}
5055
}
5156

52-
return presets, diags
57+
return foundPresets
5358
}

preview.go

Lines changed: 3 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ type Output struct {
3838

3939
Parameters []types.Parameter `json:"parameters"`
4040
WorkspaceTags types.TagBlocks `json:"workspace_tags"`
41+
Presets []types.Preset `json:"presets"`
4142
// Files is included for printing diagnostics.
4243
// They can be marshalled, but not unmarshalled. This is a limitation
4344
// of the HCL library.
@@ -162,6 +163,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn
162163

163164
diags := make(hcl.Diagnostics, 0)
164165
rp, rpDiags := parameters(modules)
166+
presets := presets(modules, rp)
165167
tags, tagDiags := workspaceTags(modules, p.Files())
166168

167169
// Add warnings
@@ -171,6 +173,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn
171173
ModuleOutput: outputs,
172174
Parameters: rp,
173175
WorkspaceTags: tags,
176+
Presets: presets,
174177
Files: p.Files(),
175178
}, diags.Extend(rpDiags).Extend(tagDiags)
176179
}
@@ -209,70 +212,3 @@ func tfVarFiles(path string, dir fs.FS) ([]string, error) {
209212
}
210213
return files, nil
211214
}
212-
213-
func PreviewPresets(ctx context.Context, dir fs.FS) ([]types.Preset, hcl.Diagnostics) {
214-
// The trivy package works with `github.com/zclconf/go-cty`. This package is
215-
// similar to `reflect` in its usage. This package can panic if types are
216-
// misused. To protect the caller, a general `recover` is used to catch any
217-
// mistakes. If this happens, there is a developer bug that needs to be resolved.
218-
var diagnostics hcl.Diagnostics
219-
defer func() {
220-
if r := recover(); r != nil {
221-
diagnostics.Extend(hcl.Diagnostics{
222-
{
223-
Severity: hcl.DiagError,
224-
Summary: "Panic occurred in preview. This should not happen, please report this to Coder.",
225-
Detail: fmt.Sprintf("panic in preview: %+v", r),
226-
},
227-
})
228-
}
229-
}()
230-
231-
logger := slog.New(slog.DiscardHandler)
232-
233-
varFiles, err := tfVarFiles("", dir)
234-
if err != nil {
235-
return nil, hcl.Diagnostics{
236-
{
237-
Severity: hcl.DiagError,
238-
Summary: "Files not found",
239-
Detail: err.Error(),
240-
},
241-
}
242-
}
243-
244-
// moduleSource is "" for a local module
245-
p := parser.New(dir, "",
246-
parser.OptionWithLogger(logger),
247-
parser.OptionStopOnHCLError(false),
248-
parser.OptionWithDownloads(false),
249-
parser.OptionWithSkipCachedModules(true),
250-
parser.OptionWithTFVarsPaths(varFiles...),
251-
)
252-
253-
err = p.ParseFS(ctx, ".")
254-
if err != nil {
255-
return nil, hcl.Diagnostics{
256-
{
257-
Severity: hcl.DiagError,
258-
Summary: "Parse terraform files",
259-
Detail: err.Error(),
260-
},
261-
}
262-
}
263-
264-
modules, err := p.EvaluateAll(ctx)
265-
if err != nil {
266-
return nil, hcl.Diagnostics{
267-
{
268-
Severity: hcl.DiagError,
269-
Summary: "Evaluate terraform files",
270-
Detail: err.Error(),
271-
},
272-
}
273-
}
274-
275-
rp, rpDiags := parameters(modules)
276-
presets, presetDiags := presets(modules, rp)
277-
return presets, diagnostics.Extend(rpDiags).Extend(presetDiags)
278-
}

preview_test.go

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func Test_Extract(t *testing.T) {
4242
expTags map[string]string
4343
unknownTags []string
4444
params map[string]assertParam
45-
presets []types.Preset
45+
presets func(t *testing.T, presets []types.Preset)
4646
warnings []*regexp.Regexp
4747
}{
4848
{
@@ -243,6 +243,80 @@ func Test_Extract(t *testing.T) {
243243
errorDiagnostics("Required"),
244244
},
245245
},
246+
{
247+
name: "empty preset",
248+
dir: "emptypreset",
249+
expTags: map[string]string{},
250+
input: preview.Input{},
251+
unknownTags: []string{},
252+
presets: func(t *testing.T, presets []types.Preset) {
253+
require.Len(t, presets, 1)
254+
preset := presets[0]
255+
require.Len(t, preset.Diagnostics, 1)
256+
require.Equal(t, preset.Diagnostics[0].Summary, "Invalid \"name\" attribute for block coder_workspace_preset.test")
257+
require.Equal(t, preset.Diagnostics[0].Detail, "Expected a string, got \"<nil>\"")
258+
},
259+
failPreview: false,
260+
},
261+
{
262+
name: "invalid presets",
263+
dir: "invalidpresets",
264+
expTags: map[string]string{},
265+
input: preview.Input{},
266+
unknownTags: []string{},
267+
params: map[string]assertParam{
268+
"valid_parameter_name": ap().
269+
optVals("valid_option_value"),
270+
},
271+
presets: func(t *testing.T, presets []types.Preset) {
272+
presetMap := map[string]func(t *testing.T, preset types.Preset){
273+
"": func(t *testing.T, preset types.Preset) {
274+
require.Len(t, preset.Diagnostics, 0)
275+
},
276+
"empty_parameters": func(t *testing.T, preset types.Preset) {
277+
require.Len(t, preset.Diagnostics, 0)
278+
},
279+
"no_parameters": func(t *testing.T, preset types.Preset) {
280+
require.Len(t, preset.Diagnostics, 0)
281+
},
282+
"invalid_parameter_name": func(t *testing.T, preset types.Preset) {
283+
require.Len(t, preset.Diagnostics, 1)
284+
require.Equal(t, preset.Diagnostics[0].Summary, "Undefined Parameter")
285+
require.Equal(t, preset.Diagnostics[0].Detail, "Preset parameter \"invalid_parameter_name\" is not defined by the template.")
286+
},
287+
"invalid_parameter_value": func(t *testing.T, preset types.Preset) {
288+
require.Len(t, preset.Diagnostics, 1)
289+
require.Equal(t, preset.Diagnostics[0].Summary, "Value must be a valid option")
290+
require.Equal(t, preset.Diagnostics[0].Detail, "the value \"invalid_value\" must be defined as one of options")
291+
},
292+
"valid_preset": func(t *testing.T, preset types.Preset) {
293+
require.Len(t, preset.Diagnostics, 0)
294+
require.Equal(t, preset.Parameters, map[string]string{
295+
"valid_parameter_name": "valid_option_value",
296+
})
297+
},
298+
}
299+
300+
for _, preset := range presets {
301+
if fn, ok := presetMap[preset.Name]; ok {
302+
fn(t, preset)
303+
}
304+
}
305+
306+
var defaultPresetsWithError int
307+
for _, preset := range presets {
308+
if preset.Name == "default_preset" || preset.Name == "another_default_preset" {
309+
for _, diag := range preset.Diagnostics {
310+
if diag.Summary == "Multiple default presets" {
311+
defaultPresetsWithError++
312+
break
313+
}
314+
}
315+
}
316+
}
317+
require.Equal(t, 1, defaultPresetsWithError, "exactly one default preset should have the multiple defaults error")
318+
},
319+
},
246320
{
247321
name: "required",
248322
dir: "required",
@@ -545,8 +619,10 @@ func Test_Extract(t *testing.T) {
545619
check(t, param)
546620
}
547621

548-
presets, diags := preview.PreviewPresets(context.Background(), dirFs)
549-
assert.ElementsMatch(t, tc.presets, presets)
622+
// Assert presets
623+
if tc.presets != nil {
624+
tc.presets(t, output.Presets)
625+
}
550626
})
551627
}
552628
}

testdata/emptypreset/main.tf

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
terraform {
2+
required_providers {
3+
coder = {
4+
source = "coder/coder"
5+
version = "2.8.0"
6+
}
7+
}
8+
}
9+
10+
data "coder_workspace_preset" "test" {
11+
}

0 commit comments

Comments
 (0)