diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cdb2b5e4ef..5ce256e385 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -56,6 +56,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 * Fixed a bug where a python3 sensor using ssl needs to be monkey patched earlier. See also #4832, #4975 and gevent/gevent#1016 (bug fix) #4976 @@ -68,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 ~~~~~~~ diff --git a/contrib/linux/actions/dig.py b/contrib/linux/actions/dig.py index 46dc970473..9a3b58a5cd 100644 --- a/contrib/linux/actions/dig.py +++ b/contrib/linux/actions/dig.py @@ -16,9 +16,10 @@ # limitations under the License. import errno +import locale import subprocess import random -import re +import sys from st2common.runners.base_action import Action @@ -34,26 +35,33 @@ 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) - for k, v in enumerate(opt_list): - cmd_args.append('+' + v) + + cmd_args.extend(['+' + option for option in opt_list]) cmd_args.append(hostname) try: - result_list = filter(None, subprocess.Popen(cmd_args, - stderr=subprocess.PIPE, - stdout=subprocess.PIPE) - .communicate()[0] - .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 " \ diff --git a/contrib/linux/tests/test_action_dig.py b/contrib/linux/tests/test_action_dig.py new file mode 100644 index 0000000000..4f363521d9 --- /dev/null +++ b/contrib/linux/tests/test_action_dig.py @@ -0,0 +1,55 @@ +#!/usr/bin/env python + +# 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. +# 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_with_empty_hostname(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) + + def test_run_with_empty_queryopts(self): + action = self.get_action_instance() + + 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() + + results = action.run(rand=False, count=0, nameserver=None, hostname='google.com', + queryopts='short') + self.assertIsInstance(results, list) + self.assertGreater(len(results), 0) + + for result in results: + self.assertIsInstance(result, str) + self.assertGreater(len(result), 0)