added --values and --set flags to lint command (#1907)

* added --values and --set flags to lint command

* Update lint_test.go

cmd/preflight/cli/docs.go

@@ -88,7 +88,7 @@ func extractDocs(templateFiles []string, valuesFiles []string, setValues []strin
 		if err != nil {
 			return errors.Wrapf(err, "failed to load values file %s", valuesFile)
 		}
-		values = mergeMaps(values, fileValues)
+		values = preflight.MergeMaps(values, fileValues)
 	}
 
 	// Normalize maps for Helm set merging
@@ -331,25 +331,6 @@ func setNestedValue(m map[string]interface{}, keys []string, value interface{})
 	}
 }
 
-func mergeMaps(base, overlay map[string]interface{}) map[string]interface{} {
-	result := make(map[string]interface{})
-	for k, v := range base {
-		result[k] = v
-	}
-	for k, v := range overlay {
-		if baseVal, exists := result[k]; exists {
-			if baseMap, ok := baseVal.(map[string]interface{}); ok {
-				if overlayMap, ok := v.(map[string]interface{}); ok {
-					result[k] = mergeMaps(baseMap, overlayMap)
-					continue
-				}
-			}
-		}
-		result[k] = v
-	}
-	return result
-}
-
 func renderTemplate(templateContent string, values map[string]interface{}) (string, error) {
 	tmpl := template.New("preflight").Funcs(sprig.FuncMap())
 	tmpl, err := tmpl.Parse(templateContent)

cmd/preflight/cli/lint.go

@@ -57,6 +57,8 @@ Exit codes:
 				FilePaths:   args,
 				Fix:         v.GetBool("fix"),
 				Format:      v.GetString("format"),
+				ValuesFiles: v.GetStringSlice("values"),
+				SetValues:   v.GetStringSlice("set"),
 			}
 
 			return runLint(opts)
@@ -65,6 +67,8 @@ Exit codes:
 
 	cmd.Flags().Bool("fix", false, "Automatically fix issues where possible")
 	cmd.Flags().String("format", "text", "Output format: text or json")
+	cmd.Flags().StringSlice("values", []string{}, "Path to YAML files with template values (required for v1beta3 specs)")
+	cmd.Flags().StringSlice("set", []string{}, "Set template values via command line (e.g., --set key=value)")
 
 	return cmd
 }

cmd/troubleshoot/cli/lint.go

@@ -57,6 +57,8 @@ Exit codes:
 				FilePaths:   args,
 				Fix:         v.GetBool("fix"),
 				Format:      v.GetString("format"),
+				ValuesFiles: v.GetStringSlice("values"),
+				SetValues:   v.GetStringSlice("set"),
 			}
 
 			return runLint(opts)
@@ -65,6 +67,8 @@ Exit codes:
 
 	cmd.Flags().Bool("fix", false, "Automatically fix issues where possible")
 	cmd.Flags().String("format", "text", "Output format: text or json")
+	cmd.Flags().StringSlice("values", []string{}, "Path to YAML files with template values (required for v1beta3 specs)")
+	cmd.Flags().StringSlice("set", []string{}, "Set template values via command line (e.g., --set key=value)")
 
 	return cmd
 }

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

@@ -0,0 +1,2 @@
+# Empty values file for v1beta3 specs without templates
+{}

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

@@ -0,0 +1 @@
+minVersion: "1.19.0"

pkg/lint/lint.go

@@ -9,6 +9,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/troubleshoot/internal/util"
 	"github.com/replicatedhq/troubleshoot/pkg/constants"
+	"github.com/replicatedhq/troubleshoot/pkg/preflight"
+	"helm.sh/helm/v3/pkg/strvals"
 	"sigs.k8s.io/yaml"
 )
 
@@ -27,6 +29,72 @@ func LintFiles(opts LintOptions) ([]LintResult, error) {
 		}
 		fileContent := string(fileBytes)
 
