Skip to content

Commit 8197dde

Browse files
NoaheCampbellNoaheCampbell
authored
added --values and --set flags to lint command (#1907)
* added --values and --set flags to lint command * Update lint_test.go
1 parent 1ff21d1 commit 8197dde

File tree

12 files changed

+284
-51
lines changed

12 files changed

+284
-51
lines changed

‎cmd/preflight/cli/docs.go‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func extractDocs(templateFiles []string, valuesFiles []string, setValues []strin
8888
if err != nil {
8989
return errors.Wrapf(err, "failed to load values file %s", valuesFile)
9090
}
91-
values = mergeMaps(values, fileValues)
91+
values = preflight.MergeMaps(values, fileValues)
9292
}
9393

9494
// Normalize maps for Helm set merging
@@ -331,25 +331,6 @@ func setNestedValue(m map[string]interface{}, keys []string, value interface{})
331331
}
332332
}
333333

334-
func mergeMaps(base, overlay map[string]interface{}) map[string]interface{} {
335-
result := make(map[string]interface{})
336-
for k, v := range base {
337-
result[k] = v
338-
}
339-
for k, v := range overlay {
340-
if baseVal, exists := result[k]; exists {
341-
if baseMap, ok := baseVal.(map[string]interface{}); ok {
342-
if overlayMap, ok := v.(map[string]interface{}); ok {
343-
result[k] = mergeMaps(baseMap, overlayMap)
344-
continue
345-
}
346-
}
347-
}
348-
result[k] = v
349-
}
350-
return result
351-
}
352-
353334
func renderTemplate(templateContent string, values map[string]interface{}) (string, error) {
354335
tmpl := template.New("preflight").Funcs(sprig.FuncMap())
355336
tmpl, err := tmpl.Parse(templateContent)

‎cmd/preflight/cli/lint.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,11 @@ Exit codes:
5454
v := viper.GetViper()
5555

5656
opts := lint.LintOptions{
57-
FilePaths: args,
58-
Fix: v.GetBool("fix"),
59-
Format: v.GetString("format"),
57+
FilePaths: args,
58+
Fix: v.GetBool("fix"),
59+
Format: v.GetString("format"),
60+
ValuesFiles: v.GetStringSlice("values"),
61+
SetValues: v.GetStringSlice("set"),
6062
}
6163

6264
return runLint(opts)
@@ -65,6 +67,8 @@ Exit codes:
6567

6668
cmd.Flags().Bool("fix", false, "Automatically fix issues where possible")
6769
cmd.Flags().String("format", "text", "Output format: text or json")
70+
cmd.Flags().StringSlice("values", []string{}, "Path to YAML files with template values (required for v1beta3 specs)")
71+
cmd.Flags().StringSlice("set", []string{}, "Set template values via command line (e.g., --set key=value)")
6872

6973
return cmd
7074
}

‎cmd/troubleshoot/cli/lint.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,11 @@ Exit codes:
5454
v := viper.GetViper()
5555

5656
opts := lint.LintOptions{
57-
FilePaths: args,
58-
Fix: v.GetBool("fix"),
59-
Format: v.GetString("format"),
57+
FilePaths: args,
58+
Fix: v.GetBool("fix"),
59+
Format: v.GetString("format"),
60+
ValuesFiles: v.GetStringSlice("values"),
61+
SetValues: v.GetStringSlice("set"),
6062
}
6163

6264
return runLint(opts)
@@ -65,6 +67,8 @@ Exit codes:
6567

6668
cmd.Flags().Bool("fix", false, "Automatically fix issues where possible")
6769
cmd.Flags().String("format", "text", "Output format: text or json")
70+
cmd.Flags().StringSlice("values", []string{}, "Path to YAML files with template values (required for v1beta3 specs)")
71+
cmd.Flags().StringSlice("set", []string{}, "Set template values via command line (e.g., --set key=value)")
6872

6973
return cmd
7074
}

‎examples/test-error-messages/values-empty.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Empty values file for v1beta3 specs without templates
2+
{}

‎examples/test-error-messages/values-helm-builtins.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
minVersion: "1.19.0"

‎pkg/lint/lint.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"github.com/pkg/errors"
1010
"github.com/replicatedhq/troubleshoot/internal/util"
1111
"github.com/replicatedhq/troubleshoot/pkg/constants"
12+
"github.com/replicatedhq/troubleshoot/pkg/preflight"
13+
"helm.sh/helm/v3/pkg/strvals"
1214
"sigs.k8s.io/yaml"
1315
)
1416

@@ -27,6 +29,72 @@ func LintFiles(opts LintOptions) ([]LintResult, error) {
2729
}
2830
fileContent := string(fileBytes)
2931

