diff --git a/extract/preset.go b/extract/preset.go index 01c9310..b6cf4f0 100644 --- a/extract/preset.go +++ b/extract/preset.go @@ -1,12 +1,34 @@ package extract import ( + "fmt" + "github.com/aquasecurity/trivy/pkg/iac/terraform" - "github.com/coder/preview/types" "github.com/hashicorp/hcl/v2" + + "github.com/coder/preview/types" ) -func PresetFromBlock(block *terraform.Block) types.Preset { +func PresetFromBlock(block *terraform.Block) (tfPreset types.Preset) { + defer func() { + // Extra safety mechanism to ensure that if a panic occurs, we do not break + // everything else. + if r := recover(); r != nil { + tfPreset = types.Preset{ + PresetData: types.PresetData{ + Name: block.Label(), + }, + Diagnostics: types.Diagnostics{ + { + Severity: hcl.DiagError, + Summary: "Panic occurred in extracting preset. This should not happen, please report this to Coder.", + Detail: fmt.Sprintf("panic in preset extract: %+v", r), + }, + }, + } + } + }() + p := types.Preset{ PresetData: types.PresetData{ Parameters: make(map[string]string), @@ -41,5 +63,13 @@ func PresetFromBlock(block *terraform.Block) types.Preset { p.Default = defaultAttr.Value().True() } + prebuildBlock := block.GetBlock("prebuilds") + if prebuildBlock != nil { + p.Prebuild = &types.PrebuildData{ + // Invalid values will be set to 0 + Instances: int(optionalInteger(prebuildBlock, "instances")), + } + } + return p } diff --git a/preview.go b/preview.go index ea4154a..2a411bd 100644 --- a/preview.go +++ b/preview.go @@ -68,6 +68,55 @@ func (o Output) MarshalJSON() ([]byte, error) { }) } +// ValidatePrebuilds will iterate over each preset, validate the inputs as a set, +// and attach any diagnostics to the preset if there are issues. This must be done +// because prebuilds have to be valid without user input. +// +// This will only validate presets that have prebuilds configured and have no existing +// error diagnostics. +func ValidatePrebuilds(ctx context.Context, input Input, preValid []types.Preset, dir fs.FS) { + for i := range preValid { + pre := &preValid[i] + if pre.Prebuild == nil || pre.Prebuild.Instances <= 0 { + // No prebuilds, so presets do not need to be valid without user input + continue + } + + if hcl.Diagnostics(pre.Diagnostics).HasErrors() { + // Piling on diagnostics is not helpful, if an error exists, leave it at that. + continue + } + + // Diagnostics are added to the existing preset. + input.ParameterValues = pre.Parameters + output, diagnostics := Preview(ctx, input, dir) + if diagnostics.HasErrors() { + pre.Diagnostics = append(pre.Diagnostics, diagnostics...) + // Do not pile on more diagnostics for individual params, it already failed + continue + } + + if output == nil { + continue + } + + // If any parameter is invalid, then the preset is invalid. + // A value must be specified for this failing parameter. + for _, param := range output.Parameters { + if hcl.Diagnostics(param.Diagnostics).HasErrors() { + for _, paramDiag := range param.Diagnostics { + if paramDiag.Severity != hcl.DiagError { + continue // Only care about errors here + } + orig := paramDiag.Summary + paramDiag.Summary = fmt.Sprintf("Parameter %s: %s", param.Name, orig) + pre.Diagnostics = append(pre.Diagnostics, paramDiag) + } + } + } + } +} + func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagnostics hcl.Diagnostics) { // The trivy package works with `github.com/zclconf/go-cty`. This package is // similar to `reflect` in its usage. This package can panic if types are @@ -180,7 +229,9 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn diags := make(hcl.Diagnostics, 0) rp, rpDiags := parameters(modules) - presets := presets(modules, rp) + // preValidPresets are extracted as written in terraform. Each individual + // param value is checked, however the preset as a whole might not valid. + preValidPresets := presets(modules, rp) tags, tagDiags := workspaceTags(modules, p.Files()) vars := variables(modules) @@ -191,7 +242,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn ModuleOutput: outputs, Parameters: rp, WorkspaceTags: tags, - Presets: presets, + Presets: preValidPresets, Files: p.Files(), Variables: vars, }, diags.Extend(rpDiags).Extend(tagDiags) diff --git a/preview_test.go b/preview_test.go index c75cf6a..32526c6 100644 --- a/preview_test.go +++ b/preview_test.go @@ -41,12 +41,13 @@ func Test_Extract(t *testing.T) { failPreview bool input preview.Input - expTags map[string]string - unknownTags []string - params map[string]assertParam - variables map[string]assertVariable - presets func(t *testing.T, presets []types.Preset) - warnings []*regexp.Regexp + expTags map[string]string + unknownTags []string + params map[string]assertParam + variables map[string]assertVariable + presetsFuncs func(t *testing.T, presets []types.Preset) + presets map[string]assertPreset + warnings []*regexp.Regexp }{ { name: "bad param values", @@ -266,6 +267,27 @@ func Test_Extract(t *testing.T) { errorDiagnostics("Required"), }, }, + { + name: "valid prebuild", + dir: "preset", + expTags: map[string]string{}, + input: preview.Input{}, + unknownTags: []string{}, + params: map[string]assertParam{ + "number": ap(), + "has_default": ap(), + }, + presets: map[string]assertPreset{ + "valid_preset": aPre(). + value("number", "9"). + value("has_default", "changed"). + prebuildCount(3), + "prebuild_instance_zero": aPre(). + prebuildCount(0), + "not_prebuild": aPre(). + prebuildCount(0), + }, + }, { name: "invalid presets", dir: "invalidpresets", @@ -276,50 +298,14 @@ func Test_Extract(t *testing.T) { "valid_parameter_name": ap(). optVals("valid_option_value"), }, - presets: func(t *testing.T, presets []types.Preset) { - presetMap := map[string]func(t *testing.T, preset types.Preset){ - "empty_parameters": func(t *testing.T, preset types.Preset) { - require.Len(t, preset.Diagnostics, 0) - }, - "no_parameters": func(t *testing.T, preset types.Preset) { - require.Len(t, preset.Diagnostics, 0) - }, - "invalid_parameter_name": func(t *testing.T, preset types.Preset) { - require.Len(t, preset.Diagnostics, 1) - require.Equal(t, preset.Diagnostics[0].Summary, "Undefined Parameter") - require.Equal(t, preset.Diagnostics[0].Detail, "Preset parameter \"invalid_parameter_name\" is not defined by the template.") - }, - "invalid_parameter_value": func(t *testing.T, preset types.Preset) { - require.Len(t, preset.Diagnostics, 1) - require.Equal(t, preset.Diagnostics[0].Summary, "Value must be a valid option") - require.Equal(t, preset.Diagnostics[0].Detail, "the value \"invalid_value\" must be defined as one of options") - }, - "valid_preset": func(t *testing.T, preset types.Preset) { - require.Len(t, preset.Diagnostics, 0) - require.Equal(t, preset.Parameters, map[string]string{ - "valid_parameter_name": "valid_option_value", - }) - }, - } - - for _, preset := range presets { - if fn, ok := presetMap[preset.Name]; ok { - fn(t, preset) - } - } - - var defaultPresetsWithError int - for _, preset := range presets { - if preset.Name == "default_preset" || preset.Name == "another_default_preset" { - for _, diag := range preset.Diagnostics { - if diag.Summary == "Multiple default presets" { - defaultPresetsWithError++ - break - } - } - } - } - require.Equal(t, 1, defaultPresetsWithError, "exactly one default preset should have the multiple defaults error") + presets: map[string]assertPreset{ + "empty_parameters": aPre(), + "no_parameters": aPre(), + "invalid_parameter_name": aPreWithDiags().errorDiagnostics("Preset parameter \"invalid_parameter_name\" is not defined by the template."), + "invalid_parameter_value": aPreWithDiags().errorDiagnostics("the value \"invalid_value\" must be defined as one of options"), + "valid_preset": aPre().value("valid_parameter_name", "valid_option_value"), + "another_default_preset": aPre().def(true), + "default_preset": aPreWithDiags().errorDiagnostics("Only one preset can be marked as default. \"another_default_preset\" is already marked as default"), }, }, { @@ -649,6 +635,9 @@ func Test_Extract(t *testing.T) { } require.False(t, diags.HasErrors()) + // Validate prebuilds too + preview.ValidatePrebuilds(context.Background(), tc.input, output.Presets, dirFs) + if len(tc.warnings) > 0 { for _, w := range tc.warnings { idx := slices.IndexFunc(diags, func(diagnostic *hcl.Diagnostic) bool { @@ -684,9 +673,10 @@ func Test_Extract(t *testing.T) { check(t, param) } - // Assert presets - if tc.presets != nil { - tc.presets(t, output.Presets) + for _, preset := range output.Presets { + check, ok := tc.presets[preset.Name] + require.True(t, ok, "unknown preset %s", preset.Name) + check(t, preset) } // Assert variables @@ -700,6 +690,55 @@ func Test_Extract(t *testing.T) { } } +func TestPresetValidation(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + dir string + input preview.Input + presetAssert map[string]assertPreset + }{ + { + name: "preset failure", + dir: "presetfail", + input: preview.Input{}, + presetAssert: map[string]assertPreset{ + "invalid_parameters": aPreWithDiags(). + errorDiagnostics("Parameter no_default: Required parameter not provided"), + "valid_preset": aPre(). + value("has_default", "changed"). + value("no_default", "custom value"). + noDiagnostics(), + "prebuild_instance_zero": aPre(), + "not_prebuild": aPre(), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dirFs := os.DirFS(filepath.Join("testdata", tc.dir)) + output, diags := preview.Preview(context.Background(), tc.input, dirFs) + if diags.HasErrors() { + t.Logf("diags: %s", diags) + } + require.False(t, diags.HasErrors()) + require.Len(t, diags, 0) + + preview.ValidatePrebuilds(context.Background(), tc.input, output.Presets, dirFs) + for _, preset := range output.Presets { + check, ok := tc.presetAssert[preset.Name] + require.True(t, ok, "unknown preset %s", preset.Name) + check(t, preset) + delete(tc.presetAssert, preset.Name) + } + + require.Len(t, tc.presetAssert, 0, "some presets were not found") + }) + } +} + type assertVariable func(t *testing.T, variable types.Variable) func av() assertVariable { @@ -890,6 +929,81 @@ func (a assertVariable) extend(f assertVariable) assertVariable { } } +type assertPreset func(t *testing.T, preset types.Preset) + +func aPre() assertPreset { + return func(t *testing.T, parameter types.Preset) { + t.Helper() + assert.Empty(t, parameter.Diagnostics, "parameter should have no diagnostics") + } +} + +func aPreWithDiags() assertPreset { + return func(t *testing.T, parameter types.Preset) {} +} + +func (a assertPreset) def(def bool) assertPreset { + return a.extend(func(t *testing.T, preset types.Preset) { + require.Equal(t, def, preset.Default) + }) +} + +func (a assertPreset) prebuildCount(exp int) assertPreset { + return a.extend(func(t *testing.T, preset types.Preset) { + if exp == 0 && preset.Prebuild == nil { + return + } + require.NotNilf(t, preset.Prebuild, "prebuild should not be nil, expected %d instances", exp) + require.Equal(t, exp, preset.Prebuild.Instances) + }) +} + +func (a assertPreset) value(key, value string) assertPreset { + return a.extend(func(t *testing.T, preset types.Preset) { + v, ok := preset.Parameters[key] + require.Truef(t, ok, "preset parameter %q existence check", key) + assert.Equalf(t, value, v, "preset parameter %q value equality check", key) + }) +} + +func (a assertPreset) errorDiagnostics(patterns ...string) assertPreset { + return a.diagnostics(hcl.DiagError, patterns...) +} + +func (a assertPreset) warnDiagnostics(patterns ...string) assertPreset { + return a.diagnostics(hcl.DiagWarning, patterns...) +} + +func (a assertPreset) diagnostics(sev hcl.DiagnosticSeverity, patterns ...string) assertPreset { + shadow := patterns + return a.extend(func(t *testing.T, preset types.Preset) { + t.Helper() + + assertDiags(t, sev, preset.Diagnostics, shadow...) + }) +} + +func (a assertPreset) noDiagnostics() assertPreset { + return a.extend(func(t *testing.T, preset types.Preset) { + t.Helper() + + assert.Empty(t, preset.Diagnostics, "parameter should have no diagnostics") + }) +} + +//nolint:revive +func (a assertPreset) extend(f assertPreset) assertPreset { + if a == nil { + a = func(t *testing.T, v types.Preset) {} + } + + return func(t *testing.T, v types.Preset) { + t.Helper() + (a)(t, v) + f(t, v) + } +} + func assertDiags(t *testing.T, sev hcl.DiagnosticSeverity, diags types.Diagnostics, patterns ...string) { t.Helper() checks := make([]string, len(patterns)) diff --git a/testdata/invalidpresets/main.tf b/testdata/invalidpresets/main.tf index d9abd56..a7afc28 100644 --- a/testdata/invalidpresets/main.tf +++ b/testdata/invalidpresets/main.tf @@ -46,18 +46,19 @@ data "coder_workspace_preset" "valid_preset" { } } -data "coder_workspace_preset" "default_preset" { - name = "default_preset" +data "coder_workspace_preset" "another_default_preset" { + name = "another_default_preset" parameters = { "valid_parameter_name" = "valid_option_value" } default = true } -data "coder_workspace_preset" "another_default_preset" { - name = "another_default_preset" +data "coder_workspace_preset" "default_preset" { + name = "default_preset" parameters = { "valid_parameter_name" = "valid_option_value" } default = true -} \ No newline at end of file +} + diff --git a/testdata/preset/main.tf b/testdata/preset/main.tf new file mode 100644 index 0000000..a8d6091 --- /dev/null +++ b/testdata/preset/main.tf @@ -0,0 +1,50 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + version = "2.8.0" + } + } +} + +data "coder_parameter" "number" { + name = "number" + default = "7" + type = "number" + + validation { + min = 4 + max = 10 + } +} + +data "coder_parameter" "has_default" { + name = "has_default" + default = "hello world" +} + + +data "coder_workspace_preset" "valid_preset" { + name = "valid_preset" + + parameters = { + "number" = "9" + "has_default" = "changed" + } + prebuilds { + instances = 3 + } +} + +data "coder_workspace_preset" "prebuild_instance_zero" { + name = "prebuild_instance_zero" + + prebuilds { + // No instances + instances = 0 + } +} + +data "coder_workspace_preset" "not_prebuild" { + name = "not_prebuild" +} \ No newline at end of file diff --git a/testdata/presetfail/main.tf b/testdata/presetfail/main.tf new file mode 100644 index 0000000..81c2e31 --- /dev/null +++ b/testdata/presetfail/main.tf @@ -0,0 +1,51 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + version = "2.8.0" + } + } +} + +data "coder_parameter" "no_default" { + name = "no_default" +} + +data "coder_parameter" "has_default" { + name = "has_default" + default = "hello world" +} + + +data "coder_workspace_preset" "invalid_parameters" { + name = "invalid_parameters" + + prebuilds { + instances = 1 + } +} + +data "coder_workspace_preset" "valid_preset" { + name = "valid_preset" + + parameters = { + "no_default" = "custom value" + "has_default" = "changed" + } + prebuilds { + instances = 1 + } +} + +data "coder_workspace_preset" "prebuild_instance_zero" { + name = "prebuild_instance_zero" + + prebuilds { + // No instances + instances = 0 + } +} + +data "coder_workspace_preset" "not_prebuild" { + name = "not_prebuild" +} \ No newline at end of file diff --git a/testdata/presetfail/skipe2e b/testdata/presetfail/skipe2e new file mode 100644 index 0000000..3ce3bec --- /dev/null +++ b/testdata/presetfail/skipe2e @@ -0,0 +1 @@ +parameters are invalid without input \ No newline at end of file diff --git a/types/preset.go b/types/preset.go index da05e41..91ee890 100644 --- a/types/preset.go +++ b/types/preset.go @@ -11,8 +11,13 @@ type Preset struct { Diagnostics Diagnostics `json:"diagnostics"` } +type PrebuildData struct { + Instances int `json:"instances"` +} + type PresetData struct { Name string `json:"name"` Parameters map[string]string `json:"parameters"` Default bool `json:"default"` + Prebuild *PrebuildData }