Bugfix for #17492 "Do not prepend PWD when path is in form user@server:path or server:path"

[pneerincx]

SUMMARY

This fixes the old bug #17492: "Do not prepend PWD when path is in form user@server:path or server:path" (again).
Fix was already in the ansible/ansible repo pull request 33998 as of December 2017, but never got merged.

There are several instances where the _get_absolute_path function is called. This should only happen for relative paths on a local file system and hence

• not for paths on a remote server
• nor for absolute paths.

The synchronize.py plugin checked before calling this function whether it was necessary to call _get_absolute_path.
It should not convert a path to an absolute path when it:

• contains a colon (path on remote server) or
• starts with a slash forward (absolute path).

In some cases the code would perform both checks and in some it failed to check for remote paths. This results in ${PWD} getting prepended to the remote path leading to something like /path/to/PWD/user@server:/path/to/what/needs/to/be/synced.

In this pull request the the two checks for remote paths and absolute paths are relocated to the _get_absolute_path function to remove code redundancy and to ensure the paths are always checked before trying to convert a local relative path into an absolute one.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

synchronize plugin

ADDITIONAL INFORMATION

Bug happens when using synchronize in pull mode. E.g.

synchronize: mode=pull src=rsync://somehost.com/path/ dest=/some/absolute/path/

will fail in any Ansible version with something like:

failed rsync from ${PWD}rsync://somehost.com/path/

[maxamillion]

Thank you for the contribution! This is great and the change looks good, would you mind adding a couple tests cases to tests/integration/targets/synchronize/tasks/main.yml?

Thank you!

[pneerincx]

I've looked at the code for the tests, but have a hard time figuring out how to make this work. All the current tests in tests/integration/targets/synchronize/tasks/main.yml run rsync with both src and dest on a local machine. Hence the r in rsync is never used. The bug described here only occurs when either src or dest is a remote path in the form user@server:/path/to/data. I've tried to simulate a remote path on the local machine with:

1. {{ lookup('env', 'USER') }}@localhost:{{ output_dir }}/foo.txt
2. {{ ansible_user }}@localhost:{{ output_dir }}/foo.txt
3. localhost:{{ output_dir }}/foo.txt

The first option fails because apparently ${USER} is undefined in the test environment as reported by ansible-test env
The second option also does work as {{ ansible_user }} does not exist in the test environment either.
Leaving the user out and only using localhost: will fail if the test environment does not have SSH login with public-private key pairs configured resulting in a password prompt. I can type that password on my laptop, but this won't fly in an automated CI environment...

[maxamillion]

!rebuild

[maxamillion]

/rebuild

[maxamillion]

Closing and reopening to rekick CI since the last time this one ran was on the old CI system

[maxamillion]

This is great, thanks! Approved and merging.