Repository: osbuild/osbuild-composer
Pull request #215 (2020-01-30 12:51:01)

Store virtual image size
Currently, osbuild-composer returns the file size of an image in its compose status/info response. We should instead return the image size that is defined either by the user when creating the compose or is the default value if the user does not define a size. The user defined size is passed to the store when the compose is created but the default image size is only stored in the pipeline's assembler options. So, in order to access the image size even if a user does not define one, I added a function to the assembler options' interface which will return the size specified for the assembler.


Patch:
diff --git a/internal/distro/distro.go b/internal/distro/distro.go
index 6e134276b..40ee85a20 100644
--- a/internal/distro/distro.go
+++ b/internal/distro/distro.go
@@ -31,6 +31,10 @@ type Distro interface {
 	// format. `outputFormat` must be one returned by
 	FilenameFromType(outputFormat string) (string, string, error)
 
+	// Returns the proper image size for a given output format. If the size
+	// is 0 the default value for the format will be returned.
+	GetSizeForOutputType(outputFormat string, size uint64) uint64
+
 	// Returns an osbuild pipeline that generates an image in the given
 	// output format with all packages and customizations specified in the
 	// given blueprint.
diff --git a/internal/distro/fedora30/distro.go b/internal/distro/fedora30/distro.go
index 34a65a21a..16522b91f 100644
--- a/internal/distro/fedora30/distro.go
+++ b/internal/distro/fedora30/distro.go
@@ -321,6 +321,18 @@ func (r *Fedora30) FilenameFromType(outputFormat string) (string, string, error)
 	return "", "", errors.New("invalid output format: " + outputFormat)
 }
 
