Cloud API: support composes/<id>/manifests endpoint for non-koji builds

Support the composes/<id>/manifests API endpoint for non-koji builds.
The endpoint will have to anyway handle `osbuild` job results once Koji
composes will start using `osbuild` job type for builds.

The endpoint previously contained a bug. If the `osbuild-koji` job had
an empty manifest attached as a static job argument (this is the default
type value), then this empty manifest was added to the endpoint
response. Since Cloud API uses the depsolve and manifest jobs, the
actual manifest was never attached to the job as a static argument. As a
result, the endpoint was always returning an empty manifest for any koji
compose. Fixing this required also adjusting unit tests, which was
relying on the buggy behavior.

Extend the unit test testing a successful compose to test the logs
endpoint.
This commit is contained in:
Tomas Hozza 2022-05-18 14:50:53 +02:00 committed by Tom Gundersen
parent 205dcd4147
commit 4032dea6d2
3 changed files with 91 additions and 23 deletions

View file

@ -917,6 +917,26 @@ func (h *apiHandlers) GetComposeLogs(ctx echo.Context, id string) error {
return ctx.JSON(http.StatusOK, resp)
}
func manifestJobResultsFromJobDeps(w *worker.Server, deps []uuid.UUID) (*worker.ManifestJobByIDResult, error) {
var manifestResult worker.ManifestJobByIDResult
for i := 0; i < len(deps); i++ {
depType, err := w.JobType(deps[i])
if err != nil {
return nil, err
}
if depType == worker.JobTypeManifestIDOnly {
_, _, err = w.ManifestJobStatus(deps[i], &manifestResult)
if err != nil {
return nil, err
}
return &manifestResult, nil
}
}
return nil, fmt.Errorf("no %q job found in the dependencies", worker.JobTypeManifestIDOnly)
}
// GetComposeIdManifests returns the Manifests for a given Compose (one for each image).
func (h *apiHandlers) GetComposeManifests(ctx echo.Context, id string) error {
jobId, err := uuid.Parse(id)
@ -929,43 +949,65 @@ func (h *apiHandlers) GetComposeManifests(ctx echo.Context, id string) error {
return HTTPError(ErrorComposeNotFound)
}
// TODO: support non-koji builds
if jobType != worker.JobTypeKojiFinalize {
return HTTPError(ErrorInvalidJobType)
}
var finalizeResult worker.KojiFinalizeJobResult
_, deps, err := h.server.workers.KojiFinalizeJobStatus(jobId, &finalizeResult)
if err != nil {
return HTTPErrorWithInternal(ErrorComposeNotFound, err)
}
var manifestBlobs []interface{}
for _, id := range deps[1:] {
var buildJob worker.OSBuildKojiJob
err = h.server.workers.OSBuildKojiJob(id, &buildJob)
switch jobType {
case worker.JobTypeKojiFinalize:
var finalizeResult worker.KojiFinalizeJobResult
_, deps, err := h.server.workers.KojiFinalizeJobStatus(jobId, &finalizeResult)
if err != nil {
return HTTPErrorWithInternal(ErrorComposeNotFound, err)
}
for i := 1; i < len(deps); i++ {
var buildJob worker.OSBuildKojiJob
err = h.server.workers.OSBuildKojiJob(deps[i], &buildJob)
if err != nil {
return HTTPErrorWithInternal(ErrorComposeNotFound, err)
}
var manifest distro.Manifest
if len(buildJob.Manifest) != 0 {
manifest = buildJob.Manifest
} else {
_, buildDeps, err := h.server.workers.OSBuildKojiJobStatus(deps[i], &worker.OSBuildKojiJobResult{})
if err != nil {
return HTTPErrorWithInternal(ErrorComposeNotFound, err)
}
manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, buildDeps)
if err != nil {
return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err))
}
manifest = manifestResult.Manifest
}
manifestBlobs = append(manifestBlobs, manifest)
}
case worker.JobTypeOSBuild:
var buildJob worker.OSBuildJob
err = h.server.workers.OSBuildJob(jobId, &buildJob)
if err != nil {
return HTTPErrorWithInternal(ErrorComposeNotFound, err)
}
var manifest distro.Manifest
if len(buildJob.Manifest) == 0 {
if len(buildJob.Manifest) != 0 {
manifest = buildJob.Manifest
} else {
_, deps, err := h.server.workers.OSBuildKojiJobStatus(id, nil)
_, deps, err := h.server.workers.OSBuildJobStatus(jobId, &worker.OSBuildJobResult{})
if err != nil {
return HTTPErrorWithInternal(ErrorComposeNotFound, err)
}
if len(deps) < 2 {
return HTTPErrorWithInternal(ErrorComposeNotFound, err)
}
var manifestResult worker.ManifestJobByIDResult
_, _, err = h.server.workers.ManifestJobStatus(deps[1], &manifestResult)
manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, deps)
if err != nil {
return HTTPErrorWithInternal(ErrorComposeNotFound, err)
return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err))
}
manifest = manifestResult.Manifest
}
manifestBlobs = append(manifestBlobs, manifest)
default:
return HTTPError(ErrorInvalidJobType)
}
resp := &ComposeManifests{

View file

@ -15,6 +15,7 @@ import (
v2 "github.com/osbuild/osbuild-composer/internal/cloudapi/v2"
"github.com/osbuild/osbuild-composer/internal/distro/test_distro"
"github.com/osbuild/osbuild-composer/internal/kojiapi/api"
"github.com/osbuild/osbuild-composer/internal/osbuild2"
osbuild "github.com/osbuild/osbuild-composer/internal/osbuild2"
"github.com/osbuild/osbuild-composer/internal/test"
"github.com/osbuild/osbuild-composer/internal/worker"
@ -510,7 +511,7 @@ func TestKojiCompose(t *testing.T) {
test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v", finalizeID), ``, http.StatusOK, c.composeStatus, `href`, `id`)
// get the manifests
test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v/manifests", finalizeID), ``, http.StatusOK, `{"manifests":[null,null],"kind":"ComposeManifests"}`, `href`, `id`)
test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v/manifests", finalizeID), ``, http.StatusOK, `{"manifests":[{"pipeline":{},"sources":{}},{"pipeline":{},"sources":{}}],"kind":"ComposeManifests"}`, `href`, `id`)
// get the logs
test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v/logs", finalizeID), ``, http.StatusOK, `{"kind":"ComposeLogs"}`, `koji`, `image_builds`, `href`, `id`)
@ -565,6 +566,9 @@ func TestKojiJobTypeValidation(t *testing.T) {
initID, err := workers.EnqueueKojiInit(&initJob, "")
require.NoError(t, err)
manifest, err := json.Marshal(osbuild2.Manifest{})
require.NoErrorf(t, err, "error marshalling empty Manifest to JSON")
buildJobs := make([]worker.OSBuildKojiJob, nImages)
buildJobIDs := make([]uuid.UUID, nImages)
filenames := make([]string, nImages)
@ -575,6 +579,12 @@ func TestKojiJobTypeValidation(t *testing.T) {
KojiServer: "test-server",
KojiDirectory: "koji-server-test-dir",
KojiFilename: fname,
// Add an empty manifest as a static job argument to make the test pass.
// Becasue of a bug in the API, the test was passing even without
// any manifest being attached to the job (static or dynamic).
// In reality, cloudapi never adds the manifest as a static job argument.
// TODO: use dependent depsolve and manifests jobs instead
Manifest: manifest,
}
buildID, err := workers.EnqueueOSBuildKoji(fmt.Sprintf("fake-arch-%d", idx), &buildJob, initID, "")
require.NoError(t, err)
@ -599,6 +609,9 @@ func TestKojiJobTypeValidation(t *testing.T) {
// ----- Jobs queued - Test API endpoints (status, manifests, logs) ----- //
t.Logf("%q job ID: %s", worker.JobTypeKojiInit, initID)
t.Logf("%q job ID: %s", worker.JobTypeKojiFinalize, finalizeID)
t.Logf("%q job IDs: %v", worker.JobTypeOSBuildKoji, buildJobIDs)
for _, path := range []string{"", "/manifests", "/logs"} {
// should return OK - actual result should be tested elsewhere
test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%s%s", finalizeID, path), ``, http.StatusOK, "*")

View file

@ -646,6 +646,19 @@ func TestComposeStatusSuccess(t *testing.T) {
}
]
}`, jobId, jobId))
test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v/manifests", jobId), ``, http.StatusOK, fmt.Sprintf(`
{
"href": "/api/image-builder-composer/v2/composes/%v/manifests",
"id": "%v",
"kind": "ComposeManifests",
"manifests": [
{
"pipeline": {},
"sources": {}
}
]
}`, jobId, jobId))
}
func TestComposeStatusFailure(t *testing.T) {