32+
// Check if this is a v1beta3 spec
33+
isV1Beta3 := detectAPIVersionFromContent(fileContent) == constants.Troubleshootv1beta3Kind
34+
isV1Beta2 := detectAPIVersionFromContent(fileContent) == constants.Troubleshootv1beta2Kind
35+
36+
// Check if the content has Helm templates (for preflight v1beta3)
37+
hasTemplates := strings.Contains(fileContent, "{{") && strings.Contains(fileContent, "}}")
38+
39+
// Track if we should add a warning about unused values for v1beta2
40+
hasUnusedValuesWarning := isV1Beta2 && (len(opts.ValuesFiles) > 0 || len(opts.SetValues) > 0)
41+
42+
// If v1beta3 with templates, require values and render the template
43+
if isV1Beta3 && hasTemplates {
44+
if len(opts.ValuesFiles) == 0 && len(opts.SetValues) == 0 {
45+
return nil, errors.New("v1beta3 specs with Helm templates require a values file. Please provide values using --values or --set flags")
46+
}
47+
48+
// Load values from files and --set flags
49+
values := make(map[string]interface{})
50+
for _, valuesFile := range opts.ValuesFiles {
51+
if valuesFile == "" {
52+
continue
53+
}
54+
data, err := os.ReadFile(valuesFile)
55+
if err != nil {
56+
return nil, errors.Wrapf(err, "failed to read values file %s", valuesFile)
57+
}
58+
59+
var fileValues map[string]interface{}
60+
if err := yaml.Unmarshal(data, &fileValues); err != nil {
61+
return nil, errors.Wrapf(err, "failed to parse values file %s", valuesFile)
62+
}
63+
64+
values = preflight.MergeMaps(values, fileValues)
65+
}
66+
67+
// Apply --set values
68+
for _, setValue := range opts.SetValues {
69+
if err := strvals.ParseInto(setValue, values); err != nil {
70+
return nil, errors.Wrapf(err, "failed to parse --set value: %s", setValue)
71+
}
72+
}
73+
74+
// Render the template
75+
preflight.SeedDefaultBooleans(fileContent, values)
76+
preflight.SeedParentMapsForValueRefs(fileContent, values)
77+
rendered, err := preflight.RenderWithHelmTemplate(fileContent, values)
78+
if err != nil {
79+
// If rendering fails, create a result with the render error
80+
// This allows us to report template syntax errors
81+
results = append(results, LintResult{
82+
FilePath: filePath,
83+
Errors: []LintError{
84+
{
85+
Line: 1,
86+
Message: fmt.Sprintf("Failed to render v1beta3 template: %v", err),
87+
Field: "template",
88+
},
89+
},
90+
})
91+
continue
92+
}
93+
94+
// Use the rendered content for linting
95+
fileContent = rendered
96+
}
97+
3098
// Split into YAML documents
3199
docs := util.SplitYAML(fileContent)
32100

@@ -101,6 +169,17 @@ func LintFiles(opts LintOptions) ([]LintResult, error) {
101169
}
102170
}
103171

