-
Notifications
You must be signed in to change notification settings - Fork 3k
fix: use pointer types for optional ignition config fields to prevent empty objects in JSON #28007
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?
fix: use pointer types for optional ignition config fields to prevent empty objects in JSON #28007
Conversation
1209747 to
bee1374
Compare
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
2 similar comments
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
a104d11 to
f97fb1e
Compare
Signed-off-by: trukhinyuri <yuri@trukhin.com>
f97fb1e to
4824af9
Compare
|
Do you have a concrete example failure? We do have a lot of users running on macos and have macos CI that all seems to run fine, if ignition would fail we could not connect to the machine so we would notice this right away? |
|
Thank you for the question, @Luap99! Concrete Failure Scenario Root Cause Analysis
// Before (problematic) This results in Why CI Might Not Catch This Specific ignition version The Fix This aligns with how optional fields should be handled in ignition configs - if not explicitly set, they should not appear in the JSON at all. Testing podman machine init completed successfully I understand this might be hard to reproduce in CI if it's environment-specific. Would it help if I provided more diagnostic information from my system, such as the full ignition config JSON before/after the fix? |
|
Please do not reply with LLM generated content to my questions, you have provided no additional content in that answer so this is useless. The ignition json generation is almost certainly 100% deterministic so either it fails always like that on all systems if ignition doesn't like that empty json object or it that is simply not the problem and we are fixing nothing here. The spec doesn't mention anything around this if it is an empty object The ignition code clearly check if the replace.Source is not nil https://github.com/coreos/ignition/blob/e66e19916d2049a300a87a5174f25d3efef8b251/internal/exec/config_fetcher.go#L48 And because we have that always as nil here AFAICT your analysis seems wrong and I don't see how this would ever be the cause for your failures, please do not trust unverified LLM output and submit PRs with that. |
Тhis PR fixes an issue where empty config.replace and verification objects were being serialized in the ignition config JSON, which could cause ignition to behave unexpectedly during VM boot.
Problem
When podman machine generates the ignition config, the IgnitionConfig.Replace field (type Resource) and Resource.Verification field (type Verification) are always serialized to JSON, even when empty:
{"ignition":{"config":{"replace":{"verification":{}}}}}The presence of an empty replace object may cause ignition to interpret this as an instruction to replace the config, leading to boot failures on macOS with Apple Silicon.
Solution
Changed fields to pointer types with omitempty JSON tags:
This ensures empty fields are omitted from JSON output.
Testing
Built and tested on macOS with Apple Silicon (M-series)
Verified podman machine starts successfully
Confirmed containers run correctly