fluxcd
/
source-controller
mirror of https://github.com/fluxcd/source-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

mitigate issue with chart validation in Helm 3.14 #1515 

Signed-off-by: ricardo.bartels@telekom.de <ricardo.bartels@telekom.de>
This commit is contained in:
ricardo.bartels@telekom.de 2024-07-19 09:06:27 +02:00
parent 58b4e6d719
commit a65f6fda92
7 changed files with 256 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
internal/helm/chart/builder_remote_test.go
View File

@ -99,6 +99,7 @@ entries:
- https://example.com/grafana.tgz
description: string
version: 6.17.4
name: grafana
`)
mockGetter := &mockIndexChartGetter{

33
internal/helm/repository/chart_repository.go
View File

@ -28,6 +28,7 @@ import (
"os"
"path"
"sort"
"strings"
"sync"
"github.com/Masterminds/semver/v3"
@ -86,18 +87,24 @@ func IndexFromBytes(b []byte) (*repo.IndexFile, error) {
return nil, repo.ErrNoAPIVersion
}
for _, cvs := range i.Entries {
for name, cvs := range i.Entries {
for idx := len(cvs) - 1; idx >= 0; idx-- {
if cvs[idx] == nil {
continue
}
// When metadata section missing, initialize with no data
if cvs[idx].Metadata == nil {
cvs[idx].Metadata = &chart.Metadata{}
}
if cvs[idx].APIVersion == "" {
cvs[idx].APIVersion = chart.APIVersionV1
}
if err := cvs[idx].Validate(); err != nil {
if err := cvs[idx].Validate(); ignoreSkippableChartValidationError(err) != nil {
cvs = append(cvs[:idx], cvs[idx+1:]...)
}
}
// adjust slice to only contain a set of valid versions
i.Entries[name] = cvs
}
i.SortEntries()
@ -501,3 +508,25 @@ 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.
//
// this code is taken from https://github.com/helm/helm/blob/v3.15.2/pkg/repo/index.go#L402
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 from index)
if strings.HasPrefix(verr.Error(), "validation: more than one dependency with name or alias") {
return nil
}
return err
}

148
internal/helm/repository/chart_repository_test.go
View File

@ -672,7 +672,7 @@ func verifyLocalIndex(t *testing.T, i *repo.IndexFile) {
g := NewWithT(t)
g.Expect(i.Entries).ToNot(BeNil())
g.Expect(i.Entries).To(HaveLen(3), "expected 3 entries in index file")
g.Expect(i.Entries).To(HaveLen(4), "expected 4 entries in index file")
alpine, ok := i.Entries["alpine"]
g.Expect(ok).To(BeTrue(), "expected 'alpine' entry to exist")
@ -682,6 +682,10 @@ func verifyLocalIndex(t *testing.T, i *repo.IndexFile) {
g.Expect(ok).To(BeTrue(), "expected 'nginx' entry to exist")
g.Expect(nginx).To(HaveLen(2), "'nginx' should have 2 entries")
broken, ok := i.Entries["xChartWithDuplicateDependenciesAndMissingAlias"]
g.Expect(ok).To(BeTrue(), "expected 'xChartWithDuplicateDependenciesAndMissingAlias' entry to exist")
g.Expect(broken).To(HaveLen(1), "'xChartWithDuplicateDependenciesAndMissingAlias' should have 1 entries")
expects := []*repo.ChartVersion{
{
Metadata: &chart.Metadata{
@ -723,8 +727,24 @@ func verifyLocalIndex(t *testing.T, i *repo.IndexFile) {
},
Digest: "sha256:1234567890abcdef",
},
{
Metadata: &chart.Metadata{
Name: "xChartWithDuplicateDependenciesAndMissingAlias",
Description: "string",
Version: "1.2.3",
Keywords: []string{"broken", "still accepted"},
Home: "https://example.com/something",
Dependencies: []*chart.Dependency{
{Name: "kube-rbac-proxy", Version: "0.9.1"},
},
},
URLs: []string{
"https://kubernetes-charts.storage.googleapis.com/nginx-1.2.3.tgz",
},
Digest: "sha256:1234567890abcdef",
},
}
tests := []*repo.ChartVersion{alpine[0], nginx[0], nginx[1]}
tests := []*repo.ChartVersion{alpine[0], nginx[0], nginx[1], broken[0]}
for i, tt := range tests {
expect := expects[i]
@ -735,5 +755,129 @@ func verifyLocalIndex(t *testing.T, i *repo.IndexFile) {
g.Expect(tt.Home).To(Equal(expect.Home))
g.Expect(tt.URLs).To(ContainElements(expect.URLs))
g.Expect(tt.Keywords).To(ContainElements(expect.Keywords))
g.Expect(tt.Dependencies).To(ContainElements(expect.Dependencies))
}
}
// This code is taken from https://github.com/helm/helm/blob/v3.15.2/pkg/repo/index_test.go#L601
// and refers to: https://github.com/helm/helm/issues/12748
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")
}
})
}
}
var indexWithFirstVersionInvalid = `
apiVersion: v1
entries:
nginx:
- urls:
- https://charts.helm.sh/stable/alpine-1.0.0.tgz
- http://storage2.googleapis.com/kubernetes-charts/alpine-1.0.0.tgz
name: nginx
version: 0..1.0
description: string
home: https://github.com/something
digest: "sha256:1234567890abcdef"
- urls:
- https://charts.helm.sh/stable/nginx-0.2.0.tgz
name: nginx
description: string
version: 0.2.0
home: https://github.com/something/else
digest: "sha256:1234567890abcdef"
`
var indexWithLastVersionInvalid = `
apiVersion: v1
entries:
nginx:
- urls:
- https://charts.helm.sh/stable/nginx-0.2.0.tgz
name: nginx
description: string
version: 0.2.0
home: https://github.com/something/else
digest: "sha256:1234567890abcdef"
- urls:
- https://charts.helm.sh/stable/alpine-1.0.0.tgz
- http://storage2.googleapis.com/kubernetes-charts/alpine-1.0.0.tgz
name: nginx
version: 0..1.0
description: string
home: https://github.com/something
digest: "sha256:1234567890abcdef"
`
func TestIndexFromBytes_InvalidEntries(t *testing.T) {
tests := []struct {
source string
data string
}{
{
source: "indexWithFirstVersionInvalid",
data: indexWithFirstVersionInvalid,
},
{
source: "indexWithLastVersionInvalid",
data: indexWithLastVersionInvalid,
},
}
for _, tc := range tests {
t.Run(tc.source, func(t *testing.T) {
idx, err := IndexFromBytes([]byte(tc.data))
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
cvs := idx.Entries["nginx"]
if len(cvs) == 0 {
t.Error("expected one chart version not to be filtered out")
}
for _, v := range cvs {
if v.Version == "0..1.0" {
t.Error("malformed version was not filtered out")
}
}
})
}
}

30
internal/helm/testdata/chartmuseum-index.json vendored
View File

@ -77,6 +77,36 @@
"created": "0001-01-01T00:00:00Z",
"digest": "sha256:1234567890abcdef"
}
],
"xChartWithDuplicateDependenciesAndMissingAlias": [
{
"name": "xChartWithDuplicateDependenciesAndMissingAlias",
"home": "https://example.com/something",
"version": "1.2.3",
"description": "string",
"keywords": [
"broken",
"still accepted"
],
"apiVersion": "v1",
"dependencies": [
{
"name": "kube-rbac-proxy",
"version": "0.9.1",
"repository": ""
},
{
"name": "kube-rbac-proxy",
"version": "0.9.1",
"repository": ""
}
],
"urls": [
"https://kubernetes-charts.storage.googleapis.com/nginx-1.2.3.tgz"
],
"created": "0001-01-01T00:00:00Z",
"digest": "sha256:1234567890abcdef"
}
]
}
}

16
internal/helm/testdata/chartmuseum-index.yaml vendored
View File

@ -48,3 +48,19 @@ entries:
- small
- sumtin
digest: "sha256:1234567890abcdef"
xChartWithDuplicateDependenciesAndMissingAlias:
- name: xChartWithDuplicateDependenciesAndMissingAlias
description: string
version: 1.2.3
home: https://example.com/something
keywords:
- broken
- still accepted
urls:
- https://kubernetes-charts.storage.googleapis.com/nginx-1.2.3.tgz
digest: "sha256:1234567890abcdef"
dependencies:
- name: kube-rbac-proxy
version: "0.9.1"
- name: kube-rbac-proxy
version: "0.9.1"

16
internal/helm/testdata/local-index-unordered.yaml vendored
View File

@ -46,3 +46,19 @@ entries:
- small
- sumtin
digest: "sha256:1234567890abcdef"
xChartWithDuplicateDependenciesAndMissingAlias:
- name: xChartWithDuplicateDependenciesAndMissingAlias
description: string
version: 1.2.3
home: https://example.com/something
keywords:
- broken
- still accepted
urls:
- https://kubernetes-charts.storage.googleapis.com/nginx-1.2.3.tgz
digest: "sha256:1234567890abcdef"
dependencies:
- name: kube-rbac-proxy
version: "0.9.1"
- name: kube-rbac-proxy
version: "0.9.1"

16
internal/helm/testdata/local-index.yaml vendored
View File

@ -46,3 +46,19 @@ entries:
- small
- sumtin
digest: "sha256:1234567890abcdef"
xChartWithDuplicateDependenciesAndMissingAlias:
- name: xChartWithDuplicateDependenciesAndMissingAlias
description: string
version: 1.2.3
home: https://example.com/something
keywords:
- broken
- still accepted
urls:
- https://kubernetes-charts.storage.googleapis.com/nginx-1.2.3.tgz
digest: "sha256:1234567890abcdef"
dependencies:
- name: kube-rbac-proxy
version: "0.9.1"
- name: kube-rbac-proxy
version: "0.9.1"