172+
// Add warning if values were provided for a v1beta2 spec
173+
if hasUnusedValuesWarning {
174+
fileResult.Warnings = append([]LintWarning{
175+
{
176+
Line: 1,
177+
Message: "Values files provided but this is a v1beta2 spec. Values are only used with v1beta3 specs. Did you mean to use apiVersion: troubleshoot.sh/v1beta3?",
178+
Field: "apiVersion",
179+
},
180+
}, fileResult.Warnings...)
181+
}
182+
104183
if writeNeeded {
105184
// Reassemble with the same delimiter used by util.SplitYAML
106185
updated := strings.Join(newDocs, "\n---\n")

‎pkg/lint/lint_test.go‎

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ func TestLintMultipleFiles(t *testing.T) {
1515
tests := []struct {
1616
name string
1717
files []string
18+
valuesFiles []string // values files for v1beta3 specs
1819
expectErrors map[string][]string // filename -> expected error substrings
1920
expectWarnings map[string][]string // filename -> expected warning substrings
2021
expectPass map[string]bool // filename -> should pass without errors
@@ -24,21 +25,23 @@ func TestLintMultipleFiles(t *testing.T) {
2425
files: []string{
2526
"helm-builtins-v1beta3.yaml",
2627
},
27-
expectErrors: map[string][]string{},
28-
expectWarnings: map[string][]string{
29-
"helm-builtins-v1beta3.yaml": {
30-
"Template values that must be provided at runtime: minVersion",
31-
},
28+
valuesFiles: []string{
29+
"values-helm-builtins.yaml",
3230
},
31+
expectErrors: map[string][]string{},
32+
expectWarnings: map[string][]string{},
3333
expectPass: map[string]bool{
34-
"helm-builtins-v1beta3.yaml": false, // has warnings
34+
"helm-builtins-v1beta3.yaml": true,
3535
},
3636
},
3737
{
3838
name: "invalid collectors and analyzers",
3939
files: []string{
4040
"invalid-collectors-analyzers.yaml",
4141
},
42+
valuesFiles: []string{
43+
"values-empty.yaml",
44+
},
4245
expectErrors: map[string][]string{
4346
"invalid-collectors-analyzers.yaml": {
4447
// The linter may stop early due to structural issues
@@ -57,6 +60,9 @@ func TestLintMultipleFiles(t *testing.T) {
5760
"missing-metadata-v1beta3.yaml",
5861
"no-analyzers-v1beta3.yaml",
5962
},
63+
valuesFiles: []string{
64+
"values-empty.yaml",
65+
},
6066
expectErrors: map[string][]string{
6167
"missing-apiversion-v1beta3.yaml": {
6268
"Missing or empty 'apiVersion' field",
@@ -95,6 +101,9 @@ func TestLintMultipleFiles(t *testing.T) {
95101
"support-bundle-no-collectors-v1beta3.yaml",
96102
"support-bundle-valid-v1beta3.yaml",
97103
},
104+
valuesFiles: []string{
105+
"values-empty.yaml",
106+
},
98107
expectErrors: map[string][]string{
99108
"support-bundle-no-collectors-v1beta3.yaml": {
100109
"SupportBundle spec must contain 'collectors' or 'hostCollectors'",
@@ -112,6 +121,9 @@ func TestLintMultipleFiles(t *testing.T) {
112121
"missing-metadata-v1beta3.yaml",
113122
"wrong-apiversion-v1beta3.yaml",
114123
},
124+
valuesFiles: []string{
125+
"values-empty.yaml",
126+
},
115127
expectErrors: map[string][]string{
116128
"missing-metadata-v1beta3.yaml": {
117129
"Missing 'metadata' section",
@@ -142,11 +154,18 @@ func TestLintMultipleFiles(t *testing.T) {
142154
}
143155
}
144156

157+
// Build values file paths
158+
var valuesFilePaths []string
159+
for _, vf := range tt.valuesFiles {
160+
valuesFilePaths = append(valuesFilePaths, filepath.Join(testDir, vf))
161+
}
162+
145163
// Run linter
146164
opts := LintOptions{
147-
FilePaths: filePaths,
148-
Fix: false,
149-
Format: "text",
165+
FilePaths: filePaths,
166+
Fix: false,
167+
Format: "text",
168+
ValuesFiles: valuesFilePaths,
150169
}
151170

152171
results, err := LintFiles(opts)
@@ -246,7 +265,7 @@ spec:
246265
fixedContent: "apiVersion: troubleshoot.sh/v1beta3",
247266
},
248267
{
249-
name: "fix missing leading dot in template",
268+
name: "v1beta3 template syntax error is reported",
250269
content: `apiVersion: troubleshoot.sh/v1beta3
251270
kind: Preflight
252271
metadata:
@@ -258,11 +277,17 @@ spec:
258277
- pass:
259278
when: '>= 1.19.0'
260279
message: OK`,
261-
expectFix: true,
262-
fixedContent: "{{ .Values.name }}",
280+
expectFix: false, // Template errors in v1beta3 are not auto-fixable, rendering will fail
281+
fixedContent: "Failed to render v1beta3 template", // Expect an error message
263282
},
264283
}
265284

285+
// Create empty values file for v1beta3 tests
286+
emptyValuesFile := filepath.Join(tmpDir, "values-empty.yaml")
287+
if err := os.WriteFile(emptyValuesFile, []byte("{}"), 0644); err != nil {
288+
t.Fatalf("Failed to write empty values file: %v", err)
289+
}
290+
266291
for _, tt := range tests {
267292
t.Run(tt.name, func(t *testing.T) {
268293
// Write test content to temp file
@@ -273,9 +298,10 @@ spec:
273298

274299
// Run linter with fix enabled
275300
opts := LintOptions{
276-
FilePaths: []string{testFile},
277-
Fix: true,
278-
Format: "text",
301+
FilePaths: []string{testFile},
302+
Fix: true,
303+
Format: "text",
304+
ValuesFiles: []string{emptyValuesFile},
279305
}
280306

281307
results, err := LintFiles(opts)
@@ -294,12 +320,27 @@ spec:
294320
}
295321
fixedContent := string(fixedBytes)
296322

297-
// Check if fix was applied
323+
// Check if fix was applied or error was reported
298324
if tt.expectFix {
299325
if !strings.Contains(fixedContent, tt.fixedContent) {
300326
t.Errorf("Expected fixed content to contain '%s', but got:\n%s",
301327
tt.fixedContent, fixedContent)
302328
}
329+
} else {
330+
// For tests that don't expect fix, check for errors
331+
if len(results[0].Errors) > 0 {
332+
errorFound := false
333+
for _, err := range results[0].Errors {
334+
if strings.Contains(err.Message, tt.fixedContent) {
335+
errorFound = true
336+
break
337+
}
338+
}
339+
if !errorFound {
340+
t.Errorf("Expected error containing '%s', but got errors: %v",
341+
tt.fixedContent, results[0].Errors)
342+
}
343+
}
303344
}
304345
})
305346
}

‎pkg/lint/types.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ type LintWarning struct {
2323
}
2424

2525
type LintOptions struct {
26-
FilePaths []string
27-
Fix bool
28-
Format string // "text" or "json"
26+
FilePaths []string
27+
Fix bool
28+
Format string // "text" or "json"
29+
ValuesFiles []string // Path to YAML files with template values (for v1beta3)
30+
SetValues []string // Template values from command line (for v1beta3)
2931
}
3032

3133
// HasErrors returns true if any of the results contain errors

0 commit comments

Comments
 (0)