From 6c001f53ea1427a956e29e5de62cd2bb7aa57d8d Mon Sep 17 00:00:00 2001 From: Laszlo Varadi Date: Wed, 27 Nov 2019 16:40:10 +0100 Subject: [PATCH] Fixes #7088 template command bug The template command (and install/upgrade) will dump the content of the problematic yaml content if it cannot be unmarshalled. Signed-off-by: Laszlo Varadi --- cmd/helm/helm.go | 3 + pkg/logs/logs.go | 39 ++++++++++ pkg/releaseutil/manifest_sorter.go | 14 +++- pkg/releaseutil/manifest_sorter_test.go | 94 +++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 pkg/logs/logs.go diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 112d5123f..e3a0110a1 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -33,6 +33,7 @@ import ( "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/gates" + "helm.sh/helm/v3/pkg/logs" ) // FeatureGateOCI is the feature gate for checking if `helm chart` and `helm registry` commands should work @@ -62,6 +63,8 @@ func initKubeLogs() { } func main() { + logs.Init(settings) + initKubeLogs() actionConfig := new(action.Configuration) diff --git a/pkg/logs/logs.go b/pkg/logs/logs.go new file mode 100644 index 000000000..9364faac6 --- /dev/null +++ b/pkg/logs/logs.go @@ -0,0 +1,39 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package logs + +import ( + "fmt" + "log" + + "helm.sh/helm/v3/pkg/cli" +) + +var settings *cli.EnvSettings + +//Init logs package with settings... used to get the Debug flag +func Init(s *cli.EnvSettings) { + settings = s +} + +//Debug log if Debug is set in HELM_DEBUG or with --debug +func Debug(format string, v ...interface{}) { + if settings.Debug { + format = fmt.Sprintf("[debug] %s\n", format) + log.Output(2, fmt.Sprintf(format, v...)) + } +} diff --git a/pkg/releaseutil/manifest_sorter.go b/pkg/releaseutil/manifest_sorter.go index 24b0c3c95..649734660 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/pkg/releaseutil/manifest_sorter.go @@ -17,6 +17,7 @@ limitations under the License. package releaseutil import ( + "fmt" "log" "path" "sort" @@ -27,6 +28,7 @@ import ( "sigs.k8s.io/yaml" "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/logs" "helm.sh/helm/v3/pkg/release" ) @@ -140,9 +142,19 @@ func (file *manifestFile) sort(result *result) error { for _, entryKey := range sortedEntryKeys { m := file.entries[entryKey] - var entry SimpleHead if err := yaml.Unmarshal([]byte(m), &entry); err != nil { + mname := "" + if len(file.entries) > 1 { + index, err := strconv.Atoi(entryKey[9:]) + if err != nil { + index = 0 + } else { + index++ + } + mname = fmt.Sprintf(" (in yaml document %d)", index) + } + logs.Debug("YAML parse error on %s%s\n---\n%s\n---\n", file.path, mname, m) return errors.Wrapf(err, "YAML parse error on %s", file.path) } diff --git a/pkg/releaseutil/manifest_sorter_test.go b/pkg/releaseutil/manifest_sorter_test.go index 0d2d6660a..491291c3c 100644 --- a/pkg/releaseutil/manifest_sorter_test.go +++ b/pkg/releaseutil/manifest_sorter_test.go @@ -17,15 +17,109 @@ limitations under the License. package releaseutil import ( + "bytes" + "log" "reflect" + "strings" "testing" "sigs.k8s.io/yaml" "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/cli" + "helm.sh/helm/v3/pkg/logs" "helm.sh/helm/v3/pkg/release" ) +func TestSortManifestErrorLog(t *testing.T) { + + logs.Init(&cli.EnvSettings{Debug: true}) + + yamlWithError := `kind: ReplicaSet +apiVersion: v1beta1 +metadata: + name: second_with_error + #indentation on next line is invalid + namespace: second +` + + data := []struct { + name []string + path string + kind []string + hooks map[string][]release.HookEvent + manifest string + }{ + { + name: []string{"first"}, + path: "one", + kind: []string{"Job"}, + hooks: map[string][]release.HookEvent{}, + manifest: `apiVersion: v1 +kind: Job +metadata: + name: first + labels: + doesnot: matter +`, + }, + { + name: []string{"second", "second_with_error", "last"}, + path: "yaml_with_error", + kind: []string{"ReplicaSet", "ReplicaSet", "Job"}, + hooks: map[string][]release.HookEvent{}, + manifest: `kind: ReplicaSet +apiVersion: v1beta1 +metadata: + name: second + namespace: second +--- +` + yamlWithError + `--- +apiVersion: v1 +kind: Job +metadata: + name: last + labels: + doesnot: matter +`, + }, + } + + manifests := make(map[string]string, len(data)) + for _, o := range data { + manifests[o.path] = o.manifest + } + + origWriter := log.Writer() + var buf bytes.Buffer + log.SetOutput(&buf) + + SortManifests(manifests, chartutil.VersionSet{"v1", "v1beta1"}, InstallOrder) + + log.SetOutput(origWriter) + + s := buf.String() + if !strings.Contains(s, "YAML parse error on yaml_with_error (in yaml document 2)") || !strings.Contains(s, yamlWithError) { + t.Log("Error log should contain the problematic yaml content") + t.Log(s) + t.FailNow() + } + + buf.Reset() + logs.Init(&cli.EnvSettings{Debug: false}) + + SortManifests(manifests, chartutil.VersionSet{"v1", "v1beta1"}, InstallOrder) + + log.SetOutput(origWriter) + + s = buf.String() + if strings.Contains(s, "YAML parse error on yaml_with_error") || strings.Contains(s, yamlWithError) { + t.Log("Error log should NOT contain the problematic yaml content") + t.Log(s) + t.FailNow() + } +} + func TestSortManifests(t *testing.T) { data := []struct {