From a7150d4fcb60aaf2dee9a83e5eb149e094fe400c Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Mon, 16 Sep 2024 11:31:08 +0200 Subject: [PATCH] Add test coverage for config cleaning Adds unit tests --- modules/common/util/template_util.go | 48 +++--- modules/common/util/template_util_test.go | 184 +++++++++++++++++++++- 2 files changed, 199 insertions(+), 33 deletions(-) diff --git a/modules/common/util/template_util.go b/modules/common/util/template_util.go index a22e61b1..97e73940 100644 --- a/modules/common/util/template_util.go +++ b/modules/common/util/template_util.go @@ -45,53 +45,43 @@ const ( // Template - config map and secret details type Template struct { - Name string // name of the cm/secret to create based of the Template. Check secret/configmap pkg on details how it is used. - Namespace string // name of the nanmespace to create the cm/secret. Check secret/configmap pkg on details how it is used. - Type TType // type of the templates, see TTtypes - InstanceType string // the CRD name in lower case, to separate the templates for each CRD in /templates - SecretType corev1.SecretType // Secrets only, defaults to "Opaque" - AdditionalTemplate map[string]string // templates which are common to multiple CRDs can be located in a shared folder and added via this type into the resulting CM/secret - CustomData map[string]string // custom data which won't get rendered as a template and just added to the resulting cm/secret - Labels map[string]string // labels to be set on the cm/secret - Annotations map[string]string // Annotations set on cm/secret - ConfigOptions map[string]interface{} // map of parameters as input data to render the templates - SkipSetOwner bool // skip setting ownership on the associated configmap - Version string // optional version string to separate templates inside the InstanceType/Type directory. E.g. placementapi/config/18.0 - PostProcessCleanup bool // optional boolean parameter to remove extra new lines from service confs, by default set to false + Name string // name of the cm/secret to create based of the Template. Check secret/configmap pkg on details how it is used. + Namespace string // name of the nanmespace to create the cm/secret. Check secret/configmap pkg on details how it is used. + Type TType // type of the templates, see TTtypes + InstanceType string // the CRD name in lower case, to separate the templates for each CRD in /templates + SecretType corev1.SecretType // Secrets only, defaults to "Opaque" + AdditionalTemplate map[string]string // templates which are common to multiple CRDs can be located in a shared folder and added via this type into the resulting CM/secret + CustomData map[string]string // custom data which won't get rendered as a template and just added to the resulting cm/secret + Labels map[string]string // labels to be set on the cm/secret + Annotations map[string]string // Annotations set on cm/secret + ConfigOptions map[string]interface{} // map of parameters as input data to render the templates + SkipSetOwner bool // skip setting ownership on the associated configmap + Version string // optional version string to separate templates inside the InstanceType/Type directory. E.g. placementapi/config/18.0 + PostProcessConfFilesCleanup bool // optional boolean parameter to remove extra new lines from service confs, by default set to false } // This function removes extra space and new-lines from conf data func removeSubsequentNewLines(rawConf string) string { lines := strings.Split(rawConf, "\n") var result []string - var prevLineWasBlank bool for _, line := range lines { trimmedLine := strings.TrimSpace(line) - if trimmedLine == "" { - prevLineWasBlank = true - // if current line is blank, no need to add it in result - } else { + if trimmedLine != "" { if strings.HasPrefix(trimmedLine, "[") && strings.HasSuffix(trimmedLine, "]") { // section-header - if len(result) > 0 && !prevLineWasBlank { - result = append(result, "") - } - var sectionHeaderLine string if len(result) > 0 { - // new section-hearder - sectionHeaderLine = "\n" + line + // new section-header + result = append(result, "\n"+line) } else { - sectionHeaderLine = line + // top section-header + result = append(result, line) } - result = append(result, sectionHeaderLine) } else { // secion-body result = append(result, line) } - // reset flag - prevLineWasBlank = false } } @@ -287,7 +277,7 @@ func GetTemplateData(t Template) (map[string]string, error) { data[filename] = renderedTemplate } - if t.PostProcessCleanup { + if t.PostProcessConfFilesCleanup { for filename, content := range data { if strings.HasSuffix(filename, ".conf") { // as of now, only clean for confs diff --git a/modules/common/util/template_util_test.go b/modules/common/util/template_util_test.go index 02285353..34f8f214 100644 --- a/modules/common/util/template_util_test.go +++ b/modules/common/util/template_util_test.go @@ -136,8 +136,8 @@ func TestGetTemplateData(t *testing.T) { "Count": 1, "Upper": "BAR", }, - AdditionalTemplate: map[string]string{}, - PostProcessCleanup: true, + AdditionalTemplate: map[string]string{}, + PostProcessConfFilesCleanup: true, }, want: map[string]string{ "bar.conf": "[DEFAULT]\nstate_path = /var/lib/nova\ndebug=true\ncompute_driver = libvirt.LibvirtDriver\n\n[oslo_concurrency]\nlock_path = /var/lib/nova/tmp\n", @@ -175,8 +175,8 @@ func TestGetTemplateData(t *testing.T) { "Upper": "BAR", "Message": "some common func", }, - AdditionalTemplate: map[string]string{"common.sh": "/common/common.sh"}, - PostProcessCleanup: true, + AdditionalTemplate: map[string]string{"common.sh": "/common/common.sh"}, + PostProcessConfFilesCleanup: true, }, want: map[string]string{ "config.json": "{\n \"command\": \"/usr/sbin/httpd -DFOREGROUND\",\n}\n", @@ -266,3 +266,179 @@ func TestGetTemplateData(t *testing.T) { }) } } + +func TestRemoveSubsequentNewLines(t *testing.T) { + tests := []struct { + name string + raw string + cleaned string + }{ + { + name: "Empty input", + raw: "", + cleaned: "", + }, + { + name: "Single empty line", + raw: "\n", + cleaned: "", + }, + { + name: "Two empty lines", + raw: "\n\n", + cleaned: "", + }, + { + name: "Insert newline at end of file", + raw: "foo", + cleaned: "foo\n", + }, + { + name: "Remove starting empty line", + raw: "\nfoo", + cleaned: "foo\n", + }, + { + name: "Remove starting empty lines", + raw: "\n\nfoo", + cleaned: "foo\n", + }, + { + name: "Remove extra empty line at the end", + raw: "foo\n\n", + cleaned: "foo\n", + }, + { + name: "Remove extra empty lines at the end", + raw: "foo\n\n\n", + cleaned: "foo\n", + }, + { + name: "Keep subsequent data lines", + raw: "foo\nbar", + cleaned: "foo\nbar\n", + }, + { + name: "Remove empty line between subsequent data", + raw: "foo\n\nbar", + cleaned: "foo\nbar\n", + }, + { + name: "Extra spaces around data lines are kept", + raw: "\n\n foo \n\n bar ", + cleaned: " foo \n bar \n", + }, + { + name: "Remove extra lines with spaces only", + raw: " \n \nfoo\n \nbar\n \n ", + cleaned: "foo\nbar\n", + }, + { + name: "Remove starting empty line from section header", + raw: "\n[foo]", + cleaned: "[foo]\n", + }, + { + name: "Remove starting empty lines from section header", + raw: "\n\n[foo]", + cleaned: "[foo]\n", + }, + { + name: "Remove extra empty line after section header", + raw: "[foo]\n\n", + cleaned: "[foo]\n", + }, + { + name: "Remove extra empty lines after section header", + raw: "[foo]\n\n\n", + cleaned: "[foo]\n", + }, + { + name: "Insert empty line after section header at the end", + raw: "[foo]", + cleaned: "[foo]\n", + }, + { + name: "Keep one empty line between section headers", + raw: "[foo]\n\n[bar]", + cleaned: "[foo]\n\n[bar]\n", + }, + + // This ws failing as it inserts two empty lines between sections. But that is inconsistent + // as if there are two empty lines between sections then one is removed. + // See next test case. + { + name: "Insert one empty line between section headers", + raw: "[foo]\n[bar]", + cleaned: "[foo]\n\n[bar]\n", + }, + { + name: "Remove more empty lines between section headers", + raw: "[foo]\n\n\n[bar]", + cleaned: "[foo]\n\n[bar]\n", + }, + { + name: "Spaces are kept around section lines but not in the empty line in between", + raw: "\n [foo] \n \n [bar] ", + cleaned: " [foo] \n\n [bar] \n", + }, + { + name: "Remove extra empty line between section header and data", + raw: "[foo]\n\nbar", + cleaned: "[foo]\nbar\n", + }, + { + name: "Remove extra empty lines between section header and data", + raw: "[foo]\n\n\nbar", + cleaned: "[foo]\nbar\n", + }, + + // This was failing as it insert two extra lines. It is inconsistent as if there are + // two empty lines between sections then one is removed. See next test case. + { + name: "Insert extra line between sections", + raw: "[foo]\nbar\n[goo]\nbaz", + cleaned: "[foo]\nbar\n\n[goo]\nbaz\n", + }, + { + name: "Remove extra lines between sections", + raw: "[foo]\nbar\n\n\n[goo]\nbaz", + cleaned: "[foo]\nbar\n\n[goo]\nbaz\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + cleaned := removeSubsequentNewLines(tt.raw) + g.Expect(cleaned).To(Equal(tt.cleaned)) + }) + } +} + +// Run the cleaning twice on an input and ensure that the second cleaning +// does nothing as the first run cleaned everything +// This was failing due to empty line handling between sections is unstable. +func TestRemoveSubsequentNewLinesIsStable(t *testing.T) { + g := NewWithT(t) + + input := ` + +[foo] +boo=1 + + +bar=1 +[goo] + + +baz=1 + + +` + cleaned := removeSubsequentNewLines(input) + cleaned2 := removeSubsequentNewLines(cleaned) + + g.Expect(cleaned2).To(Equal(cleaned)) +}