Avoid ARG_MAX overflow in bootc image-mode builds

[Lorquas]

Summary

When plan environment contains large values (e.g. base64-encoded SSH keys, certificates, and API tokens from CI systems like Testing Farm), bootc image-mode builds fail with OSError: [Errno 7] Argument list too long: 'ssh'. Two patterns combine to hit the OS ARG_MAX limit:

1. collect_command() inlines the full environment as export statements into every Containerfile RUN directive, multiplying the size
2. build_container() passes the entire generated Containerfile as a cat <<EOF SSH command-line argument

Changes

tmt/package_managers/bootc.py: Transfer the generated Containerfile to the guest via push() (rsync/SCP) instead of inlining it in a cat heredoc SSH command, removing the command-line size ceiling entirely. Uses self.guest.guest_workdir / 'Containerfile' as the local staging path.

Relates to #4518

Testing

• Integration test (tests/prepare/large-env/): End-to-end BeakerLib test that dynamically sizes a base64 environment variable based on the system's MAX_ARG_STRLEN (PAGE_SIZE * 32). The test plan uses two prepare: how: shell steps so the full environment is inlined via collect_command() into two separate RUN directives — making the combined Containerfile exceed MAX_ARG_STRLEN. This reliably triggers the old cat <<EOF failure while passing with the push() fix.
• Verified locally: old code fails with [Errno 7] Argument list too long: 'ssh', new code passes.
• All pre-commit hooks pass (mypy, pyright, ruff, shellcheck, tmt lint, codespell, flake8).

Pull Request Checklist

• implement the feature
• extend the test coverage
• include a release note

[gemini-code-assist]

Code Review

This pull request addresses an issue where large environment variables in bootc image-mode plans could exceed the OS ARG_MAX limit by being inlined into Containerfile RUN directives. The changes introduce a mechanism to emit environment variables as ENV directives once, and switch to using the guest.push() method for writing Containerfiles instead of relying on SSH command-line execution. A new test case has been added to verify this behavior. Please move the imports in tmt/package_managers/bootc.py to the top of the file to comply with PEP 8 standards.

[happz]

env being a parameter of collect_command method: is it guaranteed that two calls to collect_command(), contributing two commands to the same "session" of future Containerfile commands, would have the same env content? For example, collect_command(..., env={'FOO': 'bar'}) followed by collect_command(..., env={'FOO': 'baz'}) would mean the second command runs with unexpected environment, correct?

Would it make sense to simply ENV directives for command's env before each command as they are getting collected, to prevent this behaviour?

Is it possible to somehow clear the environment before emitting new directives for new command? I'm thinking about collect_command(..., env={'DEBUG': '1'}) followed by collect_command(...) - the latter command would inherit environment from the first command, running with DEBUG=1 which was not what the command asked for.

[happz]

Summary

When plan environment contains large values (e.g. base64-encoded SSH keys, certificates, and API tokens from CI systems like Testing Farm), bootc image-mode builds fail with OSError: [Errno 7] Argument list too long: 'ssh'. Two patterns combine to hit the OS ARG_MAX limit:

1. collect_command() inlines the full environment as export statements into every Containerfile RUN directive, multiplying the size
2. build_container() passes the entire generated Containerfile as a cat <<EOF SSH command-line argument

Changes

Fix A (tmt/guest/__init__.py): Emit plan environment variables as Containerfile ENV directives once at the top, rather than repeating export statements in every RUN directive. This is both smaller and more idiomatic for container builds.

This one is problematic, see #4793 (comment) - as long as Containerfile can host multiple commands, collected by a sequence of collect_command(), ENV can cause commands to see unexpected environment.

Fix B (tmt/package_managers/bootc.py): Transfer the generated Containerfile to the guest via push() (rsync/SCP) instead of inlining it in a cat heredoc SSH command, removing the command-line size ceiling entirely.

This one seems like a no-brainer, we absolutely can and should transfer the file via push(). I think you can use self.workdir / Containerfile instead of the temporary file, as the guest should be owned by one thread at one point in time, no other thread would be collecting commands or even thinking about rebuilding the guest, so there's no contest over such a path.

[Lorquas]

