Repository: osbuild/osbuild-composer
Pull request #182 (2019-12-19 09:05:19)

Improve compose/log routes


Patch:
diff --git a/internal/store/store.go b/internal/store/store.go
index e7cca4db7..f8ea1e9e3 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -3,10 +3,12 @@
 package store
 
 import (
+	"bytes"
 	"crypto/sha1"
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"os"
@@ -252,38 +254,6 @@ func (s *Store) ListBlueprints() []string {
 	return names
 }
 
-func (s *Store) GetCompose(id uuid.UUID) (Compose, bool) {
-	s.mu.RLock()
-	defer s.mu.RUnlock()
-
-	compose, exists := s.Composes[id]
-	return compose, exists
-}
-
-func (s *Store) GetAllComposes() map[uuid.UUID]Compose {
-	s.mu.RLock()
-	defer s.mu.RUnlock()
-
-	composes := make(map[uuid.UUID]Compose)
-
-	for id, compose := range s.Composes {
-		newCompose := compose
-		newCompose.Targets = []*target.Target{}
-
-		for _, t := range compose.Targets {
-			newTarget := *t
-			newCompose.Targets = append(newCompose.Targets, &newTarget)
-		}
-
-		newBlueprint := *compose.Blueprint
-		newCompose.Blueprint = &newBlueprint
-
-		composes[id] = newCompose
-	}
-
-	return composes
-}
-
 func (s *Store) GetBlueprint(name string) (*blueprint.Blueprint, bool) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
@@ -434,6 +404,45 @@ func (s *Store) DeleteBlueprintFromWorkspace(name string) {
 	})
 }
 
