From 0eb999d51028e3c04f970939f366f5529caea246 Mon Sep 17 00:00:00 2001
From: Achilleas Koutsou <achilleas@koutsou.net>
Date: Tue, 6 Jun 2023 17:21:46 +0200
Subject: [PATCH] cloudapi: remove ostree from imageRequest

Use ostree.ImageOptions for the request parameters instead of a
SourceSpec on the imageRequest.

When preparing the image request, add the ostree values from the API's
compose request to the ostree options on the image options of the image
request.

It's not necessary to create a source spec and it's also not necessary
to add the default ref when it's not specified in the request for an
ostree-based image type.  Both of these will be handled by the Manifest
generation based on the ostree options (imageOptions.OSTree).  The image
functions will take care of setting any missing parameters or returning
errors if any required parameters are missing.
---
 internal/cloudapi/v2/handler.go | 59 ++++++++++++---------------------
 internal/cloudapi/v2/server.go  |  7 ++++
 2 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go
index 870ef12c9..2e2e5210c 100644
--- a/internal/cloudapi/v2/handler.go
+++ b/internal/cloudapi/v2/handler.go
@@ -116,7 +116,6 @@ type imageRequest struct {
 	repositories []rpmmd.RepoConfig
 	imageOptions distro.ImageOptions
 	target       *target.Target
-	ostree       *ostree.SourceSpec
 }
 
 func (h *apiHandlers) PostCompose(ctx echo.Context) error {
@@ -459,43 +458,30 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error {
 			}
 		}
 
-		var ostreeOptions *ostree.SourceSpec
-		// assume it's an ostree image if the type has a default ostree ref
-		if imageType.OSTreeRef() != "" {
-			imageOptions.OSTree = &ostree.ImageOptions{}
-
-			ostreeOptions = &ostree.SourceSpec{}
-			if ir.Ostree != nil {
-				if ir.Ostree.Ref != nil {
-					ostreeOptions.Ref = *ir.Ostree.Ref
-				}
-				if ir.Ostree.Url != nil {
-					ostreeOptions.URL = *ir.Ostree.Url
-				}
-				if ir.Ostree.Contenturl != nil {
-					// URL must be set if content url is specified
-					if ir.Ostree.Url == nil {
-						return HTTPError(ErrorInvalidOSTreeParams)
-					}
-
-					// Contenturl is set on imageoptions directly, as it's not
-					// needed in the resolve job
-					imageOptions.OSTree.ContentURL = *ir.Ostree.Contenturl
-				}
-				if ir.Ostree.Parent != nil {
-					ostreeOptions.Parent = *ir.Ostree.Parent
-				}
-				if ir.Ostree.Rhsm != nil {
-					ostreeOptions.RHSM = *ir.Ostree.Rhsm
-
-					// RHSM needs to be set on imageoptions directly in addition
-					// to the resolve job input
-					imageOptions.OSTree.RHSM = *ir.Ostree.Rhsm
-				}
+		// Add ostree options to image options if they were included in the
+		// request
+		if ir.Ostree != nil {
+			ostreeOptions := &ostree.ImageOptions{}
+			if ir.Ostree.Ref != nil {
+				ostreeOptions.ImageRef = *ir.Ostree.Ref
 			}
-			if ostreeOptions.Ref == "" {
-				ostreeOptions.Ref = imageType.OSTreeRef()
+			if ir.Ostree.Url != nil {
+				ostreeOptions.URL = *ir.Ostree.Url
 			}
+			if ir.Ostree.Contenturl != nil {
+				// URL must be set if content url is specified
+				if ir.Ostree.Url == nil {
+					return HTTPError(ErrorInvalidOSTreeParams)
+				}
+				ostreeOptions.ContentURL = *ir.Ostree.Contenturl
+			}
+			if ir.Ostree.Parent != nil {
+				ostreeOptions.ParentRef = *ir.Ostree.Parent
+			}
+			if ir.Ostree.Rhsm != nil {
+				ostreeOptions.RHSM = *ir.Ostree.Rhsm
+			}
+			imageOptions.OSTree = ostreeOptions
 		}
 
 		var irTarget *target.Target
@@ -712,7 +698,6 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error {
 			repositories: repos,
 			imageOptions: imageOptions,
 			target:       irTarget,
-			ostree:       ostreeOptions,
 		})
 	}
 
diff --git a/internal/cloudapi/v2/server.go b/internal/cloudapi/v2/server.go
index 72ec43729..91cbd467e 100644
--- a/internal/cloudapi/v2/server.go
+++ b/internal/cloudapi/v2/server.go
@@ -531,6 +531,13 @@ func serializeManifest(ctx context.Context, manifestSource *manifest.Manifest, w
 				URL:      resultSpec.URL,
 				Checksum: resultSpec.Checksum,
 			}
+			if resultSpec.RHSM {
+				// NOTE: Older workers don't set the Secrets string in the result
+				// spec so let's add it here for backwards compatibility. This
+				// should be removed after a few versions when all workers have
+				// been updated.
+				resultSpec.Secrets = "org.osbuild.rhsm.consumer"
+			}
 		}
 		ostreeCommitSpecs = map[string][]ostree.CommitSpec{
 			ostreeCommitPipeline: commitSpecs,