tsarevvv/nmstate
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

route and route rule: Treat empty IP string as None

Browse Source 
When deleting route or route rule with empty string like

```yml
- ip-to: 2001:db8:1::2/64
  ip-from: ''
  state: absent
```

Nmstate will fail with error on `Invalid IP network: invalid IP address
syntax`.

Fixed by treating empty IP string as None during `sanitize()` and
`is_match()`.

Unit test cases and integration test cases included.

Signed-off-by: Gris Ge <fge@redhat.com>
This commit is contained in:
Gris Ge 2023-01-12 13:37:28 +08:00 committed by Fernando Fernández Mancera
parent 8fbb93a513
commit c2e69269f9
5 changed files with 234 additions and 76 deletions
rust/src/lib
route.rsroute_rule.rs
unit_tests
route.rsroute_rule.rs
tests/integration
route_test.py

21
rust/src/lib/route.rs

@ -152,6 +152,7 @@ impl RouteEntry {
pub(crate) fn is_match(&self, other: &Self) -> bool {
if self.destination.as_ref().is_some()
&& self.destination.as_deref() != Some("")
&& self.destination != other.destination
{
return false;
@ -195,14 +196,18 @@ impl RouteEntry {
pub(crate) fn sanitize(&mut self) -> Result<(), NmstateError> {
if let Some(dst) = self.destination.as_ref() {
let new_dst = sanitize_ip_network(dst)?;
if dst != &new_dst {
log::warn!(
"Route destination {} sanitized to {}",
dst,
new_dst
);
self.destination = Some(new_dst);
if dst.is_empty() {
self.destination = None;
} else {
let new_dst = sanitize_ip_network(dst)?;
if dst != &new_dst {
log::warn!(
"Route destination {} sanitized to {}",
dst,
new_dst
);
self.destination = Some(new_dst);
}
}
}
if let Some(via) = self.next_hop_addr.as_ref() {

104
rust/src/lib/route_rule.rs

@ -205,35 +205,39 @@ impl RouteRuleEntry {
pub(crate) fn is_match(&self, other: &Self) -> bool {
if let Some(ip_from) = self.ip_from.as_deref() {
let ip_from = if !ip_from.contains('/') {
match InterfaceIpAddr::try_from(ip_from) {
Ok(ref i) => i.into(),
Err(e) => {
log::error!("{}", e);
return false;
if !ip_from.is_empty() {
let ip_from = if !ip_from.contains('/') {
match InterfaceIpAddr::try_from(ip_from) {
Ok(ref i) => i.into(),
Err(e) => {
log::error!("{}", e);
return false;
}
}
} else {
ip_from.to_string()
};
if other.ip_from != Some(ip_from) {
return false;
}
} else {
ip_from.to_string()
};
if other.ip_from != Some(ip_from) {
return false;
}
}
if let Some(ip_to) = self.ip_to.as_deref() {
let ip_to = if !ip_to.contains('/') {
match InterfaceIpAddr::try_from(ip_to) {
Ok(ref i) => i.into(),
Err(e) => {
log::error!("{}", e);
return false;
if !ip_to.is_empty() {
let ip_to = if !ip_to.contains('/') {
match InterfaceIpAddr::try_from(ip_to) {
Ok(ref i) => i.into(),
Err(e) => {
log::error!("{}", e);
return false;
}
}
} else {
ip_to.to_string()
};
if other.ip_to != Some(ip_to) {
return false;
}
} else {
ip_to.to_string()
};
if other.ip_to != Some(ip_to) {
return false;
}
}
if self.priority.is_some()
@ -301,29 +305,45 @@ impl RouteRuleEntry {
pub(crate) fn sanitize(&mut self) -> Result<(), NmstateError> {
if let Some(ip) = self.ip_from.as_ref() {
let new_ip = sanitize_ip_network(ip)?;
if self.family.is_none() {
match is_ipv6_addr(new_ip.as_str()) {
true => self.family = Some(AddressFamily::IPv6),
false => self.family = Some(AddressFamily::IPv4),
};
}
if ip != &new_ip {
log::warn!("Route rule ip-from {} sanitized to {}", ip, new_ip);
self.ip_from = Some(new_ip);
if ip.is_empty() {
self.ip_from = None;
} else {
let new_ip = sanitize_ip_network(ip)?;
if self.family.is_none() {
match is_ipv6_addr(new_ip.as_str()) {
true => self.family = Some(AddressFamily::IPv6),
false => self.family = Some(AddressFamily::IPv4),
};
}
if ip != &new_ip {
log::warn!(
"Route rule ip-from {} sanitized to {}",
ip,
new_ip
);
self.ip_from = Some(new_ip);
}
}
}
if let Some(ip) = self.ip_to.as_ref() {
let new_ip = sanitize_ip_network(ip)?;
if self.family.is_none() {
match is_ipv6_addr(new_ip.as_str()) {
true => self.family = Some(AddressFamily::IPv6),
false => self.family = Some(AddressFamily::IPv4),
};
}
if ip != &new_ip {
log::warn!("Route rule ip-to {} sanitized to {}", ip, new_ip);
self.ip_to = Some(new_ip);
if ip.is_empty() {
self.ip_to = None;
} else {
let new_ip = sanitize_ip_network(ip)?;
if self.family.is_none() {
match is_ipv6_addr(new_ip.as_str()) {
true => self.family = Some(AddressFamily::IPv6),
false => self.family = Some(AddressFamily::IPv4),
};
}
if ip != &new_ip {
log::warn!(
"Route rule ip-to {} sanitized to {}",
ip,
new_ip
);
self.ip_to = Some(new_ip);
}
}
}
self.validate_ip_from_to()?;

21
rust/src/lib/unit_tests/route.rs

@ -259,3 +259,24 @@ next-hop-address: "2001:db8:a:0000:000::1"
assert_eq!(route.destination, Some("2001:db8:1::1/128".to_string()));
assert_eq!(route.next_hop_addr, Some("2001:db8:a::1".to_string()));
}
#[test]
fn test_route_treat_empty_dst_as_none_for_matching() {
let absent_route: RouteEntry = serde_yaml::from_str(
r#"
destination: ""
state: absent
"#,
)
.unwrap();
let route: RouteEntry = serde_yaml::from_str(
r#"
destination: ::/0
next-hop-address: 2001:db8:1::2
next-hop-interface: eth1
"#,
)
.unwrap();
assert!(absent_route.is_match(&route));
}

47
rust/src/lib/unit_tests/route_rule.rs

@ -135,3 +135,50 @@ family: ipv4
rule.sanitize().unwrap();
}
#[test]
fn test_route_rule_treat_empty_ip_from_as_none_for_matching() {
let absent_rule: RouteRuleEntry = serde_yaml::from_str(
r#"
ip-from: ""
state: absent
route-table: 200
"#,
)
.unwrap();
let rule: RouteRuleEntry = serde_yaml::from_str(
r#"
ip-from: 192.168.2.0/24
priority: 30000
route-table: 200
family: ipv4
"#,
)
.unwrap();
assert!(absent_rule.is_match(&rule));
}
#[test]
fn test_route_rule_treat_empty_ip_to_as_none_for_matching() {
let absent_rule: RouteRuleEntry = serde_yaml::from_str(
r#"
ip-to: ""
state: absent
route-table: 200
"#,
)
.unwrap();
let rule: RouteRuleEntry = serde_yaml::from_str(
r#"
ip-to: 192.168.2.0/24
priority: 30000
route-table: 200
family: ipv4
"#,
)
.unwrap();
assert!(absent_rule.is_match(&rule));
}

117
tests/integration/route_test.py

@ -97,14 +97,8 @@ BGP_PROTOCOL_ID = "186"
@pytest.mark.tier1
def test_add_static_routes(eth1_up):
def test_add_static_routes(static_eth1_with_routes):
routes = _get_ipv4_test_routes() + _get_ipv6_test_routes()
libnmstate.apply(
{
Interface.KEY: [ETH1_INTERFACE_STATE],
Route.KEY: {Route.CONFIG: routes},
}
)
cur_state = libnmstate.show()
_assert_routes(routes, cur_state)
@ -719,25 +713,9 @@ def test_route_rule_add_to_only(route_rule_test_env):
@pytest.mark.tier1
def test_route_rule_add(route_rule_test_env):
state = route_rule_test_env
rules = [
{
RouteRule.IP_FROM: "2001:db8:a::/64",
RouteRule.IP_TO: "2001:db8:f::/64",
RouteRule.PRIORITY: 1000,
RouteRule.ROUTE_TABLE: IPV6_ROUTE_TABLE_ID1,
},
{
RouteRule.IP_FROM: "203.0.113.0/24",
RouteRule.IP_TO: "192.0.2.0/24",
RouteRule.PRIORITY: 1000,
RouteRule.ROUTE_TABLE: IPV4_ROUTE_TABLE_ID1,
},
]
state[RouteRule.KEY] = {RouteRule.CONFIG: rules}
libnmstate.apply(state)
def test_route_rule_add(static_eth1_with_route_rules):
state = static_eth1_with_route_rules
rules = state[RouteRule.KEY][RouteRule.CONFIG]
_check_ip_rules(rules)
@ -1406,3 +1384,90 @@ def test_route_rule_action(route_rule_test_env):
libnmstate.apply({RouteRule.KEY: {RouteRule.CONFIG: desired_rules}})
_check_ip_rules(desired_rules)
@pytest.fixture
def static_eth1_with_route_rules(route_rule_test_env):
state = route_rule_test_env
rules = [
{
RouteRule.IP_FROM: "2001:db8:a::/64",
RouteRule.IP_TO: "2001:db8:f::/64",
RouteRule.PRIORITY: 1000,
RouteRule.ROUTE_TABLE: IPV6_ROUTE_TABLE_ID1,
},
{
RouteRule.IP_FROM: "203.0.113.0/24",
RouteRule.IP_TO: "192.0.2.0/24",
RouteRule.PRIORITY: 1000,
RouteRule.ROUTE_TABLE: IPV4_ROUTE_TABLE_ID1,
},
]
state[RouteRule.KEY] = {RouteRule.CONFIG: rules}
libnmstate.apply(state)
yield state
def test_absent_route_rule_with_empty_ip_from_to(static_eth1_with_route_rules):
current_state = libnmstate.show()
assert len(current_state[RouteRule.KEY][RouteRule.CONFIG]) == 2
rules = [
{
RouteRule.STATE: RouteRule.STATE_ABSENT,
RouteRule.IP_FROM: "",
RouteRule.ROUTE_TABLE: IPV4_ROUTE_TABLE_ID1,
},
{
RouteRule.STATE: RouteRule.STATE_ABSENT,
RouteRule.IP_TO: "",
RouteRule.ROUTE_TABLE: IPV6_ROUTE_TABLE_ID1,
},
]
state = {RouteRule.KEY: {RouteRule.CONFIG: rules}}
libnmstate.apply(state)
current_state = libnmstate.show()
assert len(current_state[RouteRule.KEY][RouteRule.CONFIG]) == 0
@pytest.fixture
def static_eth1_with_routes(eth1_up):
routes = _get_ipv4_test_routes() + _get_ipv6_test_routes()
state = {
Interface.KEY: [ETH1_INTERFACE_STATE],
Route.KEY: {Route.CONFIG: routes},
}
libnmstate.apply(state)
yield state
def test_absent_route_with_empty_destination(static_eth1_with_routes):
current_state = libnmstate.show()
cur_eth1_config_routes = [
rt
for rt in current_state[Route.KEY][Route.CONFIG]
if rt[Route.NEXT_HOP_INTERFACE] == "eth1"
]
assert len(cur_eth1_config_routes) != 0
libnmstate.apply(
{
Route.KEY: {
Route.CONFIG: [
{
Route.NEXT_HOP_INTERFACE: "eth1",
Route.STATE: Route.STATE_ABSENT,
Route.DESTINATION: "",
},
]
},
}
)
current_state = libnmstate.show()
cur_eth1_config_routes = [
rt
for rt in current_state[Route.KEY][Route.CONFIG]
if rt[Route.NEXT_HOP_INTERFACE] == "eth1"
]
assert len(cur_eth1_config_routes) == 0