Skip to content

Commit 3f5ab9c

Browse files
committed
doesnt harcode apiVersion line when looking and figures out which apiVersion to give if none is there
1 parent 0316bb2 commit 3f5ab9c

File tree

3 files changed

+99
-39
lines changed

3 files changed

+99
-39
lines changed

cmd/preflight/cli/lint.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,23 @@ func LintCmd() *cobra.Command {
1515
cmd := &cobra.Command{
1616
Use: "lint [spec-files...]",
1717
Args: cobra.MinimumNArgs(1),
18-
Short: "Lint v1beta3 preflight specs for syntax and structural errors",
19-
Long: `Lint v1beta3 preflight specs for syntax and structural errors.
18+
Short: "Lint preflight specs for syntax and structural errors",
19+
Long: `Lint preflight specs for syntax and structural errors.
2020
21-
This command validates v1beta3 preflight specs and checks for:
22-
- YAML syntax errors
21+
This command validates troubleshoot specs and checks for:
22+
- YAML syntax errors (missing colons, invalid structure)
2323
- Missing required fields (apiVersion, kind, metadata, spec)
24-
- Invalid template syntax ({{ .Values.* }})
24+
- Invalid template syntax ({{ .Values.* }}, {{ .Release.* }}, etc.)
2525
- Missing analyzers or collectors
2626
- Common structural issues
2727
- Missing docStrings (warning)
2828
29-
The linter only validates v1beta3 specs. For v1beta2 specs, use the 'convert' command first.
29+
Both v1beta2 and v1beta3 apiVersions are supported. Use 'convert' if you need a full structural conversion between schema versions.
30+
31+
The --fix flag can automatically repair:
32+
- Missing colons in YAML (e.g., "metadata" → "metadata:")
33+
- Missing or malformed apiVersion line. If templating ({{ }}) or docString fields are detected, apiVersion is set to v1beta3; otherwise v1beta2.
34+
- Template expressions missing leading dot (e.g., "{{ Values.x }}" → "{{ .Values.x }}")
3035
3136
Examples:
3237
# Lint a single spec file
@@ -35,7 +40,7 @@ Examples:
3540
# Lint multiple spec files
3641
preflight lint spec1.yaml spec2.yaml spec3.yaml
3742
38-
# Lint with automatic fixes
43+
# Lint with automatic fixes (may need to run multiple times for complex issues)
3944
preflight lint --fix my-preflight.yaml
4045
4146
# Lint and output as JSON for CI/CD integration

cmd/troubleshoot/cli/root.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strings"
77

88
"github.com/replicatedhq/troubleshoot/cmd/internal/util"
9+
preflightcli "github.com/replicatedhq/troubleshoot/cmd/preflight/cli"
910
"github.com/replicatedhq/troubleshoot/internal/traces"
1011
"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
1112
"github.com/replicatedhq/troubleshoot/pkg/logger"
@@ -109,7 +110,7 @@ If no arguments are provided, specs are automatically loaded from the cluster by
109110
cmd.AddCommand(Schedule())
110111
cmd.AddCommand(UploadCmd())
111112
cmd.AddCommand(util.VersionCmd())
112-
cmd.AddCommand(LintCmd())
113+
cmd.AddCommand(preflightcli.LintCmd())
113114

114115
cmd.Flags().StringSlice("redactors", []string{}, "names of the additional redactors to use")
115116
cmd.Flags().Bool("redact", true, "enable/disable default redactions")

pkg/lint/lint.go

Lines changed: 85 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ package lint
22

33
import (
44
"fmt"
5-
"io/ioutil"
65
"regexp"
76
"strings"
87

8+
"os"
9+
910
"github.com/pkg/errors"
1011
"github.com/replicatedhq/troubleshoot/pkg/constants"
1112
"sigs.k8s.io/yaml"
@@ -60,32 +61,11 @@ func lintFile(filePath string, fix bool) (LintResult, error) {
6061
}
6162

6263
// Read file
63-
content, err := ioutil.ReadFile(filePath)
64+
content, err := os.ReadFile(filePath)
6465
if err != nil {
6566
return result, errors.Wrapf(err, "failed to read file %s", filePath)
6667
}
6768

68-
// Check for v1beta3 apiVersion
69-
if !strings.Contains(string(content), constants.Troubleshootv1beta3Kind) {
70-
result.Errors = append(result.Errors, LintError{
71-
Line: 1,
72-
Message: fmt.Sprintf("File must contain apiVersion: %s", constants.Troubleshootv1beta3Kind),
73-
Field: "apiVersion",
74-
})
75-
// Try to fix wrong apiVersion
76-
if fix {
77-
fixed, err := applyFixes(filePath, string(content), result)
78-
if err != nil {
79-
return result, err
80-
}
81-
if fixed {
82-
// Re-lint to verify fixes
83-
return lintFile(filePath, false)
84-
}
85-
}
86-
return result, nil
87-
}
88-
8969
// Check if file contains template expressions
9070
hasTemplates := strings.Contains(string(content), "{{") && strings.Contains(string(content), "}}")
9171

@@ -102,6 +82,22 @@ func lintFile(filePath string, fix bool) (LintResult, error) {
10282
})
10383
// Don't return yet - we want to try to fix this error
10484
// Continue to applyFixes at the end
85+
// Try to surface apiVersion issues even if YAML failed to parse
86+
// Detect via simple textual scan
87+
avLine, avValue := findAPIVersionLineAndValue(string(content))
88+
if avLine == 0 {
89+
result.Errors = append(result.Errors, LintError{
90+
Line: 0,
91+
Field: "apiVersion",
92+
Message: "Missing or empty 'apiVersion' field",
93+
})
94+
} else if avValue != constants.Troubleshootv1beta2Kind && avValue != constants.Troubleshootv1beta3Kind {
95+
result.Errors = append(result.Errors, LintError{
96+
Line: avLine,
97+
Field: "apiVersion",
98+
Message: fmt.Sprintf("Invalid 'apiVersion' value %q; expected %s or %s", avValue, constants.Troubleshootv1beta2Kind, constants.Troubleshootv1beta3Kind),
99+
})
100+
}
105101
if fix {
106102
fixed, err := applyFixes(filePath, string(content), result)
107103
if err != nil {
@@ -116,6 +112,21 @@ func lintFile(filePath string, fix bool) (LintResult, error) {
116112
}
117113
// For templated files, we can't parse YAML strictly, so just check template syntax
118114
result.Errors = append(result.Errors, checkTemplateSyntax(string(content))...)
115+
// Surface apiVersion issues via textual scan for templated files
116+
avLine, avValue := findAPIVersionLineAndValue(string(content))
117+
if avLine == 0 {
118+
result.Errors = append(result.Errors, LintError{
119+
Line: 0,
120+
Field: "apiVersion",
121+
Message: "Missing or empty 'apiVersion' field",
122+
})
123+
} else if avValue != constants.Troubleshootv1beta2Kind && avValue != constants.Troubleshootv1beta3Kind {
124+
result.Errors = append(result.Errors, LintError{
125+
Line: avLine,
126+
Field: "apiVersion",
127+
Message: fmt.Sprintf("Invalid 'apiVersion' value %q; expected %s or %s", avValue, constants.Troubleshootv1beta2Kind, constants.Troubleshootv1beta3Kind),
128+
})
129+
}
119130
// Continue to applyFixes for templates too
120131
if fix {
121132
fixed, err := applyFixes(filePath, string(content), result)
@@ -146,6 +157,17 @@ func lintFile(filePath string, fix bool) (LintResult, error) {
146157
}
147158
}
148159

160+
// Validate apiVersion value if present
161+
if apiVersion, ok := parsed["apiVersion"].(string); ok && apiVersion != "" {
162+
if apiVersion != constants.Troubleshootv1beta2Kind && apiVersion != constants.Troubleshootv1beta3Kind {
163+
result.Errors = append(result.Errors, LintError{
164+
Line: findLineNumber(string(content), "apiVersion"),
165+
Field: "apiVersion",
166+
Message: fmt.Sprintf("Invalid 'apiVersion' value %q; expected %s or %s", apiVersion, constants.Troubleshootv1beta2Kind, constants.Troubleshootv1beta3Kind),
167+
})
168+
}
169+
}
170+
149171
// Check for common issues
150172
result.Warnings = append(result.Warnings, checkCommonIssues(parsed, string(content))...)
151173

@@ -404,6 +426,14 @@ func applyFixes(filePath, content string, result LintResult) (bool, error) {
404426
newContent := content
405427
lines := strings.Split(newContent, "\n")
406428

429+
// Determine desired apiVersion for fixes
430+
hasTemplates := strings.Contains(content, "{{") && strings.Contains(content, "}}")
431+
hasDocStrings := strings.Contains(content, "docString:")
432+
desiredAPIVersion := constants.Troubleshootv1beta2Kind
433+
if hasTemplates || hasDocStrings {
434+
desiredAPIVersion = constants.Troubleshootv1beta3Kind
435+
}
436+
407437
// Sort errors by line number (descending) to avoid line number shifts when editing
408438
errorsByLine := make(map[int][]LintError)
409439
for _, err := range result.Errors {
@@ -461,12 +491,11 @@ func applyFixes(filePath, content string, result LintResult) (bool, error) {
461491
}
462492
}
463493

464-
// Fix 3: Fix wrong apiVersion
465-
if strings.Contains(err.Message, "File must contain apiVersion:") && err.Field == "apiVersion" {
466-
if strings.Contains(line, "apiVersion:") && !strings.Contains(line, constants.Troubleshootv1beta3Kind) {
467-
// Replace existing apiVersion with correct one
494+
// Fix 3: Replace invalid apiVersion value with desiredAPIVersion
495+
if strings.Contains(err.Message, "Invalid 'apiVersion' value") && err.Field == "apiVersion" {
496+
if strings.Contains(line, "apiVersion:") {
468497
indent := line[:len(line)-len(strings.TrimLeft(line, " \t"))]
469-
line = indent + "apiVersion: " + constants.Troubleshootv1beta3Kind
498+
line = indent + "apiVersion: " + desiredAPIVersion
470499
fixed = true
471500
}
472501
}
@@ -481,8 +510,14 @@ func applyFixes(filePath, content string, result LintResult) (bool, error) {
481510
// Fix 4: Add missing required top-level fields
482511
for _, err := range result.Errors {
483512
if err.Field == "apiVersion" && strings.Contains(err.Message, "Missing or empty 'apiVersion'") {
484-
// Add apiVersion at the beginning
485-
lines = append([]string{"apiVersion: " + constants.Troubleshootv1beta3Kind}, lines...)
513+
// Replace existing empty apiVersion line if present; otherwise prepend
514+
if avLine, avVal := findAPIVersionLineAndValue(newContent); avLine > 0 && strings.TrimSpace(avVal) == "" {
515+
line := lines[avLine-1]
516+
indent := line[:len(line)-len(strings.TrimLeft(line, " \t"))]
517+
lines[avLine-1] = indent + "apiVersion: " + desiredAPIVersion
518+
} else {
519+
lines = append([]string{"apiVersion: " + desiredAPIVersion}, lines...)
520+
}
486521
fixed = true
487522
} else if err.Field == "kind" && strings.Contains(err.Message, "Missing or empty 'kind'") {
488523
// Try to determine if it should be Preflight or SupportBundle based on filename
@@ -526,7 +561,7 @@ func applyFixes(filePath, content string, result LintResult) (bool, error) {
526561
// Write fixed content back to file if changes were made
527562
if fixed {
528563
newContent = strings.Join(lines, "\n")
529-
if err := ioutil.WriteFile(filePath, []byte(newContent), 0644); err != nil {
564+
if err := os.WriteFile(filePath, []byte(newContent), 0644); err != nil {
530565
return false, errors.Wrapf(err, "failed to write fixed content to %s", filePath)
531566
}
532567
}
@@ -544,6 +579,25 @@ func findLineNumber(content, search string) int {
544579
return 0
545580
}
546581

582+
// findAPIVersionLineAndValue locates the first line that declares apiVersion and returns its
583+
// 1-based line number and the trimmed value to the right of the colon. Returns (0, "") if not found.
584+
func findAPIVersionLineAndValue(content string) (int, string) {
585+
lines := strings.Split(content, "\n")
586+
for i, line := range lines {
587+
trimmed := strings.TrimSpace(line)
588+
if strings.HasPrefix(trimmed, "apiVersion:") {
589+
// extract value after the first colon
590+
parts := strings.SplitN(trimmed, ":", 2)
591+
if len(parts) == 2 {
592+
value := strings.TrimSpace(parts[1])
593+
return i + 1, value
594+
}
595+
return i + 1, ""
596+
}
597+
}
598+
return 0, ""
599+
}
600+
547601
func findAnalyzerLine(content string, index int) int {
548602
lines := strings.Split(content, "\n")
549603
analyzerCount := 0

0 commit comments

Comments
 (0)