Skip to content

Commit 772e28e

Browse files
m-ildefonsm-ildefons
authored
fix kernel command line for management interface (#942)
* fix kernel command line for management interface Fix parsing management interface configuration from the kernel command line. The parser now accepts more different specifications for the management interface, including just the systemd name for the device. Additionally, should the configuration not be parseable (e.g. due to a typo), the installer doesn't crash anymore, but rather produces a human readable error. related-to: harvester/harvester#7470 Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com> * Add test cases for management interface details Add test case for producing a useful error when parsing management interface details given on the kernel command line fails. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com> --------- Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
1 parent 8ff5804 commit 772e28e

File tree

2 files changed

+96
-21
lines changed

2 files changed

+96
-21
lines changed

‎pkg/util/cmdline.go

+52-4
Original file line numberDiff line numberDiff line change
@@ -83,19 +83,67 @@ func toNetworkInterfaces(data map[string]interface{}) error {
8383

8484
outDetails := make([]interface{}, 0, len(ifDetails))
8585
for _, v := range ifDetails {
86-
tmpStrings := strings.SplitN(v, ":", 2)
87-
n := make(map[string]interface{})
88-
err := json.Unmarshal([]byte(fmt.Sprintf("{\"%s\":\"%s\"}", tmpStrings[0], strings.ReplaceAll(tmpStrings[1], " ", ""))), &n)
86+
n, err := parseIfDetails(v)
8987
if err != nil {
9088
return err
9189
}
92-
outDetails = append(outDetails, n)
90+
outDetails = append(outDetails, *n)
9391
}
9492

9593
values.PutValue(data, outDetails, "install", "management_interface", "interfaces")
9694
return nil
9795
}
9896

97+
// parseIfDetails accepts strings in the form of:
98+
// - "hwAddr: ab:cd:ef:gh:ij:kl"
99+
// - "name: ens3"
100+
// - "ab:cd:ef:gh:ij:kl"
101+
// - "ens3"
102+
// and returns a map of either
103+
// "hwAddr: ab:cd:ef:gh:ij:kl"
104+
// or
105+
// "name: ens3"
106+
func parseIfDetails(details string) (*map[string]interface{}, error) {
107+
var (
108+
parts []string
109+
data string
110+
)
111+
112+
for _, s := range strings.Split(details, ":") {
113+
parts = append(parts, strings.TrimSpace(s))
114+
}
115+
116+
switch len(parts) {
117+
case 7:
118+
// hwAddr: ab:cd:ef:gh:ij:kl
119+
if parts[0] != "hwAddr" {
120+
return nil, fmt.Errorf("could not parse interface details %v", details)
121+
}
122+
data = fmt.Sprintf("{\"hwAddr\":\"%v\"}", strings.Join(parts[1:], ":"))
123+
case 6:
124+
// ab:cd:ef:gh:ij:kl
125+
data = fmt.Sprintf("{\"hwAddr\":\"%v\"}", strings.Join(parts, ":"))
126+
case 2:
127+
// name: ens3
128+
if parts[0] != "name" {
129+
return nil, fmt.Errorf("could not parse interface details %v", details)
130+
}
131+
data = fmt.Sprintf("{\"name\":\"%v\"}", parts[1])
132+
case 1:
133+
// ens3
134+
data = fmt.Sprintf("{\"name\":\"%v\"}", parts[0])
135+
default:
136+
return nil, fmt.Errorf("could not parse interface details %v", details)
137+
}
138+
139+
n := make(map[string]interface{})
140+
err := json.Unmarshal([]byte(data), &n)
141+
if err != nil {
142+
return nil, err
143+
}
144+
return &n, nil
145+
}
146+
99147
func toSchemeVersion(data map[string]interface{}) error {
100148
schemeVersion, ok := values.GetValue(data, "scheme_version")
101149
if !ok {

‎pkg/util/cmdline_test.go

+44-17
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package util
22

33
import (
44
"fmt"
5+
"strings"
56
"testing"
67

78
"github.com/rancher/mapper/values"
@@ -37,33 +38,59 @@ func Test_parseCmdLineWithoutPrefix(t *testing.T) {
3738
}
3839

3940
func Test_parseCmdLineWithNetworkInterface(t *testing.T) {
40-
41-
cmdline := `harvester.os.sshAuthorizedKeys=a harvester.install.management_interface.method=dhcp harvester.install.management_interface.bond_options.mode=balance-tlb harvester.install.management_interface.bond_options.miimon=100 harvester.os.sshAuthorizedKeys=b harvester.install.mode=create harvester.install.management_interface.interfaces="hwAddr: ab:cd:ef:gh" harvester.install.management_interface.interfaces="hwAddr: de:fg:hi:jk"`
42-
43-
m, err := parseCmdLine(cmdline, "harvester")
44-
if err != nil {
45-
t.Fatal(err)
41+
type testcase struct {
42+
cmdline string
43+
expectation []interface{}
44+
expectedError error
4645
}
4746

48-
want := []interface{}{
49-
map[string]interface{}{
50-
"hwAddr": "ab:cd:ef:gh",
47+
testcases := []testcase{
48+
{
49+
cmdline: `harvester.os.sshAuthorizedKeys=a harvester.install.management_interface.method=dhcp harvester.install.management_interface.bond_options.mode=balance-tlb harvester.install.management_interface.bond_options.miimon=100 harvester.os.sshAuthorizedKeys=b harvester.install.mode=create harvester.install.management_interface.interfaces="hwAddr: ab:cd:ef:gh:ij:kl" harvester.install.management_interface.interfaces="hwAddr: de:fg:hi:jk:lm:no" harvester.install.management_interface.interfaces="ens3" harvester.install.management_interface.interfaces="name:ens5"`,
50+
expectation: []interface{}{
51+
map[string]interface{}{"hwAddr": "ab:cd:ef:gh:ij:kl"},
52+
map[string]interface{}{"hwAddr": "de:fg:hi:jk:lm:no"},
53+
map[string]interface{}{"name": "ens3"},
54+
map[string]interface{}{"name": "ens5"},
55+
},
56+
expectedError: nil,
57+
},
58+
{
59+
cmdline: `harvester.install.management_interface.interfaces="ens3"`,
60+
expectation: []interface{}{
61+
map[string]interface{}{"name": "ens3"},
62+
},
63+
expectedError: nil,
5164
},
52-
map[string]interface{}{
53-
"hwAddr": "de:fg:hi:jk",
65+
{
66+
cmdline: `harvester.os.sshAuthorizedKeys=a harvester.install.management_interface.method=dhcp harvester.install.management_interface.bond_options.mode=balance-tlb harvester.install.management_interface.bond_options.miimon=100 harvester.os.sshAuthorizedKeys=b harvester.install.mode=create harvester.install.management_interface.interfaces="foo:bar:foobar"`,
67+
expectation: []interface{}{},
68+
expectedError: fmt.Errorf("could not parse interface details"),
5469
},
5570
}
5671

57-
have, ok := values.GetValue(m, "install", "management_interface", "interfaces")
58-
if !ok {
59-
t.Fatal(fmt.Errorf("no network interfaces found"))
60-
}
72+
for _, tc := range testcases {
73+
m, err := parseCmdLine(tc.cmdline, "harvester")
74+
if err != nil {
75+
if tc.expectedError != nil {
76+
assert.True(t, strings.Contains(err.Error(), tc.expectedError.Error()), "unexpected error")
77+
} else {
78+
t.Fatal(err)
79+
}
80+
} else {
81+
want := tc.expectation
82+
have, ok := values.GetValue(m, "install", "management_interface", "interfaces")
83+
if !ok {
84+
t.Fatal(fmt.Errorf("no network interfaces found"))
85+
}
6186

62-
assert.Equal(t, want, have)
87+
assert.Equal(t, want, have)
88+
}
89+
}
6390
}
6491

6592
func Test_parseCmdLineWithSchemeVersion(t *testing.T) {
66-
cmdline := `harvester.os.sshAuthorizedKeys=a harvester.install.management_interface.method=dhcp harvester.install.management_interface.bond_options.mode=balance-tlb harvester.install.management_interface.bond_options.miimon=100 harvester.os.sshAuthorizedKeys=b harvester.install.mode=create harvester.install.management_interface.interfaces="hwAddr: ab:cd:ef:gh" harvester.install.management_interface.interfaces="hwAddr: de:fg:hi:jk" harvester.scheme_version=1`
93+
cmdline := `harvester.os.sshAuthorizedKeys=a harvester.install.management_interface.method=dhcp harvester.install.management_interface.bond_options.mode=balance-tlb harvester.install.management_interface.bond_options.miimon=100 harvester.os.sshAuthorizedKeys=b harvester.install.mode=create harvester.install.management_interface.interfaces="hwAddr: ab:cd:ef:gh:ij:kl" harvester.install.management_interface.interfaces="hwAddr: de:fg:hi:jk:lm:no" harvester.scheme_version=1`
6794

6895
m, err := parseCmdLine(cmdline, "harvester")
6996
assert.NoError(t, err, "expected no error while parsing arguments")

0 commit comments

Comments
 (0)