From 775af2a0ceadef1bc8f627cdb70fadb3c69b8d86 Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Fri, 21 Oct 2022 18:04:05 +0100 Subject: [PATCH 1/5] Update schema validation handling Signed-off-by: Martin Hickey --- pkg/chartutil/jsonschema.go | 8 ++++++- pkg/chartutil/jsonschema_test.go | 24 +++++++++++++++++++ .../testdata/test-values-invalid.schema.json | 1 + 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 pkg/chartutil/testdata/test-values-invalid.schema.json diff --git a/pkg/chartutil/jsonschema.go b/pkg/chartutil/jsonschema.go index 753dc98c1..7b9768fd3 100644 --- a/pkg/chartutil/jsonschema.go +++ b/pkg/chartutil/jsonschema.go @@ -55,7 +55,13 @@ func ValidateAgainstSchema(chrt *chart.Chart, values map[string]interface{}) err } // ValidateAgainstSingleSchema checks that values does not violate the structure laid out in this schema -func ValidateAgainstSingleSchema(values Values, schemaJSON []byte) error { +func ValidateAgainstSingleSchema(values Values, schemaJSON []byte) (reterr error) { + defer func() { + if r := recover(); r != nil { + reterr = fmt.Errorf("unable to validate schema: %s", r) + } + }() + valuesData, err := yaml.Marshal(values) if err != nil { return err diff --git a/pkg/chartutil/jsonschema_test.go b/pkg/chartutil/jsonschema_test.go index a0acd5a7f..d71668ac8 100644 --- a/pkg/chartutil/jsonschema_test.go +++ b/pkg/chartutil/jsonschema_test.go @@ -38,6 +38,30 @@ func TestValidateAgainstSingleSchema(t *testing.T) { } } +func TestValidateAgainstInvalidSingleSchema(t *testing.T) { + values, err := ReadValuesFile("./testdata/test-values.yaml") + if err != nil { + t.Fatalf("Error reading YAML file: %s", err) + } + schema, err := ioutil.ReadFile("./testdata/test-values-invalid.schema.json") + if err != nil { + t.Fatalf("Error reading YAML file: %s", err) + } + + var errString string + if err := ValidateAgainstSingleSchema(values, schema); err == nil { + t.Fatalf("Expected an error, but got nil") + } else { + errString = err.Error() + } + + expectedErrString := "unable to validate schema: runtime error: invalid " + + "memory address or nil pointer dereference" + if errString != expectedErrString { + t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) + } +} + func TestValidateAgainstSingleSchemaNegative(t *testing.T) { values, err := ReadValuesFile("./testdata/test-values-negative.yaml") if err != nil { diff --git a/pkg/chartutil/testdata/test-values-invalid.schema.json b/pkg/chartutil/testdata/test-values-invalid.schema.json new file mode 100644 index 000000000..35a16a2c4 --- /dev/null +++ b/pkg/chartutil/testdata/test-values-invalid.schema.json @@ -0,0 +1 @@ + 1E1111111 From 256e976331db4b7335ef721e411e7b59c5317ccb Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Wed, 9 Nov 2022 16:11:43 +0000 Subject: [PATCH 2/5] Update repo handling Signed-off-by: Martin Hickey --- pkg/repo/index.go | 8 ++++++++ pkg/repo/index_test.go | 33 +++++++++++++++++++++++++++++++++ pkg/repo/repo.go | 3 +++ pkg/repo/repo_test.go | 31 +++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+) diff --git a/pkg/repo/index.go b/pkg/repo/index.go index 1b65ac497..60cfe5801 100644 --- a/pkg/repo/index.go +++ b/pkg/repo/index.go @@ -118,6 +118,10 @@ func LoadIndexFile(path string) (*IndexFile, error) { // MustAdd adds a file to the index // This can leave the index in an unsorted state func (i IndexFile) MustAdd(md *chart.Metadata, filename, baseURL, digest string) error { + if i.Entries == nil { + return errors.New("entries not initialized") + } + if md.APIVersion == "" { md.APIVersion = chart.APIVersionV1 } @@ -339,6 +343,10 @@ func loadIndex(data []byte, source string) (*IndexFile, error) { for name, cvs := range i.Entries { for idx := len(cvs) - 1; idx >= 0; idx-- { + if cvs[idx] == nil { + log.Printf("skipping loading invalid entry for chart %q from %s: empty entry", name, source) + continue + } if cvs[idx].APIVersion == "" { cvs[idx].APIVersion = chart.APIVersionV1 } diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index a75a4177a..2403e9a71 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -59,6 +59,15 @@ entries: version: 1.0.0 home: https://github.com/something digest: "sha256:1234567890abcdef" +` + indexWithEmptyEntry = ` +apiVersion: v1 +entries: + grafana: + - apiVersion: v2 + name: grafana + foo: + - ` ) @@ -152,6 +161,12 @@ func TestLoadIndex_Duplicates(t *testing.T) { } } +func TestLoadIndex_EmptyEntry(t *testing.T) { + if _, err := loadIndex([]byte(indexWithEmptyEntry), "indexWithEmptyEntry"); err != nil { + t.Errorf("unexpected error: %s", err) + } +} + func TestLoadIndex_Empty(t *testing.T) { if _, err := loadIndex([]byte(""), "indexWithEmpty"); err == nil { t.Errorf("Expected an error when index.yaml is empty.") @@ -526,3 +541,21 @@ func TestIndexWrite(t *testing.T) { t.Fatal("Index files doesn't contain expected content") } } + +func TestAddFileIndexEntriesNil(t *testing.T) { + i := NewIndexFile() + i.APIVersion = chart.APIVersionV1 + i.Entries = nil + for _, x := range []struct { + md *chart.Metadata + filename string + baseURL string + digest string + }{ + {&chart.Metadata{APIVersion: "v2", Name: " ", Version: "8033-5.apinie+s.r"}, "setter-0.1.9+beta.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + } { + if err := i.MustAdd(x.md, x.filename, x.baseURL, x.digest); err == nil { + t.Errorf("expected err to be non-nil when entries not initialized") + } + } +} diff --git a/pkg/repo/repo.go b/pkg/repo/repo.go index 6f1e90dad..ee80d04f4 100644 --- a/pkg/repo/repo.go +++ b/pkg/repo/repo.go @@ -100,6 +100,9 @@ func (r *File) Remove(name string) bool { cp := []*Entry{} found := false for _, rf := range r.Repositories { + if rf == nil { + continue + } if rf.Name == name { found = true continue diff --git a/pkg/repo/repo_test.go b/pkg/repo/repo_test.go index f87d2c202..7080a7cef 100644 --- a/pkg/repo/repo_test.go +++ b/pkg/repo/repo_test.go @@ -225,3 +225,34 @@ func TestRepoNotExists(t *testing.T) { t.Errorf("expected prompt `couldn't load repositories file`") } } + +func TestRemoveRepositoryInvalidEntries(t *testing.T) { + sampleRepository := NewFile() + sampleRepository.Add( + &Entry{ + Name: "stable", + URL: "https://example.com/stable/charts", + }, + &Entry{ + Name: "incubator", + URL: "https://example.com/incubator", + }, + &Entry{}, + nil, + &Entry{ + Name: "test", + URL: "https://example.com/test", + }, + ) + + removeRepository := "stable" + found := sampleRepository.Remove(removeRepository) + if !found { + t.Errorf("expected repository %s not found", removeRepository) + } + + found = sampleRepository.Has(removeRepository) + if found { + t.Errorf("repository %s not deleted", removeRepository) + } +} From a59e58468430bf9b454426ff22f5f367185b7d77 Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Fri, 25 Nov 2022 18:16:43 +0000 Subject: [PATCH 3/5] Update string handling Signed-off-by: Martin Hickey --- pkg/strvals/parser.go | 28 ++++++++++++------ pkg/strvals/parser_test.go | 58 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/pkg/strvals/parser.go b/pkg/strvals/parser.go index 26bc0fcf2..2b815e1b9 100644 --- a/pkg/strvals/parser.go +++ b/pkg/strvals/parser.go @@ -36,6 +36,10 @@ var ErrNotList = errors.New("not a list") // The default value 65536 = 1024 * 64 var MaxIndex = 65536 +// MaxNestedNameLevel is the maximum level of nesting for a value name that +// will be allowed. +var MaxNestedNameLevel = 30 + // ToYAML takes a string of arguments and converts to a YAML document. func ToYAML(s string) (string, error) { m, err := Parse(s) @@ -155,7 +159,7 @@ func newFileParser(sc *bytes.Buffer, data map[string]interface{}, reader RunesVa func (t *parser) parse() error { for { - err := t.key(t.data) + err := t.key(t.data, 0) if err == nil { continue } @@ -174,7 +178,7 @@ func runeSet(r []rune) map[rune]bool { return s } -func (t *parser) key(data map[string]interface{}) (reterr error) { +func (t *parser) key(data map[string]interface{}, nestedNameLevel int) (reterr error) { defer func() { if r := recover(); r != nil { reterr = fmt.Errorf("unable to parse key: %s", r) @@ -204,7 +208,7 @@ func (t *parser) key(data map[string]interface{}) (reterr error) { } // Now we need to get the value after the ]. - list, err = t.listItem(list, i) + list, err = t.listItem(list, i, nestedNameLevel) set(data, kk, list) return err case last == '=': @@ -261,6 +265,12 @@ func (t *parser) key(data map[string]interface{}) (reterr error) { set(data, string(k), "") return errors.Errorf("key %q has no value (cannot end with ,)", string(k)) case last == '.': + // Check value name is within the maximum nested name level + nestedNameLevel++ + if nestedNameLevel > MaxNestedNameLevel { + return fmt.Errorf("value name nested level is greater than maximum supported nested level of %d", MaxNestedNameLevel) + } + // First, create or find the target map. inner := map[string]interface{}{} if _, ok := data[string(k)]; ok { @@ -268,12 +278,14 @@ func (t *parser) key(data map[string]interface{}) (reterr error) { } // Recurse - e := t.key(inner) + if e := t.key(inner, nestedNameLevel); e != nil { + return e + } if len(inner) == 0 { return errors.Errorf("key map %q has no value", string(k)) } set(data, string(k), inner) - return e + return nil } } } @@ -322,7 +334,7 @@ func (t *parser) keyIndex() (int, error) { return strconv.Atoi(string(v)) } -func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) { +func (t *parser) listItem(list []interface{}, i, nestedNameLevel int) ([]interface{}, error) { if i < 0 { return list, fmt.Errorf("negative %d index not allowed", i) } @@ -395,7 +407,7 @@ func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) { } } // Now we need to get the value after the ]. - list2, err := t.listItem(crtList, nextI) + list2, err := t.listItem(crtList, nextI, nestedNameLevel) if err != nil { return list, err } @@ -414,7 +426,7 @@ func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) { } // Recurse - e := t.key(inner) + e := t.key(inner, nestedNameLevel) if e != nil { return list, e } diff --git a/pkg/strvals/parser_test.go b/pkg/strvals/parser_test.go index f7eba7830..122f0ac85 100644 --- a/pkg/strvals/parser_test.go +++ b/pkg/strvals/parser_test.go @@ -16,6 +16,7 @@ limitations under the License. package strvals import ( + "fmt" "testing" "sigs.k8s.io/yaml" @@ -247,8 +248,9 @@ func TestParseSet(t *testing.T) { err: true, }, { - str: "name1.name2=", - expect: map[string]interface{}{"name1": map[string]interface{}{"name2": ""}}, + "name1.name2=", + map[string]interface{}{}, + false, }, { str: "name1.=name2", @@ -754,3 +756,55 @@ func TestToYAML(t *testing.T) { t.Errorf("Expected %q, got %q", expect, o) } } + +func TestParseSetNestedLevels(t *testing.T) { + var keyMultipleNestedLevels string + for i := 1; i <= MaxNestedNameLevel+2; i++ { + tmpStr := fmt.Sprintf("name%d", i) + if i <= MaxNestedNameLevel+1 { + tmpStr = tmpStr + "." + } + keyMultipleNestedLevels += tmpStr + } + tests := []struct { + str string + expect map[string]interface{} + err bool + }{ + { + "outer.middle.inner=value", + map[string]interface{}{"outer": map[string]interface{}{"middle": map[string]interface{}{"inner": "value"}}}, + false, + }, + { + str: keyMultipleNestedLevels + "=value", + err: true, + }, + } + + for _, tt := range tests { + got, err := Parse(tt.str) + if err != nil { + if tt.err { + continue + } + t.Fatalf("%s: %s", tt.str, err) + } + if tt.err { + t.Errorf("%s: Expected error. Got nil", tt.str) + } + + y1, err := yaml.Marshal(tt.expect) + if err != nil { + t.Fatal(err) + } + y2, err := yaml.Marshal(got) + if err != nil { + t.Fatalf("Error serializing parsed value: %s", err) + } + + if string(y1) != string(y2) { + t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.str, y1, y2) + } + } +} From b307d0fbeb42fe890450d8d3de2291817ad9b4cb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 5 Dec 2022 09:09:07 +0000 Subject: [PATCH 4/5] chore(deps): bump golang.org/x/text from 0.4.0 to 0.5.0 Bumps [golang.org/x/text](https://github.com/golang/text) from 0.4.0 to 0.5.0. - [Release notes](https://github.com/golang/text/releases) - [Commits](https://github.com/golang/text/compare/v0.4.0...v0.5.0) --- updated-dependencies: - dependency-name: golang.org/x/text dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index b284a7eb6..3a8bd6919 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.3.0 golang.org/x/term v0.2.0 - golang.org/x/text v0.4.0 + golang.org/x/text v0.5.0 k8s.io/api v0.25.2 k8s.io/apiextensions-apiserver v0.25.2 k8s.io/apimachinery v0.25.2 diff --git a/go.sum b/go.sum index 3f4f2a667..2f3b3f045 100644 --- a/go.sum +++ b/go.sum @@ -852,8 +852,9 @@ golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.5.0 h1:OLmvp0KP+FVG99Ct/qFiL/Fhk4zp4QQnZ7b2U+5piUM= +golang.org/x/text v0.5.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= From b6fef6c4665130644acf7742040ebd46f9cc957c Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Thu, 8 Dec 2022 16:00:45 +0000 Subject: [PATCH 5/5] Fix backwards compatibility Signed-off-by: Martin Hickey --- pkg/strvals/parser.go | 12 ++++++------ pkg/strvals/parser_test.go | 14 +++++++++++--- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pkg/strvals/parser.go b/pkg/strvals/parser.go index 2b815e1b9..c59925522 100644 --- a/pkg/strvals/parser.go +++ b/pkg/strvals/parser.go @@ -278,14 +278,14 @@ func (t *parser) key(data map[string]interface{}, nestedNameLevel int) (reterr e } // Recurse - if e := t.key(inner, nestedNameLevel); e != nil { - return e - } - if len(inner) == 0 { + e := t.key(inner, nestedNameLevel) + if e == nil && len(inner) == 0 { return errors.Errorf("key map %q has no value", string(k)) } - set(data, string(k), inner) - return nil + if len(inner) != 0 { + set(data, string(k), inner) + } + return e } } } diff --git a/pkg/strvals/parser_test.go b/pkg/strvals/parser_test.go index 122f0ac85..925aa97c6 100644 --- a/pkg/strvals/parser_test.go +++ b/pkg/strvals/parser_test.go @@ -248,9 +248,8 @@ func TestParseSet(t *testing.T) { err: true, }, { - "name1.name2=", - map[string]interface{}{}, - false, + str: "name1.name2=", + expect: map[string]interface{}{"name1": map[string]interface{}{"name2": ""}}, }, { str: "name1.=name2", @@ -770,15 +769,19 @@ func TestParseSetNestedLevels(t *testing.T) { str string expect map[string]interface{} err bool + errStr string }{ { "outer.middle.inner=value", map[string]interface{}{"outer": map[string]interface{}{"middle": map[string]interface{}{"inner": "value"}}}, false, + "", }, { str: keyMultipleNestedLevels + "=value", err: true, + errStr: fmt.Sprintf("value name nested level is greater than maximum supported nested level of %d", + MaxNestedNameLevel), }, } @@ -786,6 +789,11 @@ func TestParseSetNestedLevels(t *testing.T) { got, err := Parse(tt.str) if err != nil { if tt.err { + if tt.errStr != "" { + if err.Error() != tt.errStr { + t.Errorf("Expected error: %s. Got error: %s", tt.errStr, err.Error()) + } + } continue } t.Fatalf("%s: %s", tt.str, err)