Skip to content

Commit a0232da

Browse files
committed
internal/cloudapi: check JobError in ComposeManifest endpoint
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]>
1 parent 9bdc2da commit a0232da

File tree

2 files changed

+7
-3
lines changed

2 files changed

+7
-3
lines changed

internal/cloudapi/v2/handler.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,9 @@ func (h *apiHandlers) getComposeManifestsImpl(ctx echo.Context, jobId uuid.UUID)
880880
if err != nil {
881881
return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err))
882882
}
883+
if manifestResult.JobError != nil {
884+
return HTTPErrorWithInternal(ErrorFailedToMakeManifest, fmt.Errorf("job %q: %v", jobId, manifestResult.JobError))
885+
}
883886
mf = manifestResult.Manifest
884887
}
885888

@@ -909,6 +912,9 @@ func (h *apiHandlers) getComposeManifestsImpl(ctx echo.Context, jobId uuid.UUID)
909912
if err != nil {
910913
return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err))
911914
}
915+
if manifestResult.JobError != nil {
916+
return HTTPErrorWithInternal(ErrorFailedToMakeManifest, fmt.Errorf("job %q: %v", jobId, manifestResult.JobError))
917+
}
912918
mf = manifestResult.Manifest
913919
}
914920
manifestBlobs = append(manifestBlobs, mf)

internal/cloudapi/v2/v2_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -880,16 +880,14 @@ func TestComposeManifests(t *testing.T) {
880880
jobResult: worker.ManifestJobByIDResult{
881881
Manifest: manifest.OSBuildManifest([]byte(`{"version":"2","pipelines":[{"name":"build"},{"name":"os"}],"sources":{"org.osbuild.curl":{"items":{"sha256:e50ddb78a37f5851d1a5c37a4c77d59123153c156e628e064b9daa378f45a2fe":{"url":"https://pkg1.example.com/1.33-2.fc30.x86_64.rpm"}}}}}`)),
882882
}},
883-
// TODO: this case should actually fail, but it doesn't
884883
{
885884
name: "failure",
886885
jobResult: worker.ManifestJobByIDResult{
887-
Manifest: manifest.OSBuildManifest([]byte(`null`)),
888886
JobResult: worker.JobResult{
889887
JobError: clienterrors.New(clienterrors.ErrorManifestDependency, "Manifest generation test error", "Package XYZ does not have a RemoteLocation"),
890888
},
891889
},
892-
//expectedError: clienterrors.New(clienterrors.ErrorManifestDependency, "Manifest generation test error", "Package XYZ does not have a RemoteLocation"),
890+
expectedError: clienterrors.New(clienterrors.ErrorManifestDependency, "Manifest generation test error", "Package XYZ does not have a RemoteLocation"),
893891
},
894892
}
895893

0 commit comments

Comments
 (0)