Skip to content

Commit 03a1f84

Browse files
authored
Add HelmChart manifest validation for linter (#636)
* Add validation for chart-to-HelmChart manifest mapping Validates that every configured Helm chart has a corresponding HelmChart manifest (kind: HelmChart). This ensures templates can be properly rendered with builder values during preflight checks. Key changes: - New validation layer with batch error reporting for missing manifests - Refactored extraction to consolidate HelmChart discovery (eliminates duplicate calls) - Separated discovery (lenient) from validation (strict) layers - Added auto-discovery support for HelmChart manifests - Made manifest paths optional in config (validation happens at runtime) The validation runs after resource discovery but before linting, catching configuration errors early with clear, actionable error messages. * Add comprehensive tests and documentation for HelmChart validation Adds unit tests, integration tests, CLI tests, and documentation for the chart-to-HelmChart manifest validation feature to ensure correctness and provide clear guidance for users. Changes: - Add 11 unit tests in validation_test.go covering all validation scenarios - Add 6 integration tests in validation_integration_test.go with test fixtures - Add 3 CLI integration tests in lint_test.go for end-to-end validation - Create 6 test scenario fixtures in testdata/validation/ - Add HelmChart Manifest Requirements section to docs/lint-format.md - Update existing tests to reflect lenient discovery pattern The tests cover success cases, error cases (single and batch), warnings for orphaned manifests, auto-discovery mode, and edge cases. Documentation includes configuration examples, validation behavior, error messages, and troubleshooting guidance. * Fix test: update expected error message for empty manifest path
1 parent 9c17c20 commit 03a1f84

File tree

41 files changed

+1559
-113
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+1559
-113
lines changed

cli/cmd/image_extraction_test.go

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ func TestExtractImagesFromConfig_ChartWithRequiredValues_WithMatchingHelmChartMa
9191
}
9292

9393
func TestExtractImagesFromConfig_ChartWithRequiredValues_NoHelmChartManifest(t *testing.T) {
94-
// Test that extraction fails when manifests are not configured
94+
// Test that discovery returns empty map when manifests are not configured (lenient)
95+
// Validation happens at a higher level (in lint command)
9596
chartPath := getAbsTestDataPath(t, filepath.Join("testdata", "image-extraction", "chart-with-required-values-test", "chart"))
9697

9798
config := &tools.Config{
@@ -101,18 +102,19 @@ func TestExtractImagesFromConfig_ChartWithRequiredValues_NoHelmChartManifest(t *
101102
Manifests: []string{}, // No manifests configured
102103
}
103104

104-
// Try to extract HelmChart manifests - should fail because manifests are required
105-
_, err := lint2.DiscoverHelmChartManifests(config.Manifests)
106-
107-
// Should fail because manifests are required
108-
if err == nil {
109-
t.Fatal("Expected error when manifests not configured, got nil")
105+
// Discovery is lenient - returns empty map instead of error
106+
helmCharts, err := lint2.DiscoverHelmChartManifests(config.Manifests)
107+
if err != nil {
108+
t.Fatalf("DiscoverHelmChartManifests should not error on empty manifests: %v", err)
110109
}
111110

112-
// Error should mention manifests configuration
113-
if !strings.Contains(err.Error(), "no manifests configured") {
114-
t.Errorf("Expected error about manifests configuration, got: %v", err)
111+
// Should return empty map (lenient discovery)
112+
if len(helmCharts) != 0 {
113+
t.Errorf("Expected empty map, got %d HelmCharts", len(helmCharts))
115114
}
115+
116+
// Note: Validation that charts need manifests happens in the lint command
117+
// This tests only the discovery layer behavior
116118
}
117119

118120

@@ -247,7 +249,8 @@ func TestExtractImagesFromConfig_NoCharts_ReturnsError(t *testing.T) {
247249
}
248250

249251
func TestExtractImagesFromConfig_NoManifests_ReturnsError(t *testing.T) {
250-
// Test that manifests are required for image extraction
252+
// Test that discovery is lenient when manifests array is empty
253+
// Validation happens at lint command level
251254
chartPath := getAbsTestDataPath(t, filepath.Join("testdata", "image-extraction", "simple-chart-test", "chart"))
252255

253256
config := &tools.Config{
@@ -257,18 +260,19 @@ func TestExtractImagesFromConfig_NoManifests_ReturnsError(t *testing.T) {
257260
Manifests: []string{}, // No manifests configured
258261
}
259262

260-
// Try to extract HelmChart manifests - should fail because manifests are required
261-
_, err := lint2.DiscoverHelmChartManifests(config.Manifests)
262-
263-
// Should fail because manifests are required
264-
if err == nil {
265-
t.Fatal("Expected error when manifests not configured, got nil")
263+
// Discovery is lenient - returns empty map instead of error
264+
helmCharts, err := lint2.DiscoverHelmChartManifests(config.Manifests)
265+
if err != nil {
266+
t.Fatalf("DiscoverHelmChartManifests should not error on empty manifests: %v", err)
266267
}
267268

268-
// Error should mention manifests configuration
269-
if !strings.Contains(err.Error(), "no manifests configured") {
270-
t.Errorf("Expected error about manifests configuration, got: %v", err)
269+
// Should return empty map (lenient discovery)
270+
if len(helmCharts) != 0 {
271+
t.Errorf("Expected empty map, got %d HelmCharts", len(helmCharts))
271272
}
273+
274+
// Note: Validation that charts require manifests happens in runLint()
275+
// This tests only the discovery layer behavior (which is now lenient)
272276
}
273277

274278

@@ -316,7 +320,8 @@ func TestExtractImagesFromConfig_EmptyBuilder_FailsToRender(t *testing.T) {
316320
}
317321

318322
func TestExtractImagesFromConfig_NoHelmChartInManifests_FailsDiscovery(t *testing.T) {
319-
// Test that manifests with other K8s resources but no HelmChart kind fail discovery
323+
// Test that discovery is lenient when manifests contain no HelmCharts
324+
// Validation happens at lint command level
320325
chartPath := getAbsTestDataPath(t, filepath.Join("testdata", "image-extraction", "no-helmchart-test", "chart"))
321326
manifestGlob := getAbsTestDataPath(t, filepath.Join("testdata", "image-extraction", "no-helmchart-test", "manifests")) + "/*.yaml"
322327

@@ -327,16 +332,17 @@ func TestExtractImagesFromConfig_NoHelmChartInManifests_FailsDiscovery(t *testin
327332
Manifests: []string{manifestGlob},
328333
}
329334

330-
// Try to extract HelmChart manifests - should fail because manifests don't contain HelmCharts
331-
_, err := lint2.DiscoverHelmChartManifests(config.Manifests)
332-
333-
// Should fail because manifests are configured but contain no HelmCharts
334-
if err == nil {
335-
t.Fatal("Expected error when manifests configured but no HelmCharts found, got nil")
335+
// Discovery is lenient - returns empty map when no HelmCharts found
336+
helmCharts, err := lint2.DiscoverHelmChartManifests(config.Manifests)
337+
if err != nil {
338+
t.Fatalf("DiscoverHelmChartManifests should not error when no HelmCharts found: %v", err)
336339
}
337340

338-
// Error should mention no HelmChart resources found
339-
if !strings.Contains(err.Error(), "no HelmChart resources found") {
340-
t.Errorf("Expected error about no HelmCharts, got: %v", err)
341+
// Should return empty map (lenient discovery)
342+
if len(helmCharts) != 0 {
343+
t.Errorf("Expected empty map when no HelmCharts in manifests, got %d", len(helmCharts))
341344
}
345+
346+
// Note: Validation that charts need HelmChart manifests happens in ValidateChartToHelmChartMapping()
347+
// Discovery layer is lenient and allows mixed directories
342348
}

cli/cmd/lint.go

Lines changed: 50 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -81,20 +81,18 @@ func extractAllPathsAndMetadata(ctx context.Context, config *tools.Config, verbo
8181
return nil, err
8282
}
8383
result.Preflights = preflights
84+
}
8485

85-
// Get HelmChart manifests (used for v1beta3 preflight validation)
86-
// HelmChart manifests are optional - only required for v1beta3 preflights
86+
// Discover HelmChart manifests ONCE (used by preflight rendering, support bundle analysis, image extraction, validation)
87+
if len(config.Manifests) > 0 {
8788
helmChartManifests, err := lint2.DiscoverHelmChartManifests(config.Manifests)
8889
if err != nil {
89-
// Only error if it's not a "no manifests found" error
90-
if !strings.Contains(err.Error(), "no HelmChart resources found") {
91-
return nil, err
92-
}
93-
// No manifests found is OK - set empty map
94-
result.HelmChartManifests = make(map[string]*lint2.HelmChartManifest)
95-
} else {
96-
result.HelmChartManifests = helmChartManifests
90+
return nil, fmt.Errorf("failed to discover HelmChart manifests: %w", err)
9791
}
92+
result.HelmChartManifests = helmChartManifests
93+
} else {
94+
// No manifests configured - return empty map (validation will check if needed)
95+
result.HelmChartManifests = make(map[string]*lint2.HelmChartManifest)
9896
}
9997

10098
// Discover support bundles
@@ -104,29 +102,10 @@ func extractAllPathsAndMetadata(ctx context.Context, config *tools.Config, verbo
104102
return nil, err
105103
}
106104
result.SupportBundles = sbPaths
107-
108-
// Get HelmChart manifests if not already extracted
109-
if result.HelmChartManifests == nil {
110-
helmChartManifests, err := lint2.DiscoverHelmChartManifests(config.Manifests)
111-
if err != nil {
112-
// Support bundles don't require HelmChart manifests - only error if manifests are explicitly configured
113-
// but fail to parse. If no HelmChart manifests exist, that's fine (return empty map).
114-
if len(config.Manifests) > 0 {
115-
// Check if error is "no HelmChart resources found" - that's acceptable
116-
if err != nil && !strings.Contains(err.Error(), "no HelmChart resources found") {
117-
return nil, err
118-
}
119-
}
120-
// Set empty map so we don't try to extract again
121-
result.HelmChartManifests = make(map[string]*lint2.HelmChartManifest)
122-
} else {
123-
result.HelmChartManifests = helmChartManifests
124-
}
125-
}
126105
}
127106

128-
// Extract charts with metadata (ONLY for verbose mode)
129-
if verbose && len(config.Charts) > 0 {
107+
// Extract charts with metadata (needed for validation and image extraction)
108+
if len(config.Charts) > 0 {
130109
chartsWithMetadata, err := lint2.GetChartsWithMetadataFromConfig(config)
131110
if err != nil {
132111
return nil, err
@@ -197,11 +176,19 @@ func (r *runners) runLint(cmd *cobra.Command, args []string) error {
197176
// Convert to manifests glob patterns for compatibility
198177
config.Manifests = append(config.Manifests, sbPaths...)
199178

179+
// Auto-discover HelmChart manifests (needed for chart validation)
180+
helmChartPaths, err := lint2.DiscoverHelmChartPaths(filepath.Join(".", "**"))
181+
if err != nil {
182+
return errors.Wrap(err, "failed to discover HelmChart manifests")
183+
}
184+
config.Manifests = append(config.Manifests, helmChartPaths...)
185+
200186
// Print what was discovered
201187
fmt.Fprintf(r.w, "Discovered resources:\n")
202188
fmt.Fprintf(r.w, " - %d Helm chart(s)\n", len(chartPaths))
203189
fmt.Fprintf(r.w, " - %d Preflight spec(s)\n", len(preflightPaths))
204-
fmt.Fprintf(r.w, " - %d Support Bundle spec(s)\n\n", len(sbPaths))
190+
fmt.Fprintf(r.w, " - %d Support Bundle spec(s)\n", len(sbPaths))
191+
fmt.Fprintf(r.w, " - %d HelmChart manifest(s)\n\n", len(helmChartPaths))
205192
r.w.Flush()
206193

207194
// If nothing was found, exit early
@@ -231,6 +218,37 @@ func (r *runners) runLint(cmd *cobra.Command, args []string) error {
231218
return errors.Wrap(err, "failed to extract paths and metadata")
232219
}
233220

221+
// Validate chart-to-HelmChart mapping if charts are configured
222+
if len(config.Charts) > 0 {
223+
// Charts configured but no manifests - error early
224+
if len(config.Manifests) == 0 {
225+
return errors.New("charts are configured but no manifests paths provided\n\n" +
226+
"HelmChart manifests (kind: HelmChart) are required for each chart.\n" +
227+
"Add manifest paths to your .replicated config:\n\n" +
228+
"manifests:\n" +
229+
" - \"./manifests/**/*.yaml\"")
230+
}
231+
232+
// Validate mapping using already-extracted metadata
233+
validationResult, err := lint2.ValidateChartToHelmChartMapping(
234+
extracted.ChartsWithMetadata, // Already populated in extraction
235+
extracted.HelmChartManifests,
236+
)
237+
if err != nil {
238+
// Hard error - stop before linting
239+
return errors.Wrap(err, "chart validation failed")
240+
}
241+
242+
// Display warnings (orphaned HelmChart manifests)
243+
if r.outputFormat == "table" && len(validationResult.Warnings) > 0 {
244+
for _, warning := range validationResult.Warnings {
245+
fmt.Fprintf(r.w, "Warning: %s\n", warning)
246+
}
247+
fmt.Fprintln(r.w)
248+
r.w.Flush()
249+
}
250+
}
251+
234252
// Extract and display images if verbose mode is enabled
235253
if r.args.lintVerbose && len(extracted.ChartsWithMetadata) > 0 {
236254
imageResults, err := r.extractImagesFromCharts(cmd.Context(), extracted.ChartsWithMetadata, extracted.HelmChartManifests)

0 commit comments

Comments
 (0)