Fixes extra newline from confs

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
 }

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))
+}

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