Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 31 additions & 16 deletions systemvm/debian/opt/cloud/bin/cs/CsAddress.py
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,37 @@ def fw_vpcrouter(self):
"-A PREROUTING -m state --state NEW -i %s -s %s ! -d %s/32 -j ACL_OUTBOUND_%s" %
(self.dev, guestNetworkCidr, self.address['gateway'], self.dev)])

# Process static routes for this interface
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhouse-nexthop , can we make the follwing code a method? i.e process_static_routes_for_Interface(…)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a vast majority of this block was just moved up outside of the if self.is_private_gateway():, I don't think it warrants a helper function tbh. Plus, the code as it is right now has been well tested, so we'd have to go through the validation process again.

If you feel really strongly about it I'll do it, but I'd rather not :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel strongly but that is not absolute. I don’t think you should force yourself or that it is a requirement for merging (at all) or that you should do it if you feel it is a risk. I do think that such extractions make code more readable and maintainable, hence my question.

static_routes = CsStaticRoutes("staticroutes", self.config)
if static_routes:
for item in static_routes.get_bag():
if item == "id":
continue
static_route = static_routes.get_bag()[item]
if static_route['revoke']:
continue

# Check if this static route applies to this interface
# Old style: ip_address field matches this interface's public_ip
# New style (nexthop): gateway is in this interface's subnet
applies_to_interface = False
if 'ip_address' in static_route and static_route['ip_address'] == self.address['public_ip']:
applies_to_interface = True
elif 'gateway' in static_route:
device = CsHelper.find_device_for_gateway(self.config, static_route['gateway'])
if device == self.dev:
applies_to_interface = True

if applies_to_interface:
self.fw.append(["mangle", "",
"-A PREROUTING -m state --state NEW -i %s -s %s ! -d %s/32 -j ACL_OUTBOUND_%s" %
(self.dev, static_route['network'], self.address['public_ip'], self.dev)])
self.fw.append(["filter", "front", "-A FORWARD -d %s -o %s -j ACL_INBOUND_%s" %
(static_route['network'], self.dev, self.dev)])
self.fw.append(["filter", "front",
"-A FORWARD -d %s -o %s -m state --state RELATED,ESTABLISHED -j ACCEPT" %
(static_route['network'], self.dev)])

if self.is_private_gateway():
self.fw.append(["filter", "front", "-A FORWARD -d %s -o %s -j ACL_INBOUND_%s" %
(self.address['network'], self.dev, self.dev)])
Expand All @@ -597,22 +628,6 @@ def fw_vpcrouter(self):
"-A PREROUTING -s %s -d %s -m state --state NEW -j MARK --set-xmark %s/0xffffffff" %
(self.cl.get_vpccidr(), self.address['network'], hex(100 + int(self.dev[3:])))])

static_routes = CsStaticRoutes("staticroutes", self.config)
if static_routes:
for item in static_routes.get_bag():
if item == "id":
continue
static_route = static_routes.get_bag()[item]
if 'ip_address' in static_route and static_route['ip_address'] == self.address['public_ip'] and not static_route['revoke']:
self.fw.append(["mangle", "",
"-A PREROUTING -m state --state NEW -i %s -s %s ! -d %s/32 -j ACL_OUTBOUND_%s" %
(self.dev, static_route['network'], static_route['ip_address'], self.dev)])
self.fw.append(["filter", "front", "-A FORWARD -d %s -o %s -j ACL_INBOUND_%s" %
(static_route['network'], self.dev, self.dev)])
self.fw.append(["filter", "front",
"-A FORWARD -d %s -o %s -m state --state RELATED,ESTABLISHED -j ACCEPT" %
(static_route['network'], self.dev)])

