From cfc91a3eb67f4a052086e5bafcabcc775360b96e Mon Sep 17 00:00:00 2001
From: Thomas Lavocat <tlavocat@redhat.com>
Date: Fri, 16 Jul 2021 14:34:41 +0200
Subject: [PATCH] CloudAPI: Invalid package set returns 400

Previously a bad error code was returned, fixes #1477.

Testing:

I have two test cases to test the solution. The first is a request that
makes depsolve crash by changing the dnf-json script by an almost empty
one that only throws an exception. The second one fails because it
requests a non existing package. The former ends with a 500 error and
the later with a 400.

----8<-----

HTTP/1.1 500 Internal Server Error

Failed to depsolve base packages for ami/x86_64/centos-8: ailed to
depsolve base packages for ami/x86_64/centos-8: unexpected end of JSON
input

----8<-----

HTTP/1.1 400 Bad Request

Content-Length: 226Failed to depsolve base packages for
ami/x86_64/centos-8: DNF error occured: MarkingErrors: Error occurred
when marking packages for installation: Problems in request:
missing packages: jesuisunpaquetquinexistepas_idonotexist
---
 internal/cloudapi/server.go | 12 +++++++++++-
 test/cases/api.sh           | 31 +++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/internal/cloudapi/server.go b/internal/cloudapi/server.go
index 7971966a2..a61e2cb55 100644
--- a/internal/cloudapi/server.go
+++ b/internal/cloudapi/server.go
@@ -182,7 +182,17 @@ func (server *Server) Compose(w http.ResponseWriter, r *http.Request) {
 		for name, packages := range packageSets {
 			pkgs, _, err := server.rpmMetadata.Depsolve(packages, repositories, distribution.ModulePlatformID(), arch.Name())
 			if err != nil {
-				http.Error(w, fmt.Sprintf("Failed to depsolve base packages for %s/%s/%s: %s", ir.ImageType, ir.Architecture, request.Distribution, err), http.StatusInternalServerError)
+				var error_type int
+				switch err.(type) {
+				// Known DNF errors falls under BadRequest
+				case *rpmmd.DNFError:
+					error_type = http.StatusBadRequest
+				// All other kind of errors are internal server Errors.
+				// (json marshalling issues for instance)
+				case error:
+					error_type = http.StatusInternalServerError
+				}
+				http.Error(w, fmt.Sprintf("Failed to depsolve base packages for %s/%s/%s: %s", ir.ImageType, ir.Architecture, request.Distribution, err), error_type)
 				return
 			}
 			pkgSpecSets[name] = pkgs
diff --git a/test/cases/api.sh b/test/cases/api.sh
index 93fce1032..87b3df451 100755
--- a/test/cases/api.sh
+++ b/test/cases/api.sh
@@ -960,4 +960,35 @@ curl \
         --header "x-rh-identity: $INVALIDAUTHSTRING" \
         http://localhost:443/api/composer/v1/version)" = "404" ]
 
+#
+# Make sure that requesting a non existing paquet returns a 400 error
+#
+[ "$(curl \
+    --silent \
+    --output /dev/null \
+    --write-out '%{http_code}' \
+    --header "x-rh-identity: $VALIDAUTHSTRING" \
+    -H "Content-Type: application/json" \
+    -d '{ "distribution": "centos-8", "image_requests": [ { "architecture": "x86_64", "image_type": "ami", "repositories": [ { "baseurl": "http://mirror.centos.org/centos/8-stream/BaseOS/x86_64/os/", "rhsm": false }, { "baseurl": "http://mirror.centos.org/centos/8-stream/AppStream/x86_64/os/", "rhsm": false }, { "baseurl": "http://mirror.centos.org/centos/8-stream/extras/x86_64/os/", "rhsm": false } ], "upload_request": { "type": "aws.s3", "options": { "region": "somewhere", "s3": { "access_key_id": "thingy", "secret_access_key": "thing", "bucket": "thingything" } } } } ], "customizations": { "packages": [ "jesuisunpaquetquinexistepas_idonotexist" ] } }' \
+    http://localhost:443/api/composer/v1/compose)" = "400" ]
+
+#
+# Make sure that a request that makes the dnf-json crash returns a 500 error
+#
+sudo cp -f /usr/libexec/osbuild-composer/dnf-json /usr/libexec/osbuild-composer/dnf-json.bak
+sudo cat << EOF | sudo tee /usr/libexec/osbuild-composer/dnf-json
+#!/usr/bin/python3
+raise Exception()
+EOF
+[ "$(curl \
+    --silent \
+    --output /dev/null \
+    --write-out '%{http_code}' \
+    --header "x-rh-identity: $VALIDAUTHSTRING" \
+    -H "Content-Type: application/json" \
+    -d '{ "distribution": "centos-8", "image_requests": [ { "architecture": "x86_64", "image_type": "ami", "repositories": [ { "baseurl": "http://mirror.centos.org/centos/8-stream/BaseOS/x86_64/os/", "rhsm": false }, { "baseurl": "http://mirror.centos.org/centos/8-stream/AppStream/x86_64/os/", "rhsm": false }, { "baseurl": "http://mirror.centos.org/centos/8-stream/extras/x86_64/os/", "rhsm": false } ], "upload_request": { "type": "aws.s3", "options": { "region": "somewhere", "s3": { "access_key_id": "thingy", "secret_access_key": "thing", "bucket": "thingything" } } } } ], "customizations": { "packages": [ "jesuisunpaquetquinexistepas_idonotexist" ] } }' \
+    http://localhost:443/api/composer/v1/compose)" = "500" ]
+
+sudo mv -f /usr/libexec/osbuild-composer/dnf-json.bak /usr/libexec/osbuild-composer/dnf-json
+
 exit 0