From 92494dd189dd53340bde12626bcda69451aa3ddf Mon Sep 17 00:00:00 2001 From: Xichen Lin Date: Mon, 15 Jan 2024 08:11:00 +0000 Subject: [PATCH 1/6] add dhcp_server bind/unbind --- .../cli-plugin-tests/mock_config_db.json | 14 +- .../test_config_dhcp_server.py | 230 +++++++++++++++++- .../cli-plugin-tests/test_show_dhcp_server.py | 16 +- .../cli/config/plugins/dhcp_server.py | 102 ++++++++ 4 files changed, 351 insertions(+), 11 deletions(-) diff --git a/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json b/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json index 773b7e178ab5..5310532df74f 100644 --- a/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json +++ b/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json @@ -4,11 +4,11 @@ }, "VLAN_INTERFACE|Vlan100": { }, - "VLAN_INTERFACE|Vlan100|100.1.1.1/24": { + "VLAN_INTERFACE|Vlan100|100.1.1.0/24": { }, "VLAN_INTERFACE|Vlan200": { }, - "VLAN_INTERFACE|Vlan200|100.1.1.2/24": { + "VLAN_INTERFACE|Vlan200|100.1.2.0/24": { }, "VLAN_INTERFACE|Vlan300": { }, @@ -42,17 +42,23 @@ "DHCP_SERVER_IPV4_RANGE|range3": { "range": "100.1.1.10" }, + "DHCP_SERVER_IPV4_RANGE|range5": { + "range": "100.1.2.10" + }, + "DHCP_SERVER_IPV4_RANGE|range6": { + "range": "100.1.2.11" + }, "DHCP_SERVER_IPV4_IP|eth0": { "ip": "240.127.1.2" }, "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet4": { - "ips": "100.1.1.10,10.1.1.11" + "ips": "100.1.1.10,100.1.1.11" }, "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet7": { "ranges": "range1,range3" }, "DHCP_SERVER_IPV4_PORT|Vlan200|Ethernet8": { - "ranges": "range1,range4" + "ranges": "range5,range6" }, "DHCP_SERVER_IPV4_PORT|Ethernet9": { "ranges": "range5,range6" diff --git a/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py b/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py index 612f229cc8ee..ec74e5078076 100644 --- a/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py +++ b/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py @@ -51,7 +51,7 @@ def test_config_dhcp_server_ipv4_add(self, mock_db): def test_config_dhcp_server_ipv4_add_dup_gw_nm(self, mock_db): expected_value = { - "gateway": "100.1.1.2", + "gateway": "100.1.2.0", "lease_time": "1000", "mode": "PORT", "netmask": "255.255.255.0", @@ -370,3 +370,231 @@ def test_config_dhcp_server_ipv4_range_delete_referenced_force(self, mock_db): assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) assert mock_db.exists("CONFIG_DB", "DHCP_SERVER_IPV4_RANGE|range1") == False + def test_config_dhcp_server_ipv4_bind_range_nonexisting(self, mock_db): + expected_value = "range2,range3" + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet2", "--range", "range2,range3"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = mock_db.get("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet2", "ranges") + assert result and set(result.split(",")) == set(expected_value.split(",")) + + def test_config_dhcp_server_ipv4_bind_ip_nonexisting(self, mock_db): + expected_value = "100.1.1.1,100.1.1.2" + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet2", "100.1.1.1,100.1.1.2"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = mock_db.get("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet2", "ips") + assert result and set(result.split(",")) == set(expected_value.split(",")) + + def test_config_dhcp_server_ipv4_bind_range_existing_no_duplicate(self, mock_db): + expected_value = "range1,range2,range3" + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet7", "--range", "range2"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = mock_db.get("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet7", "ranges") + assert result and set(result.split(",")) == set(expected_value.split(",")) + + def test_config_dhcp_server_ipv4_bind_range_existing_duplicate(self, mock_db): + expected_value = "range1,range2,range3" + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet7", "--range", "range2,range3"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = mock_db.get("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet7", "ranges") + assert result and set(result.split(",")) == set(expected_value.split(",")) + + def test_config_dhcp_server_ipv4_bind_ip_existing_no_duplicate(self, mock_db): + expected_value = "100.1.1.10,100.1.1.11,100.1.1.12,100.1.1.13" + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet4", "100.1.1.12,100.1.1.13"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = mock_db.get("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet4", "ips") + assert result and set(result.split(",")) == set(expected_value.split(",")) + + def test_config_dhcp_server_ipv4_bind_ip_existing_duplicate(self, mock_db): + expected_value = "100.1.1.10,100.1.1.11,100.1.1.12" + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet4", "100.1.1.11,100.1.1.12"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = mock_db.get("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet4", "ips") + assert result and set(result.split(",")) == set(expected_value.split(",")) + + def test_config_dhcp_server_ipv4_bind_nonexisting_range(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet7", "--range", "range4"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_bind_range_out_of_subnet(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet7", "--range", "range5"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_bind_ip_out_of_subnet(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet4", "100.1.2.10"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_bind_range_and_ip(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet2", "100.1.1.13,100.1.1.14", "--range", "range3"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_bind_range_to_existing_ip(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet4", "--range", "range3"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_bind_ip_to_existing_range(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet7", "100.1.1.13,100.1.1.14"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_bind_nothing(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet2"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_unbind_range_with_remain(self, mock_db): + expected_value = "range1" + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet7", "--range", "range3"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = mock_db.get("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet7", "ranges") + assert result and set(result.split(",")) == set(expected_value.split(",")) + + def test_config_dhcp_server_ipv4_unbind_range_with_no_remain(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet7", "--range", "range1,range3"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + assert mock_db.exists("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet7") == False + + def test_config_dhcp_server_ipv4_unbind_range_with_all(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet7", "all"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + assert mock_db.exists("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet7") == False + + def test_config_dhcp_server_ipv4_unbind_ip_with_remain(self, mock_db): + expected_value = "100.1.1.10" + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet4", "100.1.1.11"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = mock_db.get("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet4", "ips") + assert result and set(result.split(",")) == set(expected_value.split(",")) + + def test_config_dhcp_server_ipv4_unbind_ip_with_no_remain(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet4", "100.1.1.10,100.1.1.11"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + assert mock_db.exists("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet4") == False + + def test_config_dhcp_server_ipv4_unbind_ip_all(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet4", "all"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + assert mock_db.exists("CONFIG_DB", "DHCP_SERVER_IPV4_PORT|Vlan100|Ethernet4") == False + + def test_config_dhcp_server_ipv4_unbind_range_and_ip(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet2", "100.1.1.13,100.1.1.14", "--range", "range3"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_unbind_range_to_existing_ip(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet4", "--range", "range3"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_unbind_ip_to_existing_range(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet7", "100.1.1.13,100.1.1.14"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_unbind_nothing(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet4"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_unbind_unbind_range(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet7", "--range", "range2"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + + def test_config_dhcp_server_ipv4_unbind_unbind_ip(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["unbind"], \ + ["Vlan100", "Ethernet4", "100.1.1.13,100.1.1.14"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + diff --git a/dockers/docker-dhcp-server/cli-plugin-tests/test_show_dhcp_server.py b/dockers/docker-dhcp-server/cli-plugin-tests/test_show_dhcp_server.py index f09c0a9c86ae..258c6d11cc3c 100644 --- a/dockers/docker-dhcp-server/cli-plugin-tests/test_show_dhcp_server.py +++ b/dockers/docker-dhcp-server/cli-plugin-tests/test_show_dhcp_server.py @@ -76,6 +76,10 @@ def test_show_dhcp_server_ipv4_range_without_name(self, mock_db): +---------+------------+------------+------------------------+ | range3 | 100.1.1.10 | 100.1.1.10 | 1 | +---------+------------+------------+------------------------+ +| range5 | 100.1.2.10 | 100.1.2.10 | 1 | ++---------+------------+------------+------------------------+ +| range6 | 100.1.2.11 | 100.1.2.11 | 1 | ++---------+------------+------------+------------------------+ """ runner = CliRunner() db = clicommon.Db() @@ -212,13 +216,13 @@ def test_show_dhcp_server_ipv4_port_without_intf(self, mock_db): | Interface | Bind | +===================+============+ | Vlan100|Ethernet4 | 100.1.1.10 | -| | 10.1.1.11 | +| | 100.1.1.11 | +-------------------+------------+ | Vlan100|Ethernet7 | range1 | | | range3 | +-------------------+------------+ -| Vlan200|Ethernet8 | range1 | -| | range4 | +| Vlan200|Ethernet8 | range5 | +| | range6 | +-------------------+------------+ | Ethernet9 | range5 | | | range6 | @@ -253,7 +257,7 @@ def test_show_dhcp_server_ipv4_port_with_vlan(self, mock_db): | Interface | Bind | +===================+============+ | Vlan100|Ethernet4 | 100.1.1.10 | -| | 10.1.1.11 | +| | 100.1.1.11 | +-------------------+------------+ | Vlan100|Ethernet7 | range1 | | | range3 | @@ -271,8 +275,8 @@ def test_show_dhcp_server_ipv4_port_with_port_and_vlan(self, mock_db): +-------------------+--------+ | Interface | Bind | +===================+========+ -| Vlan200|Ethernet8 | range1 | -| | range4 | +| Vlan200|Ethernet8 | range5 | +| | range6 | +-------------------+--------+ """ runner = CliRunner() diff --git a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py index 237550a4b803..1452f9c8ac88 100644 --- a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py +++ b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py @@ -258,6 +258,108 @@ def dhcp_sever_ipv4_range_del(db, range_name, force): ctx.fail("Range {} does not exist, cannot delete".format(range_name)) +@dhcp_server_ipv4.command(name="bind") +@click.argument("vlan_interface", required=True) +@click.argument("interface", required=True) +@click.option("--range", "range_", required=False) +@click.argument("ip_list", required=False) +@clicommon.pass_db +def dhcp_server_ipv4_ip_bind(db, vlan_interface, interface, range_, ip_list): + ctx = click.get_current_context() + dbconn = db.db + vlan_prefix = "VLAN_INTERFACE|" + vlan_interface + "|" + subnets = [ipaddress.ip_network(key[len(vlan_prefix):]) for key in dbconn.keys("CONFIG_DB", vlan_prefix + "*")] + if range_: + range_ = set(range_.split(",")) + for r in range_: + if not dbconn.exists("CONFIG_DB", "DHCP_SERVER_IPV4_RANGE|" + r): + ctx.fail("Cannot bind nonexistent range {} to interface".format(r)) + ip_range = dbconn.get("CONFIG_DB", "DHCP_SERVER_IPV4_RANGE|" + r, "range").split(",") + if len(ip_range) == 1: + ip_start = ip_range[0] + ip_end = ip_range[0] + if len(ip_range) == 2: + ip_start = ip_range[0] + ip_end = ip_range[1] + if not any([ipaddress.ip_address(ip_start) in subnet and ipaddress.ip_address(ip_end) in subnet for subnet in subnets]): + ctx.fail("Range {} is not in any subnet of vlan {}".format(r, vlan_interface)) + if ip_list: + ip_list = set(ip_list.split(",")) + for ip in ip_list: + if not validate_str_type("ipv4-address", ip): + ctx.fail("Illegal IP address {}".format(ip)) + if not any([ipaddress.ip_address(ip) in subnet for subnet in subnets]): + ctx.fail("IP {} is not in any subnet of vlan {}".format(ip, vlan_interface)) + if range_ and ip_list or not range_ and not ip_list: + ctx.fail("Only one of range and ip list need to be provided") + key = "DHCP_SERVER_IPV4_PORT|" + vlan_interface + "|" + interface + if dbconn.exists("CONFIG_DB", key): + ips = dbconn.get("CONFIG_DB", key, "ips") + ranges = dbconn.get("CONFIG_DB", key, "ranges") + if ips and range_ or ranges and ip_list: + ctx.fail("IP bind cannot have ip range and ip list configured at the same time") + if range_: + range_set= set(ranges.split(",")) if ranges else set() + range_set = range_set.union(range_) + dbconn.set("CONFIG_DB", key, "ranges", ",".join(range_set)) + if ip_list: + ip_set= set(ips.split(",")) if ips else set() + ip_set = ip_set.union(ip_list) + dbconn.set("CONFIG_DB", key, "ips", ",".join(ip_set)) + elif range_: + dbconn.hmset("CONFIG_DB", key, {"ranges": ",".join(range_)}) + elif ip_list: + dbconn.hmset("CONFIG_DB", key, {"ips": ",".join(ip_list)}) + + +@dhcp_server_ipv4.command(name="unbind") +@click.argument("vlan_interface", required=True) +@click.argument("interface", required=True) +@click.option("--range", "range_", required=False) +@click.argument("ip_list", required=False) +@clicommon.pass_db +def dhcp_server_ipv4_ip_unbind(db, vlan_interface, interface, range_, ip_list): + ctx = click.get_current_context() + dbconn = db.db + key = "DHCP_SERVER_IPV4_PORT|" + vlan_interface + "|" + interface + if ip_list == "all": + dbconn.delete("CONFIG_DB", key) + return + if range_: + range_ = set(range_.split(",")) + if ip_list: + ip_list = set(ip_list.split(",")) + if range_ and ip_list or not range_ and not ip_list: + ctx.fail("Only one of range and ip list need to be provided") + if dbconn.exists("CONFIG_DB", key): + ips = dbconn.get("CONFIG_DB", key, "ips") + ranges = dbconn.get("CONFIG_DB", key, "ranges") + if range_: + range_set= set(ranges.split(",")) if ranges else set() + if range_set.issuperset(range_): + range_set = range_set.difference(range_) + if range_set: + dbconn.set("CONFIG_DB", key, "ranges", ",".join(range_set)) + else: + dbconn.delete("CONFIG_DB", key) + else: + ctx.fail("Attempting to unbind range that is not binded") + if ip_list: + ip_set= set(ips.split(",")) if ips else set() + if ip_set.issuperset(ip_list): + ip_set = ip_set.difference(ip_list) + if ip_set: + dbconn.set("CONFIG_DB", key, "ips", ",".join(ip_set)) + else: + dbconn.delete("CONFIG_DB", key) + else: + ctx.fail("Attempting to unbind ip that is not binded") + elif range_: + ctx.fail("The specified vlan and interface are not bind to ip range") + elif ip_list: + ctx.fail("The specified vlan and interface are not bind to ip list") + + def register(cli): # cli.add_command(dhcp_server) pass From be8acbc85a4a2b0991b1bf31ef44ec59527733b4 Mon Sep 17 00:00:00 2001 From: Xichen Lin Date: Wed, 17 Jan 2024 16:43:34 +0000 Subject: [PATCH 2/6] change prompt --- dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py index 1452f9c8ac88..f72c88a07e34 100644 --- a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py +++ b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py @@ -252,7 +252,7 @@ def dhcp_sever_ipv4_range_del(db, range_name, force): for port in dbconn.keys("CONFIG_DB", "DHCP_SERVER_IPV4_PORT*"): ranges = dbconn.get("CONFIG_DB", port, "ranges") if ranges and range_name in ranges.split(","): - ctx.fail("Range {} is referenced in {}, cannot delete, add --force to bypass".format(range_name, port)) + ctx.fail("Range {} is referenced in {}, cannot delete, add --force to bypass or range unbind to unbind range first".format(range_name, port)) dbconn.delete("CONFIG_DB", key) else: ctx.fail("Range {} does not exist, cannot delete".format(range_name)) From 1b6f0da5f4f16721f33675dda29d9090bf0e3918 Mon Sep 17 00:00:00 2001 From: Xichen Lin Date: Thu, 18 Jan 2024 11:07:24 +0000 Subject: [PATCH 3/6] improve code --- .../cli-plugin-tests/mock_config_db.json | 4 +- .../cli/config/plugins/dhcp_server.py | 93 +++++++------------ 2 files changed, 38 insertions(+), 59 deletions(-) diff --git a/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json b/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json index 5310532df74f..9ecabd1613ca 100644 --- a/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json +++ b/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json @@ -4,11 +4,11 @@ }, "VLAN_INTERFACE|Vlan100": { }, - "VLAN_INTERFACE|Vlan100|100.1.1.0/24": { + "VLAN_INTERFACE|Vlan100|100.1.1.1/24": { }, "VLAN_INTERFACE|Vlan200": { }, - "VLAN_INTERFACE|Vlan200|100.1.2.0/24": { + "VLAN_INTERFACE|Vlan200|100.1.2.2/24": { }, "VLAN_INTERFACE|Vlan300": { }, diff --git a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py index f72c88a07e34..14139964c614 100644 --- a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py +++ b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py @@ -259,16 +259,16 @@ def dhcp_sever_ipv4_range_del(db, range_name, force): @dhcp_server_ipv4.command(name="bind") -@click.argument("vlan_interface", required=True) -@click.argument("interface", required=True) +@click.argument("dhcp_interface", required=True) +@click.argument("member_interface", required=True) @click.option("--range", "range_", required=False) @click.argument("ip_list", required=False) @clicommon.pass_db -def dhcp_server_ipv4_ip_bind(db, vlan_interface, interface, range_, ip_list): +def dhcp_server_ipv4_ip_bind(db, dhcp_interface, member_interface, range_, ip_list): ctx = click.get_current_context() dbconn = db.db - vlan_prefix = "VLAN_INTERFACE|" + vlan_interface + "|" - subnets = [ipaddress.ip_network(key[len(vlan_prefix):]) for key in dbconn.keys("CONFIG_DB", vlan_prefix + "*")] + vlan_prefix = "VLAN_INTERFACE|" + dhcp_interface + "|" + subnets = [ipaddress.ip_network(key[len(vlan_prefix):], strict=False) for key in dbconn.keys("CONFIG_DB", vlan_prefix + "*")] if range_: range_ = set(range_.split(",")) for r in range_: @@ -282,82 +282,61 @@ def dhcp_server_ipv4_ip_bind(db, vlan_interface, interface, range_, ip_list): ip_start = ip_range[0] ip_end = ip_range[1] if not any([ipaddress.ip_address(ip_start) in subnet and ipaddress.ip_address(ip_end) in subnet for subnet in subnets]): - ctx.fail("Range {} is not in any subnet of vlan {}".format(r, vlan_interface)) + ctx.fail("Range {} is not in any subnet of vlan {}".format(r, dhcp_interface)) if ip_list: ip_list = set(ip_list.split(",")) for ip in ip_list: if not validate_str_type("ipv4-address", ip): ctx.fail("Illegal IP address {}".format(ip)) if not any([ipaddress.ip_address(ip) in subnet for subnet in subnets]): - ctx.fail("IP {} is not in any subnet of vlan {}".format(ip, vlan_interface)) + ctx.fail("IP {} is not in any subnet of vlan {}".format(ip, dhcp_interface)) if range_ and ip_list or not range_ and not ip_list: ctx.fail("Only one of range and ip list need to be provided") - key = "DHCP_SERVER_IPV4_PORT|" + vlan_interface + "|" + interface - if dbconn.exists("CONFIG_DB", key): - ips = dbconn.get("CONFIG_DB", key, "ips") - ranges = dbconn.get("CONFIG_DB", key, "ranges") - if ips and range_ or ranges and ip_list: - ctx.fail("IP bind cannot have ip range and ip list configured at the same time") - if range_: - range_set= set(ranges.split(",")) if ranges else set() - range_set = range_set.union(range_) - dbconn.set("CONFIG_DB", key, "ranges", ",".join(range_set)) - if ip_list: - ip_set= set(ips.split(",")) if ips else set() - ip_set = ip_set.union(ip_list) - dbconn.set("CONFIG_DB", key, "ips", ",".join(ip_set)) - elif range_: - dbconn.hmset("CONFIG_DB", key, {"ranges": ",".join(range_)}) - elif ip_list: - dbconn.hmset("CONFIG_DB", key, {"ips": ",".join(ip_list)}) + key = "DHCP_SERVER_IPV4_PORT|" + dhcp_interface + "|" + member_interface + key_exist = dbconn.exists("CONFIG_DB", key) + for bind_value_name, bind_value in [["ips", ip_list], ["ranges", range_]]: + if key_exits: + existing_value = dbconn.get("CONFIG_DB", key, bind_value_name) + if (not not existing_value) == (not bind_value): + ctx.fail("IP bind cannot have ip range and ip list configured at the same time") + if bind_value: + value_set = set(existing_value.split(",")) if existing_value else set() + new_value_set = value_set.union(bind_value) + dbconn.set("CONFIG_DB", key, bind_value_name, ",".join(new_value_set)) + elif bind_value: + dbconn.hmset("CONFIG_DB", key, {bind_value_name: ",".join(bind_value)}) @dhcp_server_ipv4.command(name="unbind") -@click.argument("vlan_interface", required=True) -@click.argument("interface", required=True) +@click.argument("dhcp_interface", required=True) +@click.argument("member_interface", required=True) @click.option("--range", "range_", required=False) @click.argument("ip_list", required=False) @clicommon.pass_db -def dhcp_server_ipv4_ip_unbind(db, vlan_interface, interface, range_, ip_list): +def dhcp_server_ipv4_ip_unbind(db, dhcp_interface, member_interface, range_, ip_list): ctx = click.get_current_context() dbconn = db.db - key = "DHCP_SERVER_IPV4_PORT|" + vlan_interface + "|" + interface + key = "DHCP_SERVER_IPV4_PORT|" + dhcp_interface + "|" + member_interface if ip_list == "all": dbconn.delete("CONFIG_DB", key) return - if range_: - range_ = set(range_.split(",")) - if ip_list: - ip_list = set(ip_list.split(",")) if range_ and ip_list or not range_ and not ip_list: ctx.fail("Only one of range and ip list need to be provided") - if dbconn.exists("CONFIG_DB", key): - ips = dbconn.get("CONFIG_DB", key, "ips") - ranges = dbconn.get("CONFIG_DB", key, "ranges") - if range_: - range_set= set(ranges.split(",")) if ranges else set() - if range_set.issuperset(range_): - range_set = range_set.difference(range_) - if range_set: - dbconn.set("CONFIG_DB", key, "ranges", ",".join(range_set)) - else: - dbconn.delete("CONFIG_DB", key) - else: - ctx.fail("Attempting to unbind range that is not binded") - if ip_list: - ip_set= set(ips.split(",")) if ips else set() - if ip_set.issuperset(ip_list): - ip_set = ip_set.difference(ip_list) - if ip_set: - dbconn.set("CONFIG_DB", key, "ips", ",".join(ip_set)) + if not dbconn.exists("CONFIG_DB", key): + ctx.fail("The specified dhcp_interface and member interface is not bind to ip or range") + for unbind_value_name, unbind_value in [["ips", ip_list], ["ranges", range_]]: + if unbind_value: + unbind_value = set(unbind_value.split(",")) + existing_value = dbconn.get("CONFIG_DB", key, unbind_value_name) + value_set = set(existing_value.split(",")) if existing_value else set() + if value_set.issuperset(unbind_value): + new_value_set = value_set.difference(unbind_value) + if new_value_set: + dbconn.set("CONFIG_DB", key, unbind_value_name, ",".join(new_value_set)) else: dbconn.delete("CONFIG_DB", key) else: - ctx.fail("Attempting to unbind ip that is not binded") - elif range_: - ctx.fail("The specified vlan and interface are not bind to ip range") - elif ip_list: - ctx.fail("The specified vlan and interface are not bind to ip list") + ctx.fail("Attempting to unbind range or ip that is not binded") def register(cli): From 224afed9ba7a72586308bee52b8d9aeb33b5eea8 Mon Sep 17 00:00:00 2001 From: Xichen Lin Date: Thu, 18 Jan 2024 11:53:17 +0000 Subject: [PATCH 4/6] fix bug --- .../cli-plugin-tests/mock_config_db.json | 6 ++++++ .../cli-plugin-tests/test_config_dhcp_server.py | 8 ++++++++ .../docker-dhcp-server/cli/config/plugins/dhcp_server.py | 2 ++ 3 files changed, 16 insertions(+) diff --git a/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json b/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json index 9ecabd1613ca..4d0d5ee10404 100644 --- a/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json +++ b/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json @@ -12,6 +12,12 @@ }, "VLAN_INTERFACE|Vlan300": { }, + "VLAN_MEMBER|Vlan100|Ethernet4": { + }, + "VLAN_MEMBER|Vlan100|Ethernet7": { + }, + "VLAN_MEMBER|Vlan100|Ethernet8": { + }, "DHCP_SERVER_IPV4|Vlan100": { "gateway": "100.1.1.1", "lease_time": "3600", diff --git a/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py b/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py index ec74e5078076..ba1aa8a16a83 100644 --- a/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py +++ b/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py @@ -460,6 +460,14 @@ def test_config_dhcp_server_ipv4_bind_ip_out_of_subnet(self, mock_db): ["Vlan100", "Ethernet4", "100.1.2.10"], obj=db) assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + def test_config_dhcp_server_ipv4_bind_interface_not_in_vlan(self, mock_db): + runner = CliRunner() + db = clicommon.Db() + db.db = mock_db + result = runner.invoke(dhcp_server.dhcp_server.commands["ipv4"].commands["bind"], \ + ["Vlan100", "Ethernet5", "100.1.1.10"], obj=db) + assert result.exit_code == 2, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + def test_config_dhcp_server_ipv4_bind_range_and_ip(self, mock_db): runner = CliRunner() db = clicommon.Db() diff --git a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py index 14139964c614..02e7485f8431 100644 --- a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py +++ b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py @@ -267,6 +267,8 @@ def dhcp_sever_ipv4_range_del(db, range_name, force): def dhcp_server_ipv4_ip_bind(db, dhcp_interface, member_interface, range_, ip_list): ctx = click.get_current_context() dbconn = db.db + if not dbconn.exists("CONFIG_DB", "VLAN_MEMBER|" + dhcp_interface + "|" + member_interface): + ctx.fail("Cannot confirm member interface {} is really in dhcp interface {}".format(member_interface, dhcp_interface)) vlan_prefix = "VLAN_INTERFACE|" + dhcp_interface + "|" subnets = [ipaddress.ip_network(key[len(vlan_prefix):], strict=False) for key in dbconn.keys("CONFIG_DB", vlan_prefix + "*")] if range_: From 64f93cbd3690b50658e2e7dc4348fa0ce3910a22 Mon Sep 17 00:00:00 2001 From: Xichen Lin Date: Thu, 18 Jan 2024 14:22:14 +0000 Subject: [PATCH 5/6] fix bug --- .../cli-plugin-tests/test_config_dhcp_server.py | 2 +- dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py b/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py index ba1aa8a16a83..8cada47f0142 100644 --- a/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py +++ b/dockers/docker-dhcp-server/cli-plugin-tests/test_config_dhcp_server.py @@ -51,7 +51,7 @@ def test_config_dhcp_server_ipv4_add(self, mock_db): def test_config_dhcp_server_ipv4_add_dup_gw_nm(self, mock_db): expected_value = { - "gateway": "100.1.2.0", + "gateway": "100.1.2.2", "lease_time": "1000", "mode": "PORT", "netmask": "255.255.255.0", diff --git a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py index 02e7485f8431..fcf2bf1db187 100644 --- a/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py +++ b/dockers/docker-dhcp-server/cli/config/plugins/dhcp_server.py @@ -297,7 +297,7 @@ def dhcp_server_ipv4_ip_bind(db, dhcp_interface, member_interface, range_, ip_li key = "DHCP_SERVER_IPV4_PORT|" + dhcp_interface + "|" + member_interface key_exist = dbconn.exists("CONFIG_DB", key) for bind_value_name, bind_value in [["ips", ip_list], ["ranges", range_]]: - if key_exits: + if key_exist: existing_value = dbconn.get("CONFIG_DB", key, bind_value_name) if (not not existing_value) == (not bind_value): ctx.fail("IP bind cannot have ip range and ip list configured at the same time") From 437bdc19ed11a2857b54442eb84dedbdbcbc3988 Mon Sep 17 00:00:00 2001 From: Xichen Lin Date: Thu, 18 Jan 2024 14:25:44 +0000 Subject: [PATCH 6/6] fix bug --- dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json b/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json index 4d0d5ee10404..cc5256079e74 100644 --- a/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json +++ b/dockers/docker-dhcp-server/cli-plugin-tests/mock_config_db.json @@ -12,6 +12,8 @@ }, "VLAN_INTERFACE|Vlan300": { }, + "VLAN_MEMBER|Vlan100|Ethernet2": { + }, "VLAN_MEMBER|Vlan100|Ethernet4": { }, "VLAN_MEMBER|Vlan100|Ethernet7": {