@happz Thanks for the hints, that simplifies things tremendously. I tried something like

                local_containerfile = self.workdir / 'Containerfile'
                local_containerfile.write_text(containerfile)

but it seems like by then workdir never exists because when either PackageManager or PackageManagerEngine run their super().__init__(logger=logger) the __init__'s only accept kwargs guest and logger, not parent. So Common.workdir returns None if parent is None. It seems like package managers are kind of detached from the run > plan > step.. workflow? I could attempt to wire that in but it would touch way more moving parts than I am comfortable handling.

[tcornell-bus]

Hi @Lorquas, we would like to propose this for the sprint that starts on May 7th. Would you have the capacity to continue work on this based on your and @happz discussion?

[Lorquas]

@tcornell-bus Yes, I've fixed everything except the super()init problem. I can look at that this week.

[Lorquas]

Updated the branch to wire parent=guest through the PackageManager / PackageManagerEngine constructors, so self.workdir is available in BootcEngine.build_container() — replacing the tempfile approach with self.workdir / 'Containerfile' as suggested.

This means all PackageManager instances now technically have access to workdir, not just the bootc engine. This should be safe: Common.workdir is lazy and only creates its directory on first access. Since no other package manager code references self.workdir that I could find.

Other Note: BootcEngine forwards constructor kwargs to its internal aux_engine (e.g. DNF5), so parent=guest gets passed there too. The aux engine simply gains an unused parent reference. Let me know if you want me to strip that out.

[LecrisUT]

/packit build

[happz]

Updated the branch to wire parent=guest through the PackageManager / PackageManagerEngine constructors, so self.workdir is available in BootcEngine.build_container() — replacing the tempfile approach with self.workdir / 'Containerfile' as suggested.

This means all PackageManager instances now technically have access to workdir, not just the bootc engine. This should be safe: Common.workdir is lazy and only creates its directory on first access. Since no other package manager code references self.workdir that I could find.

Other Note: BootcEngine forwards constructor kwargs to its internal aux_engine (e.g. DNF5), so parent=guest gets passed there too. The aux engine simply gains an unused parent reference. Let me know if you want me to strip that out.

I had to think quite hard to recall why parent becomes needed, and it seems like an unnecessary complication. And I don't remember whether I proposed it, or whether it's based on your own discovery. Would you mind using guest.guest_workdir instead of self.workdir? A package manager is tied to the guest, as such it has no workdir of its own, it will be puzzling in the future.

[Lorquas]

Updated the branch addressing all feedback and rebased on latest main. Forgive me for not replying in each thread, I figure this is better than the notification spam.

I had to think quite hard to recall why parent becomes needed

I also forgot what the original PR even looked like at this point. Here's what I got so far:

@happz — Switched to self.guest.guest_workdir / 'Containerfile' instead of self.workdir, and reverted all the parent= constructor plumbing from PackageManager.__init__ and the call sites in tmt/guest/__init__.py.

@LecrisUT —

• parent= changes fully reverted
• Renamed tests/prepare/bootc-large-env/ to tests/prepare/large-env/, stripped refrences to bootc/image-mode
• Flattened plans.fmf and dropped the /centos-stream-10: subplan structure since test.sh handles things
• large-env.yaml now generated into $run/ temp directory instead of the source data/ tree

[thrix]

Related to #4871 #4875., the fixes there should be revisited on top of this

[LecrisUT]

I figure this is better than the notification spam.

Don't worry about that, we have plenty of notification "spam" we deal on a daily basis 😅. Also incremental commits are fine, we do squash commits.

[happz]

@Lorquas please, check out the tests, they fail in our CI: https://artifacts.dev.testing-farm.io/2689664c-7b34-4464-a180-d2a7d5f80fc2/work-preparez7vj16yz/plans/provision/virtual/prepare/execute/data/guest/default-0/tests/prepare/large-env-3/output.txt. If I'm not mistaken, the large env file needs to be injected into the tree, tmt has a limitation in place to stick to the tree when it comes to loading files.

[psss]

@Lorquas, could you please update the pull request description so that it can be used as the git commit message when squashing? Currently it contains some outdated stuff like Fix A or unit tests. Could you also add a short release note? Thanks.

[psss]

/packit build

[psss]

/packit build