+func (r *Fedora30) GetSizeForOutputType(outputFormat string, size uint64) uint64 {
+	const MegaByte = 1024 * 1024
+	// Microsoft Azure requires vhd images to be rounded up to the nearest MB
+	if outputFormat == "vhd" && size%MegaByte != 0 {
+		size = (size/MegaByte + 1) * MegaByte
+	}
+	if size == 0 {
+		size = r.outputs[outputFormat].DefaultSize
+	}
+	return size
+}
+
 func (r *Fedora30) Pipeline(b *blueprint.Blueprint, additionalRepos []rpmmd.RepoConfig, checksums map[string]string, outputArchitecture, outputFormat string, size uint64) (*pipeline.Pipeline, error) {
 	output, exists := r.outputs[outputFormat]
 	if !exists {
@@ -397,9 +409,6 @@ func (r *Fedora30) Pipeline(b *blueprint.Blueprint, additionalRepos []rpmmd.Repo
 
 	p.AddStage(pipeline.NewSELinuxStage(r.selinuxStageOptions()))
 
-	if size == 0 {
-		size = output.DefaultSize
-	}
 	p.Assembler = output.Assembler(arch.UEFI, size)
 
 	return p, nil
diff --git a/internal/distro/rhel82/distro.go b/internal/distro/rhel82/distro.go
index d387a1298..f2ed9f499 100644
--- a/internal/distro/rhel82/distro.go
+++ b/internal/distro/rhel82/distro.go
@@ -456,6 +456,18 @@ func (r *RHEL82) FilenameFromType(outputFormat string) (string, string, error) {
 	return "", "", errors.New("invalid output format: " + outputFormat)
 }
 
+func (r *RHEL82) GetSizeForOutputType(outputFormat string, size uint64) uint64 {
+	const MegaByte = 1024 * 1024
+	// Microsoft Azure requires vhd images to be rounded up to the nearest MB
+	if outputFormat == "vhd" && size%MegaByte != 0 {
+		size = (size/MegaByte + 1) * MegaByte
+	}
+	if size == 0 {
+		size = r.outputs[outputFormat].DefaultSize
+	}
+	return size
+}
+
 func (r *RHEL82) Pipeline(b *blueprint.Blueprint, additionalRepos []rpmmd.RepoConfig, checksums map[string]string, outputArchitecture, outputFormat string, size uint64) (*pipeline.Pipeline, error) {
 	output, exists := r.outputs[outputFormat]
 	if !exists {
@@ -537,9 +549,6 @@ func (r *RHEL82) Pipeline(b *blueprint.Blueprint, additionalRepos []rpmmd.RepoCo
 
 	p.AddStage(pipeline.NewSELinuxStage(r.selinuxStageOptions()))
 
-	if size == 0 {
-		size = output.DefaultSize
-	}
 	p.Assembler = output.Assembler(arch.UEFI, size)
 
 	return p, nil
diff --git a/internal/store/store.go b/internal/store/store.go
index 296ca41ac..80e44f732 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -59,6 +59,7 @@ type Compose struct {
 	JobStarted  time.Time            `json:"job_started"`
 	JobFinished time.Time            `json:"job_finished"`
 	Image       *Image               `json:"image"`
+	Size        uint64               `json:"size"`
 }
 
 // A Job contains the information about a compose a worker needs to process it.
@@ -483,6 +484,8 @@ func (s *Store) PushCompose(composeID uuid.UUID, bp *blueprint.Blueprint, checks
 		repos = append(repos, source.RepoConfig())
 	}
 
+	size = s.distro.GetSizeForOutputType(composeType, size)
+
 	pipeline, err := s.distro.Pipeline(bp, repos, checksums, arch, composeType, size)
 	if err != nil {
 		return err
@@ -495,6 +498,7 @@ func (s *Store) PushCompose(composeID uuid.UUID, bp *blueprint.Blueprint, checks
 			OutputType:  composeType,
 			Targets:     targets,
 			JobCreated:  time.Now(),
+			Size:        size,
 		}
 		return nil
 	})
diff --git a/internal/weldr/api.go b/internal/weldr/api.go
index e59e4f8eb..746ee45de 100644
--- a/internal/weldr/api.go
+++ b/internal/weldr/api.go
@@ -1127,21 +1127,29 @@ func (api *API) blueprintsWorkspaceHandler(writer http.ResponseWriter, request *
 	}
 
 	contentType := request.Header["Content-Type"]
-	if len(contentType) != 1 || contentType[0] != "application/json" {
+	if len(contentType) == 0 {
 		errors := responseError{
 			ID:  "BlueprintsError",
-			Msg: "'blueprint must be json'",
+			Msg: "missing Content-Type header",
 		}
 		statusResponseError(writer, http.StatusBadRequest, errors)
 		return
 	}
 
 	var blueprint blueprint.Blueprint
-	err := json.NewDecoder(request.Body).Decode(&blueprint)
+	var err error
+	if contentType[0] == "application/json" {
+		err = json.NewDecoder(request.Body).Decode(&blueprint)
+	} else if contentType[0] == "text/x-toml" {
+		_, err = toml.DecodeReader(request.Body, &blueprint)
+	} else {
+		err = errors.New("blueprint must be in json or toml format")
+	}
+
 	if err != nil {
 		errors := responseError{
 			ID:  "BlueprintsError",
-			Msg: "400 Bad Request: The browser (or proxy) sent a request that this server could not understand.",
+			Msg: "400 Bad Request: The browser (or proxy) sent a request that this server could not understand: " + err.Error(),
 		}
 		statusResponseError(writer, http.StatusBadRequest, errors)
 		return
@@ -1203,7 +1211,6 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request
 		BuildID uuid.UUID `json:"build_id"`
 		Status  bool      `json:"status"`
 	}
-	const MegaByte = 1024 * 1024
 
 	contentType := request.Header["Content-Type"]
 	if len(contentType) != 1 || contentType[0] != "application/json" {
@@ -1249,13 +1256,6 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request
 
 	bp := api.store.GetBlueprintCommitted(cr.BlueprintName)
 
-	size := cr.Size
-
-	// Microsoft Azure requires vhd images to be rounded up to the nearest MB
-	if cr.ComposeType == "vhd" && size%MegaByte != 0 {
-		size = (size/MegaByte + 1) * MegaByte
-	}
-
 	if bp != nil {
 		_, checksums, err := api.depsolveBlueprint(bp, true)
 		if err != nil {
@@ -1267,7 +1267,7 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request
 			return
 		}
 
-		err = api.store.PushCompose(reply.BuildID, bp, checksums, api.arch, cr.ComposeType, size, uploadTarget)
+		err = api.store.PushCompose(reply.BuildID, bp, checksums, api.arch, cr.ComposeType, cr.Size, uploadTarget)
 
 		// TODO: we should probably do some kind of blueprint validation in future
 		// for now, let's just 500 and bail out
@@ -1473,7 +1473,7 @@ func (api *API) composeInfoHandler(writer http.ResponseWriter, request *http.Req
 		Deps        Dependencies         `json:"deps"`      // empty for now
 		ComposeType string               `json:"compose_type"`
 		QueueStatus string               `json:"queue_status"`
-		ImageSize   int64                `json:"image_size"`
+		ImageSize   uint64               `json:"image_size"`
 		Uploads     []UploadResponse     `json:"uploads,omitempty"`
 	}
 
@@ -1485,7 +1485,7 @@ func (api *API) composeInfoHandler(writer http.ResponseWriter, request *http.Req
 	reply.ComposeType = compose.OutputType
 	reply.QueueStatus = compose.QueueStatus
 	if compose.Image != nil {
-		reply.ImageSize = compose.Image.Size
+		reply.ImageSize = compose.Size
 	}
 
 	if isRequestVersionAtLeast(params, 1) {
diff --git a/internal/weldr/compose.go b/internal/weldr/compose.go
index 705188f34..a4355bab3 100644
--- a/internal/weldr/compose.go
+++ b/internal/weldr/compose.go
@@ -12,7 +12,7 @@ type ComposeEntry struct {
 	Blueprint   string           `json:"blueprint"`
 	Version     string           `json:"version"`
 	ComposeType string           `json:"compose_type"`
-	ImageSize   int64            `json:"image_size"`
+	ImageSize   uint64           `json:"image_size"`
 	QueueStatus string           `json:"queue_status"`
 	JobCreated  float64          `json:"job_created"`
 	JobStarted  float64          `json:"job_started,omitempty"`
@@ -43,7 +43,7 @@ func composeToComposeEntry(id uuid.UUID, compose store.Compose, includeUploads b
 
 	case "FINISHED":
 		if compose.Image != nil {
-			composeEntry.ImageSize = compose.Image.Size
+			composeEntry.ImageSize = compose.Size
 		} else {
 			log.Printf("finished compose with id %s has nil image\n", id.String())
 			composeEntry.ImageSize = 0


Test patch:
diff --git a/internal/distro/distro_test.go b/internal/distro/distro_test.go
index 9672f15be..ba4f578fc 100644
--- a/internal/distro/distro_test.go
+++ b/internal/distro/distro_test.go
@@ -48,7 +48,8 @@ func TestDistro_Pipeline(t *testing.T) {
 				t.Errorf("unknown distro: %v", tt.Compose.Distro)
 				return
 			}
-			got, err := d.Pipeline(tt.Compose.Blueprint, nil, tt.Compose.Checksums, tt.Compose.Arch, tt.Compose.OutputFormat, 0)
+			size := d.GetSizeForOutputType(tt.Compose.OutputFormat, 0)
+			got, err := d.Pipeline(tt.Compose.Blueprint, nil, tt.Compose.Checksums, tt.Compose.Arch, tt.Compose.OutputFormat, size)
 			if (err != nil) != (tt.Pipeline == nil) {
 				t.Errorf("distro.Pipeline() error = %v", err)
 				return
diff --git a/internal/distro/test/distro.go b/internal/distro/test/distro.go
index b6f1af18c..c062a8b9b 100644
--- a/internal/distro/test/distro.go
+++ b/internal/distro/test/distro.go
@@ -42,6 +42,10 @@ func (d *TestDistro) FilenameFromType(outputFormat string) (string, string, erro
 	}
 }
 
+func (r *TestDistro) GetSizeForOutputType(outputFormat string, size uint64) uint64 {
+	return 0
+}
+
 func (d *TestDistro) Pipeline(b *blueprint.Blueprint, additionalRepos []rpmmd.RepoConfig, checksums map[string]string, outputArch, outputFormat string, size uint64) (*pipeline.Pipeline, error) {
 	if outputFormat == "test_output" && outputArch == "test_arch" {
 		return &pipeline.Pipeline{}, nil
diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go
index b0cb33a87..ad22e3d9b 100644
--- a/internal/weldr/api_test.go
+++ b/internal/weldr/api_test.go
@@ -101,7 +101,7 @@ version = "2.4.*"`
 	}
 }
 
-func TestBlueprintsWorkspace(t *testing.T) {
+func TestBlueprintsWorkspaceJSON(t *testing.T) {
 	var cases = []struct {
 		Method         string
 		Path           string
@@ -119,6 +119,29 @@ func TestBlueprintsWorkspace(t *testing.T) {
 	}
 }
 
+func TestBlueprintsWorkspaceTOML(t *testing.T) {
+	blueprint := `
+name = "test"
+description = "Test"
+version = "0.0.0"
+
+[[packages]]
+name = "httpd"
+version = "2.4.*"`
+
+	req := httptest.NewRequest("POST", "/api/v0/blueprints/workspace", bytes.NewReader([]byte(blueprint)))
+	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.StatusOK {
+		t.Fatalf("unexpected status %v", r.StatusCode)
+	}
+}
+
 func TestBlueprintsInfo(t *testing.T) {
 	var cases = []struct {
 		Method         string