Repository: helm/helm
Pull request #12789 (2024-02-08 17:11:22)

Fix: Ignore alias validation error for index load
<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/main/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:

This PR skips the alias validation error introduced in https://github.com/helm/helm/pull/9176 when loading chart indexes. 
fixes: https://github.com/helm/helm/issues/12735
fixes: https://github.com/helm/helm/issues/12748

**Special notes for your reviewer**:
Checking the error type and string is super brittle. And IMHO super hacky. Happy for other suggestions. Without introducing a full "chart validation" type/interface, I didn't really see another (easy) option :(. But I do think we should attempt to not revert https://github.com/helm/helm/pull/9176 .

**If applicable**:
- [ ] this PR contains documentation
- [x] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility



Patch:
diff --git a/pkg/repo/index.go b/pkg/repo/index.go
index 8a23ba060..2f062b028 100644
--- a/pkg/repo/index.go
+++ b/pkg/repo/index.go
@@ -362,7 +362,7 @@ func loadIndex(data []byte, source string) (*IndexFile, error) {
 			if cvs[idx].APIVersion == "" {
 				cvs[idx].APIVersion = chart.APIVersionV1
 			}
-			if err := cvs[idx].Validate(); err != nil {
+			if err := cvs[idx].Validate(); ignoreSkippableChartValidationError(err) != nil {
 				log.Printf("skipping loading invalid entry for chart %q %q from %s: %s", name, cvs[idx].Version, source, err)
 				cvs = append(cvs[:idx], cvs[idx+1:]...)
 			}
@@ -388,3 +388,23 @@ func jsonOrYamlUnmarshal(b []byte, i interface{}) error {
 	}
 	return yaml.UnmarshalStrict(b, i)
 }
+
+// ignoreSkippableChartValidationError inspect the given error and returns nil if
+// the error isn't important for index loading
+//
+// In particular, charts may introduce validations that don't impact repository indexes
+// And repository indexes may be generated by older/non-complient software, which doesn't
+// conform to all validations.
+func ignoreSkippableChartValidationError(err error) error {
+	verr, ok := err.(chart.ValidationError)
+	if !ok {
+		return err
+	}
+
+	// https://github.com/helm/helm/issues/12748 (JFrog repository strips alias field)
+	if strings.HasPrefix(verr.Error(), "validation: more than one dependency with name or alias") {
+		return nil
+	}
+
+	return err
+}


Test patch:
diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go
index cb9317f7e..3852dc15d 100644
--- a/pkg/repo/index_test.go
+++ b/pkg/repo/index_test.go
@@ -20,6 +20,7 @@ import (
 	"bufio"
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -592,3 +593,50 @@ func TestAddFileIndexEntriesNil(t *testing.T) {
 		}
 	}
 }
+
+func TestIgnoreSkippableChartValidationError(t *testing.T) {
+	type TestCase struct {
+		Input        error
+		ErrorSkipped bool
+	}
+	testCases := map[string]TestCase{
+		"nil": {
+			Input: nil,
+		},
+		"generic_error": {
+			Input: fmt.Errorf("foo"),
+		},
+		"non_skipped_validation_error": {
+			Input: chart.ValidationError("chart.metadata.type must be application or library"),
+		},
+		"skipped_validation_error": {
+			Input:        chart.ValidationErrorf("more than one dependency with name or alias %q", "foo"),
+			ErrorSkipped: true,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			result := ignoreSkippableChartValidationError(tc.Input)
+
+			if tc.Input == nil {
+				if result != nil {
+					t.Error("expected nil result for nil input")
+				}
+				return
+			}
+
+			if tc.ErrorSkipped {
+				if result != nil {
+					t.Error("expected nil result for skipped error")
+				}
+				return
+			}
+
+			if tc.Input != result {
+				t.Error("expected the result equal to input")
+			}
+
+		})
+	}
+}