-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Preview can now show presets and validate them #149
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
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 |
---|---|---|
|
@@ -274,15 +274,18 @@ func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic | |
} | ||
|
||
diag := &hcl.Diagnostic{ | ||
Severity: hcl.DiagError, | ||
Summary: fmt.Sprintf("Invalid %q attribute for block %s", key, block.Label()), | ||
Detail: fmt.Sprintf("Expected a string, got %q", typeName), | ||
Subject: &(tyAttr.HCLAttribute().Range), | ||
//Context: &(block.HCLBlock().DefRange), | ||
Expression: tyAttr.HCLAttribute().Expr, | ||
Severity: hcl.DiagError, | ||
Summary: fmt.Sprintf("Invalid %q attribute for block %s", key, block.Label()), | ||
Detail: fmt.Sprintf("Expected a string, got %q", typeName), | ||
EvalContext: block.Context().Inner(), | ||
} | ||
|
||
if tyAttr.IsNotNil() { | ||
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. I found a nilpointer here during testing. |
||
diag.Subject = &(tyAttr.HCLAttribute().Range) | ||
// diag.Context = &(block.HCLBlock().DefRange) | ||
diag.Expression = tyAttr.HCLAttribute().Expr | ||
} | ||
|
||
if !tyVal.IsWhollyKnown() { | ||
refs := hclext.ReferenceNames(tyAttr.HCLAttribute().Expr) | ||
if len(refs) > 0 { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
package extract | ||
|
||
import ( | ||
"github.com/aquasecurity/trivy/pkg/iac/terraform" | ||
"github.com/coder/preview/types" | ||
"github.com/hashicorp/hcl/v2" | ||
) | ||
|
||
func PresetFromBlock(block *terraform.Block) types.Preset { | ||
p := types.Preset{ | ||
PresetData: types.PresetData{ | ||
Parameters: make(map[string]string), | ||
}, | ||
Diagnostics: types.Diagnostics{}, | ||
} | ||
|
||
if !block.IsResourceType(types.BlockTypePreset) { | ||
p.Diagnostics = append(p.Diagnostics, &hcl.Diagnostic{ | ||
Severity: hcl.DiagError, | ||
Summary: "Invalid Preset", | ||
Detail: "Block is not a preset", | ||
}) | ||
return p | ||
} | ||
|
||
pName, nameDiag := requiredString(block, "name") | ||
if nameDiag != nil { | ||
p.Diagnostics = append(p.Diagnostics, nameDiag) | ||
} | ||
p.Name = pName | ||
|
||
// GetAttribute and AsMapValue both gracefully handle `nil`, `null` and `unknown` values. | ||
// All of these return an empty map, which then makes the loop below a no-op. | ||
params := block.GetAttribute("parameters").AsMapValue() | ||
for presetParamName, presetParamValue := range params.Value() { | ||
p.Parameters[presetParamName] = presetParamValue | ||
} | ||
SasSwart marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
defaultAttr := block.GetAttribute("default") | ||
if defaultAttr != nil { | ||
p.Default = defaultAttr.Value().True() | ||
} | ||
|
||
return p | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,58 @@ | ||||||
package preview | ||||||
|
||||||
import ( | ||||||
"fmt" | ||||||
"slices" | ||||||
|
||||||
"github.com/aquasecurity/trivy/pkg/iac/terraform" | ||||||
"github.com/hashicorp/hcl/v2" | ||||||
|
||||||
"github.com/coder/preview/extract" | ||||||
"github.com/coder/preview/types" | ||||||
) | ||||||
|
||||||
// presets extracts all presets from the given modules. It then validates the name, | ||||||
// parameters and default preset. | ||||||
func presets(modules terraform.Modules, parameters []types.Parameter) []types.Preset { | ||||||
foundPresets := make([]types.Preset, 0) | ||||||
var defaultPreset *types.Preset | ||||||
|
||||||
for _, mod := range modules { | ||||||
blocks := mod.GetDatasByType(types.BlockTypePreset) | ||||||
for _, block := range blocks { | ||||||
preset := extract.PresetFromBlock(block) | ||||||
switch true { | ||||||
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.
Suggested change
|
||||||
case defaultPreset != nil && preset.Default: | ||||||
preset.Diagnostics = append(preset.Diagnostics, &hcl.Diagnostic{ | ||||||
Severity: hcl.DiagError, | ||||||
Summary: "Multiple default presets", | ||||||
Detail: fmt.Sprintf("Only one preset can be marked as default. %q is already marked as default", defaultPreset.Name), | ||||||
}) | ||||||
case defaultPreset == nil && preset.Default: | ||||||
defaultPreset = &preset | ||||||
} | ||||||
|
||||||
for paramName, paramValue := range preset.Parameters { | ||||||
templateParamIndex := slices.IndexFunc(parameters, func(p types.Parameter) bool { | ||||||
return p.Name == paramName | ||||||
}) | ||||||
if templateParamIndex == -1 { | ||||||
preset.Diagnostics = append(preset.Diagnostics, &hcl.Diagnostic{ | ||||||
Severity: hcl.DiagError, | ||||||
Summary: "Undefined Parameter", | ||||||
Detail: fmt.Sprintf("Preset parameter %q is not defined by the template.", paramName), | ||||||
}) | ||||||
continue | ||||||
} | ||||||
templateParam := parameters[templateParamIndex] | ||||||
for _, diag := range templateParam.Valid(types.StringLiteral(paramValue)) { | ||||||
SasSwart marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
preset.Diagnostics = append(preset.Diagnostics, diag) | ||||||
} | ||||||
} | ||||||
|
||||||
foundPresets = append(foundPresets, preset) | ||||||
} | ||||||
} | ||||||
|
||||||
return foundPresets | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,7 @@ func Test_Extract(t *testing.T) { | |
expTags map[string]string | ||
unknownTags []string | ||
params map[string]assertParam | ||
presets func(t *testing.T, presets []types.Preset) | ||
warnings []*regexp.Regexp | ||
}{ | ||
{ | ||
|
@@ -242,6 +243,62 @@ func Test_Extract(t *testing.T) { | |
errorDiagnostics("Required"), | ||
}, | ||
}, | ||
{ | ||
name: "invalid presets", | ||
dir: "invalidpresets", | ||
expTags: map[string]string{}, | ||
input: preview.Input{}, | ||
unknownTags: []string{}, | ||
params: map[string]assertParam{ | ||
"valid_parameter_name": ap(). | ||
optVals("valid_option_value"), | ||
}, | ||
presets: func(t *testing.T, presets []types.Preset) { | ||
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. There is probably a way to make these assertions a bit more reusable. Fine for now though |
||
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") | ||
}, | ||
}, | ||
{ | ||
name: "required", | ||
dir: "required", | ||
|
@@ -543,6 +600,11 @@ func Test_Extract(t *testing.T) { | |
require.True(t, ok, "unknown parameter %s", param.Name) | ||
check(t, param) | ||
} | ||
|
||
// Assert presets | ||
if tc.presets != nil { | ||
tc.presets(t, output.Presets) | ||
} | ||
}) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. The preview cli is very much a dev tool still. 👍