+func (s *Store) GetCompose(id uuid.UUID) (Compose, bool) {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	compose, exists := s.Composes[id]
+	return compose, exists
+}
+
+func (s *Store) GetAllComposes() map[uuid.UUID]Compose {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	composes := make(map[uuid.UUID]Compose)
+
+	for id, compose := range s.Composes {
+		newCompose := compose
+		newCompose.Targets = []*target.Target{}
+
+		for _, t := range compose.Targets {
+			newTarget := *t
+			newCompose.Targets = append(newCompose.Targets, &newTarget)
+		}
+
+		newBlueprint := *compose.Blueprint
+		newCompose.Blueprint = &newBlueprint
+
+		composes[id] = newCompose
+	}
+
+	return composes
+}
+
+func (s *Store) GetComposeResult(id uuid.UUID) (io.ReadCloser, error) {
+	if s.stateDir == nil {
+		return ioutil.NopCloser(bytes.NewBuffer([]byte("{}"))), nil
+	}
+	return os.Open(*s.stateDir + "/outputs/" + id.String() + "/result.json")
+}
+
 func (s *Store) PushCompose(composeID uuid.UUID, bp *blueprint.Blueprint, checksums map[string]string, arch, composeType string, uploadTarget *target.Target) error {
 	targets := []*target.Target{}
 
diff --git a/internal/weldr/api.go b/internal/weldr/api.go
index b0694f781..e587ecc40 100644
--- a/internal/weldr/api.go
+++ b/internal/weldr/api.go
@@ -2,6 +2,7 @@ package weldr
 
 import (
 	"archive/tar"
+	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -97,6 +98,7 @@ func New(rpmmd rpmmd.RPMMD, arch string, distro distro.Distro, logger *log.Logge
 	api.router.GET("/api/v:version/compose/failed", api.composeFailedHandler)
 	api.router.GET("/api/v:version/compose/image/:uuid", api.composeImageHandler)
 	api.router.GET("/api/v:version/compose/logs/:uuid", api.composeLogsHandler)
+	api.router.GET("/api/v:version/compose/log/:uuid", api.composeLogHandler)
 	api.router.POST("/api/v:version/compose/uploads/schedule/:uuid", api.uploadsScheduleHandler)
 
 	api.router.DELETE("/api/v:version/upload/delete/:uuid", api.uploadsDeleteHandler)
@@ -1581,22 +1583,116 @@ func (api *API) composeLogsHandler(writer http.ResponseWriter, request *http.Req
 		return
 	}
 
+	resultReader, err := api.store.GetComposeResult(id)
+
+	if err != nil {
+		errors := responseError{
+			ID:  "ComposeError",
+			Msg: fmt.Sprintf("Opening log for compose %s failed", uuidString),
+		}
+		statusResponseError(writer, http.StatusBadRequest, errors)
+		return
+	}
+
+	var result ComposeResult
+	err = json.NewDecoder(resultReader).Decode(&result)
+	if err != nil {
+		errors := responseError{
+			ID:  "ComposeError",
+			Msg: fmt.Sprintf("Parsing log for compose %s failed", uuidString),
+		}
+		statusResponseError(writer, http.StatusBadRequest, errors)
+		return
+	}
+
+	resultReader.Close()
+
 	writer.Header().Set("Content-Disposition", "attachment; filename="+id.String()+"-logs.tar")
 	writer.Header().Set("Content-Type", "application/x-tar")
 
 	tw := tar.NewWriter(writer)
 
-	// TODO: return real log from osbuild
-	fileContents := []byte("SUCCESS\n")
+	// tar format needs to contain file size before the actual file content, therefore the intermediate buffer
+	var fileContents bytes.Buffer
+	result.WriteLog(&fileContents)
 
 	header := &tar.Header{
 		Name: "logs/osbuild.log",
 		Mode: 0644,
-		Size: int64(len(fileContents)),
+		Size: int64(fileContents.Len()),
 	}
 
 	tw.WriteHeader(header)
-	tw.Write(fileContents)
+	io.Copy(tw, &fileContents)
+	tw.Close()
+}
+
+func (api *API) composeLogHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
+	// TODO: implement size param
+	if !verifyRequestVersion(writer, params, 0) {
+		return
+	}
+
+	uuidString := params.ByName("uuid")
+	id, err := uuid.Parse(uuidString)
+	if err != nil {
+		errors := responseError{
+			ID:  "UnknownUUID",
+			Msg: fmt.Sprintf("%s is not a valid build uuid", uuidString),
+		}
+		statusResponseError(writer, http.StatusBadRequest, errors)
+		return
+	}
+
+	compose, exists := api.store.GetCompose(id)
+	if !exists {
+		errors := responseError{
+			ID:  "UnknownUUID",
+			Msg: fmt.Sprintf("Compose %s doesn't exist", uuidString),
+		}
+		statusResponseError(writer, http.StatusBadRequest, errors)
+		return
+	}
+
+	if compose.QueueStatus == "WAITING" {
+		errors := responseError{
+			ID:  "BuildInWrongState",
+			Msg: fmt.Sprintf("Build %s has not started yet. No logs to view.", uuidString),
+		}
+		statusResponseError(writer, http.StatusOK, errors) // weirdly, Lorax returns 200 in this case
+		return
+	}
+
+	if compose.QueueStatus == "RUNNING" {
+		fmt.Fprintf(writer, "Running...\n")
+		return
+	}
+
+	resultReader, err := api.store.GetComposeResult(id)
+
+	if err != nil {
+		errors := responseError{
+			ID:  "ComposeError",
+			Msg: fmt.Sprintf("Opening log for compose %s failed", uuidString),
+		}
+		statusResponseError(writer, http.StatusBadRequest, errors)
+		return
+	}
+
+	var result ComposeResult
+	err = json.NewDecoder(resultReader).Decode(&result)
+	if err != nil {
+		errors := responseError{
+			ID:  "ComposeError",
+			Msg: fmt.Sprintf("Parsing log for compose %s failed", uuidString),
+		}
+		statusResponseError(writer, http.StatusBadRequest, errors)
+		return
+	}
+
+	resultReader.Close()
+
+	result.WriteLog(writer)
 }
 
 func (api *API) composeFinishedHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
diff --git a/internal/weldr/result.go b/internal/weldr/result.go
new file mode 100644
index 000000000..18490ff79
--- /dev/null
+++ b/internal/weldr/result.go
@@ -0,0 +1,72 @@
+package weldr
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+)
+
+type ComposeResult struct {
+	TreeID   string `json:"tree_id"`
+	OutputID string `json:"output_id"`
+	Build    *struct {
+		Stages []struct {
+			Name    string          `json:"name"`
+			Options json.RawMessage `json:"options"`
+			Success bool            `json:"success"`
+			Output  string          `json:"output"`
+		} `json:"stages"`
+		TreeID  string `json:"tree_id"`
+		Success bool   `json:"success"`
+	} `json:"build"`
+	Stages []struct {
+		Name    string          `json:"name"`
+		Options json.RawMessage `json:"options"`
+		Success bool            `json:"success"`
+		Output  string          `json:"output"`
+	} `json:"stages"`
+	Assembler *struct {
+		Name    string          `json:"name"`
+		Options json.RawMessage `json:"options"`
+		Success bool            `json:"success"`
+		Output  string          `json:"output"`
+	} `json:"assembler"`
+	Success bool `json:"success"`
+}
+
+func (cr *ComposeResult) WriteLog(writer io.Writer) {
+	if cr.Build == nil && len(cr.Stages) == 0 && cr.Assembler == nil {
+		fmt.Fprintf(writer, "The compose result is empty.\n")
+	}
+
+	if cr.Build != nil {
+		fmt.Fprintf(writer, "Build pipeline:\n")
+
+		for _, stage := range cr.Build.Stages {
+			fmt.Fprintf(writer, "Stage %s\n", stage.Name)
+			enc := json.NewEncoder(writer)
+			enc.SetIndent("", "  ")
+			enc.Encode(stage.Options)
+			fmt.Fprintf(writer, "\nOutput:\n%s\n", stage.Output)
+		}
+	}
+
+	if len(cr.Stages) > 0 {
+		fmt.Fprintf(writer, "Stages:\n")
+		for _, stage := range cr.Stages {
+			fmt.Fprintf(writer, "Stage: %s\n", stage.Name)
+			enc := json.NewEncoder(writer)
+			enc.SetIndent("", "  ")
+			enc.Encode(stage.Options)
+			fmt.Fprintf(writer, "\nOutput:\n%s\n", stage.Output)
+		}
+	}
+
+	if cr.Assembler != nil {
+		fmt.Fprintf(writer, "Assembler %s:\n", cr.Assembler.Name)
+		enc := json.NewEncoder(writer)
+		enc.SetIndent("", "  ")
+		enc.Encode(cr.Assembler.Options)
+		fmt.Fprintf(writer, "\nOutput:\n%s\n", cr.Assembler.Output)
+	}
+}


