Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add send string to wait_for module #54026

Open
wants to merge 5 commits into
base: devel
Choose a base branch
from
Open

Conversation

@heldner
Copy link

@heldner heldner commented Mar 19, 2019

SUMMARY

Useful for checking ssh server. You can send a string with the ssh version and have clear auth.log, without messages like

Did not receive identification string from

Fixes #54024

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

module wait_for

ADDITIONAL INFORMATION

Add the ability to send any string to the socket. This may remove annoying errors in auth.log.

Without this patch during ssh server checks you'll see in auth.log:

sshd[20751]: Did not receive identification string from ::1 port 54116

With patch and with the given parameter send_string equal to the "SSH-2.0-OpenSSH_7.4p1\n" there is no errors:

sshd[20993]: Connection closed by ::1 port 54120 [preauth]
@ansibot
Copy link
Contributor

@ansibot ansibot commented Mar 19, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:464:7: singleton-comparison Comparison to None should be 'expr is not None'

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:464:20: E711 comparison to None should be 'if cond is not None:'

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: E309 version_added for new option (send_string) should be '2.8'. Currently StrictVersion ('2.7')

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

test/integration/targets/wait_for/tasks/main.yml:165:1: empty-lines too many blank lines (1 > 0)

click here for bot help

@ansibot
Copy link
Contributor

@ansibot ansibot commented Mar 20, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:464:7: singleton-comparison Comparison to None should be 'expr is not None'

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:464:20: E711 comparison to None should be 'if cond is not None:'

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: E309 version_added for new option (send_string) should be '2.8'. Currently StrictVersion ('2.7')

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

test/integration/targets/wait_for/tasks/main.yml:165:1: empty-lines too many blank lines (1 > 0)

click here for bot help

Copy link
Contributor

@gregswift gregswift left a comment

Does the version have to match the server's version for ssh? or can it just be anything. Are there other usecases for this aside from ssh?

@@ -82,6 +82,12 @@
- Defaults to a multiline regex.
type: str
version_added: "1.4"
send_string:
description:
- String to send to socket

This comment has been minimized.

@gregswift

gregswift Mar 20, 2019
Contributor

Can you describe the usecase a bit more here? i had to read your PR to understand why i'd use this.

This comment has been minimized.

@heldner

heldner Mar 27, 2019
Author

I expanded the description.

@heldner
Copy link
Author

@heldner heldner commented Mar 27, 2019

It can be used for any service which expect some handshake message. May be some webserver with healthcheck uri, that ensures that service is up and can accept connections.

@ansibot
Copy link
Contributor

@ansibot ansibot commented Mar 27, 2019

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: missing documentation (or could not parse documentation): expected string or buffer

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: missing documentation (or could not parse documentation): expected string or buffer

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.8 [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/utilities/logic/wait_for.py:0:0: E305 DOCUMENTATION.options.send_string.description.1: expected str @ data['options']['send_string']['description'][1]. Got {'For example': 'You can send a string with the ssh version and have clear auth.log.'}

click here for bot help

Copy link
Member

@samdoran samdoran left a comment

Sorry for not looking at this is so long. We're ok adding this feature but it needs a few things before we can merge it. Please address the few comments and create a changelog fragment. See this fragment as an example.

- For example, You can send a string with the ssh version and have clear auth.log.
- Defaults to None.
type: str
version_added: "2.8"

This comment has been minimized.

@samdoran

samdoran Sep 25, 2020
Member

Suggested change
version_added: "2.8"
version_added: "2.11"
@@ -454,6 +462,8 @@ def _create_connection(host, port, connect_timeout):
connect_socket.connect((host, port))
else:
connect_socket = socket.create_connection((host, port), connect_timeout)
if send_string is not None:
connect_socket.sendall(bytes(send_string))

This comment has been minimized.

@samdoran

samdoran Sep 25, 2020
Member

Use our to_bytes() function.

Suggested change
connect_socket.sendall(bytes(send_string))
connect_socket.sendall(to_bytes(send_string))
host: 'localhost'
port: 22
search_regex: OpenSSH
request: 'SSH-2.0-OpenSSH_7.4p1\n'

This comment has been minimized.

@samdoran

samdoran Sep 25, 2020
Member

This may need to change based on the OS it is running against.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants