Repository: osbuild/osbuild-composer
Pull request #1921 (2021-10-18 11:58:56)

customizations: add support for fs sizes with custom units
Replaces: #1907

This pull request includes:

- [ ] adequate testing for the new functionality or fixed issue
- [ ] adequate documentation informing people about the change such as
  - [ ] create a file in [news/unreleased](https://github.com/osbuild/osbuild-composer/tree/main/docs/news/unreleased) directory if this change should be mentioned in the release news
  - [ ] submit a PR for the [guides](https://github.com/osbuild/guides) repository if this PR changed any behavior described there: https://www.osbuild.org/guides/

<!--
Thanks for proposing a change to osbuild-composer!

Please don't remove the above check list. These are things that each pull
request must have before it is merged. It helps maintainers to not forget
anything.

If the reason for ticking any of the boxes is ambiguous, please add a short
note explaining why.

For user-visible changes, "adequate documentation" is an entry describing the
change for users in docs/news. Please refer to docs/news/README.md for details.

In addition, if this pull request fixes a downstream issue, please refer to
test/README.md and add these additional items:

- [ ] 1st commit of any `rhbz#` related PR contains bug reproducer; CI reports FAIL or
- [ ] PR contains automated tests for new functionality and
- [ ] QE has approved reproducer/new tests and
- [ ] Subsequent commits provide bug fixes without modifying the reproducer; CI reports PASS and
- [ ] QE approves this PR; RHBZ status is set to `MODIFIED + Verified=Tested`
-->


ondrejbudai: I think that base tests are failing for this PR, can you look at them, please?
msehnout: > I think that base tests are failing for this PR, can you look at them, please?

Fixed ;-)

Patch:
diff --git a/internal/blueprint/customizations.go b/internal/blueprint/customizations.go
index 3b95983b9..58ace04ad 100644
--- a/internal/blueprint/customizations.go
+++ b/internal/blueprint/customizations.go
@@ -1,8 +1,11 @@
 package blueprint
 
 import (
+	"encoding/json"
 	"fmt"
 	"reflect"
+
+	"github.com/osbuild/osbuild-composer/internal/common"
 )
 
 type Customizations struct {
@@ -76,6 +79,64 @@ type FilesystemCustomization struct {
 	MinSize    uint64 `json:"minsize,omitempty" toml:"size,omitempty"`
 }
 
+func (fsc *FilesystemCustomization) UnmarshalTOML(data interface{}) error {
+	d, _ := data.(map[string]interface{})
+
+	switch d["mountpoint"].(type) {
+	case string:
+		fsc.Mountpoint = d["mountpoint"].(string)
+	default:
+		return fmt.Errorf("TOML unmarshal: mountpoint must be string, got %v of type %T", d["mountpoint"], d["mountpoint"])
+	}
+
+	switch d["size"].(type) {
+	case int64:
+		fsc.MinSize = uint64(d["size"].(int64))
+	case string:
+		size, err := common.DataSizeToUint64(d["size"].(string))
+		if err != nil {
+			return fmt.Errorf("TOML unmarshal: size is not valid filesystem size (%w)", err)
+		}
+		fsc.MinSize = size
+	default:
+		return fmt.Errorf("TOML unmarshal: size must be integer or string, got %v of type %T", d["size"], d["size"])
+	}
+
+	return nil
+}
+
+func (fsc *FilesystemCustomization) UnmarshalJSON(data []byte) error {
+	var v interface{}
+	if err := json.Unmarshal(data, &v); err != nil {
+		return err
+	}
+	d, _ := v.(map[string]interface{})
+
+	switch d["mountpoint"].(type) {
+	case string:
+		fsc.Mountpoint = d["mountpoint"].(string)
+	default:
+		return fmt.Errorf("JSON unmarshal: mountpoint must be string, got %v of type %T", d["mountpoint"], d["mountpoint"])
+	}
+
+	// The JSON specification only mentions float64 and Go defaults to it: https://go.dev/blog/json
+	switch d["minsize"].(type) {
+	case float64:
+		// Note that it uses different key than the TOML version
+		fsc.MinSize = uint64(d["minsize"].(float64))
+	case string:
+		size, err := common.DataSizeToUint64(d["minsize"].(string))
+		if err != nil {
+			return fmt.Errorf("JSON unmarshal: size is not valid filesystem size (%w)", err)
+		}
+		fsc.MinSize = size
+	default:
+		return fmt.Errorf("JSON unmarshal: minsize must be float64 number or string, got %v of type %T", d["minsize"], d["minsize"])
+	}
+
+	return nil
+}
+
 type CustomizationError struct {
 	Message string
 }
diff --git a/internal/common/helpers.go b/internal/common/helpers.go
index 04e8a291a..339e5b73f 100644
--- a/internal/common/helpers.go
+++ b/internal/common/helpers.go
@@ -1,8 +1,12 @@
 package common
 
 import (
+	"fmt"
+	"regexp"
 	"runtime"
 	"sort"
+	"strconv"
+	"strings"
 )
 
 var RuntimeGOARCH = runtime.GOARCH
@@ -36,3 +40,51 @@ func IsStringInSortedSlice(slice []string, s string) bool {
 	}
 	return false
 }
+
+// DataSizeToUint64 converts a size specified as a string in KB/KiB/MB/etc. to
+// a number of bytes represented by uint64.
+func DataSizeToUint64(size string) (uint64, error) {
+	// Pre-process the input
+	size = strings.TrimSpace(size)
+
+	// Get the number from the string
+	plain_number := regexp.MustCompile(`[[:digit:]]+`)
+	number_as_str := plain_number.FindString(size)
+	if number_as_str == "" {
+		return 0, fmt.Errorf("the size string doesn't contain any number: %s", size)
+	}
+
+	// Parse the number into integer
+	return_size, err := strconv.ParseUint(number_as_str, 10, 64)
+	if err != nil {
+		return 0, fmt.Errorf("failed to parse size as integer: %s", number_as_str)
+	}
+
+	// List of all supported units (from kB to TB and KiB to TiB)
+	supported_units := []struct {
+		re       *regexp.Regexp
+		multiple uint64
+	}{
+		{regexp.MustCompile(`^\s*[[:digit:]]+\s*kB$`), 1000},
+		{regexp.MustCompile(`^\s*[[:digit:]]+\s*KiB$`), 1024},
+		{regexp.MustCompile(`^\s*[[:digit:]]+\s*MB$`), 1000 * 1000},
+		{regexp.MustCompile(`^\s*[[:digit:]]+\s*MiB$`), 1024 * 1024},
+		{regexp.MustCompile(`^\s*[[:digit:]]+\s*GB$`), 1000 * 1000 * 1000},
+		{regexp.MustCompile(`^\s*[[:digit:]]+\s*GiB$`), 1024 * 1024 * 1024},
+		{regexp.MustCompile(`^\s*[[:digit:]]+\s*TB$`), 1000 * 1000 * 1000 * 1000},
+		{regexp.MustCompile(`^\s*[[:digit:]]+\s*TiB$`), 1024 * 1024 * 1024 * 1024},
+		{regexp.MustCompile(`^\s*[[:digit:]]+$`), 1},
+	}
+
+	for _, unit := range supported_units {
+		if unit.re.MatchString(size) {
+			return_size *= unit.multiple
+			return return_size, nil
+		}
+	}
+
+	// In case the strign didn't match any of the above regexes, return nil
+	// even if a number was found. This is to prevent users from submitting
+	// unknown units.
+	return 0, fmt.Errorf("unknown data size units in string: %s", size)
+}


Test patch:
diff --git a/internal/blueprint/blueprint_test.go b/internal/blueprint/blueprint_test.go
index 2eca4b527..719ae3ffa 100644
--- a/internal/blueprint/blueprint_test.go
+++ b/internal/blueprint/blueprint_test.go
@@ -1,12 +1,58 @@
 package blueprint
 
 import (
+	"encoding/json"
 	"testing"
 
+	"github.com/BurntSushi/toml"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestBlueprintParse(t *testing.T) {
+	blueprint := `
+name = "test"
+description = "Test"
+version = "0.0.0"
+
+[[packages]]
+name = "httpd"
+version = "2.4.*"
+
+[[customizations.filesystem]]
+mountpoint = "/var"
+size = 2147483648
+
+[[customizations.filesystem]]
+mountpoint = "/opt"
+size = "20 GB"
+`
+
+	var bp Blueprint
+	err := toml.Unmarshal([]byte(blueprint), &bp)
+	require.Nil(t, err)
+	assert.Equal(t, bp.Name, "test")
+	assert.Equal(t, "/var", bp.Customizations.Filesystem[0].Mountpoint)
+	assert.Equal(t, uint64(2147483648), bp.Customizations.Filesystem[0].MinSize)
+	assert.Equal(t, "/opt", bp.Customizations.Filesystem[1].Mountpoint)
+	assert.Equal(t, uint64(20*1000*1000*1000), bp.Customizations.Filesystem[1].MinSize)
+
+	blueprint = `{
+		"name": "test",
+		"customizations": {
+		  "filesystem": [{
+			"mountpoint": "/opt",
+			"minsize": "20 GiB"
+		  }]
+		}
+	  }`
+	err = json.Unmarshal([]byte(blueprint), &bp)
+	require.Nil(t, err)
+	assert.Equal(t, bp.Name, "test")
+	assert.Equal(t, "/opt", bp.Customizations.Filesystem[0].Mountpoint)
+	assert.Equal(t, uint64(20*1024*1024*1024), bp.Customizations.Filesystem[0].MinSize)
+}
+
 func TestDeepCopy(t *testing.T) {
 	bpOrig := Blueprint{
 		Name:        "deepcopy-test",
diff --git a/internal/common/helpers_test.go b/internal/common/helpers_test.go
index 467093221..e8ecaf67e 100644
--- a/internal/common/helpers_test.go
+++ b/internal/common/helpers_test.go
@@ -2,8 +2,10 @@ package common
 
 import (
 	"errors"
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCurrentArchAMD64(t *testing.T) {
@@ -52,3 +54,40 @@ func TestIsStringInSortedSlice(t *testing.T) {
 	assert.False(t, IsStringInSortedSlice([]string{"bart", "lisa", "marge"}, ""))
 	assert.False(t, IsStringInSortedSlice([]string{}, "homer"))
 }
+
+func TestDataSizeToUint64(t *testing.T) {
+	cases := []struct {
+		input   string
+		success bool
+		output  uint64
+	}{
+		{"123", true, 123},
+		{"123 kB", true, 123000},
+		{"123 KiB", true, 123 * 1024},
+		{"123 MB", true, 123 * 1000 * 1000},
+		{"123 MiB", true, 123 * 1024 * 1024},
+		{"123 GB", true, 123 * 1000 * 1000 * 1000},
+		{"123 GiB", true, 123 * 1024 * 1024 * 1024},
+		{"123 TB", true, 123 * 1000 * 1000 * 1000 * 1000},
+		{"123 TiB", true, 123 * 1024 * 1024 * 1024 * 1024},
+		{"123kB", true, 123000},
+		{"123KiB", true, 123 * 1024},
+		{" 123  ", true, 123},
+		{"  123kB  ", true, 123000},
+		{"  123KiB  ", true, 123 * 1024},
+		{"123 KB", false, 0},
+		{"123 mb", false, 0},
+		{"123 PB", false, 0},
+		{"123 PiB", false, 0},
+	}
+
+	for _, c := range cases {
+		result, err := DataSizeToUint64(c.input)
+		if c.success {
+			require.Nil(t, err)
+			assert.EqualValues(t, c.output, result)
+		} else {
+			assert.NotNil(t, err)
+		}
+	}
+}