From 89182761546f85b3579baef2133f2b7dd3fd0d3f Mon Sep 17 00:00:00 2001 From: blag Date: Wed, 15 Jul 2020 13:38:04 -0700 Subject: [PATCH 01/13] Fix bytes -> str handling in linux.dig action --- contrib/linux/actions/dig.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/contrib/linux/actions/dig.py b/contrib/linux/actions/dig.py index ccf7e9a318..fb195cd63c 100644 --- a/contrib/linux/actions/dig.py +++ b/contrib/linux/actions/dig.py @@ -15,6 +15,7 @@ # limitations under the License. import errno +import locale import subprocess import random import re @@ -42,12 +43,15 @@ def run(self, rand, count, nameserver, hostname, queryopts): cmd_args.append(hostname) + # This function might call getpreferred encoding unless we pass + # do_setlocale=False. + encoding = locale.getpreferredencoding(do_setlocale=False) try: - result_list = filter(None, subprocess.Popen(cmd_args, - stderr=subprocess.PIPE, - stdout=subprocess.PIPE) - .communicate()[0] - .split('\n')) + result_list = list(filter(None, subprocess.Popen(cmd_args, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE) + .communicate()[0].decode(encoding) + .split('\n'))) # NOTE: Python3 supports the FileNotFoundError, the errono.ENOENT is for py2 compat # for Python3: From 85c68236cf9aa75e7f7d2d9c4ceada6379626376 Mon Sep 17 00:00:00 2001 From: blag Date: Wed, 15 Jul 2020 17:14:12 -0700 Subject: [PATCH 02/13] Update changelog --- CHANGELOG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c010743614..8feb89b92b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -35,6 +35,8 @@ Fixed up correctly, specifically when specifying a bastion host / jump box. (bug fix) #4973 Contributed by Nick Maludy (@nmaludy Encore Technologies) +* Fixed a bytes/string encoding bug in the ``linux.dig`` action so it should work on Python 3 + (bug fix) #4993 Removed ~~~~~~~ From 681871b7b7271d8431e8d92a29d8ab02e9d9ba0d Mon Sep 17 00:00:00 2001 From: blag Date: Tue, 4 Aug 2020 14:22:06 -0700 Subject: [PATCH 03/13] Add initial tests for linux.dig action --- contrib/linux/tests/test_action_dig.py | 37 ++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 contrib/linux/tests/test_action_dig.py diff --git a/contrib/linux/tests/test_action_dig.py b/contrib/linux/tests/test_action_dig.py new file mode 100644 index 0000000000..2d8da611ae --- /dev/null +++ b/contrib/linux/tests/test_action_dig.py @@ -0,0 +1,37 @@ +#!/usr/bin/env python + +# Copyright 2020 The StackStorm Developers +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import +from st2tests.base import BaseActionTestCase + +from dig import DigAction + + +class DigActionTestCase(BaseActionTestCase): + action_cls = DigAction + + def test_run(self): + action = self.get_action_instance() + + # Use the defaults from dig.yaml + result = action.run(rand=False, count=0, nameserver=None, hostname='', queryopts='short') + self.assertIsInstance(result, list) + self.assertEqual(len(result), 0) + + result = action.run(rand=False, count=0, nameserver=None, hostname='google.com', + queryopts='') + self.assertIsInstance(result, list) + self.assertGreater(len(result), 0) From 54454f7f963730aa3fc91c5b1baf44762b07fb47 Mon Sep 17 00:00:00 2001 From: blag Date: Wed, 5 Aug 2020 13:42:27 -0700 Subject: [PATCH 04/13] Use "The StackStorm Authors" in copyright Co-authored-by: Eugen C. --- contrib/linux/tests/test_action_dig.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/linux/tests/test_action_dig.py b/contrib/linux/tests/test_action_dig.py index 2d8da611ae..c3fbb408b0 100644 --- a/contrib/linux/tests/test_action_dig.py +++ b/contrib/linux/tests/test_action_dig.py @@ -1,6 +1,6 @@ #!/usr/bin/env python -# Copyright 2020 The StackStorm Developers +# Copyright 2020 The StackStorm Authors. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From cbde6c382f9218aa7e433fc24e32a8ec766a92d3 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 14:19:37 -0700 Subject: [PATCH 05/13] Don't use re.search to check for contains substring --- contrib/linux/actions/dig.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contrib/linux/actions/dig.py b/contrib/linux/actions/dig.py index fb195cd63c..38bdd16b05 100644 --- a/contrib/linux/actions/dig.py +++ b/contrib/linux/actions/dig.py @@ -18,7 +18,6 @@ import locale import subprocess import random -import re from st2common.runners.base_action import Action @@ -34,7 +33,7 @@ def run(self, rand, count, nameserver, hostname, queryopts): nameserver = '@' + nameserver cmd_args.append(nameserver) - if re.search(',', queryopts): + if isinstance(queryopts, str) and ',' in queryopts: opt_list = queryopts.split(',') else: opt_list.append(queryopts) From cd92fe65e52bf49b97547b2e53785a74461eaaf1 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 14:21:29 -0700 Subject: [PATCH 06/13] Use list comprehension and extend cmd_args with options --- contrib/linux/actions/dig.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/linux/actions/dig.py b/contrib/linux/actions/dig.py index 38bdd16b05..abaee26a15 100644 --- a/contrib/linux/actions/dig.py +++ b/contrib/linux/actions/dig.py @@ -37,8 +37,8 @@ def run(self, rand, count, nameserver, hostname, queryopts): opt_list = queryopts.split(',') else: opt_list.append(queryopts) - for k, v in enumerate(opt_list): - cmd_args.append('+' + v) + + cmd_args.extend(['+' + option for option in opt_list]) cmd_args.append(hostname) From 5a6c8b1c9c13078462bec7ba254c6a6f95dd3c42 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 14:46:08 -0700 Subject: [PATCH 07/13] Test that returned result is a str instance --- contrib/linux/tests/test_action_dig.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/linux/tests/test_action_dig.py b/contrib/linux/tests/test_action_dig.py index c3fbb408b0..e7c648a0fe 100644 --- a/contrib/linux/tests/test_action_dig.py +++ b/contrib/linux/tests/test_action_dig.py @@ -35,3 +35,7 @@ def test_run(self): queryopts='') self.assertIsInstance(result, list) self.assertGreater(len(result), 0) + + first_result = result[0] + self.assertIsInstance(first_result, str) + self.assertGreater(len(first_result)) From a93b03286caa8ebe0c5a1b2cc86bb156197395b7 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 14:46:37 -0700 Subject: [PATCH 08/13] Fix to maintain Python 2 support --- contrib/linux/actions/dig.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/contrib/linux/actions/dig.py b/contrib/linux/actions/dig.py index abaee26a15..155011b6c8 100644 --- a/contrib/linux/actions/dig.py +++ b/contrib/linux/actions/dig.py @@ -18,6 +18,7 @@ import locale import subprocess import random +import sys from st2common.runners.base_action import Action @@ -42,20 +43,24 @@ def run(self, rand, count, nameserver, hostname, queryopts): cmd_args.append(hostname) - # This function might call getpreferred encoding unless we pass - # do_setlocale=False. - encoding = locale.getpreferredencoding(do_setlocale=False) try: - result_list = list(filter(None, subprocess.Popen(cmd_args, - stderr=subprocess.PIPE, - stdout=subprocess.PIPE) - .communicate()[0].decode(encoding) - .split('\n'))) + raw_result = subprocess.Popen(cmd_args, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE).communicate()[0] + + if sys.version_info >= (3,): + # This function might call getpreferred encoding unless we pass + # do_setlocale=False. + encoding = locale.getpreferredencoding(do_setlocale=False) + result_list_str = raw_result.decode(encoding) + else: + result_list_str = str(raw_result) + + result_list = list(filter(None, result_list_str.split('\n'))) # NOTE: Python3 supports the FileNotFoundError, the errono.ENOENT is for py2 compat # for Python3: # except FileNotFoundError as e: - except OSError as e: if e.errno == errno.ENOENT: return False, "Can't find dig installed in the path (usually /usr/bin/dig). If " \ From 9035ea5a599dd45f71df6423c43a9d6a9fc07929 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 15:14:22 -0700 Subject: [PATCH 09/13] Split up tests --- contrib/linux/tests/test_action_dig.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/linux/tests/test_action_dig.py b/contrib/linux/tests/test_action_dig.py index e7c648a0fe..eca6e7f47f 100644 --- a/contrib/linux/tests/test_action_dig.py +++ b/contrib/linux/tests/test_action_dig.py @@ -23,7 +23,7 @@ class DigActionTestCase(BaseActionTestCase): action_cls = DigAction - def test_run(self): + def test_run_with_empty_hostname(self): action = self.get_action_instance() # Use the defaults from dig.yaml @@ -31,6 +31,9 @@ def test_run(self): self.assertIsInstance(result, list) self.assertEqual(len(result), 0) + def test_run(self): + action = self.get_action_instance() + result = action.run(rand=False, count=0, nameserver=None, hostname='google.com', queryopts='') self.assertIsInstance(result, list) From eac75cda5624db65e7532bf5b7727c9fcaf2e4e2 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 15:15:50 -0700 Subject: [PATCH 10/13] Fix happy path test --- contrib/linux/tests/test_action_dig.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/linux/tests/test_action_dig.py b/contrib/linux/tests/test_action_dig.py index eca6e7f47f..e45ddd28ff 100644 --- a/contrib/linux/tests/test_action_dig.py +++ b/contrib/linux/tests/test_action_dig.py @@ -35,7 +35,7 @@ def test_run(self): action = self.get_action_instance() result = action.run(rand=False, count=0, nameserver=None, hostname='google.com', - queryopts='') + queryopts='short') self.assertIsInstance(result, list) self.assertGreater(len(result), 0) From f1f68ec39eca05d6653f6d5e4bb8dd24bb1ab6ed Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 15:16:32 -0700 Subject: [PATCH 11/13] Explicitly test empty queryopts string --- contrib/linux/tests/test_action_dig.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/contrib/linux/tests/test_action_dig.py b/contrib/linux/tests/test_action_dig.py index e45ddd28ff..743099cfd1 100644 --- a/contrib/linux/tests/test_action_dig.py +++ b/contrib/linux/tests/test_action_dig.py @@ -31,6 +31,14 @@ def test_run_with_empty_hostname(self): self.assertIsInstance(result, list) self.assertEqual(len(result), 0) + def test_run_with_empty_queryopts(self): + action = self.get_action_instance() + + result = action.run(rand=False, count=0, nameserver=None, hostname='google.com', + queryopts='') + self.assertIsInstance(result, list) + self.assertEqual(len(result), 0) + def test_run(self): action = self.get_action_instance() From f430b4ede925609b97d1963cb979926c51a9432f Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 16:38:41 -0700 Subject: [PATCH 12/13] Fix test assertions --- contrib/linux/tests/test_action_dig.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/contrib/linux/tests/test_action_dig.py b/contrib/linux/tests/test_action_dig.py index 743099cfd1..4f363521d9 100644 --- a/contrib/linux/tests/test_action_dig.py +++ b/contrib/linux/tests/test_action_dig.py @@ -34,19 +34,22 @@ def test_run_with_empty_hostname(self): def test_run_with_empty_queryopts(self): action = self.get_action_instance() - result = action.run(rand=False, count=0, nameserver=None, hostname='google.com', - queryopts='') - self.assertIsInstance(result, list) - self.assertEqual(len(result), 0) + results = action.run(rand=False, count=0, nameserver=None, hostname='google.com', + queryopts='') + self.assertIsInstance(results, list) + + for result in results: + self.assertIsInstance(result, str) + self.assertGreater(len(result), 0) def test_run(self): action = self.get_action_instance() - result = action.run(rand=False, count=0, nameserver=None, hostname='google.com', - queryopts='short') - self.assertIsInstance(result, list) - self.assertGreater(len(result), 0) + results = action.run(rand=False, count=0, nameserver=None, hostname='google.com', + queryopts='short') + self.assertIsInstance(results, list) + self.assertGreater(len(results), 0) - first_result = result[0] - self.assertIsInstance(first_result, str) - self.assertGreater(len(first_result)) + for result in results: + self.assertIsInstance(result, str) + self.assertGreater(len(result), 0) From 7a1fda4b13e49d62bca2fdbd7a21a4f367c46015 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 10 Aug 2020 18:14:27 -0700 Subject: [PATCH 13/13] Update CHANGELOG --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a1d55d5683..5ce256e385 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -70,6 +70,9 @@ Fixed Contributed by Bradley Bishop (@bishopbm1 Encore Technologies) * Fix a regression when updated ``dnspython`` pip dependency resulted in st2 services unable to connect to mongodb remote host (bug fix) #4997 +* Fixed a regression in the ``linux.dig`` action on Python 3. (bug fix) #4993 + + Contributed by @blag Changed ~~~~~~~