Add check O346 to disallow backslash line continuation

Also remove our own check that we were ignoring.

Change-Id: Icc8d74f0ee85df742340dc147b9252eaa3eaa739

octavia/hacking/checks.py

@@ -95,26 +95,6 @@ def assert_equal_or_not_none(logical_line):
         yield (0, msg)
 
 
-def use_jsonutils(logical_line, filename):
-    msg = "O321: jsonutils.%(fun)s must be used instead of json.%(fun)s"
-
-    # Some files in the tree are not meant to be run from inside Octavia
-    # itself, so we should not complain about them not using jsonutils
-    json_check_skipped_patterns = [
-    ]
-
-    for pattern in json_check_skipped_patterns:
-        if pattern in filename:
-            return
-
-    if "json." in logical_line:
-        json_funcs = ['dumps(', 'dump(', 'loads(', 'load(']
-        for f in json_funcs:
-            pos = logical_line.find('json.%s' % f)
-            if pos != -1:
-                yield (pos, msg % {'fun': f[:-1]})
-
-
 def no_author_tags(physical_line):
     for regex in author_tag_re:
         if regex.match(physical_line):
@@ -267,11 +247,31 @@ def check_no_eventlet_imports(logical_line):
         yield logical_line.index('eventlet'), msg
 
 
+def check_line_continuation_no_backslash(logical_line, tokens):
+    """O346 - Don't use backslashes for line continuation.
+
+    :param logical_line: The logical line to check. Not actually used.
+    :param tokens: List of tokens to check.
+    :returns: None if the tokens don't contain any issues, otherwise a tuple
+    is yielded that contains the offending index in the logical line and a
+    message describe the check validation failure.
+    """
+    backslash = None
+    for token_type, text, start, end, orig_line in tokens:
+        m = re.match(r'.*(\\)\n', orig_line)
+        if m:
+            backslash = (start[0], m.start(1))
+            break
+
+    if backslash is not None:
+        msg = 'O346 Backslash line continuations not allowed'
+        yield backslash, msg
+
+
 def factory(register):
     register(assert_true_instance)
     register(assert_equal_or_not_none)
     register(no_translate_logs)
-    register(use_jsonutils)
     register(no_author_tags)
     register(assert_equal_true_or_false)
     register(no_mutable_default_args)
@@ -282,3 +282,4 @@ def factory(register):
     register(check_no_basestring)
     register(check_python3_no_iteritems)
     register(check_no_eventlet_imports)
+    register(check_line_continuation_no_backslash)

octavia/tests/functional/api/v1/test_load_balancer.py

@@ -162,10 +162,11 @@ class TestLoadBalancer(base.BaseAPITest):
         vip = {'network_id': network.id}
         lb_json = {'vip': vip,
                    'project_id': self.project_id}
-        with mock.patch("octavia.network.drivers.noop_driver.driver"
+        with mock.patch(
-                        ".NoopManager.get_network") as mock_get_network, \
+                "octavia.network.drivers.noop_driver.driver.NoopManager"
-                mock.patch("octavia.network.drivers.noop_driver.driver"
+                ".get_network") as mock_get_network, mock.patch(
-                           ".NoopManager.get_subnet") as mock_get_subnet:
+            "octavia.network.drivers.noop_driver.driver.NoopManager"
+                ".get_subnet") as mock_get_subnet:
             mock_get_network.return_value = network
             mock_get_subnet.side_effect = [subnet1, subnet2]
             response = self.post(self.LBS_PATH, lb_json)
@@ -186,10 +187,11 @@ class TestLoadBalancer(base.BaseAPITest):
         vip = {'network_id': network.id}
         lb_json = {'vip': vip,
                    'project_id': self.project_id}
-        with mock.patch("octavia.network.drivers.noop_driver.driver"
+        with mock.patch(
-                        ".NoopManager.get_network") as mock_get_network, \
+                "octavia.network.drivers.noop_driver.driver.NoopManager"
-                mock.patch("octavia.network.drivers.noop_driver.driver"
+                ".get_network") as mock_get_network, mock.patch(
-                           ".NoopManager.get_subnet") as mock_get_subnet:
+            "octavia.network.drivers.noop_driver.driver.NoopManager"
+                ".get_subnet") as mock_get_subnet:
             mock_get_network.return_value = network
             mock_get_subnet.return_value = subnet
             response = self.post(self.LBS_PATH, lb_json)