+		// Check if this is a v1beta3 spec
+		isV1Beta3 := detectAPIVersionFromContent(fileContent) == constants.Troubleshootv1beta3Kind
+		isV1Beta2 := detectAPIVersionFromContent(fileContent) == constants.Troubleshootv1beta2Kind
+
+		// Check if the content has Helm templates (for preflight v1beta3)
+		hasTemplates := strings.Contains(fileContent, "{{") && strings.Contains(fileContent, "}}")
+
+		// Track if we should add a warning about unused values for v1beta2
+		hasUnusedValuesWarning := isV1Beta2 && (len(opts.ValuesFiles) > 0 || len(opts.SetValues) > 0)
+
+		// If v1beta3 with templates, require values and render the template
+		if isV1Beta3 && hasTemplates {
+			if len(opts.ValuesFiles) == 0 && len(opts.SetValues) == 0 {
+				return nil, errors.New("v1beta3 specs with Helm templates require a values file. Please provide values using --values or --set flags")
+			}
+
+			// Load values from files and --set flags
+			values := make(map[string]interface{})
+			for _, valuesFile := range opts.ValuesFiles {
+				if valuesFile == "" {
+					continue
+				}
+				data, err := os.ReadFile(valuesFile)
+				if err != nil {
+					return nil, errors.Wrapf(err, "failed to read values file %s", valuesFile)
+				}
+
+				var fileValues map[string]interface{}
+				if err := yaml.Unmarshal(data, &fileValues); err != nil {
+					return nil, errors.Wrapf(err, "failed to parse values file %s", valuesFile)
+				}
+
+				values = preflight.MergeMaps(values, fileValues)
+			}
+
+			// Apply --set values
+			for _, setValue := range opts.SetValues {
+				if err := strvals.ParseInto(setValue, values); err != nil {
+					return nil, errors.Wrapf(err, "failed to parse --set value: %s", setValue)
+				}
+			}
+
+			// Render the template
+			preflight.SeedDefaultBooleans(fileContent, values)
+			preflight.SeedParentMapsForValueRefs(fileContent, values)
+			rendered, err := preflight.RenderWithHelmTemplate(fileContent, values)
+			if err != nil {
+				// If rendering fails, create a result with the render error
+				// This allows us to report template syntax errors
+				results = append(results, LintResult{
+					FilePath: filePath,
+					Errors: []LintError{
+						{
+							Line:    1,
+							Message: fmt.Sprintf("Failed to render v1beta3 template: %v", err),
+							Field:   "template",
+						},
+					},
+				})
+				continue
+			}
+
+			// Use the rendered content for linting
+			fileContent = rendered
+		}
+
 		// Split into YAML documents
 		docs := util.SplitYAML(fileContent)
 
@@ -101,6 +169,17 @@ func LintFiles(opts LintOptions) ([]LintResult, error) {
 			}
 		}
 
