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

fix network_cli connection plugin: ConnectionError: unsupported operand type(s) for ^: 'str' and 'str' #59174

Closed

Conversation

smnmtzgr
Copy link
Contributor

@smnmtzgr smnmtzgr commented Jul 17, 2019

SUMMARY

Fixes ansible/awx#4325 (comment)
Fixes #48093

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

network_cli.py

ADDITIONAL INFORMATION

network_cli.py breaks when used in AWX 6.0.0. AWX seems to define credentials in special the become with some string, e.g. "true". This leads to a failure:

The full traceback is:
Traceback (most recent call last):
  File "/usr/bin/ansible-connection", line 302, in main
    conn.update_play_context(pc_data)
  File "/usr/lib/python2.7/site-packages/ansible/module_utils/connection.py", line 186, in __rpc__
    raise ConnectionError(to_text(msg, errors='surrogate_then_replace'), code=code)
ConnectionError: unsupported operand type(s) for ^: 'str' and 'str'

With this bugfix the become gets converted to boolean and checked correctly.

@ansibot
Copy link
Contributor

ansibot commented Jul 17, 2019

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. networking Network category small_patch support:network This issue/PR relates to code supported by the Ansible Network Team. traceback This issue/PR includes a traceback. labels Jul 17, 2019
@smnmtzgr
Copy link
Contributor Author

I think this should be a backport bugfix, because it is existent since the ^-operator was introduced in the network_cli plugin. Thats the case since Ansible 2.7 https://github.com/ansible/ansible/blob/stable-2.7/lib/ansible/plugins/connection/network_cli.py

In the earlier versions we had the following logic, which also worked correctly with AWX:

        messages = ['updating play_context for connection']
        if self._play_context.become is False and play_context.become is True:
            auth_pass = play_context.become_pass
            self._terminal.on_become(passwd=auth_pass)
            messages.append('authorizing connection')

        elif self._play_context.become is True and not play_context.become:
            self._terminal.on_unbecome()
            messages.append('deauthorizing connection')

        self._play_context = play_context
        return messages

@ansible-zuul
Copy link

ansible-zuul bot commented Jul 17, 2019

@@ -299,7 +300,7 @@ def update_play_context(self, pc_data):
play_context.deserialize(pc_data)

self.queue_message('vvvv', 'updating play_context for connection')
if self._play_context.become ^ play_context.become:
if boolean(self._play_context.become) ^ boolean(play_context.become):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this should not be needed as become is defined as a boolean attribute, it is an indication that 'validation' is not happening on the play_context objects, which can lead to many of the same issues for other fields.

The proper fix would be to ensure both objects trigger full validation of their attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, can you give me a hint where this validation should be done?

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. labels Jul 17, 2019
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 31, 2019
@justjais justjais removed their assignment Oct 19, 2019
@Qalthos
Copy link
Contributor

Qalthos commented Dec 11, 2019

Can you verify that the issue still occurs without this patch in Ansible 2.9?

needs_info

@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label Dec 11, 2019
@ansibot
Copy link
Contributor

ansibot commented Jan 12, 2020

@smnmtzgr This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Feb 13, 2020

@smnmtzgr You have not responded to information requests in this pullrequest so we will assume it no longer affects you. If you are still interested in this, please create a new pullrequest with the requested information.

click here for bot help

@ansibot ansibot closed this Feb 13, 2020
@ansible ansible locked and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_info This issue requires further information. Please answer any outstanding questions. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. networking Network category small_patch stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:network This issue/PR relates to code supported by the Ansible Network Team. traceback This issue/PR includes a traceback.
Projects
None yet
5 participants