Test patch:
diff --git a/internal/test/helpers.go b/internal/test/helpers.go
index 3aa0db197..cd6360e89 100644
--- a/internal/test/helpers.go
+++ b/internal/test/helpers.go
@@ -146,6 +146,25 @@ func TestRoute(t *testing.T, api API, external bool, method, path, body string,
 	}
 }
 
+func TestNonJsonRoute(t *testing.T, api API, external bool, method, path, body string, expectedStatus int, expectedResponse string) {
+	response := SendHTTP(api, external, method, path, body)
+
+	if response.StatusCode != expectedStatus {
+		t.Errorf("%s: expected status %v, but got %v", path, expectedStatus, response.StatusCode)
+	}
+
+	responseBodyBytes, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		t.Errorf("%s: could not read response body: %v", path, err)
+	}
+
+	responseBody := string(responseBodyBytes)
+
+	if responseBody != expectedResponse {
+		t.Errorf("%s: expected response \"%s\", but got \"%s\"", path, expectedResponse, responseBody)
+	}
+}
+
 func IgnoreDates() cmp.Option {
 	return cmp.Comparer(func(a, b time.Time) bool { return true })
 }
diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go
index e4e835183..340528f29 100644
--- a/internal/weldr/api_test.go
+++ b/internal/weldr/api_test.go
@@ -455,14 +455,18 @@ func TestComposeLogs(t *testing.T) {
 		ExpectedFileName           string
 		ExpectedFileContent        string
 	}{
-		{"/api/v0/compose/logs/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-logs.tar", "application/x-tar", "logs/osbuild.log", "SUCCESS\n"},
-		{"/api/v1/compose/logs/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-logs.tar", "application/x-tar", "logs/osbuild.log", "SUCCESS\n"},
+		{"/api/v0/compose/logs/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-logs.tar", "application/x-tar", "logs/osbuild.log", "The compose result is empty.\n"},
+		{"/api/v1/compose/logs/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-logs.tar", "application/x-tar", "logs/osbuild.log", "The compose result is empty.\n"},
 	}
 
 	for _, c := range successCases {
 		api, _ := createWeldrAPI(rpmmd_mock.BaseFixture)
 
 		response := test.SendHTTP(api, false, "GET", c.Path, "")
+		if response.StatusCode != http.StatusOK {
+			t.Errorf("%s: expected status code: %d, but got: %d", c.Path, 200, response.StatusCode)
+		}
+
 		if response.Header.Get("content-disposition") != c.ExpectedContentDisposition {
 			t.Errorf("%s: expected content-disposition: %s, but got: %s", c.Path, c.ExpectedContentDisposition, response.Header.Get("content-disposition"))
 		}
@@ -510,6 +514,32 @@ func TestComposeLogs(t *testing.T) {
 	}
 }
 