@@ -213,10 +215,11 @@ class TestLoadBalancer(base.BaseAPITest):
         lb_json = {'name': 'test1', 'description': 'test1_desc',
                    'vip': vip, 'enabled': False,
                    'project_id': self.project_id}
-        with mock.patch("octavia.network.drivers.noop_driver.driver"
+        with mock.patch(
-                        ".NoopManager.get_network") as mock_get_network, \
+                "octavia.network.drivers.noop_driver.driver.NoopManager"
-                mock.patch("octavia.network.drivers.noop_driver.driver"
+                ".get_network") as mock_get_network, mock.patch(
-                           ".NoopManager.get_port") as mock_get_port:
+            "octavia.network.drivers.noop_driver.driver.NoopManager"
+                ".get_port") as mock_get_port:
             mock_get_network.return_value = network
             mock_get_port.return_value = port
             response = self.post(self.LBS_PATH, lb_json)
@@ -288,10 +291,11 @@ class TestLoadBalancer(base.BaseAPITest):
                'subnet_id': subnet.id,
                'network_id': network.id,
                'port_id': port.id}
-        with mock.patch("octavia.network.drivers.noop_driver.driver"
+        with mock.patch(
-                        ".NoopManager.get_network") as mock_get_network, \
+                "octavia.network.drivers.noop_driver.driver.NoopManager"
-                mock.patch("octavia.network.drivers.noop_driver.driver"
+                ".get_network") as mock_get_network, mock.patch(
-                           ".NoopManager.get_port") as mock_get_port:
+            "octavia.network.drivers.noop_driver.driver.NoopManager."
+                "get_port") as mock_get_port:
             mock_get_network.return_value = network
             mock_get_port.return_value = port
             lb = self.create_load_balancer(vip, name='lb1',

octavia/tests/functional/api/v2/test_load_balancer.py

@@ -159,10 +159,11 @@ class TestLoadBalancer(base.BaseAPITest):
         lb_json = {'vip_network_id': network.id,
                    'project_id': self.project_id}
         body = self._build_body(lb_json)
-        with mock.patch("octavia.network.drivers.noop_driver.driver"
+        with mock.patch(
-                        ".NoopManager.get_network") as mock_get_network, \
+                "octavia.network.drivers.noop_driver.driver.NoopManager"
-                mock.patch("octavia.network.drivers.noop_driver.driver"
+                ".get_network") as mock_get_network, mock.patch(
-                           ".NoopManager.get_subnet") as mock_get_subnet:
+            "octavia.network.drivers.noop_driver.driver.NoopManager"
+                ".get_subnet") as mock_get_subnet:
             mock_get_network.return_value = network
             mock_get_subnet.side_effect = [subnet1, subnet2]
             response = self.post(self.LBS_PATH, body)
@@ -181,10 +182,11 @@ class TestLoadBalancer(base.BaseAPITest):
         lb_json = {'vip_network_id': network_id,
                    'project_id': self.project_id}
         body = self._build_body(lb_json)
-        with mock.patch("octavia.network.drivers.noop_driver.driver"
+        with mock.patch(
-                        ".NoopManager.get_network") as mock_get_network, \
+                "octavia.network.drivers.noop_driver.driver.NoopManager"
-                mock.patch("octavia.network.drivers.noop_driver.driver"
+                ".get_network") as mock_get_network, mock.patch(
-                           ".NoopManager.get_subnet") as mock_get_subnet:
+            "octavia.network.drivers.noop_driver.driver.NoopManager"
+                ".get_subnet") as mock_get_subnet:
             mock_get_network.return_value = network
             mock_get_subnet.return_value = subnet
             response = self.post(self.LBS_PATH, body)
@@ -205,10 +207,11 @@ class TestLoadBalancer(base.BaseAPITest):
             'vip_network_id': network.id, 'vip_port_id': port.id,
             'admin_state_up': False, 'project_id': self.project_id}
         body = self._build_body(lb_json)
-        with mock.patch("octavia.network.drivers.noop_driver.driver"
+        with mock.patch(
-                        ".NoopManager.get_network") as mock_get_network, \
+                "octavia.network.drivers.noop_driver.driver.NoopManager"
-                mock.patch("octavia.network.drivers.noop_driver.driver"
+                ".get_network") as mock_get_network, mock.patch(
-                           ".NoopManager.get_port") as mock_get_port:
+            "octavia.network.drivers.noop_driver.driver.NoopManager"
+                ".get_port") as mock_get_port:
             mock_get_network.return_value = network
             mock_get_port.return_value = port
             response = self.post(self.LBS_PATH, body)
@@ -380,10 +383,11 @@ class TestLoadBalancer(base.BaseAPITest):
                                          subnets=[subnet])
         port = network_models.Port(id=uuidutils.generate_uuid(),
                                    network_id=network.id)
-        with mock.patch("octavia.network.drivers.noop_driver.driver"
+        with mock.patch(
-                        ".NoopManager.get_network") as mock_get_network, \
+                "octavia.network.drivers.noop_driver.driver.NoopManager"
-                mock.patch("octavia.network.drivers.noop_driver.driver"
+                ".get_network") as mock_get_network, mock.patch(
-                           ".NoopManager.get_port") as mock_get_port:
+            "octavia.network.drivers.noop_driver.driver.NoopManager"
+                ".get_port") as mock_get_port:
             mock_get_network.return_value = network
             mock_get_port.return_value = port
 

octavia/tests/tempest/common/manager.py

@@ -156,9 +156,9 @@ class ScenarioTest(tempest.test.BaseTestCase):
                 # Convert security group names to security group ids
                 # to pass to create_port
                 if 'security_groups' in kwargs:
-                    security_groups = \
+                    security_groups = (
                         clients.security_groups_client.list_security_groups(
-                        ).get('security_groups')
+                        ).get('security_groups'))
                     sec_dict = dict([(s['name'], s['id'])
                                     for s in security_groups])
 
@@ -1235,12 +1235,12 @@ class EncryptionScenarioTest(ScenarioTest):
         super(EncryptionScenarioTest, cls).setup_clients()
         if CONF.volume_feature_enabled.api_v2:
             cls.admin_volume_types_client = cls.os_adm.volume_types_v2_client
-            cls.admin_encryption_types_client =\
+            cls.admin_encryption_types_client = (
-                cls.os_adm.encryption_types_v2_client
+                cls.os_adm.encryption_types_v2_client)
         else:
             cls.admin_volume_types_client = cls.os_adm.volume_types_client
-            cls.admin_encryption_types_client =\
+            cls.admin_encryption_types_client = (
-                cls.os_adm.encryption_types_client
+                cls.os_adm.encryption_types_client)
 
     def create_encryption_type(self, client=None, type_id=None, provider=None,
                                key_size=None, cipher=None,

octavia/tests/unit/test_hacking.py

@@ -142,3 +142,10 @@ class HackingTestCase(base.BaseTestCase):
 
         self.assertEqual(0, len(list(checks.no_xrange(
             "range(45)"))))
+
+    def test_line_continuation_no_backslash(self):
+        results = list(checks.check_line_continuation_no_backslash(
+            '', [(1, 'import', (2, 0), (2, 6), 'import \\\n'),
+                 (1, 'os', (3, 4), (3, 6), '    os\n')]))
+        self.assertEqual(1, len(results))
+        self.assertEqual((2, 7), results[0][0])

tox.ini

@@ -101,9 +101,7 @@ commands =
 commands = bandit -r octavia -ll -ii -x octavia/tests {posargs}
 
 [flake8]
-# Ignoring O321 because it's unnecessarily restricting use of json package.
+ignore =
-# jsonutils version doesn't add additional value
-ignore = O321
 show-source = true
 builtins = _
 exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build