Resolves: rhbz#1081982 Signed-off-by: Martin Kolman mkolman@redhat.com --- pykickstart/commands/network.py | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
diff --git a/pykickstart/commands/network.py b/pykickstart/commands/network.py index 7b1341c..d48aec2 100644 --- a/pykickstart/commands/network.py +++ b/pykickstart/commands/network.py @@ -523,3 +523,45 @@ class RHEL7_Network(F20_Network): op.add_option("--interfacename", dest="interfacename", action="store", default="") return op + + def parse(self, args): + # call the overridden command to do it's job first + retval = F20_Network.parse(self, args) + + # validate the device name + # (for reference see the NetworkManager source code: + # NetworkManager/src/settings/plugins/ifcfg-rh/reader.c + # and the make_vlan_setting function) + + min_vlan_id = 0 + max_vlan_id = 4095 + vlan_id = None + + # if it contains '.', vlan id should follow (eg 'ens7.171', 'mydev.171') + split_name = retval.interfacename.split(".", 1) + if len(split_name) > 1: + # 'vlan' can't be followed by a '.' + if split_name[0] == "vlan": + raise KickstartValueError(formatErrorMsg(self.lineno, + msg=_("for network --interfacename 'vlan' can't be followed by a '.'"))) + try: + vlan_id = int(split_name[1]) + except ValueError: + raise KickstartValueError(formatErrorMsg(self.lineno, + msg=_("if network --interfacename contains a '.', valid vlan id should follow"))) + + # if it starts with 'vlan', vlan id should follow ('vlan171') + if retval.interfacename.startswith("vlan"): + try: + vlan_id = int(retval.interfacename.strip("vlan")) + except ValueError: + raise KickstartValueError(formatErrorMsg(self.lineno, + msg=_("if network --interfacename starts with 'vlan', valid vlan id should follow"))) + + # check if the vlan id is in range + if vlan_id is not None: + if not(min_vlan_id <= vlan_id <= max_vlan_id): + raise KickstartValueError(formatErrorMsg(self.lineno, + msg=_("vlan id out of range"))) + + return retval
Related: rhbz#1081982 Signed-off-by: Martin Kolman mkolman@redhat.com --- tests/commands/network.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/tests/commands/network.py b/tests/commands/network.py index 9643c74..e421c0c 100644 --- a/tests/commands/network.py +++ b/tests/commands/network.py @@ -69,5 +69,34 @@ class F20_TestCase(CommandTest): nd2 = self.assert_parse(s) self.assertEquals(value, nd2.teamslaves)
+class RHEL7_TestCase(CommandTest): + command = "network" + + def runTest(self): + # there needs to be a vlan id after a dot & only one dot is allowed + self.assert_parse_error("network --interfacename=abc.", KickstartValueError) + self.assert_parse_error("network --interfacename=abc.def", KickstartValueError) + self.assert_parse_error("network --interfacename=abc..", KickstartValueError) + self.assert_parse_error("network --interfacename=abc.123.456", KickstartValueError) + # 'vlan' can't be followed by a '.' + self.assert_parse_error("network --interfacename=vlan.123", KickstartValueError) + self.assert_parse_error("network --interfacename=vlan.", KickstartValueError) + self.assert_parse_error("network --interfacename=vlan..", KickstartValueError) + self.assert_parse_error("network --interfacename=vlan.abc", KickstartValueError) + self.assert_parse("network --interfacename=abc.123") + + # if the device name begins with 'vlan', vlan id needs to follow + self.assert_parse_error("network --interfacename=vlan", KickstartValueError) + self.assert_parse_error("network --interfacename=vlanabcd", KickstartValueError) + self.assert_parse("network --interfacename=vlan123") + + # vlan ids go from 0 to 4095 + self.assert_parse("network --interfacename=vlan0") + self.assert_parse("network --interfacename=vlan4095") + self.assert_parse("network --interfacename=abc.0") + self.assert_parse("network --interfacename=abc.4095") + self.assert_parse_error("network --interfacename=vlan9001", KickstartValueError) + self.assert_parse_error("network --interfacename=abc.9001", KickstartValueError) + if __name__ == "__main__": unittest.main()
+class RHEL7_TestCase(CommandTest):
- command = "network"
This class should be a subclass of F20_TestCase and runTest should call its parent. While it seems a little like overkill, setting things up this way has caught bugs in the past.
- Chris
On Mon, 2014-09-08 at 11:51 -0400, Chris Lumens wrote:
+class RHEL7_TestCase(CommandTest):
- command = "network"
This class should be a subclass of F20_TestCase and runTest should call its parent. While it seems a little like overkill, setting things up this way has caught bugs in the past.
Makes sense, will do! :)
- Chris
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
Resolves: rhbz#1081982
If this patch resolves this bug, the bug should be reassigned to avoid process nightmares.
# if it contains '.', vlan id should follow (eg 'ens7.171', 'mydev.171')
split_name = retval.interfacename.split(".", 1)
if len(split_name) > 1:
# 'vlan' can't be followed by a '.'
if split_name[0] == "vlan":
raise KickstartValueError(formatErrorMsg(self.lineno,
msg=_("for network --interfacename 'vlan' can't be followed by a '.'")))
I don't really understand this error message. Could it be made more clear?
try:
vlan_id = int(split_name[1])
except ValueError:
raise KickstartValueError(formatErrorMsg(self.lineno,
msg=_("if network --interfacename contains a '.', valid vlan id should follow")))
# if it starts with 'vlan', vlan id should follow ('vlan171')
if retval.interfacename.startswith("vlan"):
try:
vlan_id = int(retval.interfacename.strip("vlan"))
except ValueError:
raise KickstartValueError(formatErrorMsg(self.lineno,
msg=_("if network --interfacename starts with 'vlan', valid vlan id should follow")))
I'd like these two error messages to look more like sentences - capitalize the If, put a period at the end.
# check if the vlan id is in range
if vlan_id is not None:
if not(min_vlan_id <= vlan_id <= max_vlan_id):
raise KickstartValueError(formatErrorMsg(self.lineno,
msg=_("vlan id out of range")))
return retval
It'd help the user if you said what the valid range is.
Also this should probably all be in its own method. That way if some later version of the network command needs to override the parse method, it doesn't also need to duplicate all the work you've done here.
- Chris
On Mon, 2014-09-08 at 12:06 -0400, Chris Lumens wrote:
Resolves: rhbz#1081982
If this patch resolves this bug, the bug should be reassigned to avoid process nightmares.
Done!
# if it contains '.', vlan id should follow (eg 'ens7.171', 'mydev.171')
split_name = retval.interfacename.split(".", 1)
if len(split_name) > 1:
# 'vlan' can't be followed by a '.'
if split_name[0] == "vlan":
raise KickstartValueError(formatErrorMsg(self.lineno,
msg=_("for network --interfacename 'vlan' can't be followed by a '.'")))
I don't really understand this error message. Could it be made more clear?
After studying the related NetworkManager source code, there are two ways of how to include a vlan id in a network interface name: * use the vlan prefix, followed by the vlan number: "vlan123" * use a prefix followed by a dot ('.') and then the vlan number, with the condition that the prefix can't equal to "vlan"
So what about: "When using the <prefix>.<vlan id> interface name notation <prefix> can't be equal to 'vlan'."
Suggestions for an even more understandable error message are welcome! :)
try:
vlan_id = int(split_name[1])
except ValueError:
raise KickstartValueError(formatErrorMsg(self.lineno,
msg=_("if network --interfacename contains a '.', valid vlan id should follow")))
# if it starts with 'vlan', vlan id should follow ('vlan171')
if retval.interfacename.startswith("vlan"):
try:
vlan_id = int(retval.interfacename.strip("vlan"))
except ValueError:
raise KickstartValueError(formatErrorMsg(self.lineno,
msg=_("if network --interfacename starts with 'vlan', valid vlan id should follow")))
I'd like these two error messages to look more like sentences - capitalize the If, put a period at the end.
OK
# check if the vlan id is in range
if vlan_id is not None:
if not(min_vlan_id <= vlan_id <= max_vlan_id):
raise KickstartValueError(formatErrorMsg(self.lineno,
msg=_("vlan id out of range")))
return retval
It'd help the user if you said what the valid range is.
Sure! :)
Also this should probably all be in its own method. That way if some later version of the network command needs to override the parse method, it doesn't also need to duplicate all the work you've done here.
Doing just that.
- Chris
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
anaconda-patches@lists.fedorahosted.org