From 5afa4925fcfd74046382014385025d889e4a99c0 Mon Sep 17 00:00:00 2001 From: Roman Dobosz Date: Thu, 24 Mar 2022 09:02:44 +0100 Subject: [PATCH] Fix potential issue with network/subnet name length. In OpenStack Neutron and Octavia, name and descriptions of the objects are limited to the 255 characters, while on Kubernetes names are limited to the 253 characters. Kuryr often creates names for the networks and subnets using Kubernetes object name with additional prefix and suffix, which may exceed allowed character limit. In this patch there is proposed solution for this issue by simply truncate kubernetes resource name, while keeping prefix and suffix intact, to fit the Neutron name limit. Change-Id: I6e404104034f11593fc313797ad464458bbdf82d --- .../controller/drivers/lbaasv2.py | 8 +++- .../controller/drivers/namespace_subnet.py | 6 ++- .../controller/drivers/network_policy.py | 6 ++- kuryr_kubernetes/controller/drivers/utils.py | 23 +++++++++- .../unit/controller/drivers/test_utils.py | 45 +++++++++++++++++++ 5 files changed, 81 insertions(+), 7 deletions(-) diff --git a/kuryr_kubernetes/controller/drivers/lbaasv2.py b/kuryr_kubernetes/controller/drivers/lbaasv2.py index fdb909e80..928894f78 100644 --- a/kuryr_kubernetes/controller/drivers/lbaasv2.py +++ b/kuryr_kubernetes/controller/drivers/lbaasv2.py @@ -26,6 +26,7 @@ from kuryr_kubernetes import clients from kuryr_kubernetes import config from kuryr_kubernetes import constants as k_const from kuryr_kubernetes.controller.drivers import base +from kuryr_kubernetes.controller.drivers import utils as c_utils from kuryr_kubernetes import exceptions as k_exc from kuryr_kubernetes import utils @@ -868,7 +869,8 @@ class LBaaSv2Driver(base.LBaaSDriver): svc_name = service['metadata']['name'] svc_ports = service['spec'].get('ports', []) - lbaas_name = "%s/%s" % (svc_namespace, svc_name) + lbaas_name = c_utils.get_resource_name(svc_name, + prefix=svc_namespace + "/") endpoints_link = utils.get_endpoints_link(service) k8s = clients.get_kubernetes_client() @@ -913,7 +915,9 @@ class LBaaSv2Driver(base.LBaaSDriver): port_protocol = port['protocol'] lbaas_port = port['port'] target_port = port['targetPort'] - sg_rule_name = "%s:%s:%s" % (lbaas_name, port_protocol, lbaas_port) + suffix = f"{port_protocol}:{lbaas_port}" + sg_rule_name = c_utils.get_resource_name(lbaas_name, + suffix=':' + suffix) listener_id = lsnr_ids.get((port_protocol, lbaas_port)) if listener_id is None: LOG.warning("There is no listener associated to the protocol " diff --git a/kuryr_kubernetes/controller/drivers/namespace_subnet.py b/kuryr_kubernetes/controller/drivers/namespace_subnet.py index 515de1ac6..0c432d4d0 100644 --- a/kuryr_kubernetes/controller/drivers/namespace_subnet.py +++ b/kuryr_kubernetes/controller/drivers/namespace_subnet.py @@ -119,7 +119,8 @@ class NamespacePodSubnetDriver(default_subnet.DefaultPodSubnetDriver): def create_network(self, ns_name, project_id): os_net = clients.get_network_client() - net_name = 'ns/' + ns_name + '-net' + net_name = c_utils.get_resource_name(ns_name, prefix='ns/', + suffix='-net') tags = oslo_cfg.CONF.neutron_defaults.resource_tags if tags: networks = os_net.networks(name=net_name, tags=tags) @@ -148,7 +149,8 @@ class NamespacePodSubnetDriver(default_subnet.DefaultPodSubnetDriver): def create_subnet(self, ns_name, project_id, net_id): os_net = clients.get_network_client() - subnet_name = "ns/" + ns_name + "-subnet" + subnet_name = c_utils.get_resource_name(ns_name, prefix='ns/', + suffix='-subnet') tags = oslo_cfg.CONF.neutron_defaults.resource_tags if tags: subnets = os_net.subnets(name=subnet_name, tags=tags) diff --git a/kuryr_kubernetes/controller/drivers/network_policy.py b/kuryr_kubernetes/controller/drivers/network_policy.py index 8c2a53229..3195d65b9 100644 --- a/kuryr_kubernetes/controller/drivers/network_policy.py +++ b/kuryr_kubernetes/controller/drivers/network_policy.py @@ -54,8 +54,10 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): return self.namespaced_pods(policy) def create_security_group(self, knp, project_id): - sg_name = ("sg-" + knp['metadata']['namespace'] + "-" + - knp['metadata']['name']) + sg_name = driver_utils.get_resource_name(knp['metadata']['namespace'] + + '-' + + knp['metadata']['name'], + prefix='sg/') desc = ("Kuryr-Kubernetes Network Policy %s SG" % utils.get_res_unique_name(knp)) try: diff --git a/kuryr_kubernetes/controller/drivers/utils.py b/kuryr_kubernetes/controller/drivers/utils.py index 48b6ffeec..710cd3f71 100644 --- a/kuryr_kubernetes/controller/drivers/utils.py +++ b/kuryr_kubernetes/controller/drivers/utils.py @@ -50,7 +50,8 @@ def get_network_id(subnets): def get_port_name(pod): - return "%(namespace)s/%(name)s" % pod['metadata'] + return get_resource_name(pod['metadata']['name'], + prefix=pod['metadata']['namespace'] + "/") def get_device_id(pod): @@ -749,3 +750,23 @@ def delete_port(leftover_port): "continue with the other " "rest.", leftover_port.id) return False + + +def get_resource_name(name, prefix='', suffix=''): + """Get OpenStack resource name out of Kubernetes resources + + Return name for the OpenStack resource, which usually is up to 255 chars + long. And while Kubernetes allows to set resource names up to 253 + characters, that makes a risk to have too long name. This function will + prefix and suffix over name of the k8s resource, which will get truncated + if needed. + + https://kubernetes.io/docs/concepts/overview/working-with-objects/names/ + """ + + length = len(f'{prefix}{name}{suffix}') + + if length > 255: + name = name[:254-(length-254)] + + return f'{prefix}{name}{suffix}' diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_utils.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_utils.py index 3e2781466..1273ef7d8 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_utils.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_utils.py @@ -115,3 +115,48 @@ class TestUtils(test_base.TestCase): group='kubernetes') self.assertTrue(utils.is_network_policy_enabled()) + + def test_get_resource_name_with_too_long_name(self): + name = 253 * "a" + prefix = 'ns/' + suffix = '-net' + + new_name = utils.get_resource_name(name, prefix, suffix) + + self.assertEqual(new_name, + prefix + 248 * 'a' + suffix) + self.assertEqual(len(new_name), 255) + + def test_get_resource_name_with_sane_name(self): + name = 'myns' + prefix = 'ns/' + suffix = '-foo' + + new_name = utils.get_resource_name(name, prefix, suffix) + + self.assertEqual(new_name, f'{prefix}{name}{suffix}') + + def test_get_resource_name_with_prefix(self): + name = 'fun_name' + prefix = 'something/' + + new_name = utils.get_resource_name(name, prefix) + + self.assertEqual(new_name, f'{prefix}{name}') + + def test_get_resource_name_with_sufix(self): + name = 'another' + suffix = '/something-else' + + new_name = utils.get_resource_name(name, suffix=suffix) + + self.assertEqual(new_name, f'{name}{suffix}') + + def test_get_resource_name_non_ascii(self): + name = 'Ру́сский вое́нный кора́бль, иди́ на хуй!' + prefix = 'bar:' + suffix = ':baz' + + new_name = utils.get_resource_name(name, prefix, suffix) + + self.assertEqual(new_name, f'{prefix}{name}{suffix}')