From 40701500b5256f6e30ff31a0a8771eda99dda507 Mon Sep 17 00:00:00 2001 From: Vladimir Grevtsev Date: Tue, 9 Oct 2018 17:54:19 +0300 Subject: [PATCH] Enable support for security group logging Add support to enabling logging of security groups for OpenStack Queens or later; this feature is enabled via the neutron-api charm, with local charm configuration options to allow control of rate and burst limits and to set a local log output directory if require (allowing log data to be written to a separate partition for example). The feature is only compatible with the openvswitch firewall driver and will not be enabled if this configuration option is not set. Basic deployment tests changes is included here since nova-cloud-controller unit and relation was missing before, and it leads to CI constantly failing. Corresponding charm-helpers change: https://github.com/juju/charm-helpers/pull/228 Change-Id: Id6ed09f714981e87838186d51a4f5e693bedb1d3 Closes-Bug: #1787397 Depends-On: https://review.openstack.org/602355 --- .gitignore | 1 + config.yaml | 26 ++++++++- .../charmhelpers/contrib/openstack/context.py | 11 ++-- hooks/neutron_ovs_context.py | 53 +++++++++++++++++-- templates/queens/openvswitch_agent.ini | 46 ++++++++++++++++ tests/basic_deployment.py | 10 ++++ unit_tests/test_neutron_ovs_context.py | 38 +++++++++++-- 7 files changed, 173 insertions(+), 12 deletions(-) create mode 100644 templates/queens/openvswitch_agent.ini diff --git a/.gitignore b/.gitignore index 166f12a2..b5db5c04 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ func-results.json .stestr __pycache__ joined-string +.idea diff --git a/config.yaml b/config.yaml index 4bfedc99..724a9ddb 100644 --- a/config.yaml +++ b/config.yaml @@ -250,7 +250,31 @@ options: configured will be used. ipfix-target: type: string - default: + default: description: | IPFIX target wit the format "IP_Address:Port". This will enable IPFIX exporting on all OVS bridges to the target, including br-int and br-ext. + security-group-log-output-base: + type: string + default: + description: | + This option allows setting a path for Network Security Group logs. + A valid file system path must be provided. If this option is not + provided Neutron will use syslog as a destination. + (Available from Queens) + security-group-log-rate-limit: + type: int + default: + description: | + Log entries are queued for writing to a log file when a packet rate + exceeds the limit set by this option. + Possible values: null (no rate limitation), integer values greater than 100. + WARNING: Should be NOT LESS than 100, if set + (or, if null, logging will log unlimited.) + security-group-log-burst-limit: + type: int + default: 25 + description: | + This option sets the maximum queue size for log entries. + Can be used to avoid excessive memory consumption. + WARNING: Should be NOT LESS than 25. \ No newline at end of file diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index 3e4e82a7..09353e41 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -1534,10 +1534,15 @@ class NeutronAPIContext(OSContextGenerator): if 'l2-population' in rdata: ctxt.update(self.get_neutron_options(rdata)) + extension_drivers = [] + if ctxt['enable_qos']: - ctxt['extension_drivers'] = 'qos' - else: - ctxt['extension_drivers'] = '' + extension_drivers.append('qos') + + if ctxt['enable_nsg_logging']: + extension_drivers.append('log') + + ctxt['extension_drivers'] = ','.join(extension_drivers) return ctxt diff --git a/hooks/neutron_ovs_context.py b/hooks/neutron_ovs_context.py index 571e1e4a..b2e87b77 100644 --- a/hooks/neutron_ovs_context.py +++ b/hooks/neutron_ovs_context.py @@ -51,7 +51,7 @@ OPENVSWITCH = 'openvswitch' VALID_FIREWALL_DRIVERS = (IPTABLES_HYBRID, OPENVSWITCH) -def _get_firewall_driver(): +def _get_firewall_driver(ovs_ctxt): ''' Determine the firewall driver to use based on configuration, OpenStack and Ubuntu releases. @@ -62,6 +62,12 @@ def _get_firewall_driver(): release = lsb_release()['DISTRIB_CODENAME'] if driver not in VALID_FIREWALL_DRIVERS: return IPTABLES_HYBRID + + if driver == IPTABLES_HYBRID and ovs_ctxt['enable_nsg_logging']: + msg = "NSG logging can not be enabled - need to set " \ + "firewall driver to 'openvswitch' explicitly" + log(msg, "WARN") + if (driver == OPENVSWITCH and CompareHostReleases(release) < 'xenial'): # NOTE(jamespage): Switch back to iptables_hybrid for @@ -69,9 +75,37 @@ def _get_firewall_driver(): # to requirements for Linux >= 4.4 and # Open vSwitch >= 2.5 return IPTABLES_HYBRID + return driver +def get_nsg_log_path(desired_nsg_log_path): + if not desired_nsg_log_path: + # None means "we need to use syslog" - no need + # to check anything on filesystem + return None + + dst_dir, _ = os.path.split(desired_nsg_log_path) + path_exists = os.path.exists(dst_dir) + if not path_exists: + log( + "Desired NSG log directory {} not exists! " + "falling back to syslog".format(dst_dir), + "WARN" + ) + return None + + if path_exists and os.path.isdir(desired_nsg_log_path): + log( + "Desired NSG log path {} should be file, not directory! " + "falling back to syslog".format(desired_nsg_log_path), + "WARN" + ) + return None + + return desired_nsg_log_path + + class OVSPluginContext(context.NeutronContext): interfaces = [] @@ -132,7 +166,6 @@ class OVSPluginContext(context.NeutronContext): ovs_ctxt['use_syslog'] = conf['use-syslog'] ovs_ctxt['verbose'] = conf['verbose'] ovs_ctxt['debug'] = conf['debug'] - cmp_release = CompareOpenStackReleases( os_release('neutron-common', base='icehouse')) if conf['prevent-arp-spoofing'] and cmp_release >= 'ocata': @@ -182,7 +215,21 @@ class OVSPluginContext(context.NeutronContext): if vlan_ranges: ovs_ctxt['vlan_ranges'] = ','.join(vlan_ranges.split()) - ovs_ctxt['firewall_driver'] = _get_firewall_driver() + ovs_ctxt['enable_nsg_logging'] = \ + neutron_api_settings['enable_nsg_logging'] + + ovs_ctxt['nsg_log_output_base'] = get_nsg_log_path( + config('security-group-log-output-base') + ) + ovs_ctxt['nsg_log_rate_limit'] = \ + config('security-group-log-rate-limit') + ovs_ctxt['nsg_log_burst_limit'] = \ + config('security-group-log-burst-limit') + + ovs_ctxt['firewall_driver'] = _get_firewall_driver(ovs_ctxt) + + if ovs_ctxt['firewall_driver'] != OPENVSWITCH: + ovs_ctxt['enable_nsg_logging'] = False return ovs_ctxt diff --git a/templates/queens/openvswitch_agent.ini b/templates/queens/openvswitch_agent.ini new file mode 100644 index 00000000..1cb9b75b --- /dev/null +++ b/templates/queens/openvswitch_agent.ini @@ -0,0 +1,46 @@ +# queens +############################################################################### +# [ WARNING ] +# Configuration file maintained by Juju. Local changes may be overwritten. +# Config managed by neutron-openvswitch charm +############################################################################### +[ovs] +enable_tunneling = True +local_ip = {{ local_ip }} +bridge_mappings = {{ bridge_mappings }} +{% if enable_dpdk -%} +datapath_type = netdev +vhostuser_socket_dir = /run/libvirt-vhost-user +{% endif -%} + +[agent] +tunnel_types = {{ overlay_network_type }} +l2_population = {{ l2_population }} +enable_distributed_routing = {{ distributed_routing }} +prevent_arp_spoofing = {{ prevent_arp_spoofing }} +{% if veth_mtu -%} +veth_mtu = {{ veth_mtu }} +{% endif -%} +polling_interval = {{ polling_interval }} +{% if extension_drivers -%} +extensions = {{ extension_drivers }} +{% endif -%} + +[securitygroup] +{% if neutron_security_groups and not enable_dpdk -%} +enable_security_group = True +firewall_driver = {{ firewall_driver }} +{% else -%} +enable_security_group = False +{% endif -%} + +{% if enable_nsg_logging -%} +[network_log] +{% if nsg_log_rate_limit -%} +rate_limit = {{ nsg_log_rate_limit }} +{% endif -%} +burst_limit = {{ nsg_log_burst_limit }} +{% if nsg_log_output_base -%} +local_output_log_base = {{ nsg_log_output_base }} +{% endif -%} +{% endif -%} diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index dbfdcf3c..3f5594fb 100644 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -67,6 +67,7 @@ class NeutronOVSBasicDeployment(OpenStackAmuletDeployment): this_service = {'name': 'neutron-openvswitch'} other_services = [ {'name': 'nova-compute'}, + {'name': 'nova-cloud-controller'}, {'name': 'rabbitmq-server'}, {'name': 'keystone'}, {'name': 'glance'}, @@ -94,6 +95,13 @@ class NeutronOVSBasicDeployment(OpenStackAmuletDeployment): 'glance:shared-db': 'percona-cluster:shared-db', 'glance:amqp': 'rabbitmq-server:amqp', 'keystone:shared-db': 'percona-cluster:shared-db', + 'nova-cloud-controller:shared-db': 'percona-cluster:shared-db', + 'nova-cloud-controller:amqp': 'rabbitmq-server:amqp', + 'nova-cloud-controller:identity-service': 'keystone:' + 'identity-service', + 'nova-cloud-controller:cloud-compute': 'nova-compute:' + 'cloud-compute', + 'nova-cloud-controller:image-service': 'glance:image-service', } super(NeutronOVSBasicDeployment, self)._add_relations(relations) @@ -109,9 +117,11 @@ class NeutronOVSBasicDeployment(OpenStackAmuletDeployment): 'root-password': 'ChangeMe123', 'sst-password': 'ChangeMe123', } + nova_cc_config = {'network-manager': 'Neutron'} configs = { 'neutron-openvswitch': neutron_ovs_config, 'percona-cluster': pxc_config, + 'nova-cloud-controller': nova_cc_config, } super(NeutronOVSBasicDeployment, self)._configure_services(configs) diff --git a/unit_tests/test_neutron_ovs_context.py b/unit_tests/test_neutron_ovs_context.py index 9085a3b2..fb496c95 100644 --- a/unit_tests/test_neutron_ovs_context.py +++ b/unit_tests/test_neutron_ovs_context.py @@ -129,7 +129,10 @@ class OVSPluginContextTest(CharmTestCase): 'bridge-mappings': "physnet1:br-data physnet2:br-data", 'flat-network-providers': 'physnet3 physnet4', 'prevent-arp-spoofing': False, - 'enable-dpdk': False} + 'enable-dpdk': False, + 'security-group-log-output-base': '/var/log/nsg.log', + 'security-group-log-rate-limit': None, + 'security-group-log-burst-limit': 25} def mock_config(key=None): if key: @@ -183,6 +186,10 @@ class OVSPluginContextTest(CharmTestCase): 'bridge_mappings': 'physnet1:br-data,physnet2:br-data', 'vlan_ranges': 'physnet1:1000:1500,physnet2:2000:2500', 'prevent_arp_spoofing': False, + 'enable_nsg_logging': False, + 'nsg_log_output_base': '/var/log/nsg.log', + 'nsg_log_rate_limit': None, + 'nsg_log_burst_limit': 25, } self.assertEqual(expect, napi_ctxt()) @@ -259,6 +266,10 @@ class OVSPluginContextTest(CharmTestCase): 'bridge_mappings': 'physnet1:br-data', 'vlan_ranges': 'physnet1:1000:2000', 'prevent_arp_spoofing': True, + 'enable_nsg_logging': False, + 'nsg_log_output_base': None, + 'nsg_log_rate_limit': None, + 'nsg_log_burst_limit': 25, } self.maxDiff = None self.assertEqual(expect, napi_ctxt()) @@ -755,24 +766,41 @@ class TestFirewallDriver(CharmTestCase): self.config.side_effect = self.test_config.get def test_get_firewall_driver_xenial_unset(self): + ctxt = {'enable_nsg_logging': False} self.lsb_release.return_value = _LSB_RELEASE_XENIAL - self.assertEqual(context._get_firewall_driver(), + self.assertEqual(context._get_firewall_driver(ctxt), context.IPTABLES_HYBRID) def test_get_firewall_driver_xenial_openvswitch(self): + ctxt = {'enable_nsg_logging': False} self.test_config.set('firewall-driver', 'openvswitch') self.lsb_release.return_value = _LSB_RELEASE_XENIAL - self.assertEqual(context._get_firewall_driver(), + self.assertEqual(context._get_firewall_driver(ctxt), context.OPENVSWITCH) def test_get_firewall_driver_xenial_invalid(self): + ctxt = {'enable_nsg_logging': False} self.test_config.set('firewall-driver', 'foobar') self.lsb_release.return_value = _LSB_RELEASE_XENIAL - self.assertEqual(context._get_firewall_driver(), + self.assertEqual(context._get_firewall_driver(ctxt), context.IPTABLES_HYBRID) def test_get_firewall_driver_trusty_openvswitch(self): + ctxt = {'enable_nsg_logging': False} self.test_config.set('firewall-driver', 'openvswitch') self.lsb_release.return_value = _LSB_RELEASE_TRUSTY - self.assertEqual(context._get_firewall_driver(), + self.assertEqual(context._get_firewall_driver(ctxt), + context.IPTABLES_HYBRID) + + def test_get_firewall_driver_nsg_logging(self): + ctxt = {'enable_nsg_logging': True} + self.lsb_release.return_value = _LSB_RELEASE_XENIAL + self.test_config.set('firewall-driver', 'openvswitch') + self.assertEqual(context._get_firewall_driver(ctxt), + context.OPENVSWITCH) + + def test_get_firewall_driver_nsg_logging_iptables_hybrid(self): + ctxt = {'enable_nsg_logging': True} + self.lsb_release.return_value = _LSB_RELEASE_XENIAL + self.assertEqual(context._get_firewall_driver(ctxt), context.IPTABLES_HYBRID)