Skip to content

Commit f71c642

Browse files
mrIncompetentkubermatic-bot
mrIncompetent
authored and
kubermatic-bot
committed
Fix ini escaping (#370)
* fix ini escaping by going back to txt templates * vendor: add password lib & remove gopkg.in/ini.v1 * fix cyclic dependency * rename func * add failed password to test fatal message
1 parent ca262b1 commit f71c642

File tree

23 files changed

+561
-3310
lines changed

23 files changed

+561
-3310
lines changed

‎Gopkg.lock‎

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Gopkg.toml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,10 @@ required = [
107107
name = "github.com/golang/protobuf"
108108
version = "v1.1.*"
109109

110-
[[constraint]]
111-
name = "gopkg.in/ini.v1"
112-
version = "1.38.2"
113-
114110
[[constraint]]
115111
name = "gopkg.in/gcfg.v1"
116112
version = "1.2.3"
113+
114+
[[constraint]]
115+
name = "github.com/sethvargo/go-password"
116+
version = "0.1.2"

‎pkg/cloudprovider/provider/openstack/cloudconfig.go‎

Lines changed: 29 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,30 @@ package openstack
33
import (
44
"bytes"
55
"fmt"
6-
"strconv"
7-
"strings"
6+
"text/template"
87

9-
"gopkg.in/ini.v1"
8+
"github.com/kubermatic/machine-controller/pkg/ini"
9+
10+
"github.com/Masterminds/sprig"
1011
)
1112

12-
// Allowed escaping characters by gopkg.in/gcfg.v1 - the lib kubernetes uses
13-
var escaper = strings.NewReplacer(
14-
`\`, `\\`,
15-
`"`, `\"`,
13+
const (
14+
cloudConfigTpl = `[Global]
15+
auth-url = {{ .Global.AuthURL | iniEscape }}
16+
username = {{ .Global.Username | iniEscape }}
17+
password = {{ .Global.Password | iniEscape }}
18+
tenant-name = {{ .Global.TenantName | iniEscape }}
19+
domain-name = {{ .Global.DomainName | iniEscape }}
20+
region = {{ .Global.Region | iniEscape }}
21+
22+
[LoadBalancer]
23+
manage-security-groups = {{ .LoadBalancer.ManageSecurityGroups }}
24+
25+
[BlockStorage]
26+
ignore-volume-az = {{ .BlockStorage.IgnoreVolumeAZ }}
27+
trust-device-path = {{ .BlockStorage.TrustDevicePath }}
28+
bs-version = {{ .BlockStorage.BSVersion | iniEscape }}
29+
`
1630
)
1731

1832
type LoadBalancerOpts struct {
@@ -42,59 +56,18 @@ type CloudConfig struct {
4256
}
4357

4458
func CloudConfigToString(c *CloudConfig) (string, error) {
45-
cfg := ini.Empty()
46-
47-
// Global
48-
gsec, err := cfg.NewSection("Global")
49-
if err != nil {
50-
return "", fmt.Errorf("failed to create the global section in ini: %v", err)
51-
}
52-
if _, err := gsec.NewKey("auth-url", escaper.Replace(c.Global.AuthURL)); err != nil {
53-
return "", fmt.Errorf("failed to write global.auth-url: %v", err)
54-
}
55-
if _, err := gsec.NewKey("username", escaper.Replace(c.Global.Username)); err != nil {
56-
return "", fmt.Errorf("failed to write global.username: %v", err)
57-
}
58-
if _, err := gsec.NewKey("password", escaper.Replace(c.Global.Password)); err != nil {
59-
return "", fmt.Errorf("failed to write global.password: %v", err)
60-
}
61-
if _, err := gsec.NewKey("tenant-name", escaper.Replace(c.Global.TenantName)); err != nil {
62-
return "", fmt.Errorf("failed to write global.tenant-name: %v", err)
63-
}
64-
if _, err := gsec.NewKey("domain-name", escaper.Replace(c.Global.DomainName)); err != nil {
65-
return "", fmt.Errorf("failed to write global.domain-name: %v", err)
66-
}
67-
if _, err := gsec.NewKey("region", escaper.Replace(c.Global.Region)); err != nil {
68-
return "", fmt.Errorf("failed to write global.region: %v", err)
69-
}
59+
funcMap := sprig.TxtFuncMap()
60+
funcMap["iniEscape"] = ini.Escape
7061

71-
// LoadBalancer
72-
lbsec, err := cfg.NewSection("LoadBalancer")
62+
tpl, err := template.New("cloud-config").Funcs(funcMap).Parse(cloudConfigTpl)
7363
if err != nil {
74-
return "", fmt.Errorf("failed to create the LoadBalancer section in ini: %v", err)
75-
}
76-
if _, err := lbsec.NewKey("manage-security-groups", strconv.FormatBool(c.LoadBalancer.ManageSecurityGroups)); err != nil {
77-
return "", fmt.Errorf("failed to write LoadBalancer.manage-security-groups: %v", err)
64+
return "", fmt.Errorf("failed to parse the cloud config template: %v", err)
7865
}
7966

80-
// BlockStorage
81-
bssec, err := cfg.NewSection("BlockStorage")
82-
if err != nil {
83-
return "", fmt.Errorf("failed to create the BlockStorage section in ini: %v", err)
84-
}
85-
if _, err := bssec.NewKey("ignore-volume-az", strconv.FormatBool(c.BlockStorage.IgnoreVolumeAZ)); err != nil {
86-
return "", fmt.Errorf("failed to write BlockStorage.ignore-volume-az: %v", err)
87-
}
88-
if _, err := bssec.NewKey("trust-device-path", strconv.FormatBool(c.BlockStorage.TrustDevicePath)); err != nil {
89-
return "", fmt.Errorf("failed to write BlockStorage.trust-device-path: %v", err)
90-
}
91-
if _, err := bssec.NewKey("bs-version", c.BlockStorage.BSVersion); err != nil {
92-
return "", fmt.Errorf("failed to write BlockStorage.bs-version: %v", err)
67+
buf := &bytes.Buffer{}
68+
if err := tpl.Execute(buf, c); err != nil {
69+
return "", fmt.Errorf("failed to execute cloud config template: %v", err)
9370
}
9471

95-
b := &bytes.Buffer{}
96-
if _, err := cfg.WriteTo(b); err != nil {
97-
return "", fmt.Errorf("failed to write ini to buffer: %v", err)
98-
}
99-
return b.String(), nil
72+
return buf.String(), nil
10073
}

‎pkg/cloudprovider/provider/openstack/cloudconfig_test.go‎

Lines changed: 14 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,23 @@
11
package openstack
22

33
import (
4-
"github.com/go-test/deep"
5-
"github.com/pmezard/go-difflib/difflib"
6-
"gopkg.in/gcfg.v1"
4+
"flag"
75
"testing"
6+
7+
"gopkg.in/gcfg.v1"
8+
9+
testhelper "github.com/kubermatic/machine-controller/pkg/test"
810
)
911

12+
var update = flag.Bool("update", false, "update .golden files")
13+
1014
func TestCloudConfigToString(t *testing.T) {
1115
tests := []struct {
12-
name string
13-
config *CloudConfig
14-
expected string
16+
name string
17+
config *CloudConfig
1518
}{
1619
{
17-
name: "simple config",
18-
expected: expectedSimpleConfig,
20+
name: "simple-config",
1921
config: &CloudConfig{
2022
Global: GlobalOpts{
2123
AuthURL: "https://127.0.0.1:8443",
@@ -36,13 +38,12 @@ func TestCloudConfigToString(t *testing.T) {
3638
},
3739
},
3840
{
39-
name: "config with special chars",
40-
expected: expectedSpecialCharactersConfig,
41+
name: "config-with-special-chars",
4142
config: &CloudConfig{
4243
Global: GlobalOpts{
4344
AuthURL: "https://127.0.0.1:8443",
4445
Username: "admin",
45-
Password: "\"f;o'ob\\a`r=#",
46+
Password: `.)\^x[tt0L@};p<KJ|f.VQ]7r9u;"ZF|`,
4647
DomainName: "Default",
4748
TenantName: "Test",
4849
Region: "eu-central1",
@@ -68,66 +69,11 @@ func TestCloudConfigToString(t *testing.T) {
6869

6970
nc := &CloudConfig{}
7071
if err := gcfg.ReadStringInto(nc, s); err != nil {
72+
t.Logf("\n%s", s)
7173
t.Fatalf("failed to load string into config object: %v", err)
7274
}
7375

74-
ddiff := deep.Equal(test.config, nc)
75-
if ddiff != nil {
76-
t.Fatal(ddiff)
77-
}
78-
79-
diff := difflib.UnifiedDiff{
80-
A: difflib.SplitLines(string(test.expected)),
81-
B: difflib.SplitLines(s),
82-
FromFile: "Expected",
83-
ToFile: "Current",
84-
Context: 3,
85-
}
86-
diffStr, err := difflib.GetUnifiedDiffString(diff)
87-
if err != nil {
88-
t.Error(err)
89-
}
90-
91-
if diffStr != "" {
92-
t.Errorf("got diff between expected and actual result: \n%s\n", diffStr)
93-
}
76+
testhelper.CompareOutput(t, test.name, s, *update)
9477
})
9578
}
9679
}
97-
98-
var (
99-
expectedSimpleConfig = `[Global]
100-
auth-url = https://127.0.0.1:8443
101-
username = admin
102-
password = password
103-
tenant-name = Test
104-
domain-name = Default
105-
region = eu-central1
106-
107-
[LoadBalancer]
108-
manage-security-groups = true
109-
110-
[BlockStorage]
111-
ignore-volume-az = true
112-
trust-device-path = true
113-
bs-version = v2
114-
115-
`
116-
expectedSpecialCharactersConfig = `[Global]
117-
auth-url = https://127.0.0.1:8443
118-
username = admin
119-
password = """\"f;o'ob\\a` + "`" + `r=#"""
120-
tenant-name = Test
121-
domain-name = Default
122-
region = eu-central1
123-
124-
[LoadBalancer]
125-
manage-security-groups = true
126-
127-
[BlockStorage]
128-
ignore-volume-az = true
129-
trust-device-path = true
130-
bs-version = v2
131-
132-
`
133-
)

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
[Global]
2+
auth-url = "https://127.0.0.1:8443"
3+
username = "admin"
4+
password = ".)\\^x[tt0L@};p<KJ|f.VQ]7r9u;\"ZF|"
5+
tenant-name = "Test"
6+
domain-name = "Default"
7+
region = "eu-central1"
8+
9+
[LoadBalancer]
10+
manage-security-groups = true
11+
12+
[BlockStorage]
13+
ignore-volume-az = true
14+
trust-device-path = true
15+
bs-version = "v2"

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
[Global]
2+
auth-url = "https://127.0.0.1:8443"
3+
username = "admin"
4+
password = "password"
5+
tenant-name = "Test"
6+
domain-name = "Default"
7+
region = "eu-central1"
8+
9+
[LoadBalancer]
10+
manage-security-groups = true
11+
12+
[BlockStorage]
13+
ignore-volume-az = true
14+
trust-device-path = true
15+
bs-version = "v2"

0 commit comments

Comments
 (0)