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

Resolve Ansible Builder Errors for Podman on Windows #717

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

Conversation

kevinshurtz
Copy link

Hey team!

I recently tried using Ansible Builder to create Ansible execution environments on my Windows machine with Podman, and I ran into a few issues.

  1. By default, Podman on Windows creates files with insufficient permissions to execute copied scripts.
  2. The Containerfile includes Windows path separators in the ENTRYPOINT path, preventing the container from running.

These changes add the --chmod flag to the COPY commands in the Containerfile, ensuring the scripts run correctly and ensuring behavioral parity between Podman and Docker. They also use the POSIX path separator when creating the ENTRYPOINT directive.

I've added unit tests to validate the path character behavior, and I've updated existing tests for the COPY directive. After applying these changes, I was able to use Ansible Builder to create a container with a usable Ansible installation inside (using Fedora as the base image).

If you guys could incorporate these changes into the project when you get the chance, I would deeply appreciate it (and please let me know if there is anything else I can do)!

Sincerely,
Kevin

This fixes an issue for Podman on Windows, which copies over executable
files without the executable bit on, which causes subsequent
Containerfile steps to fail.

By explicitly marking copied files as executable, this issue is
remedied, and Ansible Builder is made usable on more platforms.
Since Containerfiles use a POSIX-like syntax, and I don't believe that
Windows containers are supported base images, I think it's safe to say
that Windows hosts should not insert backslash characters into
entrypoint path for their Containerfiles.
This adds a unit test to make sure that when Windows NT path separator
characters are preferred by os.path, the entrypoint will still be
defined with POSIX path separators.
@kevinshurtz kevinshurtz requested a review from a team as a code owner October 31, 2024 04:04
Copy link

@Shrews
Copy link
Contributor

Shrews commented Dec 5, 2024

Thanks for the contribution!

Unfortunately, we don't really support Windows for builder, and do not have any testing for that platform in our CI. There are currently no plans to add support either. However, I'm totally fine with the addition of the --chmod portion of the change. If you care to remove the Windows-specific bits (mainly the new test, and stick to os.path), then I think I'd be ok with this change.

@kevinshurtz
Copy link
Author

Thanks for the contribution!

Unfortunately, we don't really support Windows for builder, and do not have any testing for that platform in our CI. There are currently no plans to add support either. However, I'm totally fine with the addition of the --chmod portion of the change. If you care to remove the Windows-specific bits (mainly the new test, and stick to os.path), then I think I'd be ok with this change.

Hey @Shrews, thanks for the reply!

While I'm happy to remove the path-handling changes, if there is no intention to support Windows for Ansible Builder, what is the purpose of using os.path anywhere in the codebase? As I understand, the purpose of this module is to select POSIX path separators on Unix-like systems and use Windows path conventions on Windows.

Since the path-handling changes in the PR merely force the usage of POSIX conventions when creating the Containerfile, it feels ironic and self-defeating to reintroduce Windows-specific path separators for the sole purpose of ensuring the project continues to misbehave on Windows.

If you would still prefer that I remove path-handling changes to ensure continued misbehavior on Windows, I'm of course still happy to do so. Thank you again for the feedback, and please let me know if there is anything else I can share!

Sincerely,
Kevin

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

Successfully merging this pull request may close these issues.

2 participants