From 8c86f56defd56c5ea02eb6952ba4b852599100d5 Mon Sep 17 00:00:00 2001 From: Konrad Bańka Date: Thu, 12 Nov 2020 09:13:37 +0100 Subject: Improve early-detection of empty template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous empty template detection pattern matched only against templates resolved to empty-or-whitespace-only files. This change makes it handle other case of empty yaml correctly, namely, yaml containing comments only. Issue-ID: MULTICLOUD-1252 Signed-off-by: Konrad Bańka Change-Id: I9132e167ec607c8a4a4ca5584141ed043c6ddd4f --- src/k8splugin/internal/helm/helm.go | 34 ++++++++++++++-------- src/k8splugin/internal/helm/helm_test.go | 29 +++++++++++++++++- .../mock_files/mock_charts/testchart3/Chart.yaml | 4 +++ .../testchart3/templates/always-empty.yaml | 9 ++++++ .../mock_charts/testchart3/templates/multi.yaml | 34 ++++++++++++++++++++++ .../testchart3/templates/only-comment.yaml | 6 ++++ .../mock_files/mock_charts/testchart3/values.yaml | 1 + 7 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 src/k8splugin/mock_files/mock_charts/testchart3/Chart.yaml create mode 100644 src/k8splugin/mock_files/mock_charts/testchart3/templates/always-empty.yaml create mode 100644 src/k8splugin/mock_files/mock_charts/testchart3/templates/multi.yaml create mode 100644 src/k8splugin/mock_files/mock_charts/testchart3/templates/only-comment.yaml create mode 100644 src/k8splugin/mock_files/mock_charts/testchart3/values.yaml diff --git a/src/k8splugin/internal/helm/helm.go b/src/k8splugin/internal/helm/helm.go index 2150758b..d3715fce 100644 --- a/src/k8splugin/internal/helm/helm.go +++ b/src/k8splugin/internal/helm/helm.go @@ -23,6 +23,7 @@ import ( "os" "path/filepath" "regexp" + "sort" "strings" utils "github.com/onap/multicloud-k8s/src/k8splugin/internal" @@ -55,16 +56,17 @@ type Template interface { // TemplateClient implements the Template interface // It will also be used to maintain any localized state type TemplateClient struct { - whitespaceRegex *regexp.Regexp - kubeVersion string - kubeNameSpace string - releaseName string + emptyRegex *regexp.Regexp + kubeVersion string + kubeNameSpace string + releaseName string } // NewTemplateClient returns a new instance of TemplateClient func NewTemplateClient(k8sversion, namespace, releasename string) *TemplateClient { return &TemplateClient{ - whitespaceRegex: regexp.MustCompile(`^\s*$`), + // emptyRegex defines template content that could be considered empty yaml-wise + emptyRegex: regexp.MustCompile(`(?m)\A(^(\s*#.*|\s*)$\n?)*\z`), // defaultKubeVersion is the default value of --kube-version flag kubeVersion: k8sversion, kubeNameSpace: namespace, @@ -209,11 +211,19 @@ func (h *TemplateClient) GenerateKubernetesArtifacts(inputPath string, valueFile continue } rmap := releaseutil.SplitManifests(v) - count := 0 - for _, v1 := range rmap { - key := fmt.Sprintf("%s-%d", k, count) - newRenderedTemplates[key] = v1 - count = count + 1 + + // Iterating over map can yield different order at times + // so first we'll sort keys + sortedKeys := make([]string, len(rmap)) + for k1, _ := range rmap { + sortedKeys = append(sortedKeys, k1) + } + // This makes empty files have the lowest indices + sort.Strings(sortedKeys) + + for k1, v1 := range sortedKeys { + key := fmt.Sprintf("%s-%d", k, k1) + newRenderedTemplates[key] = rmap[v1] } } @@ -232,7 +242,7 @@ func (h *TemplateClient) GenerateKubernetesArtifacts(inputPath string, valueFile } // blank template after execution - if h.whitespaceRegex.MatchString(data) { + if h.emptyRegex.MatchString(data) { continue } @@ -260,7 +270,7 @@ func (h *TemplateClient) GenerateKubernetesArtifacts(inputPath string, valueFile func getGroupVersionKind(data string) (schema.GroupVersionKind, error) { out, err := k8syaml.ToJSON([]byte(data)) if err != nil { - return schema.GroupVersionKind{}, pkgerrors.Wrap(err, "Converting yaml to json") + return schema.GroupVersionKind{}, pkgerrors.Wrap(err, "Converting yaml to json:\n"+data) } simpleMeta := json.SimpleMetaFactory{} diff --git a/src/k8splugin/internal/helm/helm_test.go b/src/k8splugin/internal/helm/helm_test.go index 1e676c52..817bbaa3 100644 --- a/src/k8splugin/internal/helm/helm_test.go +++ b/src/k8splugin/internal/helm/helm_test.go @@ -155,6 +155,33 @@ func TestGenerateKubernetesArtifacts(t *testing.T) { }, expectedError: "", }, + { + label: "Generate artifacts from multi-template and empty files v1", + chartPath: "../../mock_files/mock_charts/testchart3", + valueFiles: []string{}, + values: []string{ + "goingEmpty=false", + }, + expectedHashMap: map[string]string{ + "testchart3/templates/multi.yaml-2": "e24cbbefac2c2f700880b8fd041838f2dd48bbc1e099e7c1d2485ae7feb3da0d", + "testchart3/templates/multi.yaml-3": "592a8e5b2c35b8469aa45703a835bc00657bfe36b51eb08427a46e7d22fb1525", + }, + expectedError: "", + }, + { + label: "Generate artifacts from multi-template and empty files v2", + chartPath: "../../mock_files/mock_charts/testchart3", + valueFiles: []string{}, + values: []string{ + "goingEmpty=true", + }, + expectedHashMap: map[string]string{ + "testchart3/templates/multi.yaml-3": "e24cbbefac2c2f700880b8fd041838f2dd48bbc1e099e7c1d2485ae7feb3da0d", + "testchart3/templates/multi.yaml-4": "0bea01e65148584609ede5000c024241ba1c35b440b32ec0a4f7013015715bfe", + "testchart3/templates/multi.yaml-5": "6a5af22538c273b9d4a3156e3b6bb538c655041eae31e93db21a9e178f73ecf0", + }, + expectedError: "", + }, } h := sha256.New() @@ -192,7 +219,7 @@ func TestGenerateKubernetesArtifacts(t *testing.T) { } } if gotHash != expectedHash { - t.Fatalf("Got unexpected hash for %s", f) + t.Fatalf("Got unexpected hash for %s: '%s'; expected: '%s'", f, gotHash, expectedHash) } } } diff --git a/src/k8splugin/mock_files/mock_charts/testchart3/Chart.yaml b/src/k8splugin/mock_files/mock_charts/testchart3/Chart.yaml new file mode 100644 index 00000000..adf4e2fe --- /dev/null +++ b/src/k8splugin/mock_files/mock_charts/testchart3/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +description: A Helm chart for Kubernetes +name: testchart3 +version: 0.1.0 diff --git a/src/k8splugin/mock_files/mock_charts/testchart3/templates/always-empty.yaml b/src/k8splugin/mock_files/mock_charts/testchart3/templates/always-empty.yaml new file mode 100644 index 00000000..121130fc --- /dev/null +++ b/src/k8splugin/mock_files/mock_charts/testchart3/templates/always-empty.yaml @@ -0,0 +1,9 @@ +--- +{{ if eq 0 1 }} +apiVersion: v1 +kind: ConfigMap +metadata: + name: dummy +data: + key1: value1 +{{ end }} diff --git a/src/k8splugin/mock_files/mock_charts/testchart3/templates/multi.yaml b/src/k8splugin/mock_files/mock_charts/testchart3/templates/multi.yaml new file mode 100644 index 00000000..0539cfb4 --- /dev/null +++ b/src/k8splugin/mock_files/mock_charts/testchart3/templates/multi.yaml @@ -0,0 +1,34 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: dummy +data: + key1: value1 +--- +{{ if .Values.goingEmpty }} +apiVersion: apps/v1 +kind: Deployment +metadata: + name: dummy +spec: + template: + metadata: + labels: + app: dummy + spec: + container: + - name: dummy + image: dummy +{{ end }} +--- +apiVersion: v1 +kind: Service +metadata: + name: dummy +spec: + ports: + - port: 80 + protocol: TCP + selector: + app: dummy diff --git a/src/k8splugin/mock_files/mock_charts/testchart3/templates/only-comment.yaml b/src/k8splugin/mock_files/mock_charts/testchart3/templates/only-comment.yaml new file mode 100644 index 00000000..aaacc787 --- /dev/null +++ b/src/k8splugin/mock_files/mock_charts/testchart3/templates/only-comment.yaml @@ -0,0 +1,6 @@ +#not so empty? +#Copyright or something similiar +#Some license info +{{/* + empty +*/}} diff --git a/src/k8splugin/mock_files/mock_charts/testchart3/values.yaml b/src/k8splugin/mock_files/mock_charts/testchart3/values.yaml new file mode 100644 index 00000000..912d0dbc --- /dev/null +++ b/src/k8splugin/mock_files/mock_charts/testchart3/values.yaml @@ -0,0 +1 @@ +goingEmpty: true -- cgit 1.2.3-korg