+func TestComposeLog(t *testing.T) {
+	var cases = []struct {
+		Fixture          rpmmd_mock.FixtureGenerator
+		Method           string
+		Path             string
+		ExpectedStatus   int
+		ExpectedResponse string
+	}{
+		{rpmmd_mock.BaseFixture, "GET", "/api/v0/compose/log/30000000-0000-0000-0000-000000000000", http.StatusOK, `{"status":false,"errors":[{"id":"BuildInWrongState","msg":"Build 30000000-0000-0000-0000-000000000000 has not started yet. No logs to view."}]}` + "\n"},
+		{rpmmd_mock.BaseFixture, "GET", "/api/v0/compose/log/30000000-0000-0000-0000-000000000001", http.StatusOK, `Running...` + "\n"},
+		{rpmmd_mock.BaseFixture, "GET", "/api/v0/compose/log/30000000-0000-0000-0000-000000000002", http.StatusOK, `The compose result is empty.` + "\n"},
+		{rpmmd_mock.BaseFixture, "GET", "/api/v1/compose/log/30000000-0000-0000-0000-000000000002", http.StatusOK, `The compose result is empty.` + "\n"},
+		{rpmmd_mock.BaseFixture, "GET", "/api/v1/compose/log/30000000-0000-0000-0000", http.StatusBadRequest, `{"status":false,"errors":[{"id":"UnknownUUID","msg":"30000000-0000-0000-0000 is not a valid build uuid"}]}` + "\n"},
+		{rpmmd_mock.BaseFixture, "GET", "/api/v1/compose/log/42000000-0000-0000-0000-000000000000", http.StatusBadRequest, `{"status":false,"errors":[{"id":"UnknownUUID","msg":"Compose 42000000-0000-0000-0000-000000000000 doesn't exist"}]}` + "\n"},
+	}
+
+	if len(os.Getenv("OSBUILD_COMPOSER_TEST_EXTERNAL")) > 0 {
+		t.Skip("This test is for internal testing only")
+	}
+
+	for _, c := range cases {
+		api, _ := createWeldrAPI(rpmmd_mock.BaseFixture)
+		test.TestNonJsonRoute(t, api, false, "GET", c.Path, "", c.ExpectedStatus, c.ExpectedResponse)
+	}
+}
+
 func TestComposeQueue(t *testing.T) {
 	var cases = []struct {
 		Fixture        rpmmd_mock.FixtureGenerator
@@ -629,8 +659,8 @@ func TestSourcesInfo(t *testing.T) {
 
 	api, _ := createWeldrAPI(rpmmd_mock.BaseFixture)
 	test.SendHTTP(api, true, "POST", "/api/v0/projects/source/new", sourceStr)
-	test.TestRoute(t, api, true, "GET", "/api/v0/projects/source/info/fish", ``, 200, `{"sources":{"fish":` + sourceStr + `},"errors":[]}`)
-	test.TestRoute(t, api, true, "GET", "/api/v0/projects/source/info/fish?format=json", ``, 200, `{"sources":{"fish":` + sourceStr + `},"errors":[]}`)
+	test.TestRoute(t, api, true, "GET", "/api/v0/projects/source/info/fish", ``, 200, `{"sources":{"fish":`+sourceStr+`},"errors":[]}`)
+	test.TestRoute(t, api, true, "GET", "/api/v0/projects/source/info/fish?format=json", ``, 200, `{"sources":{"fish":`+sourceStr+`},"errors":[]}`)
 	test.TestRoute(t, api, true, "GET", "/api/v0/projects/source/info/fish?format=son", ``, 400, `{"status":false,"errors":[{"id":"InvalidChars","msg":"invalid format parameter: son"}]}`)
 }