Skip to content

Conversation

thozza
Copy link
Member

@thozza thozza commented Oct 6, 2025

While porting osbuild/images#1907 to osbuild-composer, I ran into the problem that the CloudAPI endpoint for compose manifests was not checking manifest generation job results and would return a 'nil' value instead of an error if the manifest generation failed.

Moreover, the mock data used by unit tests for depsolved packages was not complete and was lacking basic properties, such as the remote location URL, which is needed to generate an osbuild manifest.

This PR addresses the issues mentioned above, it also adds a unit test for compose manifests Cloud API endpoint and improves unit tests where failure debugging was previously unnecessarily complicated.

/jira-epic HMS-8910

JIRA: HMS-9501

thozza added 6 commits October 6, 2025 19:56
When checking the response status code in APICall.Do(), print response
body if the check fails. This helps with debugging test failures.

Signed-off-by: Tomáš Hozza <[email protected]>
The CloudAPI unit tests use mock data for depsolve job results.
Previously, the returned depsolve job results were using dummy package
which was missing certain required properties that are needed in order
to generate an osbuild manifest, specifically the package's remote
location URL.

Ensure that the mock depsolve job results contain the minimal, but
complete, dummy package specification.

Signed-off-by: Tomáš Hozza <[email protected]>
The multi-tenancy test case uses a simplified flow when all jobs are
finished with empty results. Subsequently, all relevant compose
endpoints are checked.

As a result, the manifest generation would always fail on empty
depsolved packages list, because the depsolve job results were
empty. This didn't used to cause any issues, because the compose
manifests API endpoint was not checking for manifest job errors and
would happily return a 'nil' manifest. Fixing the compose manifests
endpoint to not return 'nil', but an error instead breaks the unit
test.

Fix the unit test by ensuring that a reasonable minimal depsolve job
results are returned, so that a manifest can be successfully generated.

Signed-off-by: Tomáš Hozza <[email protected]>
Add a new unit test TestComposeManifests to verify the compose manifests
API endpoint responses when the manifest generation job succeeds and
fails. Due to the handling of the manifest generation job on the server
side, I had to modify the server code to allow overriding the
serializeManifest() function for testing purposes.

Note that the ComposeManifests endpoint incorrectly returns 'null' as a
manifest value in case the manifest generation job failed. This will be
fixed later.

Signed-off-by: Tomáš Hozza <[email protected]>
When CloudAPI serializes the osbuild manifest, it throws away the error
details in case the serialization fails. Add the serialization error to
the JobError details. This will make debugging of manifest serialization
much easier.

Signed-off-by: Tomáš Hozza <[email protected]>
The API endpoint used to check only for errors when getting the osbuild
manifest from job dependencies results. However, if the actual manifest
serialization in the respective job failed, the endpoint would return no
error, but instead `nil` as the manifest value.

Make sure to check also the manifest generation job JobError, before
returning the manifest, and return the error instead if it is set.

Signed-off-by: Tomáš Hozza <[email protected]>
@thozza thozza requested a review from a team as a code owner October 6, 2025 18:45
@thozza thozza requested review from achilleas-k, mvo5 and supakeen and removed request for a team October 6, 2025 18:45
@schutzbot schutzbot changed the title CloudAPI: fix /compose/{id}/manifests endpoint behavior on manifest generation errors CloudAPI: fix /compose/{id}/manifests endpoint behavior on manifest generation errors (HMS-9501) Oct 6, 2025
@thozza thozza requested a review from croissanne October 6, 2025 18:51
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice fixes here. Thank you.

LGTM!

@thozza thozza enabled auto-merge (rebase) October 7, 2025 08:27
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! A tiny nitpick inline for your consideration

@thozza thozza merged commit a0232da into osbuild:main Oct 7, 2025
52 checks passed
@thozza thozza deleted the cloudapi-fix-compose-manifests branch October 7, 2025 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants