Return compose request details as part of metadata response #4451

bcl · 2024-10-29T18:44:04Z

This pull request includes:

adequate testing for the new functionality or fixed issue
adequate documentation informing people about the change such as
- submit a PR for the READMEs listed here
- submit a PR for the osbuild.org website repository if this PR changed any behavior not covered by the automatically updated READMEs

schuellerf

If I got it correct, this is only for on-prem as it will be empty in the service, right?
To get it consistent we would need to extend image-builder to also return the request by just getting it from the image-builder DB (json or postgres) in a subsequent PR.
otherwise it LGTM

schuellerf · 2024-11-27T10:54:16Z

Two questions aside:

where does the issue (triggering this PR) come from, which problem does it solve?
is this ComposeMetadata → request (similar to the ComposeMetadata → packages) big? would it make sense to be able to mask it in the getComposeMetadata request by something like getComposeMetadata → hide_{packages,request}

(it's a bit confusing that there is a request in the response, but I hope my comment is somehow readable)

bcl · 2024-12-02T20:59:09Z

This is part of https://issues.redhat.com/browse/COMPOSER-2131 (which is also related to cockpit-composer using cloudapi instead of weldr).

The request is in the response because I want to reuse the openapi structure instead of reinventing and copying, etc. I don't think size is a consideration, it will be as big as the original request so I don't see any reason to complicate things by trying to hide it.

ETA: And yes, it is on-prem only for now, conditional on the artifact directory being used. Using the db is beyond the scope of this PR and can be added later if needed.

internal/cloudapi/v2/handler.go

Because of how cloud composes are started they may, or may not, have information about the blueprint used to create them. Handle this by leaving those fields blank in the output until PR osbuild/osbuild-composer#4451 is merged. This also translates the existing filter strings so that cloud composes can also be filtered -- but there is no difference between waiting and running so they both return pending cloudapi composes. Related: RHEL-60123

Because of how cloud composes are started they may, or may not, have information about the blueprint used to create them. Handle this by leaving those fields blank in the output until PR osbuild/osbuild-composer#4451 is merged. Related: RHEL-60123

This will allow clients to display more information about a compose, including the image type created, arch and distro, and blueprint to customizations used to create it. Related: RHEL-60120

Related: RHEL-60120

This will help make it easier to write the original compose request json to the same directory tree. Related: RHEL-60120

The original compose request contains useful details that are not preserved when it is converted to a manifest. Things like the distribution, arch, image type, blueprint or customizations are useful when examining builds later. This saves the original request json using the job id and a new directory (ComposeRequest) under the artifacts directory. The original request, if present, is then added to the compose/<id>/metadata response alongside the package list. Related: RHEL-60120

Related: RHEL-60120

thozza

EDIT: the original review was not really relevant, so I deleted it.

thozza · 2025-02-26T12:19:43Z

internal/cloudapi/v2/handler.go

@@ -630,6 +644,7 @@ func (h *apiHandlers) getComposeMetadataImpl(ctx echo.Context, id string) error
 				Id:   jobId.String(),
 				Kind: "ComposeMetadata",
 			},
+			Request: request,


Nitpick: I don't think that the compose request should be returned as is. It can contain e.g. activation key, which is considered a kind of a secret. I'd suggest to filter some fields out.

EDIT: I consider this acceptable if being used only on-prem for now.

internal/cloudapi/v2/handler.go

thozza

Ups, I missed the fact that the save / read function returns nil when the Artifacts Dir is not set... 🙄 😇

All fine...

bcl force-pushed the main-metadata-request branch from ca6d2fb to df51355 Compare November 15, 2024 18:12

bcl requested a review from schuellerf November 26, 2024 23:28

bcl force-pushed the main-metadata-request branch from df51355 to fd7d358 Compare November 26, 2024 23:40

schuellerf previously approved these changes Nov 27, 2024

View reviewed changes

mvo5 reviewed Dec 3, 2024

View reviewed changes

internal/cloudapi/v2/handler.go Outdated Show resolved Hide resolved

bcl dismissed schuellerf’s stale review via 61fab81 December 17, 2024 16:51

bcl force-pushed the main-metadata-request branch from fd7d358 to 61fab81 Compare December 17, 2024 16:51

bcl force-pushed the main-metadata-request branch from 61fab81 to 546fb8b Compare January 11, 2025 00:15

bcl force-pushed the main-metadata-request branch 2 times, most recently from fe0fe5b to 50fa879 Compare February 5, 2025 16:46

bcl force-pushed the main-metadata-request branch from 50fa879 to 9aef543 Compare February 13, 2025 20:01

bcl requested review from thozza and a team as code owners February 13, 2025 20:01

bcl requested review from achilleas-k and schuellerf and removed request for a team February 13, 2025 20:01

schuellerf mentioned this pull request Feb 25, 2025

Add cloudapi support to compose list command osbuild/weldr-client#159

Open

schuellerf force-pushed the main-metadata-request branch from 9aef543 to a597592 Compare February 25, 2025 14:19

bcl added 5 commits February 25, 2025 08:55

openapi: Add ComposeRequest as part of the ComposeMetadata response

ed45f84

This will allow clients to display more information about a compose, including the image type created, arch and distro, and blueprint to customizations used to create it. Related: RHEL-60120

cloudapi: Fix href for metadata responses

ec50b90

Related: RHEL-60120

worker: Expose the ArtifactsDir path

600f66f

This will help make it easier to write the original compose request json to the same directory tree. Related: RHEL-60120

cloudapi: Add a test for metadata returning the compose request

4faf0e2

Related: RHEL-60120

bcl force-pushed the main-metadata-request branch from a597592 to 4faf0e2 Compare February 25, 2025 17:06

thozza requested changes Feb 27, 2025

View reviewed changes

thozza approved these changes Feb 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return compose request details as part of metadata response #4451

Return compose request details as part of metadata response #4451

bcl commented Oct 29, 2024

schuellerf left a comment

schuellerf commented Nov 27, 2024 •

edited

Loading

bcl commented Dec 2, 2024 •

edited

Loading

thozza left a comment •

edited

Loading

thozza Feb 26, 2025 •

edited

Loading

thozza left a comment

Return compose request details as part of metadata response #4451

Are you sure you want to change the base?

Return compose request details as part of metadata response #4451

Conversation

bcl commented Oct 29, 2024

schuellerf left a comment

Choose a reason for hiding this comment

schuellerf commented Nov 27, 2024 • edited Loading

bcl commented Dec 2, 2024 • edited Loading

thozza left a comment • edited Loading

Choose a reason for hiding this comment

thozza Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

thozza left a comment

Choose a reason for hiding this comment

schuellerf commented Nov 27, 2024 •

edited

Loading

bcl commented Dec 2, 2024 •

edited

Loading

thozza left a comment •

edited

Loading

thozza Feb 26, 2025 •

edited

Loading