From 57aab5a065730f1e9bccd944c9519047502fac73 Mon Sep 17 00:00:00 2001 From: Feilong Wang Date: Mon, 30 Mar 2020 14:12:06 +1300 Subject: [PATCH] [k8s] Add label 'master_lb_allowed_cidrs' A new label named `master_lb_allowed_cidrs` is added to control the IP range which can access the k8s api and etcd load balancers. It's a good security enhancement. Task: 39188 Story: 2007414 Change-Id: I157a3b01d169e550e79b94316803fde8ddf77b03 (cherry picked from commit 3b87c5cc6f7aaca40089b74d1ea1c4d6eaa91bb5) --- doc/source/user/index.rst | 8 +++ magnum/drivers/common/templates/lb_api.yaml | 28 ++++++++- magnum/drivers/common/templates/lb_etcd.yaml | 28 ++++++++- magnum/drivers/heat/k8s_template_def.py | 15 ++++- .../templates/kubecluster.yaml | 7 +++ .../handlers/test_k8s_cluster_conductor.py | 6 ++ .../unit/drivers/test_template_definition.py | 60 ++++++++++++++++++- ...ter-lb-allowed-cidrs-cc599da4eb96e983.yaml | 7 +++ 8 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/master-lb-allowed-cidrs-cc599da4eb96e983.yaml diff --git a/doc/source/user/index.rst b/doc/source/user/index.rst index 2c19342c89..34c85ea9f6 100644 --- a/doc/source/user/index.rst +++ b/doc/source/user/index.rst @@ -436,6 +436,8 @@ the table are linked to more details elsewhere in the user guide. | `master_lb_floating_ip_enabled`_ | - true | see below | | | - false | | +---------------------------------------+--------------------+---------------+ +| `master_lb_allowed_cidrs`_ | see below | "" | ++---------------------------------------+--------------------+---------------+ | `auto_healing_enabled`_ | - true | false | | | - false | | +---------------------------------------+--------------------+---------------+ @@ -1515,6 +1517,12 @@ _`master_lb_floating_ip_enabled` ``master_lb_enabled`` is set. If not specified, the default value is the same as template property ``floating_ip_enabled``. +_`master_lb_allowed_cidrs` + A CIDR list which can be used to control the access for the load balancer of + master nodes. The input format is comma delimited list. For example, + 192.168.0.0/16,10.0.0.0/24. Default value is "" which means opening to + 0.0.0.0/0. + _`auto_healing_enabled` If set to true, auto healing feature will be enabled. Defaults to false. diff --git a/magnum/drivers/common/templates/lb_api.yaml b/magnum/drivers/common/templates/lb_api.yaml index ef4c65ea38..2a09eb6a6f 100644 --- a/magnum/drivers/common/templates/lb_api.yaml +++ b/magnum/drivers/common/templates/lb_api.yaml @@ -1,4 +1,14 @@ -heat_template_version: 2014-10-16 +heat_template_version: queens + +conditions: + allowed_cidrs_enabled: + not: + allowed_cidrs_disabled + + allowed_cidrs_disabled: + equals: + - get_param: allowed_cidrs + - [] parameters: @@ -17,6 +27,10 @@ parameters: port: type: number + allowed_cidrs: + type: comma_delimited_list + description: The allowed CIDR list for master load balancer + resources: loadbalancer: type: Magnum::Optional::Neutron::LBaaS::LoadBalancer @@ -24,17 +38,27 @@ resources: vip_subnet: {get_param: fixed_subnet} listener: + condition: allowed_cidrs_disabled type: Magnum::Optional::Neutron::LBaaS::Listener properties: loadbalancer: {get_resource: loadbalancer} protocol: {get_param: protocol} protocol_port: {get_param: port} + listener_with_allowed_cidrs: + condition: allowed_cidrs_enabled + type: Magnum::Optional::Neutron::LBaaS::Listener + properties: + loadbalancer: {get_resource: loadbalancer} + protocol: {get_param: protocol} + protocol_port: {get_param: port} + allowed_cidrs: {get_param: allowed_cidrs} + pool: type: Magnum::Optional::Neutron::LBaaS::Pool properties: lb_algorithm: ROUND_ROBIN - listener: {get_resource: listener} + listener: {if: ["allowed_cidrs_enabled", {get_resource: listener_with_allowed_cidrs}, {get_resource: listener}]} protocol: {get_param: protocol} monitor: diff --git a/magnum/drivers/common/templates/lb_etcd.yaml b/magnum/drivers/common/templates/lb_etcd.yaml index db6314568b..39b8733de5 100644 --- a/magnum/drivers/common/templates/lb_etcd.yaml +++ b/magnum/drivers/common/templates/lb_etcd.yaml @@ -1,5 +1,15 @@ # etcd service load balancer doesn't have floating IP associated. -heat_template_version: 2014-10-16 +heat_template_version: queens + +conditions: + allowed_cidrs_enabled: + not: + allowed_cidrs_disabled + + allowed_cidrs_disabled: + equals: + - get_param: allowed_cidrs + - [] parameters: @@ -15,6 +25,10 @@ parameters: port: type: number + allowed_cidrs: + type: comma_delimited_list + description: The allowed CIDR list for master load balancer + resources: loadbalancer: type: Magnum::Optional::Neutron::LBaaS::LoadBalancer @@ -22,17 +36,27 @@ resources: vip_subnet: {get_param: fixed_subnet} listener: + condition: allowed_cidrs_disabled type: Magnum::Optional::Neutron::LBaaS::Listener properties: loadbalancer: {get_resource: loadbalancer} protocol: {get_param: protocol} protocol_port: {get_param: port} + listener_with_allowed_cidrs: + condition: allowed_cidrs_enabled + type: Magnum::Optional::Neutron::LBaaS::Listener + properties: + loadbalancer: {get_resource: loadbalancer} + protocol: {get_param: protocol} + protocol_port: {get_param: port} + allowed_cidrs: {get_param: allowed_cidrs} + pool: type: Magnum::Optional::Neutron::LBaaS::Pool properties: lb_algorithm: ROUND_ROBIN - listener: {get_resource: listener} + listener: {if: ["allowed_cidrs_enabled", {get_resource: listener_with_allowed_cidrs}, {get_resource: listener}]} protocol: {get_param: protocol} monitor: diff --git a/magnum/drivers/heat/k8s_template_def.py b/magnum/drivers/heat/k8s_template_def.py index 11d1454431..a1bbcb9912 100644 --- a/magnum/drivers/heat/k8s_template_def.py +++ b/magnum/drivers/heat/k8s_template_def.py @@ -230,7 +230,8 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): 'kubeproxy_options', 'kubecontroller_options', 'kubescheduler_options', - 'influx_grafana_dashboard_enabled'] + 'influx_grafana_dashboard_enabled', + 'master_lb_allowed_cidrs'] labels = self._get_relevant_labels(cluster, kwargs) @@ -262,11 +263,23 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): extra_params['master_kube_tag'] = kube_tag extra_params['minion_kube_tag'] = kube_tag + self._set_master_lb_allowed_cidrs(context, cluster, extra_params) + return super(K8sTemplateDefinition, self).get_params(context, cluster_template, cluster, extra_params=extra_params, **kwargs) + def _set_master_lb_allowed_cidrs(self, context, cluster, extra_params): + if extra_params.get("master_lb_allowed_cidrs"): + subnet_cidr = (cluster.labels.get("fixed_network_cidr") or + "10.0.0.0/24") + if extra_params.get("fixed_subnet"): + subnet_cidr = neutron.get_subnet(context, + extra_params["fixed_subnet"], + "id", "cidr") + extra_params["master_lb_allowed_cidrs"] += "," + subnet_cidr + def get_scale_params(self, context, cluster, scale_manager=None, nodes_to_remove=None): scale_params = dict() diff --git a/magnum/drivers/k8s_fedora_coreos_v1/templates/kubecluster.yaml b/magnum/drivers/k8s_fedora_coreos_v1/templates/kubecluster.yaml index eabdefad6f..100f1dc3ed 100644 --- a/magnum/drivers/k8s_fedora_coreos_v1/templates/kubecluster.yaml +++ b/magnum/drivers/k8s_fedora_coreos_v1/templates/kubecluster.yaml @@ -938,6 +938,11 @@ parameters: specific configs default: "" + master_lb_allowed_cidrs: + type: comma_delimited_list + description: The allowed CIDR list for master load balancer + default: [] + resources: ###################################################################### @@ -967,6 +972,7 @@ resources: external_network: {get_param: external_network} protocol: {get_param: loadbalancing_protocol} port: {get_param: kubernetes_port} + allowed_cidrs: {get_param: master_lb_allowed_cidrs} etcd_lb: condition: create_cluster_resources @@ -975,6 +981,7 @@ resources: fixed_subnet: {get_attr: [network, fixed_subnet]} protocol: {get_param: loadbalancing_protocol} port: 2379 + allowed_cidrs: {get_param: master_lb_allowed_cidrs} ###################################################################### # diff --git a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py index cef7bbff75..329c4c9e56 100644 --- a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py @@ -373,6 +373,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'master_nodegroup_name': 'master_ng', 'worker_nodegroup_name': 'worker_ng', 'post_install_manifest_url': '', + 'master_lb_allowed_cidrs': None, } if missing_attr is not None: @@ -525,6 +526,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'master_nodegroup_name': 'master_ng', 'worker_nodegroup_name': 'worker_ng', 'post_install_manifest_url': '', + 'master_lb_allowed_cidrs': None, } self.assertEqual(expected, definition) @@ -658,6 +660,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'master_nodegroup_name': 'master_ng', 'worker_nodegroup_name': 'worker_ng', 'post_install_manifest_url': '', + 'master_lb_allowed_cidrs': None, } self.assertEqual(expected, definition) self.assertEqual( @@ -770,6 +773,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'worker_role': 'worker', 'master_nodegroup_name': 'master_ng', 'worker_nodegroup_name': 'worker_ng', + 'master_lb_allowed_cidrs': None, } self.assertEqual(expected, definition) self.assertEqual( @@ -880,6 +884,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'worker_role': 'worker', 'master_nodegroup_name': 'master_ng', 'worker_nodegroup_name': 'worker_ng', + 'master_lb_allowed_cidrs': None, } self.assertEqual(expected, definition) self.assertEqual( @@ -1114,6 +1119,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'master_nodegroup_name': 'master_ng', 'worker_nodegroup_name': 'worker_ng', 'post_install_manifest_url': '', + 'master_lb_allowed_cidrs': None, } self.assertEqual(expected, definition) self.assertEqual( diff --git a/magnum/tests/unit/drivers/test_template_definition.py b/magnum/tests/unit/drivers/test_template_definition.py index 022415fe4c..8b98955c23 100644 --- a/magnum/tests/unit/drivers/test_template_definition.py +++ b/magnum/tests/unit/drivers/test_template_definition.py @@ -432,6 +432,8 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): expected_scale_params = {'minions_to_remove': ['node1', 'node2']} self.assertEqual(scale_params, expected_scale_params) + @mock.patch('magnum.drivers.heat.k8s_template_def.K8sTemplateDefinition' + '._set_master_lb_allowed_cidrs') @mock.patch('magnum.common.neutron.get_fixed_network_name') @mock.patch('magnum.common.keystone.is_octavia_enabled') @mock.patch('magnum.common.clients.OpenStackClients') @@ -449,7 +451,8 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_get_output, mock_get_params, mock_get_discovery_url, mock_osc_class, mock_enable_octavia, - mock_get_fixed_network_name): + mock_get_fixed_network_name, + mock_set_master_lb_allowed_cidrs): mock_generate_csr_and_key.return_value = {'csr': 'csr', 'private_key': 'private_key', 'public_key': 'public_key'} @@ -640,6 +643,8 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): 'containerd_tarball_sha256') kube_image_digest = mock_cluster.labels.get('kube_image_digest') metrics_scraper_tag = mock_cluster.labels.get('metrics_scraper_tag') + master_lb_allowed_cidrs = mock_cluster.labels.get( + 'master_lb_allowed_cidrs') k8s_def = k8sa_tdef.AtomicK8sTemplateDefinition() @@ -754,6 +759,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): 'containerd_tarball_sha256': containerd_tarball_sha256, 'post_install_manifest_url': '', 'metrics_scraper_tag': metrics_scraper_tag, + 'master_lb_allowed_cidrs': master_lb_allowed_cidrs, }} mock_get_params.assert_called_once_with(mock_context, mock_cluster_template, @@ -950,6 +956,8 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): actual_params.get("ingress_controller") ) + @mock.patch('magnum.drivers.heat.k8s_template_def.K8sTemplateDefinition' + '._set_master_lb_allowed_cidrs') @mock.patch('magnum.common.keystone.is_octavia_enabled') @mock.patch('magnum.common.clients.OpenStackClients') @mock.patch('magnum.drivers.heat.template_def' @@ -965,7 +973,8 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_sign_node_certificate, mock_get_output, mock_get_params, mock_get_discovery_url, mock_osc_class, - mock_enable_octavia): + mock_enable_octavia, + mock_set_master_lb_allowed_cidrs): mock_generate_csr_and_key.return_value = {'csr': 'csr', 'private_key': 'private_key', 'public_key': 'public_key'} @@ -1155,6 +1164,9 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): kube_image_digest = mock_cluster.labels.get('kube_image_digest') metrics_scraper_tag = mock_cluster.labels.get('metrics_scraper_tag') + master_lb_allowed_cidrs = mock_cluster.labels.get( + 'master_lb_allowed_cidrs') + k8s_def = k8sa_tdef.AtomicK8sTemplateDefinition() k8s_def.get_params(mock_context, mock_cluster_template, mock_cluster) @@ -1270,6 +1282,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): 'containerd_tarball_sha256': containerd_tarball_sha256, 'post_install_manifest_url': '', 'metrics_scraper_tag': metrics_scraper_tag, + 'master_lb_allowed_cidrs': master_lb_allowed_cidrs, }} mock_get_params.assert_called_once_with(mock_context, mock_cluster_template, @@ -1533,6 +1546,49 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): is_master=False ) + def test_set_master_lb_allowed_cidrs(self): + definition = self.get_definition() + extra_params = {"master_lb_allowed_cidrs": "192.168.0.0/16"} + mock_cluster = mock.MagicMock() + mock_context = mock.MagicMock() + mock_cluster.labels = {} + + definition._set_master_lb_allowed_cidrs(mock_context, + mock_cluster, extra_params) + + self.assertEqual(extra_params["master_lb_allowed_cidrs"], + "192.168.0.0/16,10.0.0.0/24") + + def test_set_master_lb_allowed_cidrs_fixed_network_cidr(self): + definition = self.get_definition() + extra_params = {"master_lb_allowed_cidrs": "192.168.0.0/16"} + mock_cluster = mock.MagicMock() + mock_context = mock.MagicMock() + mock_cluster.labels = {"fixed_network_cidr": "100.0.0.0/24"} + + definition._set_master_lb_allowed_cidrs(mock_context, + mock_cluster, extra_params) + + self.assertEqual(extra_params["master_lb_allowed_cidrs"], + "192.168.0.0/16,100.0.0.0/24") + + @mock.patch('magnum.common.neutron.get_subnet') + def test_set_master_lb_allowed_cidrs_find_subnet_cidr(self, + mock_get_subnet): + definition = self.get_definition() + extra_params = {"master_lb_allowed_cidrs": "192.168.0.0/16", + "fixed_subnet": "fake_subnet_id"} + mock_cluster = mock.MagicMock() + mock_context = mock.MagicMock() + mock_cluster.labels = {} + mock_get_subnet.return_value = "172.24.0.0/16" + + definition._set_master_lb_allowed_cidrs(mock_context, + mock_cluster, extra_params) + + self.assertEqual(extra_params["master_lb_allowed_cidrs"], + "192.168.0.0/16,172.24.0.0/16") + class FedoraK8sIronicTemplateDefinitionTestCase(base.TestCase): diff --git a/releasenotes/notes/master-lb-allowed-cidrs-cc599da4eb96e983.yaml b/releasenotes/notes/master-lb-allowed-cidrs-cc599da4eb96e983.yaml new file mode 100644 index 0000000000..be776e5548 --- /dev/null +++ b/releasenotes/notes/master-lb-allowed-cidrs-cc599da4eb96e983.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Add a new label named `master_lb_allowed_cidrs` to control the IP ranges + which can access the k8s API and etcd load balancers of master. To get + this feature, the minimum version of Heat is stable/ussuri and minimum + version of Octavia is stable/train.