From a69f02db794a6c6aa7784241ea7b24f4e5a04160 Mon Sep 17 00:00:00 2001 From: chynasan Date: Thu, 6 Feb 2025 12:31:55 -0600 Subject: [PATCH 1/6] Add generate_mark() function and corresponding unit test. --- plugins/connection/aws_ssm.py | 17 ++++++++++++----- .../plugins/connection/aws_ssm/test_aws_ssm.py | 10 ++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/plugins/connection/aws_ssm.py b/plugins/connection/aws_ssm.py index e50c43e495d..3cfe6873d62 100644 --- a/plugins/connection/aws_ssm.py +++ b/plugins/connection/aws_ssm.py @@ -377,7 +377,7 @@ def chunks(lst, n): for i in range(0, len(lst), n): yield lst[i:i + n] # fmt: skip - + def filter_ansi(line: str, is_windows: bool) -> str: """Remove any ANSI terminal control codes. @@ -704,20 +704,27 @@ def exec_communicate(self, cmd: str, mark_start: str, mark_begin: str, mark_end: # see https://github.com/pylint-dev/pylint/issues/8909) return (returncode, stdout, self._flush_stderr(self._session)) # pylint: disable=unreachable + + def generate_mark() -> str: + """Generates a random string of characters to delimit SSM CLI commands""" + mark = "".join([random.choice(string.ascii_letters) for i in range(Connection.MARK_LENGTH)]) + return(mark) + + @_ssm_retry def exec_command(self, cmd: str, in_data: bool = None, sudoable: bool = True) -> Tuple[int, str, str]: - """run a command on the ssm host""" + """When running a command on the SSM host, uses generate_mark to get delimiting strings""" super().exec_command(cmd, in_data=in_data, sudoable=sudoable) self._vvv(f"EXEC: {to_text(cmd)}") - mark_begin = "".join([random.choice(string.ascii_letters) for i in range(self.MARK_LENGTH)]) + mark_begin = self.generate_mark() if self.is_windows: mark_start = mark_begin + " $LASTEXITCODE" else: mark_start = mark_begin - mark_end = "".join([random.choice(string.ascii_letters) for i in range(self.MARK_LENGTH)]) + mark_end = self.generate_mark() # Wrap command in markers accordingly for the shell used cmd = self._wrap_command(cmd, mark_start, mark_end) @@ -745,7 +752,7 @@ def _prepare_terminal(self): disable_echo_cmd = to_bytes("stty -echo\n", errors="surrogate_or_strict") disable_prompt_complete = None - end_mark = "".join([random.choice(string.ascii_letters) for i in range(self.MARK_LENGTH)]) + end_mark = self.generate_mark() disable_prompt_cmd = to_bytes( "PS1='' ; bind 'set enable-bracketed-paste off'; printf '\\n%s\\n' '" + end_mark + "'\n", errors="surrogate_or_strict", diff --git a/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py b/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py index f6e4480277e..4344b9efc62 100644 --- a/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py +++ b/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py @@ -10,6 +10,7 @@ from ansible.plugins.loader import connection_loader from ansible_collections.amazon.aws.plugins.module_utils.botocore import HAS_BOTO3 +from ansible_collections.community.aws.plugins.connection.aws_ssm import Connection if not HAS_BOTO3: pytestmark = pytest.mark.skip("test_data_pipeline.py requires the python modules 'boto3' and 'botocore'") @@ -257,3 +258,12 @@ def test_plugins_connection_aws_ssm_close(self, s_check_output): conn._session_id.return_value = "a" conn._client = MagicMock() conn.close() + + def test_generate_mark(self): + """Testing string generation""" + test_a = Connection.generate_mark() + test_b = Connection.generate_mark() + + assert test_a != test_b + assert len(test_a) == Connection.MARK_LENGTH + assert len(test_b) == Connection.MARK_LENGTH \ No newline at end of file From e5d92a87f32d99bf5bb8d93b1db49538b201ae1c Mon Sep 17 00:00:00 2001 From: chynasan Date: Thu, 6 Feb 2025 13:26:20 -0600 Subject: [PATCH 2/6] Add changelog fragment --- changelogs/fragments/2096-refactor-random-helper-function.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/2096-refactor-random-helper-function.yml diff --git a/changelogs/fragments/2096-refactor-random-helper-function.yml b/changelogs/fragments/2096-refactor-random-helper-function.yml new file mode 100644 index 00000000000..ab479c0640a --- /dev/null +++ b/changelogs/fragments/2096-refactor-random-helper-function.yml @@ -0,0 +1,2 @@ +minor_changes: + - ssm - add function to generate random strings for SSM CLI delimitation (https://github.com/ansible-collections/community.aws/pull/2235). From a9ccdf95fd80965d4dd91e2374baf0d14c90f6b4 Mon Sep 17 00:00:00 2001 From: Chyna Sanders Date: Thu, 6 Feb 2025 14:30:38 -0600 Subject: [PATCH 3/6] Apply suggestions from code review Fixing whitespace errors as suggested Co-authored-by: Mandar Kulkarni --- plugins/connection/aws_ssm.py | 2 +- tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/connection/aws_ssm.py b/plugins/connection/aws_ssm.py index 3cfe6873d62..5247a8b3249 100644 --- a/plugins/connection/aws_ssm.py +++ b/plugins/connection/aws_ssm.py @@ -377,7 +377,7 @@ def chunks(lst, n): for i in range(0, len(lst), n): yield lst[i:i + n] # fmt: skip - + def filter_ansi(line: str, is_windows: bool) -> str: """Remove any ANSI terminal control codes. diff --git a/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py b/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py index 4344b9efc62..53ee1c96ab3 100644 --- a/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py +++ b/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py @@ -266,4 +266,5 @@ def test_generate_mark(self): assert test_a != test_b assert len(test_a) == Connection.MARK_LENGTH - assert len(test_b) == Connection.MARK_LENGTH \ No newline at end of file + assert len(test_b) == Connection.MARK_LENGTH + \ No newline at end of file From 87b0819a3846ec42c9181cb4592859a939e48b49 Mon Sep 17 00:00:00 2001 From: chynasan Date: Thu, 6 Feb 2025 14:45:24 -0600 Subject: [PATCH 4/6] tox lint and format fixes --- plugins/connection/aws_ssm.py | 6 ++---- tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/plugins/connection/aws_ssm.py b/plugins/connection/aws_ssm.py index 5247a8b3249..9eb51448764 100644 --- a/plugins/connection/aws_ssm.py +++ b/plugins/connection/aws_ssm.py @@ -377,7 +377,7 @@ def chunks(lst, n): for i in range(0, len(lst), n): yield lst[i:i + n] # fmt: skip - + def filter_ansi(line: str, is_windows: bool) -> str: """Remove any ANSI terminal control codes. @@ -704,12 +704,10 @@ def exec_communicate(self, cmd: str, mark_start: str, mark_begin: str, mark_end: # see https://github.com/pylint-dev/pylint/issues/8909) return (returncode, stdout, self._flush_stderr(self._session)) # pylint: disable=unreachable - def generate_mark() -> str: """Generates a random string of characters to delimit SSM CLI commands""" mark = "".join([random.choice(string.ascii_letters) for i in range(Connection.MARK_LENGTH)]) - return(mark) - + return mark @_ssm_retry def exec_command(self, cmd: str, in_data: bool = None, sudoable: bool = True) -> Tuple[int, str, str]: diff --git a/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py b/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py index 53ee1c96ab3..c2d7cba3fbe 100644 --- a/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py +++ b/tests/unit/plugins/connection/aws_ssm/test_aws_ssm.py @@ -10,6 +10,7 @@ from ansible.plugins.loader import connection_loader from ansible_collections.amazon.aws.plugins.module_utils.botocore import HAS_BOTO3 + from ansible_collections.community.aws.plugins.connection.aws_ssm import Connection if not HAS_BOTO3: @@ -267,4 +268,3 @@ def test_generate_mark(self): assert test_a != test_b assert len(test_a) == Connection.MARK_LENGTH assert len(test_b) == Connection.MARK_LENGTH - \ No newline at end of file From 3066598d14d7b44e80a246c0130290846751477e Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 7 Feb 2025 07:53:06 +0100 Subject: [PATCH 5/6] need to add self as an argument to object methods. --- plugins/connection/aws_ssm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/connection/aws_ssm.py b/plugins/connection/aws_ssm.py index 9eb51448764..b6be3a80291 100644 --- a/plugins/connection/aws_ssm.py +++ b/plugins/connection/aws_ssm.py @@ -704,7 +704,7 @@ def exec_communicate(self, cmd: str, mark_start: str, mark_begin: str, mark_end: # see https://github.com/pylint-dev/pylint/issues/8909) return (returncode, stdout, self._flush_stderr(self._session)) # pylint: disable=unreachable - def generate_mark() -> str: + def generate_mark(self) -> str: """Generates a random string of characters to delimit SSM CLI commands""" mark = "".join([random.choice(string.ascii_letters) for i in range(Connection.MARK_LENGTH)]) return mark From 51c7cd232fc9678455534da866722d3d54c432fc Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 7 Feb 2025 07:57:59 +0100 Subject: [PATCH 6/6] Update plugins/connection/aws_ssm.py --- plugins/connection/aws_ssm.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/connection/aws_ssm.py b/plugins/connection/aws_ssm.py index b6be3a80291..af9133ac96e 100644 --- a/plugins/connection/aws_ssm.py +++ b/plugins/connection/aws_ssm.py @@ -704,7 +704,8 @@ def exec_communicate(self, cmd: str, mark_start: str, mark_begin: str, mark_end: # see https://github.com/pylint-dev/pylint/issues/8909) return (returncode, stdout, self._flush_stderr(self._session)) # pylint: disable=unreachable - def generate_mark(self) -> str: + @staticmethod + def generate_mark() -> str: """Generates a random string of characters to delimit SSM CLI commands""" mark = "".join([random.choice(string.ascii_letters) for i in range(Connection.MARK_LENGTH)]) return mark