+		// Add warning if values were provided for a v1beta2 spec
+		if hasUnusedValuesWarning {
+			fileResult.Warnings = append([]LintWarning{
+				{
+					Line:    1,
+					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?",
+					Field:   "apiVersion",
+				},
+			}, fileResult.Warnings...)
+		}
+
 		if writeNeeded {
 			// Reassemble with the same delimiter used by util.SplitYAML
 			updated := strings.Join(newDocs, "\n---\n")

pkg/lint/lint_test.go

@@ -15,6 +15,7 @@ func TestLintMultipleFiles(t *testing.T) {
 	tests := []struct {
 		name           string
 		files          []string
+		valuesFiles    []string            // values files for v1beta3 specs
 		expectErrors   map[string][]string // filename -> expected error substrings
 		expectWarnings map[string][]string // filename -> expected warning substrings
 		expectPass     map[string]bool     // filename -> should pass without errors
@@ -24,14 +25,13 @@ func TestLintMultipleFiles(t *testing.T) {
 			files: []string{
 				"helm-builtins-v1beta3.yaml",
 			},
+			valuesFiles: []string{
+				"values-helm-builtins.yaml",
+			},
 			expectErrors:   map[string][]string{},
-			expectWarnings: map[string][]string{
+			expectWarnings: map[string][]string{},
-				"helm-builtins-v1beta3.yaml": {
-					"Template values that must be provided at runtime: minVersion",
-				},
-			},
 			expectPass: map[string]bool{
-				"helm-builtins-v1beta3.yaml": false, // has warnings
+				"helm-builtins-v1beta3.yaml": true,
 			},
 		},
 		{
@@ -39,6 +39,9 @@ func TestLintMultipleFiles(t *testing.T) {
 			files: []string{
 				"invalid-collectors-analyzers.yaml",
 			},
+			valuesFiles: []string{
+				"values-empty.yaml",
+			},
 			expectErrors: map[string][]string{
 				"invalid-collectors-analyzers.yaml": {
 					// The linter may stop early due to structural issues
@@ -57,6 +60,9 @@ func TestLintMultipleFiles(t *testing.T) {
 				"missing-metadata-v1beta3.yaml",
 				"no-analyzers-v1beta3.yaml",
 			},
+			valuesFiles: []string{
+				"values-empty.yaml",
+			},
 			expectErrors: map[string][]string{
 				"missing-apiversion-v1beta3.yaml": {
 					"Missing or empty 'apiVersion' field",
@@ -95,6 +101,9 @@ func TestLintMultipleFiles(t *testing.T) {
 				"support-bundle-no-collectors-v1beta3.yaml",
 				"support-bundle-valid-v1beta3.yaml",
 			},
+			valuesFiles: []string{
+				"values-empty.yaml",
+			},
 			expectErrors: map[string][]string{
 				"support-bundle-no-collectors-v1beta3.yaml": {
 					"SupportBundle spec must contain 'collectors' or 'hostCollectors'",
@@ -112,6 +121,9 @@ func TestLintMultipleFiles(t *testing.T) {
 				"missing-metadata-v1beta3.yaml",
 				"wrong-apiversion-v1beta3.yaml",
 			},
+			valuesFiles: []string{
+				"values-empty.yaml",
+			},
 			expectErrors: map[string][]string{
 				"missing-metadata-v1beta3.yaml": {
 					"Missing 'metadata' section",
@@ -142,11 +154,18 @@ func TestLintMultipleFiles(t *testing.T) {
 				}
 			}
 
+			// Build values file paths
+			var valuesFilePaths []string
+			for _, vf := range tt.valuesFiles {
+				valuesFilePaths = append(valuesFilePaths, filepath.Join(testDir, vf))
+			}
+
 			// Run linter
 			opts := LintOptions{
 				FilePaths:   filePaths,
 				Fix:         false,
 				Format:      "text",
+				ValuesFiles: valuesFilePaths,
 			}
 
 			results, err := LintFiles(opts)
@@ -246,7 +265,7 @@ spec:
 			fixedContent: "apiVersion: troubleshoot.sh/v1beta3",
 		},
 		{
-			name: "fix missing leading dot in template",
+			name: "v1beta3 template syntax error is reported",
 			content: `apiVersion: troubleshoot.sh/v1beta3
 kind: Preflight
 metadata:
@@ -258,11 +277,17 @@ spec:
           - pass:
               when: '>= 1.19.0'
               message: OK`,
-			expectFix:    true,
+			expectFix:    false,                               // Template errors in v1beta3 are not auto-fixable, rendering will fail
-			fixedContent: "{{ .Values.name }}",
+			fixedContent: "Failed to render v1beta3 template", // Expect an error message
 		},
 	}
 
+	// Create empty values file for v1beta3 tests
+	emptyValuesFile := filepath.Join(tmpDir, "values-empty.yaml")
+	if err := os.WriteFile(emptyValuesFile, []byte("{}"), 0644); err != nil {
+		t.Fatalf("Failed to write empty values file: %v", err)
+	}
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			// Write test content to temp file
@@ -276,6 +301,7 @@ spec:
 				FilePaths:   []string{testFile},
 				Fix:         true,
 				Format:      "text",
+				ValuesFiles: []string{emptyValuesFile},
 			}
 
 			results, err := LintFiles(opts)
@@ -294,12 +320,27 @@ spec:
 			}
 			fixedContent := string(fixedBytes)
 
-			// Check if fix was applied
+			// Check if fix was applied or error was reported
 			if tt.expectFix {
 				if !strings.Contains(fixedContent, tt.fixedContent) {
 					t.Errorf("Expected fixed content to contain '%s', but got:\n%s",
 						tt.fixedContent, fixedContent)
 				}
+			} else {
+				// For tests that don't expect fix, check for errors
+				if len(results[0].Errors) > 0 {
+					errorFound := false
+					for _, err := range results[0].Errors {
+						if strings.Contains(err.Message, tt.fixedContent) {
+							errorFound = true
+							break
+						}
+					}
+					if !errorFound {
+						t.Errorf("Expected error containing '%s', but got errors: %v",
+							tt.fixedContent, results[0].Errors)
+					}
+				}
 			}
 		})
 	}

pkg/lint/types.go

@@ -26,6 +26,8 @@ type LintOptions struct {
 	FilePaths   []string
 	Fix         bool
 	Format      string   // "text" or "json"
+	ValuesFiles []string // Path to YAML files with template values (for v1beta3)
+	SetValues   []string // Template values from command line (for v1beta3)
 }
 
 // HasErrors returns true if any of the results contain errors

pkg/preflight/read_specs.go

@@ -88,6 +88,52 @@ func preprocessV1Beta3Specs(args []string) ([]string, []string, error) {
 	valuesFiles := viper.GetStringSlice("values")
 	setValues := viper.GetStringSlice("set")
 
+	// Check if any args contain v1beta3 specs
+	hasV1Beta3 := false
+	for _, arg := range args {
+		// Skip non-file arguments
+		if arg == "-" || strings.HasPrefix(arg, "http://") || strings.HasPrefix(arg, "https://") ||
+			strings.HasPrefix(arg, "secret/") || strings.HasPrefix(arg, "configmap/") {
+			continue
+		}
+
+		// Check if file exists
+		if _, err := os.Stat(arg); err != nil {
+			continue
+		}
+
+		// Read the file
+		content, err := os.ReadFile(arg)
+		if err != nil {
+			continue
+		}
+
+		// Check if it's a v1beta3 spec with templates
+		contentStr := string(content)
+		var parsed map[string]interface{}
+		if err := yaml.Unmarshal(content, &parsed); err == nil {
+			if apiVersion, ok := parsed["apiVersion"]; ok && apiVersion == constants.Troubleshootv1beta3Kind {
+				// Only require values if the spec has Helm templates
+				if strings.Contains(contentStr, "{{") && strings.Contains(contentStr, "}}") {
+					hasV1Beta3 = true
+					break
+				}
+			}
+		} else {
+			// If YAML parsing fails, check raw content for v1beta3 with templates
+			if strings.Contains(contentStr, "apiVersion: troubleshoot.sh/v1beta3") &&
+				strings.Contains(contentStr, "{{") && strings.Contains(contentStr, "}}") {
+				hasV1Beta3 = true
+				break
+			}
+		}
+	}
+
+	// If v1beta3 spec with templates found but no values provided, return error
+	if hasV1Beta3 && len(valuesFiles) == 0 && len(setValues) == 0 {
+		return nil, nil, errors.New("v1beta3 specs with Helm templates require a values file. Please provide values using --values or --set flags")
+	}
+
 	// If no values provided, return args unchanged
 	if len(valuesFiles) == 0 && len(setValues) == 0 {
 		return args, nil, nil
@@ -109,7 +155,7 @@ func preprocessV1Beta3Specs(args []string) ([]string, []string, error) {
 			return nil, nil, errors.Wrapf(err, "failed to parse values file %s", valuesFile)
 		}
 
-		values = mergeMaps(values, fileValues)
+		values = MergeMaps(values, fileValues)
 	}
 
 	// Apply --set values

pkg/preflight/read_specs_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/replicatedhq/troubleshoot/internal/testutils"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
 	"github.com/replicatedhq/troubleshoot/pkg/loader"
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -410,3 +411,75 @@ func singleTestPreflightSpecsRead(t *testing.T, tt *PreflightSpecsReadTest) (*lo
 
 	return kinds, err
 }
+
+func TestPreprocessV1Beta3Specs_RequiresValues(t *testing.T) {
+	// Save and restore viper state
+	oldValues := viper.Get("values")
+	oldSet := viper.Get("set")
+	defer func() {
+		viper.Set("values", oldValues)
+		viper.Set("set", oldSet)
+	}()
+
+	t.Run("v1beta3 without values should error", func(t *testing.T) {
+		// Clear viper values
+		viper.Set("values", []string{})
+		viper.Set("set", []string{})
+
+		v1beta3File := filepath.Join(testutils.FileDir(), "../../examples/preflight/simple-v1beta3.yaml")
+		_, _, err := preprocessV1Beta3Specs([]string{v1beta3File})
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "v1beta3 specs with Helm templates require a values file")
+	})
+
+	t.Run("v1beta3 with values file should succeed", func(t *testing.T) {
+		valuesFile := filepath.Join(testutils.FileDir(), "../../examples/preflight/values-v1beta3-1.yaml")
+		viper.Set("values", []string{valuesFile})
+		viper.Set("set", []string{})
+
+		v1beta3File := filepath.Join(testutils.FileDir(), "../../examples/preflight/simple-v1beta3.yaml")
+		processedArgs, tempFiles, err := preprocessV1Beta3Specs([]string{v1beta3File})
+
+		// Clean up temp files
+		defer func() {
+			for _, f := range tempFiles {
+				_ = os.Remove(f)
+			}
+		}()
+
+		require.NoError(t, err)
+		assert.NotNil(t, processedArgs)
+	})
+
+	t.Run("v1beta3 with --set values should succeed", func(t *testing.T) {
+		viper.Set("values", []string{})
+		viper.Set("set", []string{"kubernetes.enabled=true"})
+
+		v1beta3File := filepath.Join(testutils.FileDir(), "../../examples/preflight/simple-v1beta3.yaml")
+		processedArgs, tempFiles, err := preprocessV1Beta3Specs([]string{v1beta3File})
+
+		// Clean up temp files
+		defer func() {
+			for _, f := range tempFiles {
+				_ = os.Remove(f)
+			}
+		}()
+
+		require.NoError(t, err)
+		assert.NotNil(t, processedArgs)
+	})
+
+	t.Run("v1beta2 without values should succeed", func(t *testing.T) {
+		viper.Set("values", []string{})
+		viper.Set("set", []string{})
+
+		v1beta2File := filepath.Join(testutils.FileDir(), "../../testdata/preflightspec/troubleshoot_v1beta2_preflight_gotest.yaml")
+		processedArgs, tempFiles, err := preprocessV1Beta3Specs([]string{v1beta2File})
+
+		require.NoError(t, err)
+		assert.NotNil(t, processedArgs)
+		assert.Empty(t, tempFiles)
+		assert.Equal(t, []string{v1beta2File}, processedArgs)
+	})
+}

pkg/preflight/template.go

@@ -33,7 +33,7 @@ func RunTemplate(templateFile string, valuesFiles []string, setValues []string,
 		if err != nil {
 			return errors.Wrapf(err, "failed to load values file %s", valuesFile)
 		}
-		values = mergeMaps(values, fileValues)
+		values = MergeMaps(values, fileValues)
 	}
 
 	// Apply --set values (Helm semantics)
@@ -165,8 +165,8 @@ func cleanRenderedYAML(content string) string {
 	return strings.Join(cleaned, "\n") + "\n"
 }
 
-// mergeMaps recursively merges two maps
+// MergeMaps recursively merges two maps, with overlay taking precedence
-func mergeMaps(base, overlay map[string]interface{}) map[string]interface{} {
+func MergeMaps(base, overlay map[string]interface{}) map[string]interface{} {
 	result := make(map[string]interface{})
 
 	// Copy base map
@@ -180,7 +180,7 @@ func mergeMaps(base, overlay map[string]interface{}) map[string]interface{} {
 			// If both are maps, merge recursively
 			if baseMap, ok := baseVal.(map[string]interface{}); ok {
 				if overlayMap, ok := v.(map[string]interface{}); ok {
-					result[k] = mergeMaps(baseMap, overlayMap)
+					result[k] = MergeMaps(baseMap, overlayMap)
 					continue
 				}
 			}

pkg/preflight/template_test.go

@@ -430,7 +430,7 @@ func TestRender_V1Beta3_MergeMultipleValuesFiles_And_SetPrecedence(t *testing.T)
 	for _, f := range []string{minimalFile, file1, file3} {
 		m, err := loadValuesFile(f)
 		require.NoError(t, err)
-		vals = mergeMaps(vals, m)
+		vals = MergeMaps(vals, m)
 	}
 
 	// First render without --set; expect NO kubernetes analyzer