Fix ini escaping

Gopkg.toml

@@ -107,10 +107,10 @@ required = [
   name = "github.com/golang/protobuf"
   version = "v1.1.*"
 
-[[constraint]]
-  name = "gopkg.in/ini.v1"
-  version = "1.38.2"
-
 [[constraint]]
   name = "gopkg.in/gcfg.v1"
   version = "1.2.3"
+
+[[constraint]]
+  name = "github.com/sethvargo/go-password"
+  version = "0.1.2"

pkg/cloudprovider/provider/openstack/cloudconfig.go

@@ -3,16 +3,30 @@ package openstack
 import (
 	"bytes"
 	"fmt"
-	"strconv"
-	"strings"
+	"text/template"
 
-	"gopkg.in/ini.v1"
+	"github.com/kubermatic/machine-controller/pkg/ini"
+
+	"github.com/Masterminds/sprig"
 )
 
-// Allowed escaping characters by gopkg.in/gcfg.v1 - the lib kubernetes uses
-var escaper = strings.NewReplacer(
-	`\`, `\\`,
-	`"`, `\"`,
+const (
+	cloudConfigTpl = `[Global]
+auth-url    = {{ .Global.AuthURL | iniEscape }}
+username    = {{ .Global.Username | iniEscape }}
+password    = {{ .Global.Password | iniEscape }}
+tenant-name = {{ .Global.TenantName | iniEscape }}
+domain-name = {{ .Global.DomainName | iniEscape }}
+region      = {{ .Global.Region | iniEscape }}
+
+[LoadBalancer]
+manage-security-groups = {{ .LoadBalancer.ManageSecurityGroups }}
+
+[BlockStorage]
+ignore-volume-az  = {{ .BlockStorage.IgnoreVolumeAZ }}
+trust-device-path = {{ .BlockStorage.TrustDevicePath }}
+bs-version        = {{ .BlockStorage.BSVersion | iniEscape }}
+`
 )
 
 type LoadBalancerOpts struct {
@@ -42,59 +56,18 @@ type CloudConfig struct {
 }
 
 func CloudConfigToString(c *CloudConfig) (string, error) {
-	cfg := ini.Empty()
-
-	// Global
-	gsec, err := cfg.NewSection("Global")
-	if err != nil {
-		return "", fmt.Errorf("failed to create the global section in ini: %v", err)
-	}
-	if _, err := gsec.NewKey("auth-url", escaper.Replace(c.Global.AuthURL)); err != nil {
-		return "", fmt.Errorf("failed to write global.auth-url: %v", err)
-	}
-	if _, err := gsec.NewKey("username", escaper.Replace(c.Global.Username)); err != nil {
-		return "", fmt.Errorf("failed to write global.username: %v", err)
-	}
-	if _, err := gsec.NewKey("password", escaper.Replace(c.Global.Password)); err != nil {
-		return "", fmt.Errorf("failed to write global.password: %v", err)
-	}
-	if _, err := gsec.NewKey("tenant-name", escaper.Replace(c.Global.TenantName)); err != nil {
-		return "", fmt.Errorf("failed to write global.tenant-name: %v", err)
-	}
-	if _, err := gsec.NewKey("domain-name", escaper.Replace(c.Global.DomainName)); err != nil {
-		return "", fmt.Errorf("failed to write global.domain-name: %v", err)
-	}
-	if _, err := gsec.NewKey("region", escaper.Replace(c.Global.Region)); err != nil {
-		return "", fmt.Errorf("failed to write global.region: %v", err)
-	}
+	funcMap := sprig.TxtFuncMap()
+	funcMap["iniEscape"] = ini.Escape
 
-	// LoadBalancer
-	lbsec, err := cfg.NewSection("LoadBalancer")
+	tpl, err := template.New("cloud-config").Funcs(funcMap).Parse(cloudConfigTpl)
 	if err != nil {
-		return "", fmt.Errorf("failed to create the LoadBalancer section in ini: %v", err)
-	}
-	if _, err := lbsec.NewKey("manage-security-groups", strconv.FormatBool(c.LoadBalancer.ManageSecurityGroups)); err != nil {
-		return "", fmt.Errorf("failed to write LoadBalancer.manage-security-groups: %v", err)
+		return "", fmt.Errorf("failed to parse the cloud config template: %v", err)
 	}
 
-	// BlockStorage
-	bssec, err := cfg.NewSection("BlockStorage")
-	if err != nil {
-		return "", fmt.Errorf("failed to create the BlockStorage section in ini: %v", err)
-	}
-	if _, err := bssec.NewKey("ignore-volume-az", strconv.FormatBool(c.BlockStorage.IgnoreVolumeAZ)); err != nil {
-		return "", fmt.Errorf("failed to write BlockStorage.ignore-volume-az: %v", err)
-	}
-	if _, err := bssec.NewKey("trust-device-path", strconv.FormatBool(c.BlockStorage.TrustDevicePath)); err != nil {
-		return "", fmt.Errorf("failed to write BlockStorage.trust-device-path: %v", err)
-	}
-	if _, err := bssec.NewKey("bs-version", c.BlockStorage.BSVersion); err != nil {
-		return "", fmt.Errorf("failed to write BlockStorage.bs-version: %v", err)
+	buf := &bytes.Buffer{}
+	if err := tpl.Execute(buf, c); err != nil {
+		return "", fmt.Errorf("failed to execute cloud config template: %v", err)
 	}
 
-	b := &bytes.Buffer{}
-	if _, err := cfg.WriteTo(b); err != nil {
-		return "", fmt.Errorf("failed to write ini to buffer: %v", err)
-	}
-	return b.String(), nil
+	return buf.String(), nil
 }

pkg/cloudprovider/provider/openstack/cloudconfig_test.go

@@ -1,21 +1,23 @@
 package openstack
 
 import (
-	"github.com/go-test/deep"
-	"github.com/pmezard/go-difflib/difflib"
-	"gopkg.in/gcfg.v1"
+	"flag"
 	"testing"
+
+	"gopkg.in/gcfg.v1"
+
+	testhelper "github.com/kubermatic/machine-controller/pkg/test"
 )
 
+var update = flag.Bool("update", false, "update .golden files")
+
 func TestCloudConfigToString(t *testing.T) {
 	tests := []struct {
-		name     string
-		config   *CloudConfig
-		expected string
+		name   string
+		config *CloudConfig
 	}{
 		{
-			name:     "simple config",
-			expected: expectedSimpleConfig,
+			name: "simple-config",
 			config: &CloudConfig{
 				Global: GlobalOpts{
 					AuthURL:    "https://127.0.0.1:8443",
@@ -36,13 +38,12 @@ func TestCloudConfigToString(t *testing.T) {
 			},
 		},
 		{
-			name:     "config with special chars",
-			expected: expectedSpecialCharactersConfig,
+			name: "config-with-special-chars",
 			config: &CloudConfig{
 				Global: GlobalOpts{
 					AuthURL:    "https://127.0.0.1:8443",
 					Username:   "admin",
-					Password:   "\"f;o'ob\\a`r=#",
+					Password:   `.)\^x[tt0L@};p<KJ|f.VQ]7r9u;"ZF|`,
 					DomainName: "Default",
 					TenantName: "Test",
 					Region:     "eu-central1",
@@ -68,66 +69,11 @@ func TestCloudConfigToString(t *testing.T) {
 
 			nc := &CloudConfig{}
 			if err := gcfg.ReadStringInto(nc, s); err != nil {
+				t.Logf("\n%s", s)
 				t.Fatalf("failed to load string into config object: %v", err)
 			}
 
-			ddiff := deep.Equal(test.config, nc)
-			if ddiff != nil {
-				t.Fatal(ddiff)
-			}
-
-			diff := difflib.UnifiedDiff{
-				A:        difflib.SplitLines(string(test.expected)),
-				B:        difflib.SplitLines(s),
-				FromFile: "Expected",
-				ToFile:   "Current",
-				Context:  3,
-			}
-			diffStr, err := difflib.GetUnifiedDiffString(diff)
-			if err != nil {
-				t.Error(err)
-			}
-
-			if diffStr != "" {
-				t.Errorf("got diff between expected and actual result: \n%s\n", diffStr)
-			}
+			testhelper.CompareOutput(t, test.name, s, *update)
 		})
 	}
 }
-
-var (
-	expectedSimpleConfig = `[Global]
-auth-url    = https://127.0.0.1:8443
-username    = admin
-password    = password
-tenant-name = Test
-domain-name = Default
-region      = eu-central1
-
-[LoadBalancer]
-manage-security-groups = true
-
-[BlockStorage]
-ignore-volume-az  = true
-trust-device-path = true
-bs-version        = v2
-
-`
-	expectedSpecialCharactersConfig = `[Global]
-auth-url    = https://127.0.0.1:8443
-username    = admin
-password    = """\"f;o'ob\\a` + "`" + `r=#"""
-tenant-name = Test
-domain-name = Default
-region      = eu-central1
-
-[LoadBalancer]
-manage-security-groups = true
-
-[BlockStorage]
-ignore-volume-az  = true
-trust-device-path = true
-bs-version        = v2
-
-`
-)

pkg/cloudprovider/provider/openstack/testdata/config-with-special-chars.golden

@@ -0,0 +1,15 @@
+[Global]
+auth-url    = "https://127.0.0.1:8443"
+username    = "admin"
+password    = ".)\\^x[tt0L@};p<KJ|f.VQ]7r9u;\"ZF|"
+tenant-name = "Test"
+domain-name = "Default"
+region      = "eu-central1"
+
+[LoadBalancer]
+manage-security-groups = true
+
+[BlockStorage]
+ignore-volume-az  = true
+trust-device-path = true
+bs-version        = "v2"

pkg/cloudprovider/provider/openstack/testdata/simple-config.golden

@@ -0,0 +1,15 @@
+[Global]
+auth-url    = "https://127.0.0.1:8443"
+username    = "admin"
+password    = "password"
+tenant-name = "Test"
+domain-name = "Default"
+region      = "eu-central1"
+
+[LoadBalancer]
+manage-security-groups = true
+
+[BlockStorage]
+ignore-volume-az  = true
+trust-device-path = true
+bs-version        = "v2"