Skip to content

Commit

Permalink
Merge pull request #1104 from cutwater/feature/multiple-iface-prefixes
Browse files Browse the repository at this point in the history
Allow using multiple interface prefixes
  • Loading branch information
Neil Jerram committed Sep 22, 2016
2 parents 8f350c5 + 6758ae7 commit d1a6656
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 71 deletions.
5 changes: 3 additions & 2 deletions calico/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,11 @@ def validate_endpoint(config, combined_id, endpoint):
issues.append("Missing 'name' field.")
elif (isinstance(endpoint['name'], StringTypes)
and combined_id.host == config.HOSTNAME
and not endpoint["name"].startswith(config.IFACE_PREFIX)):
and not any(endpoint["name"].startswith(prefix)
for prefix in config.IFACE_PREFIX)):
# Only test the interface for local endpoints - remote hosts may have
# a different interface prefix.
issues.append("Interface %r does not start with %r." %
issues.append("Interface %r does not start with any of %r." %
(endpoint["name"], config.IFACE_PREFIX))
if "state" not in endpoint:
issues.append("Missing 'state' field.")
Expand Down
11 changes: 9 additions & 2 deletions calico/felix/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ class ConfigParameter(object):
"""
def __init__(self, name, description, default,
sources=DEFAULT_SOURCES, value_is_int=False,
value_is_bool=False, value_is_int_list=False):
value_is_bool=False, value_is_int_list=False,
value_is_str_list=False):
"""
Create a configuration parameter.
:param str description: Description for logging
Expand All @@ -105,6 +106,7 @@ def __init__(self, name, description, default,
self.value_is_int = value_is_int
self.value_is_bool = value_is_bool
self.value_is_int_list = value_is_int_list
self.value_is_str_list = value_is_str_list

def set(self, value, source):
"""
Expand Down Expand Up @@ -157,6 +159,9 @@ def set(self, value, source):
else:
raise ConfigException("Invalid list of ints", self)
self.value = ints
elif self.value_is_str_list:
splits = str(value).split(',')
self.value = [s.strip() for s in splits]
else:
# Calling str in principle can throw an exception, but it's
# hard to see how in practice, so don't catch and wrap.
Expand Down Expand Up @@ -226,7 +231,8 @@ def __init__(self, config_path):
"127.0.0.1")
self.add_parameter("MetadataPort", "Metadata Port",
8775, value_is_int=True)
self.add_parameter("InterfacePrefix", "Interface name prefix", "cali")
self.add_parameter("InterfacePrefix", "Interface name prefix",
["cali"], value_is_str_list=True)
self.add_parameter("DefaultEndpointToHostAction",
"Action to take for packets that arrive from"
"an endpoint to the host.", "DROP")
Expand Down Expand Up @@ -469,6 +475,7 @@ def _finish_update(self, final=False):
set_bits = find_set_bits(mark_mask)
self.IPTABLES_MARK_ACCEPT = "0x%x" % next(set_bits)
self.IPTABLES_MARK_NEXT_TIER = "0x%x" % next(set_bits)
self.IPTABLES_MARK_ENDPOINTS = "0x%x" % next(set_bits)

for plugin in self.plugins.itervalues():
# Plugins don't get loaded and registered until we've read config
Expand Down
6 changes: 4 additions & 2 deletions calico/felix/endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,13 @@ def _poll_interfaces(self, known_interfaces):
:return:
"""
# We only care about host interfaces, not workload ones.
exclude_prefix = self.config.IFACE_PREFIX
exclude_prefixes = self.config.IFACE_PREFIX
# Get the IPs for each interface.
ips_by_iface = devices.list_ips_by_iface(self.ip_type)
for iface, ips in ips_by_iface.items():
if iface.startswith(exclude_prefix):
ignore_iface = any(iface.startswith(prefix)
for prefix in exclude_prefixes)
if ignore_iface:
# Ignore non-host interfaces.
ips_by_iface.pop(iface)
else:
Expand Down
28 changes: 15 additions & 13 deletions calico/felix/frules.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,6 @@ def install_global_rules(config, filter_updater, nat_updater, ip_version,
- applies any changes required.
"""

# The interface matching string; for example, if interfaces start "tap"
# then this string is "tap+".
iface_match = config.IFACE_PREFIX + "+"

# If enabled, create the IP-in-IP device, but only for IPv4
if ip_version == 4:
if config.IP_IN_IP_ENABLED:
Expand Down Expand Up @@ -294,11 +290,15 @@ def install_global_rules(config, filter_updater, nat_updater, ip_version,
{CHAIN_PREROUTING: raw_prerouting_deps},
async=False)

raw_updater.ensure_rule_inserted(
"PREROUTING --in-interface %s --match rpfilter --invert "
"--jump %s" %
(iface_match, CHAIN_PREROUTING),
async=False)
for iface_prefix in config.IFACE_PREFIX:
# The interface matching string; for example,
# if interfaces start "tap" then this string is "tap+".
iface_match = iface_prefix + '+'
raw_updater.ensure_rule_inserted(
"PREROUTING --in-interface %s --match rpfilter --invert "
"--jump %s" %
(iface_match, CHAIN_PREROUTING),
async=False)

# Both IPV4 and IPV6 nat tables need felix-PREROUTING and
# felix-POSTROUTING, along with the dependent DNAT and SNAT tables
Expand Down Expand Up @@ -409,7 +409,9 @@ def interface_to_chain_suffix(config, iface_name):
:param iface_name: The interface name
:returns string: the suffix (shortened if necessary)
"""
suffix = iface_name.replace(config.IFACE_PREFIX, "", 1)
# The suffix is surely not very long, but make sure.
suffix = futils.uniquely_shorten(suffix, 16)
return suffix
for prefix in sorted(config.IFACE_PREFIX, reverse=True):
if iface_name.startswith(prefix):
iface_name = iface_name[len(prefix):]
break
iface_name = futils.uniquely_shorten(iface_name, 16)
return iface_name
79 changes: 52 additions & 27 deletions calico/felix/plugins/fiptgenerator.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def __init__(self):
self.METADATA_PORT = None
self.IPTABLES_MARK_ACCEPT = None
self.IPTABLES_MARK_NEXT_TIER = None
self.IPTABLES_MARK_ENDPOINTS = None
self.FAILSAFE_INBOUND_PORTS = None
self.FAILSAFE_OUTBOUND_PORTS = None
self.ACTION_ON_DROP = None
Expand All @@ -92,12 +93,13 @@ def store_and_validate_config(self, config):
super(FelixIptablesGenerator, self).store_and_validate_config(config)

self.IFACE_PREFIX = config.IFACE_PREFIX
self.IFACE_MATCH = self.IFACE_PREFIX + "+"
self.IFACE_MATCH = [prefix + "+" for prefix in self.IFACE_PREFIX]
self.METADATA_IP = config.METADATA_IP
self.METADATA_PORT = config.METADATA_PORT
self.DEFAULT_INPUT_CHAIN_ACTION = config.DEFAULT_INPUT_CHAIN_ACTION
self.IPTABLES_MARK_ACCEPT = config.IPTABLES_MARK_ACCEPT
self.IPTABLES_MARK_NEXT_TIER = config.IPTABLES_MARK_NEXT_TIER
self.IPTABLES_MARK_ENDPOINTS = config.IPTABLES_MARK_ENDPOINTS
self.FAILSAFE_INBOUND_PORTS = config.FAILSAFE_INBOUND_PORTS
self.FAILSAFE_OUTBOUND_PORTS = config.FAILSAFE_OUTBOUND_PORTS
self.ACTION_ON_DROP = config.ACTION_ON_DROP
Expand Down Expand Up @@ -231,11 +233,23 @@ def filter_input_chain(self, ip_version, hosts_set_name=None):
chain.append("--append %s --match conntrack "
"--ctstate RELATED,ESTABLISHED --jump ACCEPT" %
CHAIN_INPUT)

chain.append(
"--append {chain} --jump MARK --set-mark 0/{mark}".format(
chain=CHAIN_INPUT, mark=self.IPTABLES_MARK_ENDPOINTS)
)
for iface_match in self.IFACE_MATCH:
chain.append(
"--append {chain} --in-interface {iface} "
"--jump MARK --set-mark {mark}/{mark}".format(
chain=CHAIN_INPUT, iface=iface_match,
mark=self.IPTABLES_MARK_ENDPOINTS)
)
# Incoming traffic on host endpoints.
chain.append(
"--append %s --goto %s ! --in-interface %s" %
(CHAIN_INPUT, CHAIN_FROM_IFACE, self.IFACE_MATCH)
"--append {chain} --goto {goto} --match mark "
"--mark 0/{mark}".format(
chain=CHAIN_INPUT, goto=CHAIN_FROM_IFACE,
mark=self.IPTABLES_MARK_ENDPOINTS)
)
deps.add(CHAIN_FROM_IFACE)

Expand Down Expand Up @@ -337,11 +351,23 @@ def filter_output_chain(self, ip_version, hosts_set_name=None):
chain.append("--append %s --match conntrack "
"--ctstate RELATED,ESTABLISHED --jump ACCEPT" %
CHAIN_OUTPUT)

chain.append(
"--append {chain} --jump MARK --set-mark 0/{mark}".format(
chain=CHAIN_OUTPUT, mark=self.IPTABLES_MARK_ENDPOINTS)
)
# Outgoing traffic on host endpoints.
for iface_match in self.IFACE_MATCH:
chain.append(
"--append {chain} --out-interface {iface} "
"--jump MARK --set-mark {mark}/{mark}".format(
chain=CHAIN_OUTPUT, iface=iface_match,
mark=self.IPTABLES_MARK_ENDPOINTS)
)
chain.append(
"--append %s --goto %s ! --out-interface %s" %
(CHAIN_OUTPUT, CHAIN_TO_IFACE, self.IFACE_MATCH)
"--append {chain} --goto {goto} --match mark "
"--mark 0/{mark}".format(
chain=CHAIN_OUTPUT, goto=CHAIN_TO_IFACE,
mark=self.IPTABLES_MARK_ENDPOINTS)
)
deps.add(CHAIN_TO_IFACE)

Expand All @@ -361,50 +387,49 @@ def filter_forward_chain(self, ip_version):
:param ip_version. Whether this is the IPv4 or IPv6 FILTER table.
:returns Tuple: list of rules, set of deps.
"""
forward_chain = (
self.drop_rules(ip_version,
CHAIN_FORWARD,
"--in-interface %s --match conntrack --ctstate "
"INVALID" % self.IFACE_MATCH,
None) +
self.drop_rules(ip_version,
CHAIN_FORWARD,
"--out-interface %s --match conntrack --ctstate "
"INVALID" % self.IFACE_MATCH,
None) +
[
forward_chain = []
for iface_match in self.IFACE_MATCH:
forward_chain.extend(self.drop_rules(
ip_version, CHAIN_FORWARD,
"--in-interface %s --match conntrack --ctstate "
"INVALID" % iface_match, None))
forward_chain.extend(
self.drop_rules(
ip_version, CHAIN_FORWARD,
"--out-interface %s --match conntrack --ctstate "
"INVALID" % iface_match, None))
forward_chain.extend([
# First, a pair of conntrack rules, which accept established
# flows to/from workload interfaces.
"--append %s --in-interface %s --match conntrack "
"--ctstate RELATED,ESTABLISHED --jump ACCEPT" %
(CHAIN_FORWARD, self.IFACE_MATCH),
(CHAIN_FORWARD, iface_match),
"--append %s --out-interface %s --match conntrack "
"--ctstate RELATED,ESTABLISHED --jump ACCEPT" %
(CHAIN_FORWARD, self.IFACE_MATCH),
(CHAIN_FORWARD, iface_match),

# Then, for traffic from a workload interface, jump to the
# from endpoint chain. It will either DROP the packet or,
# if policy allows, return it to this chain for further
# processing.
"--append %s --jump %s --in-interface %s" %
(CHAIN_FORWARD, CHAIN_FROM_ENDPOINT, self.IFACE_MATCH),
(CHAIN_FORWARD, CHAIN_FROM_ENDPOINT, iface_match),

# Then, for traffic to a workload interface, jump to the
# "to endpoint" chain. Note: a packet that's going from one
# endpoint to another on the same host will go through both
# the "from" and "to" chains.
"--append %s --jump %s --out-interface %s" %
(CHAIN_FORWARD, CHAIN_TO_ENDPOINT, self.IFACE_MATCH),
(CHAIN_FORWARD, CHAIN_TO_ENDPOINT, iface_match),

# Finally, if the packet is from/to a workload and it passes
# both the "from" and "to" chains without being dropped, it
# must be allowed by policy; ACCEPT it.
"--append %s --jump ACCEPT --in-interface %s" %
(CHAIN_FORWARD, self.IFACE_MATCH),
(CHAIN_FORWARD, iface_match),
"--append %s --jump ACCEPT --out-interface %s" %
(CHAIN_FORWARD, self.IFACE_MATCH),
]
)
(CHAIN_FORWARD, iface_match),
])
return forward_chain, set([CHAIN_FROM_ENDPOINT, CHAIN_TO_ENDPOINT])

def endpoint_chain_names(self, endpoint_suffix):
Expand Down
33 changes: 24 additions & 9 deletions calico/felix/test/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def test_default_config(self):
self.assertEqual(config.ETCD_CA_FILE,
"/etc/ssl/certs/ca-certificates.crt")
self.assertEqual(config.HOSTNAME, socket.gethostname())
self.assertEqual(config.IFACE_PREFIX, "blah")
self.assertEqual(config.IFACE_PREFIX, ["blah"])
self.assertEqual(config.METADATA_PORT, 123)
self.assertEqual(config.METADATA_IP, "1.2.3.4")
self.assertEqual(config.REPORTING_INTERVAL_SECS, 30)
Expand Down Expand Up @@ -316,7 +316,7 @@ def test_blank_metadata_addr(self):

def test_no_iface_prefix(self):
config = load_config("felix_missing.cfg", host_dict={})
self.assertEqual(config.IFACE_PREFIX, "cali")
self.assertEqual(config.IFACE_PREFIX, ["cali"])

def test_file_sections(self):
"""
Expand All @@ -333,7 +333,7 @@ def test_file_sections(self):
self.assertEqual(config.ETCD_ADDRS, ["localhost:4001"])
self.assertEqual(config.HOSTNAME, socket.gethostname())
self.assertEqual(config.LOGFILE, "/log/nowhere.log")
self.assertEqual(config.IFACE_PREFIX, "whatever")
self.assertEqual(config.IFACE_PREFIX, ["whatever"])
self.assertEqual(config.METADATA_PORT, 246)
self.assertEqual(config.METADATA_IP, "1.2.3.4")
self.assertEqual(config.REPORTING_INTERVAL_SECS, 5)
Expand Down Expand Up @@ -374,7 +374,7 @@ def test_env_var_override(self):
self.assertEqual(config.ETCD_ADDRS, ["9.9.9.9:1234"])
self.assertEqual(config.HOSTNAME, socket.gethostname())
self.assertEqual(config.LOGFILE, "/log/nowhere.log")
self.assertEqual(config.IFACE_PREFIX, "whatever")
self.assertEqual(config.IFACE_PREFIX, ["whatever"])
self.assertEqual(config.METADATA_PORT, 999)
self.assertEqual(config.METADATA_IP, "1.2.3.4")
self.assertEqual(config.STARTUP_CLEANUP_DELAY, 42)
Expand Down Expand Up @@ -458,6 +458,7 @@ def test_insufficient_mark_bits(self):
self.assertEqual(config.IPTABLES_MARK_MASK, 0xff000000)
self.assertEqual(config.IPTABLES_MARK_ACCEPT, "0x1000000")
self.assertEqual(config.IPTABLES_MARK_NEXT_TIER, "0x2000000")
self.assertEqual(config.IPTABLES_MARK_ENDPOINTS, "0x4000000")

def test_exact_mark_bits(self):
"""
Expand All @@ -466,13 +467,14 @@ def test_exact_mark_bits(self):
"""
# This test is intended to catch if _validate_cfg() isn't updated when
# new mark bits are added.
cfg_dict = { "InterfacePrefix": "blah",
"IptablesMarkMask": "12" }
cfg_dict = {"InterfacePrefix": "blah",
"IptablesMarkMask": "28"}
config = load_config("felix_missing.cfg", host_dict=cfg_dict)

self.assertEqual(config.IPTABLES_MARK_MASK, 0x0000000c)
self.assertEqual(config.IPTABLES_MARK_MASK, 0x0000001c)
self.assertEqual(config.IPTABLES_MARK_ACCEPT, "0x4")
self.assertEqual(config.IPTABLES_MARK_NEXT_TIER, "0x8")
self.assertEqual(config.IPTABLES_MARK_ENDPOINTS, "0x10")

def test_too_many_mark_bits(self):
"""
Expand All @@ -485,18 +487,20 @@ def test_too_many_mark_bits(self):
self.assertEqual(config.IPTABLES_MARK_MASK, 0xff000000)
self.assertEqual(config.IPTABLES_MARK_ACCEPT, "0x1000000")
self.assertEqual(config.IPTABLES_MARK_NEXT_TIER, "0x2000000")
self.assertEqual(config.IPTABLES_MARK_ENDPOINTS, "0x4000000")

def test_hex_mark(self):
"""
Test that the IptablesMarkMask accepts hexadecimal values.
"""
cfg_dict = { "InterfacePrefix": "blah",
"IptablesMarkMask": "0x60" }
"IptablesMarkMask": "0xe0" }
config = load_config("felix_missing.cfg", host_dict=cfg_dict)

self.assertEqual(config.IPTABLES_MARK_MASK, 0x00000060)
self.assertEqual(config.IPTABLES_MARK_MASK, 0x000000e0)
self.assertEqual(config.IPTABLES_MARK_ACCEPT, "0x20")
self.assertEqual(config.IPTABLES_MARK_NEXT_TIER, "0x40")
self.assertEqual(config.IPTABLES_MARK_ENDPOINTS, "0x80")

def test_default_ttl(self):
"""
Expand Down Expand Up @@ -666,3 +670,14 @@ def test_drop_valid(self):
}
config = load_config("felix_missing.cfg", host_dict=cfg_dict)
self.assertEqual(config.ACTION_ON_DROP, value)

def test_interface_prefix(self):
cfg_dict = {"InterfacePrefix": "foo"}
config = load_config("felix_interface_prefix.cfg",
host_dict=cfg_dict)
self.assertEqual(config.IFACE_PREFIX, ['foo'])

cfg_dict = {"InterfacePrefix": "foo,bar"}
config = load_config("felix_interface_prefix.cfg",
host_dict=cfg_dict)
self.assertEqual(config.IFACE_PREFIX, ['foo', 'bar'])
4 changes: 2 additions & 2 deletions calico/felix/test/test_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ def test_resolve_host_eps_multiple_conflicting_matches(self):

def test_poll_interfaces(self):
known_interfaces = {}
self.mgr.config.IFACE_PREFIX = "tap"
self.mgr.config.IFACE_PREFIX = ["tap"]

with mock.patch("calico.felix.devices.list_ips_by_iface",
autospec=True) as m_list_ips, \
Expand Down Expand Up @@ -1361,7 +1361,7 @@ class TestHostEndpoint(BaseTestCase):
def setUp(self):
super(TestHostEndpoint, self).setUp()
self.config = mock.Mock()
self.config.IFACE_PREFIX = "tap"
self.config.IFACE_PREFIX = ["tap"]
self.m_ipt_gen = Mock(spec=FelixIptablesGenerator)
self.config.plugins = {"iptables_generator": self.m_ipt_gen}
self.updates = ({"chain": ["rule"]}, {"chain": set(["deps"])})
Expand Down
Loading

0 comments on commit d1a6656

Please sign in to comment.