diff --git a/modules/common/util/template_util.go b/modules/common/util/template_util.go index a641bb89..97e73940 100644 --- a/modules/common/util/template_util.go +++ b/modules/common/util/template_util.go @@ -45,18 +45,50 @@ 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 + 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 + + for _, line := range lines { + trimmedLine := strings.TrimSpace(line) + + if trimmedLine != "" { + if strings.HasPrefix(trimmedLine, "[") && strings.HasSuffix(trimmedLine, "]") { + // section-header + if len(result) > 0 { + // new section-header + result = append(result, "\n"+line) + } else { + // top section-header + result = append(result, line) + } + } else { + // secion-body + result = append(result, line) + } + } + } + + // add an extra line at EOF + result = append(result, "") + + return strings.Join(result, "\n") } // GetTemplatesPath get path to templates, either running local or deployed as container @@ -135,6 +167,7 @@ func ExecuteTemplate(templateFile string, data interface{}) (string, error) { if err != nil { return "", err } + return renderedTemplate, nil } @@ -244,5 +277,14 @@ func GetTemplateData(t Template) (map[string]string, error) { data[filename] = renderedTemplate } + if t.PostProcessConfFilesCleanup { + for filename, content := range data { + if strings.HasSuffix(filename, ".conf") { + // as of now, only clean for confs + data[filename] = removeSubsequentNewLines(content) + } + } + } + return data, nil } diff --git a/modules/common/util/template_util_test.go b/modules/common/util/template_util_test.go index ef0a8989..34f8f214 100644 --- a/modules/common/util/template_util_test.go +++ b/modules/common/util/template_util_test.go @@ -74,6 +74,7 @@ func TestGetAllTemplates(t *testing.T) { tmplType: TemplateTypeConfig, version: "", want: []string{ + filepath.Join(path.Dir(filename), templatePath, "testservice", "config", "bar.conf"), filepath.Join(path.Dir(filename), templatePath, "testservice", "config", "config.json"), filepath.Join(path.Dir(filename), templatePath, "testservice", "config", "foo.conf"), }, @@ -135,9 +136,11 @@ func TestGetTemplateData(t *testing.T) { "Count": 1, "Upper": "BAR", }, - AdditionalTemplate: map[string]string{}, + 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", "config.json": "{\n \"command\": \"/usr/sbin/httpd -DFOREGROUND\",\n}\n", "foo.conf": "username = foo\ncount = 1\nadd = 3\nlower = bar\n", }, @@ -172,12 +175,14 @@ func TestGetTemplateData(t *testing.T) { "Upper": "BAR", "Message": "some common func", }, - AdditionalTemplate: map[string]string{"common.sh": "/common/common.sh"}, + 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", "foo.conf": "username = foo\ncount = 1\nadd = 3\nlower = bar\n", "common.sh": "#!/bin/bash\nset -e\n\nfunction common_func {\n echo some common func\n}\n", + "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", }, error: false, }, @@ -261,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)) +} diff --git a/modules/common/util/testdata/templates/testservice/config/bar.conf b/modules/common/util/testdata/templates/testservice/config/bar.conf new file mode 100644 index 00000000..8ee5944f --- /dev/null +++ b/modules/common/util/testdata/templates/testservice/config/bar.conf @@ -0,0 +1,11 @@ +[DEFAULT] +state_path = /var/lib/nova + + +debug=true + +compute_driver = libvirt.LibvirtDriver + + +[oslo_concurrency] +lock_path = /var/lib/nova/tmp