-
Notifications
You must be signed in to change notification settings - Fork 160
feat: add QEMU_ADDITIONAL_PACKAGES environment variable #2266
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
base: main
Are you sure you want to change the base?
feat: add QEMU_ADDITIONAL_PACKAGES environment variable #2266
Conversation
Add support for QEMU_ADDITIONAL_PACKAGES environment variable that allows users to specify additional packages to install in the QEMU microVM during initialization. The variable accepts a comma-separated list of package names (e.g., "hello-wolfi,nginx-stable,strace") and passes them to microvm-init via the kernel command line as melange.additional_packages=<list>. Input validation prevents injection attacks by only allowing alphanumeric characters, hyphens, underscores, commas, and dots. Invalid input is rejected with a warning. Usage: QEMU_ADDITIONAL_PACKAGES=hello-wolfi,strace melange build mypackage.yaml Note: This requires a corresponding update to microvm-init package to read and process the melange.additional_packages kernel parameter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update to use strings.SplitSeq() instead of strings.Split() for better efficiency with the iterator pattern in Go 1.24+. Addresses golangci-lint modernize check.
pkg/container/qemu_runner.go
Outdated
|
|
||
| // Check for QEMU_ADDITIONAL_PACKAGES environment variable | ||
| // Add packages to the initramfs image so they're available during boot | ||
| if additionalPkgs, ok := os.LookupEnv("QEMU_ADDITIONAL_PACKAGES"); ok && additionalPkgs != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth moving this to a new function (getAdditionalPackages) and writing a test to make sure this does what it's supposed to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(and likely the new TESTING code as well although it's not doing nearly as much)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree on the separate function and tests
Also I'd avoid shadowing variables in general
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On it. Added a new function and tests for it
pkg/container/qemu_runner.go
Outdated
| // Use sanitized package list as cache key (replace commas and dots with dashes) | ||
| sanitized := strings.NewReplacer(",", "-", ".", "-").Replace(additionalPkgs) | ||
| if len(sanitized) > 32 { | ||
| sanitized = sanitized[:32] // Limit length for reasonable filenames |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we foresee hitting the edge case where we install enough packages to make this consistent even though we're installing new/different packages after the cutoff?
Would something like a checksum work better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree on checksum, makes it simpler and less likely to have strange chars in it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think that smart and gets us out of dealing with the 4K cmdline problem I think
…L_PACKAGES Addresses PR feedback from @egibs and @89luca89: - Extract getAdditionalPackages() function for parsing env var - Extract getPackageCacheSuffix() function for cache key generation - Use SHA256 hash instead of truncation to avoid collisions - Add comprehensive test coverage for both functions - Fix variable shadowing issue Tests verify: - Package parsing and validation - Security (injection prevention) - Cache suffix generation with SHA256 - Hash determinism and collision prevention 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary
This PR adds support for the
QEMU_ADDITIONAL_PACKAGESenvironment variable, which allows users to specify additional packages to include in the initramfs image during QEMU VM initialization. This complements the existingTESTINGenvironment variable feature gate.Changes
QEMU_ADDITIONAL_PACKAGESsupport inpkg/container/qemu_runner.gohello-wolfi,nginx-stable,strace)^[a-zA-Z0-9_,.-]+$)How It Works
Without QEMU_ADDITIONAL_PACKAGES:
With QEMU_ADDITIONAL_PACKAGES=hello-wolfi,strace:
Packages are installed into the initramfs during the apko build, so they're available immediately when the VM boots - no runtime
apk addneeded!Usage
Use Cases
strace,gdb,tcpdumpfor debugging buildsSecurity
The implementation validates input to prevent shell injection attacks. Only alphanumeric characters, hyphens, underscores, commas, and dots are allowed in package names. Suspicious input is rejected with a warning.
Test Results
Verified that packages are successfully added to the initramfs:
The package is installed in the initramfs and available at boot time ✅