if self.address["source_nat"]:
self.fw.append(["nat", "front",
"-A POSTROUTING -o %s -j SNAT --to-source %s" %
Expand Down
30 changes: 30 additions & 0 deletions systemvm/debian/opt/cloud/bin/cs/CsHelper.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@
import os.path
import re
import shutil
from typing import Optional, TYPE_CHECKING
from netaddr import *

if TYPE_CHECKING:
from .CsConfig import CsConfig
Comment thread
bhouse-nexthop marked this conversation as resolved.

PUBLIC_INTERFACES = {"router": "eth2", "vpcrouter": "eth1"}

STATE_COMMANDS = {"router": "ip addr show dev eth0 | grep inet | wc -l | xargs bash -c 'if [ $0 == 2 ]; then echo \"PRIMARY\"; else echo \"BACKUP\"; fi'",
Expand Down Expand Up @@ -270,3 +274,29 @@ def copy(src, dest):
logging.error("Could not copy %s to %s" % (src, dest))
else:
logging.info("Copied %s to %s" % (src, dest))


def find_device_for_gateway(config: 'CsConfig', gateway_ip: str) -> Optional[str]:
"""
Find which ethernet device the gateway IP belongs to by checking
if the gateway is in any of the configured interface subnets.

Args:
config: CsConfig instance containing network configuration
gateway_ip: IP address of the gateway to locate

Returns:
Device name (e.g., 'eth2') or None if not found
"""
try:
interfaces = config.address().get_interfaces()
for interface in interfaces:
if not interface.is_added():
continue
if interface.ip_in_subnet(gateway_ip):
return interface.get_device()
logging.debug("No matching device found for gateway %s" % gateway_ip)
return None
except Exception as e:
logging.error("Error finding device for gateway %s: %s" % (gateway_ip, e))
return None
40 changes: 37 additions & 3 deletions systemvm/debian/opt/cloud/bin/cs/CsStaticRoutes.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import logging
from . import CsHelper
from .CsDatabag import CsDataBag
from .CsRoute import CsRoute


class CsStaticRoutes(CsDataBag):
Expand All @@ -31,13 +32,46 @@ def process(self):
continue
self.__update(self.dbag[item])



def __update(self, route):
network = route['network']
gateway = route['gateway']

if route['revoke']:
command = "ip route del %s via %s" % (route['network'], route['gateway'])
# Delete from main table
command = "ip route del %s via %s" % (network, gateway)
CsHelper.execute(command)

# Delete from PBR table if applicable
device = CsHelper.find_device_for_gateway(self.config, gateway)
if device:
cs_route = CsRoute()
table_name = cs_route.get_tablename(device)
command = "ip route del %s via %s table %s" % (network, gateway, table_name)
CsHelper.execute(command)
logging.info("Deleted static route %s via %s from PBR table %s" % (network, gateway, table_name))
else:
command = "ip route show | grep %s | awk '{print $1, $3}'" % route['network']
# Add to main table (existing logic)
command = "ip route show | grep '^%s' | awk '{print $1, $3}'" % network
result = CsHelper.execute(command)
if not result:
route_command = "ip route add %s via %s" % (route['network'], route['gateway'])
route_command = "ip route add %s via %s" % (network, gateway)
CsHelper.execute(route_command)
logging.info("Added static route %s via %s to main table" % (network, gateway))

# Add to PBR table if applicable
device = CsHelper.find_device_for_gateway(self.config, gateway)
if device:
cs_route = CsRoute()
table_name = cs_route.get_tablename(device)
# Check if route already exists in the PBR table
check_command = "ip route show table %s | grep '^%s' | awk '{print $1, $3}'" % (table_name, network)
result = CsHelper.execute(check_command)
if not result:
# Add route to the interface-specific table
route_command = "ip route add %s via %s dev %s table %s" % (network, gateway, device, table_name)
CsHelper.execute(route_command)
logging.info("Added static route %s via %s to PBR table %s" % (network, gateway, table_name))
else:
logging.info("Static route %s via %s added to main table only (no matching interface found for PBR table)" % (network, gateway))
Loading