fix(agent): separate download and upload paths for agent package transfer

[aewtemp]

Fixes #741

The win_copy and copy modules always expect their source file to be located either on the Ansible controller, or on the target host itself when remote_src: true is set. They cannot transparently bridge a file between two different remote hosts.

Previously, a single path variable (__checkmk_agent_agent.file.*) was used both as the destination for the initial package download (which may run on a delegated host, e.g. localhost, depending on checkmk_agent_delegate_download) and as the source/destination for the subsequent copy task onto the actual target host. When the download was delegated to a host different from the target, this caused the copy task to fail, since the file did not exist at the expected location on the target.

This change splits the previously shared path into two explicit, independent path sets:

• __checkmk_agent_agent.file.download.*: where the package is downloaded to (on the delegated/download host).
• __checkmk_agent_agent.file.upload.*: where the package is copied/uploaded to on the target host, and subsequently installed from.

All download tasks (get_url/win_get_url via
checkmk_agent_delegate_download) now write to file.download.*, while the copy/win_copy tasks explicitly use file.download.* as src and file.upload.* as dest. Installation tasks (package, zypper, win_package) now consistently reference file.upload.*, since that is the final location on the target host.

Additionally:

• checkmk_agent_delegate_download is changed from a hostname-based variable (defaulting to inventory_hostname) to a boolean, making the delegation intent explicit rather than implicit through hostname comparison.
• __checkmk_agent_host_tmp_dir (used for the download destination) is now defined centrally in vars/main.yml, while OS-specific __checkmk_agent_lin_tmp_dir / __checkmk_agent_win_tmp_dir variables are introduced for the upload destination on Linux and Windows targets respectively.

Provide a brief summary of your changes as a title in the textbox above

Use the devel branch as the merge target (see dropdown above)!
We use that branch as a staging area, to make sure the main branch
stays as stable as possible, in case people are using it to install the collection directly.

Pull request type

Try to limit your pull request to one type, submit multiple pull requests if needed.

Check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build related changes
• Documentation content changes
• Other (describe what kind of change you performed):

What is the current behavior?

Describe the current behavior that you are modifying, or link to a relevant issue.

The role "agent" currently is build upon the belief that setting a target_name on "checkmk_agent_delegate_download" would delegate the download and copy transaction of the agent to that host. But the module "windows.win_copy" and "builtin.copy" expect the file to be copied either on the controller or the target host.
Issue Number: #741

What is the new behavior?

Describe the behavior or changes that are being added by this PR.

The changes introduced by this request allow the controller to either be linux or windows based Operating systems.
The "checkmk_agent_delegate_download" variable was changed to a boolean. If true, the download of the file will be handled by the controller. Otherwise the download will be done on the target.

Other information

Any other information that is important to this PR, e.g screenshots of how the component looks before and after the change.

[github-actions]

Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA or my organization already has a signed CLA.

---

1 out of 2 committers have signed the CLA.
✅ (robin-checkmk)[https://github.com/robin-checkmk]
❌ @alexander Temp
Alexander Temp seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.}

[robin-checkmk]

@aewtemp the name/email you made your commits with, does not match the one in your GitHub account. Can you either add the name or make the commit with a name, that matches yours? Worst case you need to copy the files and create a new commit and PR. Let me know, if I can help with that.

We do appreciate your contribution though! I am currently reviewing, so if you need to re-create the PR, maybe wait for my feedback to land.

[robin-checkmk]

I like the idea of splitting the file path into two sets:

__checkmk_agent_agent.file.download.*: where the package is downloaded to (on the delegated/download host).
__checkmk_agent_agent.file.upload.*: where the package is copied/uploaded to on the target host, and subsequently installed from.

What I have doubts and concerns about:

checkmk_agent_delegate_download is changed from a hostname-based variable (defaulting to inventory_hostname) to a boolean, making the delegation intent explicit rather than implicit through hostname comparison.

This is a breaking change, which will affect every single user, who uses the delegation to this day. And while I understand the idea, I would ask you to revert to the previous behavior. Also consider my more detailed comments on this matter.

__checkmk_agent_host_tmp_dir (used for the download destination) is now defined centrally in vars/main.yml, while OS-specific __checkmk_agent_lin_tmp_dir / __checkmk_agent_win_tmp_dir variables are introduced for the upload destination on Linux and Windows targets respectively.

I do not see merrit in this change, why is it necessary?

[robin-checkmk]

This is a breaking change. Can we somehow avoid it?
Coming back to this after reviewing further: I think we can avoid this, as the conditionals you introduced further down are unnecessary, if you keep this filed reserved for the hostname to delegate to.

[aewtemp]

The problem here is, that this configuration option leaves the impression one can randomly choose a delegate to have the files downloaded and then copied from to the target host.

This violates ansibles own assumption for the modules ansible.builtin.copy and ansible.windows.win_copy, whose expectations are that the file to copy has to reside either on the controller or on the target host.
One can only download files directly to the target host, or the controller, to then be able to copy the files to their destination.

So I don't think it would be good to leave the logic behind the delegation as is.

[robin-checkmk]

I think I see your point and I remember considering the option to actually delegate things to a third party host, so it can act as an intermediary. But I am still very hesitent to introduce such a breaking change. Especially, as the issue you describe has not surficed yet. I would rather extend on documentation, instead of an unnecessary breaking change.

[aewtemp]

I fully understand your point.
But hear me out: Just because nobody has encountered the error yet, doesn't mean it's not there. And I am the first person then to have hit that problem.
I am targeting 1700+ hosts with this Playbook. Windows and Linux hosts. I tried delegating the operations for Windows hosts to a third (not controller nor target) Windows machine. It fails.

This is my approach to fix this misconception and the resulting error.

To me this wont break any setup already using this collection in their setups. Because they have not had this error yet.

There is no way this would be working if you choose a third host as delegate for the download and copy operation. Try it yourself.
Besides that Problem, if the controller is a Linux host, you only will be able to transfer files to Linux hosts. The current state does not work when cross-transferring files from Linux to Windows and vice versa.
This will also be fixed with my approach.

I'll fix and commit the outstanding things.

I would appreciate it if you could reconsider this once more.

[robin-checkmk]

Why is this necessary? This feels awefully complex and seems to do the same as the original code.

[aewtemp]

I just came to notice that I left the original inclusion of os_family specific variables above this block.
I'll check if it works without explicitly loading the needed variables for the controller host.