Repository: osbuild/osbuild-composer
Pull request #469 (2020-04-06 12:42:21)

jobqeue: worker API fixups
Make the worker API behave a bit better, especially not returning plain text when advertising `application/json`. See commits messages for details.


Patch:
diff --git a/internal/jobqueue/api.go b/internal/jobqueue/api.go
index 4196c0f0b..ed71e152d 100644
--- a/internal/jobqueue/api.go
+++ b/internal/jobqueue/api.go
@@ -2,6 +2,7 @@ package jobqueue
 
 import (
 	"encoding/json"
+	"fmt"
 	"log"
 	"net"
 	"net/http"
@@ -58,43 +59,36 @@ func (api *API) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
 	api.router.ServeHTTP(writer, request)
 }
 
-func methodNotAllowedHandler(writer http.ResponseWriter, request *http.Request) {
-	writer.WriteHeader(http.StatusMethodNotAllowed)
-}
+// jsonErrorf() is similar to http.Error(), but returns the message in a json
+// object with a "message" field.
+func jsonErrorf(writer http.ResponseWriter, code int, message string, args ...interface{}) {
+	writer.WriteHeader(code)
 
-func notFoundHandler(writer http.ResponseWriter, request *http.Request) {
-	writer.WriteHeader(http.StatusNotFound)
+	// ignore error, because we cannot do anything useful with it
+	_ = json.NewEncoder(writer).Encode(&errorResponse{
+		Message: fmt.Sprintf(message, args...),
+	})
 }
 
-func statusResponseOK(writer http.ResponseWriter) {
-	writer.WriteHeader(http.StatusOK)
+func methodNotAllowedHandler(writer http.ResponseWriter, request *http.Request) {
+	jsonErrorf(writer, http.StatusMethodNotAllowed, "method not allowed")
 }
 
-func statusResponseError(writer http.ResponseWriter, code int, errors ...string) {
-	writer.WriteHeader(code)
-
-	for _, err := range errors {
-		_, e := writer.Write([]byte(err))
-		if e != nil {
-			panic(e)
-		}
-	}
+func notFoundHandler(writer http.ResponseWriter, request *http.Request) {
+	jsonErrorf(writer, http.StatusNotFound, "not found")
 }
 
 func (api *API) addJobHandler(writer http.ResponseWriter, request *http.Request, _ httprouter.Params) {
-	type requestBody struct {
-	}
-
 	contentType := request.Header["Content-Type"]
 	if len(contentType) != 1 || contentType[0] != "application/json" {
-		statusResponseError(writer, http.StatusUnsupportedMediaType)
+		jsonErrorf(writer, http.StatusUnsupportedMediaType, "request must contain application/json data")
 		return
 	}
 
-	var body requestBody
+	var body addJobRequest
 	err := json.NewDecoder(request.Body).Decode(&body)
 	if err != nil {
-		statusResponseError(writer, http.StatusBadRequest, "invalid request: "+err.Error())
+		jsonErrorf(writer, http.StatusBadRequest, "%v", err)
 		return
 	}
 
@@ -113,60 +107,57 @@ func (api *API) addJobHandler(writer http.ResponseWriter, request *http.Request,
 func (api *API) updateJobHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
 	contentType := request.Header["Content-Type"]
 	if len(contentType) != 1 || contentType[0] != "application/json" {
-		statusResponseError(writer, http.StatusUnsupportedMediaType)
+		jsonErrorf(writer, http.StatusUnsupportedMediaType, "request must contain application/json data")
 		return
 	}
 
 	id, err := uuid.Parse(params.ByName("job_id"))
 	if err != nil {
-		statusResponseError(writer, http.StatusBadRequest, "invalid compose id: "+err.Error())
+		jsonErrorf(writer, http.StatusBadRequest, "cannot parse compose id: %v", err)
 		return
 	}
 
 	imageBuildId, err := strconv.Atoi(params.ByName("build_id"))
 
 	if err != nil {
-		statusResponseError(writer, http.StatusBadRequest, "invalid image build id: "+err.Error())
+		jsonErrorf(writer, http.StatusBadRequest, "cannot parse image build id: %v", err)
 		return
 	}
 
 	var body updateJobRequest
 	err = json.NewDecoder(request.Body).Decode(&body)
 	if err != nil {
-		statusResponseError(writer, http.StatusBadRequest, "invalid status: "+err.Error())
+		jsonErrorf(writer, http.StatusBadRequest, "cannot parse request body: %v", err)
 		return
 	}
 
 	err = api.store.UpdateImageBuildInCompose(id, imageBuildId, body.Status, body.Result)
 	if err != nil {
 		switch err.(type) {
-		case *store.NotFoundError:
-			statusResponseError(writer, http.StatusNotFound, err.Error())
-		case *store.NotPendingError:
-			statusResponseError(writer, http.StatusNotFound, err.Error())
-		case *store.NotRunningError:
-			statusResponseError(writer, http.StatusBadRequest, err.Error())
-		case *store.InvalidRequestError:
-			statusResponseError(writer, http.StatusBadRequest, err.Error())
+		case *store.NotFoundError, *store.NotPendingError:
+			jsonErrorf(writer, http.StatusNotFound, "%v", err)
+		case *store.NotRunningError, *store.InvalidRequestError:
+			jsonErrorf(writer, http.StatusBadRequest, "%v", err)
 		default:
-			statusResponseError(writer, http.StatusInternalServerError, err.Error())
+			jsonErrorf(writer, http.StatusInternalServerError, "%v", err)
 		}
 		return
 	}
-	statusResponseOK(writer)
+
+	_ = json.NewEncoder(writer).Encode(updateJobResponse{})
 }
 
 func (api *API) addJobImageHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
 	id, err := uuid.Parse(params.ByName("job_id"))
 	if err != nil {
-		statusResponseError(writer, http.StatusBadRequest, "invalid compose id: "+err.Error())
+		jsonErrorf(writer, http.StatusBadRequest, "cannot parse compose id: %v", err)
 		return
 	}
 
 	imageBuildId, err := strconv.Atoi(params.ByName("build_id"))
 
 	if err != nil {
-		statusResponseError(writer, http.StatusBadRequest, "invalid build id: "+err.Error())
+		jsonErrorf(writer, http.StatusBadRequest, "cannot parse image build id: %v", err)
 		return
 	}
 
@@ -175,14 +166,12 @@ func (api *API) addJobImageHandler(writer http.ResponseWriter, request *http.Req
 	if err != nil {
 		switch err.(type) {
 		case *store.NotFoundError:
-			statusResponseError(writer, http.StatusNotFound, err.Error())
+			jsonErrorf(writer, http.StatusNotFound, "%v", err)
 		case *store.NoLocalTargetError:
-			statusResponseError(writer, http.StatusBadRequest, err.Error())
+			jsonErrorf(writer, http.StatusBadRequest, "%v", err)
 		default:
-			statusResponseError(writer, http.StatusInternalServerError, err.Error())
+			jsonErrorf(writer, http.StatusInternalServerError, "%v", err)
 		}
 		return
 	}
-
-	statusResponseOK(writer)
 }
diff --git a/internal/jobqueue/client.go b/internal/jobqueue/client.go
index b9f1f4f1f..96fa7965f 100644
--- a/internal/jobqueue/client.go
+++ b/internal/jobqueue/client.go
@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net"
 	"net/http"
 
@@ -62,11 +61,8 @@ func NewClientUnix(path string) *Client {
 }
 
 func (c *Client) AddJob() (*Job, error) {
-	type request struct {
-	}
-
 	var b bytes.Buffer
-	err := json.NewEncoder(&b).Encode(request{})
+	err := json.NewEncoder(&b).Encode(addJobRequest{})
 	if err != nil {
 		panic(err)
 	}
@@ -77,9 +73,9 @@ func (c *Client) AddJob() (*Job, error) {
 	defer response.Body.Close()
 
 	if response.StatusCode != http.StatusCreated {
-		rawR, _ := ioutil.ReadAll(response.Body)
-		r := string(rawR)
-		return nil, fmt.Errorf("couldn't create job, got %d: %s", response.StatusCode, r)
+		var er errorResponse
+		_ = json.NewDecoder(response.Body).Decode(&er)
+		return nil, fmt.Errorf("couldn't create job, got %d: %s", response.StatusCode, er.Message)
 	}
 
 	var jr addJobResponse
diff --git a/internal/jobqueue/json.go b/internal/jobqueue/json.go
index f324479cd..c6a8f2df9 100644
--- a/internal/jobqueue/json.go
+++ b/internal/jobqueue/json.go
@@ -8,6 +8,13 @@ import (
 	"github.com/osbuild/osbuild-composer/internal/target"
 )
 
+type errorResponse struct {
+	Message string `json:"message"`
+}
+
+type addJobRequest struct {
+}
+
 type addJobResponse struct {
 	ComposeID    uuid.UUID         `json:"compose_id"`
 	ImageBuildID int               `json:"image_build_id"`
@@ -19,3 +26,6 @@ type updateJobRequest struct {
 	Status common.ImageBuildState `json:"status"`
 	Result *common.ComposeResult  `json:"result"`
 }
+
+type updateJobResponse struct {
+}
diff --git a/internal/store/store.go b/internal/store/store.go
index a073eab59..784cc1ff9 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -57,12 +57,12 @@ type Job struct {
 }
 
 type SourceConfig struct {
-	Name     string `json:"name"`
-	Type     string `json:"type"`
-	URL      string `json:"url"`
-	CheckGPG bool   `json:"check_gpg"`
-	CheckSSL bool   `json:"check_ssl"`
-	System   bool   `json:"system"`
+	Name     string `json:"name" toml:"name"`
+	Type     string `json:"type" toml:"type"`
+	URL      string `json:"url" toml:"url"`
+	CheckGPG bool   `json:"check_gpg" toml:"check_gpg"`
+	CheckSSL bool   `json:"check_ssl" toml:"check_ssl"`
+	System   bool   `json:"system" toml:"system"`
 }
 
 type NotFoundError struct {
diff --git a/internal/weldr/api.go b/internal/weldr/api.go
index b4696d89e..93a5e831c 100644
--- a/internal/weldr/api.go
+++ b/internal/weldr/api.go
@@ -379,6 +379,15 @@ func (api *API) sourceNewHandler(writer http.ResponseWriter, request *http.Reque
 		err = errors_package.New("blueprint must be in json or toml format")
 	}
 
+	// Basic check of the source, should at least have a name and type
+	if err == nil {
+		if len(source.Name) == 0 {
+			err = errors_package.New("'name' field is missing from API v0 request")
+		} else if len(source.Type) == 0 {
+			err = errors_package.New("'type' field is missing from API v0 request")
+		}
+	}
+
 	if err != nil {
 		errors := responseError{
 			ID:  "ProjectsError",
diff --git a/internal/weldr/json.go b/internal/weldr/json.go
index be81cce4c..ed767e243 100644
--- a/internal/weldr/json.go
+++ b/internal/weldr/json.go
@@ -100,14 +100,14 @@ func (s *SourceInfoV0) SourceConfig(sourceName string) (ssc store.SourceConfig,
 
 // SourceConfigV0 holds the source repository information
 type SourceConfigV0 struct {
-	Name     string   `json:"name"`
-	Type     string   `json:"type"`
-	URL      string   `json:"url"`
-	CheckGPG bool     `json:"check_gpg"`
-	CheckSSL bool     `json:"check_ssl"`
-	System   bool     `json:"system"`
-	Proxy    string   `json:"proxy"`
-	GPGUrls  []string `json:"gpgkey_urls"`
+	Name     string   `json:"name" toml:"name"`
+	Type     string   `json:"type" toml:"type"`
+	URL      string   `json:"url" toml:"url"`
+	CheckGPG bool     `json:"check_gpg" toml:"check_gpg"`
+	CheckSSL bool     `json:"check_ssl" toml:"check_ssl"`
+	System   bool     `json:"system" toml:"system"`
+	Proxy    string   `json:"proxy" toml:"proxy"`
+	GPGUrls  []string `json:"gpgkey_urls" toml:"gpgkey_urls"`
 }
 
 // SourceConfig returns a SourceConfig struct populated with the supported variables


Test patch:
diff --git a/internal/client/source_test.go b/internal/client/source_test.go
index e8a5eedcd..a38d78eaa 100644
--- a/internal/client/source_test.go
+++ b/internal/client/source_test.go
@@ -109,6 +109,25 @@ func TestPOSTInvalidTOMLSourceV0(t *testing.T) {
 	require.False(t, resp.Status, "did not return an error")
 }
 
+// POST a wrong TOML source
+func TestPOSTWrongTOMLSourceV0(t *testing.T) {
+	// Should not have a [] section
+	source := `
+		[package-repo-toml-v0]
+		name = "package-repo-toml-v0"
+		url = "file://REPO-PATH
+		type = "yum-baseurl"
+		proxy = "https://proxy-url/"
+		check_ssl = true
+		check_gpg = true
+		gpgkey_urls = ["https://url/path/to/gpg-key"]
+	`
+
+	resp, err := PostTOMLSourceV0(testState.socket, source)
+	require.NoError(t, err, "POST source failed with a client error")
+	require.False(t, resp.Status, "did not return an error")
+}
+
 // list sources
 func TestListSourcesV0(t *testing.T) {
 	sources := []string{`{
@@ -183,3 +202,102 @@ func TestGetSourceInfoV0(t *testing.T) {
 	require.NoError(t, err, "DELETE source failed with a client error")
 	require.True(t, resp.Status, "DELETE source failed: %#v", resp)
 }
+
+func UploadUserDefinedSources(t *testing.T, sources []string) {
+	for i := range sources {
+		source := strings.Replace(sources[i], "REPO-PATH", testState.repoDir, 1)
+		resp, err := PostJSONSourceV0(testState.socket, source)
+		require.NoError(t, err, "POST source failed with a client error")
+		require.True(t, resp.Status, "POST source failed: %#v", resp)
+	}
+}
+
+// verify user defined sources are not present
+func VerifyNoUserDefinedSources(t *testing.T, source_names []string) {
+	list, api, err := ListSourcesV0(testState.socket)
+	require.NoError(t, err, "GET source failed with a client error")
+	require.Nil(t, api, "ListSources failed: %#v", api)
+	require.GreaterOrEqual(t, len(list), 1, "Not enough sources returned")
+	for i := range source_names {
+		require.NotContains(t, list, source_names[i])
+	}
+}
+
+func TestDeleteUserDefinedSourcesV0(t *testing.T) {
+	source_names := []string{"package-repo-1", "package-repo-2"}
+	sources := []string{`{
+		"name": "package-repo-1",
+		"url": "file://REPO-PATH",
+		"type": "yum-baseurl",
+		"proxy": "https://proxy-url/",
+		"check_ssl": true,
+		"check_gpg": true,
+		"gpgkey_urls": ["https://url/path/to/gpg-key"]
+	}`,
+		`{
+		"name": "package-repo-2",
+		"url": "file://REPO-PATH",
+		"type": "yum-baseurl",
+		"proxy": "https://proxy-url/",
+		"check_ssl": true,
+		"check_gpg": true,
+		"gpgkey_urls": ["https://url/path/to/gpg-key"]
+	}`}
+
+	// verify test starts without user defined sources
+	VerifyNoUserDefinedSources(t, source_names)
+
+	// post user defined sources
+	UploadUserDefinedSources(t, sources)
+	// note: not verifying user defined sources have been pushed b/c correct
+	// operation of PostJSONSourceV0 is validated in the test functions above
+
+	// Remove the test sources
+	for _, n := range source_names {
+		resp, err := DeleteSourceV0(testState.socket, n)
+		require.NoError(t, err, "DELETE source failed with a client error")
+		require.True(t, resp.Status, "DELETE source failed: %#v", resp)
+	}
+
+	// verify removed sources are not present after removal
+	VerifyNoUserDefinedSources(t, source_names)
+}
+
+func Index(vs []string, t string) int {
+	for i, v := range vs {
+		if v == t {
+			return i
+		}
+	}
+	return -1
+}
+
+func Include(vs []string, t string) bool {
+	return Index(vs, t) >= 0
+}
+
+func TestDeleteSystemSourcesV0(t *testing.T) {
+	sources_list, api, err := ListSourcesV0(testState.socket)
+	require.NoError(t, err, "GET source failed with a client error")
+	require.Nil(t, api, "ListSources failed: %#v", api)
+
+	for _, repo_name := range []string{"test-system-repo", "fedora", "baseos"} {
+		// skip repository names which are not present b/c this test can be
+		// executed both as a unit test and as an integration test
+		if !Include(sources_list, repo_name) {
+			continue
+		}
+
+		// try removing system source
+		resp, err := DeleteSourceV0(testState.socket, repo_name)
+		require.NoError(t, err, "DELETE source failed with a client error")
+		require.True(t, resp.Status, "DELETE source failed: %#v", resp)
+
+		// verify that system sources are still there
+		list, api, err := ListSourcesV0(testState.socket)
+		require.NoError(t, err, "GET source failed with a client error")
+		require.Nil(t, api, "ListSources failed: %#v", api)
+		require.GreaterOrEqual(t, len(list), 1, "Not enough sources returned")
+		require.Contains(t, list, repo_name)
+	}
+}
diff --git a/internal/client/unit_test.go b/internal/client/unit_test.go
index 00e33d87d..d1cde6bf6 100644
--- a/internal/client/unit_test.go
+++ b/internal/client/unit_test.go
@@ -45,7 +45,7 @@ func TestMain(m *testing.M) {
 	if err != nil {
 		panic(err)
 	}
-	repos := []rpmmd.RepoConfig{{Id: "test-id", BaseURL: "http://example.com/test/os/test_arch"}}
+	repos := []rpmmd.RepoConfig{{Id: "test-system-repo", BaseURL: "http://example.com/test/os/test_arch"}}
 	logger := log.New(os.Stdout, "", 0)
 	api := weldr.New(rpm, arch, distro, repos, logger, fixture.Store)
 	server := http.Server{Handler: api}
diff --git a/internal/jobqueue/api_test.go b/internal/jobqueue/api_test.go
index c1a28d413..37d25d818 100644
--- a/internal/jobqueue/api_test.go
+++ b/internal/jobqueue/api_test.go
@@ -13,28 +13,30 @@ import (
 	"github.com/osbuild/osbuild-composer/internal/test"
 )
 
-func TestBasic(t *testing.T) {
+func TestErrors(t *testing.T) {
 	var cases = []struct {
-		Method           string
-		Path             string
-		Body             string
-		ExpectedStatus   int
-		ExpectedResponse string
+		Method         string
+		Path           string
+		Body           string
+		ExpectedStatus int
 	}{
+		// Bogus path
+		{"GET", "/foo", ``, http.StatusNotFound},
 		// Create job with invalid body
-		{"POST", "/job-queue/v1/jobs", ``, http.StatusBadRequest, `invalid request: EOF`},
+		{"POST", "/job-queue/v1/jobs", ``, http.StatusBadRequest},
+		// Wrong method
+		{"GET", "/job-queue/v1/jobs", ``, http.StatusMethodNotAllowed},
 		// Update job with invalid ID
-		{"PATCH", "/job-queue/v1/jobs/foo/builds/0", `{"status":"RUNNING"}`, http.StatusBadRequest, `invalid compose id: invalid UUID length: 3`},
+		{"PATCH", "/job-queue/v1/jobs/foo/builds/0", `{"status":"RUNNING"}`, http.StatusBadRequest},
 		// Update job that does not exist, with invalid body
-		{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", ``, http.StatusBadRequest, `invalid status: EOF`},
+		{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", ``, http.StatusBadRequest},
 		// Update job that does not exist
-		{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", `{"status":"RUNNING"}`, http.StatusNotFound, `compose does not exist`},
+		{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", `{"status":"RUNNING"}`, http.StatusNotFound},
 	}
 
 	for _, c := range cases {
 		api := jobqueue.New(nil, store.New(nil))
-
-		test.TestNonJsonRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedResponse)
+		test.TestRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, "{}", "message")
 	}
 }
 
@@ -60,7 +62,7 @@ func TestCreate(t *testing.T) {
 		`{"compose_id":"`+id.String()+`","image_build_id":0,"manifest":{"sources":{},"pipeline":{}},"targets":[]}`, "created")
 }
 
-func testUpdateTransition(t *testing.T, from, to string, expectedStatus int, expectedResponse string) {
+func testUpdateTransition(t *testing.T, from, to string, expectedStatus int) {
 	distroStruct := fedoratest.New()
 	arch, err := distroStruct.GetArch("x86_64")
 	if err != nil {
@@ -88,39 +90,38 @@ func testUpdateTransition(t *testing.T, from, to string, expectedStatus int, exp
 		}
 	}
 
-	test.TestNonJsonRoute(t, api, false, "PATCH", "/job-queue/v1/jobs/"+id.String()+"/builds/0", `{"status":"`+to+`"}`, expectedStatus, expectedResponse)
+	test.TestRoute(t, api, false, "PATCH", "/job-queue/v1/jobs/"+id.String()+"/builds/0", `{"status":"`+to+`"}`, expectedStatus, "{}", "message")
 }
 
 func TestUpdate(t *testing.T) {
 	var cases = []struct {
-		From             string
-		To               string
-		ExpectedStatus   int
-		ExpectedResponse string
+		From           string
+		To             string
+		ExpectedStatus int
 	}{
-		{"VOID", "WAITING", http.StatusNotFound, "compose does not exist"},
-		{"VOID", "RUNNING", http.StatusNotFound, "compose does not exist"},
-		{"VOID", "FINISHED", http.StatusNotFound, "compose does not exist"},
-		{"VOID", "FAILED", http.StatusNotFound, "compose does not exist"},
-		{"WAITING", "WAITING", http.StatusNotFound, "compose has not been popped"},
-		{"WAITING", "RUNNING", http.StatusNotFound, "compose has not been popped"},
-		{"WAITING", "FINISHED", http.StatusNotFound, "compose has not been popped"},
-		{"WAITING", "FAILED", http.StatusNotFound, "compose has not been popped"},
-		{"RUNNING", "WAITING", http.StatusBadRequest, "invalid state transition: image build cannot be moved into waiting state"},
-		{"RUNNING", "RUNNING", http.StatusOK, ""},
-		{"RUNNING", "FINISHED", http.StatusOK, ""},
-		{"RUNNING", "FAILED", http.StatusOK, ""},
-		{"FINISHED", "WAITING", http.StatusBadRequest, "invalid state transition: image build cannot be moved into waiting state"},
-		{"FINISHED", "RUNNING", http.StatusBadRequest, "invalid state transition: only waiting image build can be transitioned into running state"},
-		{"FINISHED", "FINISHED", http.StatusBadRequest, "invalid state transition: only running image build can be transitioned into finished or failed state"},
-		{"FINISHED", "FAILED", http.StatusBadRequest, "invalid state transition: only running image build can be transitioned into finished or failed state"},
-		{"FAILED", "WAITING", http.StatusBadRequest, "invalid state transition: image build cannot be moved into waiting state"},
-		{"FAILED", "RUNNING", http.StatusBadRequest, "invalid state transition: only waiting image build can be transitioned into running state"},
-		{"FAILED", "FINISHED", http.StatusBadRequest, "invalid state transition: only running image build can be transitioned into finished or failed state"},
-		{"FAILED", "FAILED", http.StatusBadRequest, "invalid state transition: only running image build can be transitioned into finished or failed state"},
+		{"VOID", "WAITING", http.StatusNotFound},
+		{"VOID", "RUNNING", http.StatusNotFound},
+		{"VOID", "FINISHED", http.StatusNotFound},
+		{"VOID", "FAILED", http.StatusNotFound},
+		{"WAITING", "WAITING", http.StatusNotFound},
+		{"WAITING", "RUNNING", http.StatusNotFound},
+		{"WAITING", "FINISHED", http.StatusNotFound},
+		{"WAITING", "FAILED", http.StatusNotFound},
+		{"RUNNING", "WAITING", http.StatusBadRequest},
+		{"RUNNING", "RUNNING", http.StatusOK},
+		{"RUNNING", "FINISHED", http.StatusOK},
+		{"RUNNING", "FAILED", http.StatusOK},
+		{"FINISHED", "WAITING", http.StatusBadRequest},
+		{"FINISHED", "RUNNING", http.StatusBadRequest},
+		{"FINISHED", "FINISHED", http.StatusBadRequest},
+		{"FINISHED", "FAILED", http.StatusBadRequest},
+		{"FAILED", "WAITING", http.StatusBadRequest},
+		{"FAILED", "RUNNING", http.StatusBadRequest},
+		{"FAILED", "FINISHED", http.StatusBadRequest},
+		{"FAILED", "FAILED", http.StatusBadRequest},
 	}
 
 	for _, c := range cases {
-		testUpdateTransition(t, c.From, c.To, c.ExpectedStatus, c.ExpectedResponse)
+		testUpdateTransition(t, c.From, c.To, c.ExpectedStatus)
 	}
 }
diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go
index c5c29efbc..2f7f680c4 100644
--- a/internal/weldr/api_test.go
+++ b/internal/weldr/api_test.go
@@ -878,6 +878,31 @@ check_gpg = false
 	test.SendHTTP(api, true, "DELETE", "/api/v0/projects/source/delete/fish", ``)
 }
 
+// Empty TOML, and invalid TOML should return an error
+func TestSourcesNewWrongToml(t *testing.T) {
+	sources := []string{``, `
+[fish]
+name = "fish"
+url = "https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/"
+type = "yum-baseurl"
+check_ssl = false
+check_gpg = false
+`}
+	for _, source := range sources {
+		req := httptest.NewRequest("POST", "/api/v0/projects/source/new", bytes.NewReader([]byte(source)))
+		req.Header.Set("Content-Type", "text/x-toml")
+		recorder := httptest.NewRecorder()
+
+		api, _ := createWeldrAPI(rpmmd_mock.BaseFixture)
+		api.ServeHTTP(recorder, req)
+
+		r := recorder.Result()
+		if r.StatusCode != http.StatusBadRequest {
+			t.Errorf("unexpected status %v", r.StatusCode)
+		}
+	}
+}
+
 func TestSourcesInfo(t *testing.T) {
 	sourceStr := `{"name":"fish","type":"yum-baseurl","url":"https://download.opensuse.org/repositories/shells:/fish:/release:/3/Fedora_29/","check_gpg":false,"check_ssl":false,"system":false}`
 
diff --git a/jenkins/run_tests.sh b/jenkins/run_tests.sh
index 6ca3a047d..042e299ba 100644
--- a/jenkins/run_tests.sh
+++ b/jenkins/run_tests.sh
@@ -21,7 +21,7 @@ pushd ansible-osbuild
 popd
 
 # Run the integration tests.
-# /usr/libexec/tests/osbuild-composer/osbuild-dnf-json-tests
+/usr/libexec/tests/osbuild-composer/osbuild-dnf-json-tests -test.v
 # /usr/libexec/tests/osbuild-composer/osbuild-image-tests
 /usr/libexec/tests/osbuild-composer/osbuild-rcm-tests -test.v
 # /usr/libexec/tests/osbuild-composer/osbuild-tests -test.v
diff --git a/tools/test-case-generators/generate-test-case b/tools/test-case-generators/generate-test-case
index 7ce4e7705..d1ae3713c 100755
--- a/tools/test-case-generators/generate-test-case
+++ b/tools/test-case-generators/generate-test-case
@@ -26,9 +26,18 @@ It then outputs a json test case as standard output.
 '''
 
 
+def get_subprocess_stdout(*args, **kwargs):
+    sp = subprocess.run(*args, **kwargs, stdout=subprocess.PIPE)
+    if sp.returncode != 0:
+        sys.stderr.write(sp.stdout)
+        sys.exit(1)
+    
+    return sp.stdout
+
+
 def run_osbuild(manifest, store):
     osbuild_cmd = ["osbuild", "--store", store, "--json", "-"]
-    result = json.loads(subprocess.check_output(osbuild_cmd, encoding="utf-8", input=json.dumps(manifest)))
+    result = json.loads(get_subprocess_stdout(osbuild_cmd, encoding="utf-8", input=json.dumps(manifest)))
     return result.get("output_id")
 
 
@@ -37,10 +46,10 @@ def main(test_case, store):
     compose_request = json.dumps(test_case["compose-request"])
 
     pipeline_command = ["go", "run", "./cmd/osbuild-pipeline", "-"]
-    test_case["manifest"] = json.loads(subprocess.check_output(pipeline_command, input=compose_request, encoding="utf-8"))
+    test_case["manifest"] = json.loads(get_subprocess_stdout(pipeline_command, input=compose_request, encoding="utf-8"))
 
     pipeline_command = ["go", "run", "./cmd/osbuild-pipeline", "-rpmmd", "-"]
-    test_case["rpmmd"] = json.loads(subprocess.check_output(pipeline_command, input=compose_request, encoding="utf-8"))
+    test_case["rpmmd"] = json.loads(get_subprocess_stdout(pipeline_command, input=compose_request, encoding="utf-8"))
 
     if boot_type != "nspawn-extract":
         output_id = run_osbuild(test_case["manifest"], store)
@@ -53,7 +62,7 @@ def main(test_case, store):
                 with open(fn, "w") as f:
                     subprocess.run(["xz", "--decompress", "--stdout", image_file], stdout=f)
                 image_file = fn
-        test_case["image-info"] = json.loads(subprocess.check_output(["tools/image-info", image_file]))
+        test_case["image-info"] = json.loads(get_subprocess_stdout(["tools/image-info", image_file], encoding="utf-8"))
 
     return test_case
 
diff --git a/tools/test-case-generators/generate-test-cases b/tools/test-case-generators/generate-test-cases
index 022ed3839..3be0b528d 100755
--- a/tools/test-case-generators/generate-test-cases
+++ b/tools/test-case-generators/generate-test-cases
@@ -6,6 +6,14 @@ import json
 import os
 import sys
 
+def get_subprocess_stdout(*args, **kwargs):
+    sp = subprocess.run(*args, **kwargs, stdout=subprocess.PIPE)
+    if sp.returncode != 0:
+        sys.stderr.write(sp.stdout)
+        sys.exit(1)
+    
+    return sp.stdout
+
 def main(distro, arch, store, output):
     with open("tools/test-case-generators/format-request-map.json") as format_request_json:
         format_request_dict = json.load(format_request_json)
@@ -15,7 +23,7 @@ def main(distro, arch, store, output):
         test_case_request["compose-request"]["distro"] = distro
         test_case_request["compose-request"]["arch"] = arch
         test_case_request["compose-request"]["repositories"] = repos_dict[distro][arch]
-        test_case = json.loads(subprocess.check_output(["tools/test-case-generators/generate-test-case", store], input=json.dumps(test_case_request), encoding="utf-8"))
+        test_case = json.loads(get_subprocess_stdout(["tools/test-case-generators/generate-test-case", store], input=json.dumps(test_case_request), encoding="utf-8"))
         name = distro.replace("-", "_") + "-" + arch + "-" + output_format.replace("-", "_") + "-boot.json"
         file_name = output + "/" + name
         with open(file_name, 'w') as case_file: