From a4e61f7b412f70cde76b426b0240d50c2e142c6b Mon Sep 17 00:00:00 2001 From: Chris Bui Date: Sun, 23 Aug 2020 09:16:57 -0500 Subject: [PATCH] Fix linting `kind: List` metadata Helm v2 didn't validate that resource names were valid. Helm v3 introduces this check, failing lint checks that were previously passing. This PR adds a check to validate metadata for a list of resources if present, otherwise falling back to previous behavior of validating the metadata. Fixes https://github.com/helm/helm/issues/8615 Signed-off-by: Chris Bui --- pkg/lint/rules/template.go | 24 +++++++++++++------ pkg/lint/rules/template_test.go | 17 +++++++++++++ .../testdata/bad_list_of_resources/Chart.yaml | 5 ++++ .../templates/list_resource.yaml | 8 +++++++ .../bad_list_of_resources/values.yaml | 1 + .../good_list_of_resources/Chart.yaml | 5 ++++ .../templates/list_resource.yaml | 11 +++++++++ .../good_list_of_resources/values.yaml | 1 + 8 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 pkg/lint/rules/testdata/bad_list_of_resources/Chart.yaml create mode 100644 pkg/lint/rules/testdata/bad_list_of_resources/templates/list_resource.yaml create mode 100644 pkg/lint/rules/testdata/bad_list_of_resources/values.yaml create mode 100644 pkg/lint/rules/testdata/good_list_of_resources/Chart.yaml create mode 100644 pkg/lint/rules/testdata/good_list_of_resources/templates/list_resource.yaml create mode 100644 pkg/lint/rules/testdata/good_list_of_resources/values.yaml diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 9a3a2a1ba..05b120c1b 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -126,11 +126,17 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace // Even though K8sYamlStruct only defines a few fields, an error in any other // key will be raised as well err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct) - // If YAML linting fails, we sill progress. So we don't capture the returned state // on this linter run. linter.RunLinterRule(support.ErrorSev, fpath, validateYamlContent(err)) - linter.RunLinterRule(support.ErrorSev, fpath, validateMetadataName(&yamlStruct)) + if isListOfResources(&yamlStruct) { + for i, item := range yamlStruct.Items { + err := errors.Wrapf(validateMetadataName(&item), "at item index %d", i) + linter.RunLinterRule(support.ErrorSev, fpath, err) + } + } else { + linter.RunLinterRule(support.ErrorSev, fpath, validateMetadataName(&yamlStruct)) + } linter.RunLinterRule(support.ErrorSev, fpath, validateNoDeprecations(&yamlStruct)) linter.RunLinterRule(support.ErrorSev, fpath, validateMatchSelector(&yamlStruct, renderedContent)) } @@ -164,13 +170,16 @@ func validateYamlContent(err error) error { return errors.Wrap(err, "unable to parse YAML") } -func validateMetadataName(obj *K8sYamlStruct) error { - // This will return an error if the characters do not abide by the standard OR if the - // name is left empty. - if validName.MatchString(obj.Metadata.Name) { +func isListOfResources(yamlStruct *K8sYamlStruct) bool { + return len(yamlStruct.Items) > 0 +} + +func validateMetadataName(yamlStruct *K8sYamlStruct) error { + if validName.MatchString(yamlStruct.Metadata.Name) { return nil } - return fmt.Errorf("object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name) + + return fmt.Errorf("object name does not conform to Kubernetes naming requirements: %q", yamlStruct.Metadata.Name) } func validateNoCRDHooks(manifest []byte) error { @@ -208,6 +217,7 @@ type K8sYamlStruct struct { APIVersion string `json:"apiVersion"` Kind string Metadata k8sYamlMetadata + Items []K8sYamlStruct } type k8sYamlMetadata struct { diff --git a/pkg/lint/rules/template_test.go b/pkg/lint/rules/template_test.go index ae82c8922..b0d20fae4 100644 --- a/pkg/lint/rules/template_test.go +++ b/pkg/lint/rules/template_test.go @@ -134,6 +134,23 @@ func TestValidateMetadataName(t *testing.T) { } } +func TestListOfResources(t *testing.T) { + testData := map[string]int{ + "./testdata/good_list_of_resources": 0, + "./testdata/bad_list_of_resources": 2, + } + + for testDir, expectedErrs := range testData { + linter := support.Linter{ChartDir: testDir} + Templates(&linter, values, namespace, strict) + res := linter.Messages + + if len(res) != expectedErrs { + t.Fatalf("Expected %d error(s) in %s, got %d, %v", expectedErrs, testDir, len(res), res) + } + } +} + func TestDeprecatedAPIFails(t *testing.T) { mychart := chart.Chart{ Metadata: &chart.Metadata{ diff --git a/pkg/lint/rules/testdata/bad_list_of_resources/Chart.yaml b/pkg/lint/rules/testdata/bad_list_of_resources/Chart.yaml new file mode 100644 index 000000000..c70f73f8a --- /dev/null +++ b/pkg/lint/rules/testdata/bad_list_of_resources/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +name: bad list of resources +description: list of resources testing chart +version: 199.44.12345-Alpha.1+cafe009 +icon: http://riverrun.io diff --git a/pkg/lint/rules/testdata/bad_list_of_resources/templates/list_resource.yaml b/pkg/lint/rules/testdata/bad_list_of_resources/templates/list_resource.yaml new file mode 100644 index 000000000..3744173a4 --- /dev/null +++ b/pkg/lint/rules/testdata/bad_list_of_resources/templates/list_resource.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: List +items: + - apiVersion: v1 + kind: ConfigMap + metadata: + - apiVersion: v1 + kind: ConfigMap \ No newline at end of file diff --git a/pkg/lint/rules/testdata/bad_list_of_resources/values.yaml b/pkg/lint/rules/testdata/bad_list_of_resources/values.yaml new file mode 100644 index 000000000..05b5d63e4 --- /dev/null +++ b/pkg/lint/rules/testdata/bad_list_of_resources/values.yaml @@ -0,0 +1 @@ +name: "bad list of resources" diff --git a/pkg/lint/rules/testdata/good_list_of_resources/Chart.yaml b/pkg/lint/rules/testdata/good_list_of_resources/Chart.yaml new file mode 100644 index 000000000..c9bd9a190 --- /dev/null +++ b/pkg/lint/rules/testdata/good_list_of_resources/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +name: good list of resources +description: list of resources testing chart +version: 199.44.12345-Alpha.1+cafe009 +icon: http://riverrun.io diff --git a/pkg/lint/rules/testdata/good_list_of_resources/templates/list_resource.yaml b/pkg/lint/rules/testdata/good_list_of_resources/templates/list_resource.yaml new file mode 100644 index 000000000..358d4e253 --- /dev/null +++ b/pkg/lint/rules/testdata/good_list_of_resources/templates/list_resource.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: List +items: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: config1 + - apiVersion: v1 + kind: ConfigMap + metadata: + name: config2 \ No newline at end of file diff --git a/pkg/lint/rules/testdata/good_list_of_resources/values.yaml b/pkg/lint/rules/testdata/good_list_of_resources/values.yaml new file mode 100644 index 000000000..711ca7c76 --- /dev/null +++ b/pkg/lint/rules/testdata/good_list_of_resources/values.yaml @@ -0,0 +1 @@ +name: "good list of resources"