datasize, blueprint: add new datasizes.Size type and use in blueprints

pkg/blueprint/blueprint_test.go

@@ -65,9 +65,9 @@ minsize = "20 GiB"
 	assert.Equal(t, bp.Version, "0.0.0")
 	assert.Equal(t, bp.Packages, []Package{{Name: "httpd", Version: "2.4.*"}})
 	assert.Equal(t, "/var", bp.Customizations.Filesystem[0].Mountpoint)
-	assert.Equal(t, uint64(2147483648), bp.Customizations.Filesystem[0].MinSize)
+	assert.Equal(t, uint64(2147483648), bp.Customizations.Filesystem[0].MinSize.Uint64())
 	assert.Equal(t, "/opt", bp.Customizations.Filesystem[1].Mountpoint)
-	assert.Equal(t, uint64(20*datasizes.GiB), bp.Customizations.Filesystem[1].MinSize)
+	assert.Equal(t, uint64(20*datasizes.GiB), bp.Customizations.Filesystem[1].MinSize.Uint64())
 }
 
 func TestGetPackages(t *testing.T) {

pkg/blueprint/customizations.go

@@ -304,7 +304,7 @@ func (c *Customizations) GetFilesystemsMinSize() uint64 {
 	}
 	var agg uint64
 	for _, m := range c.Filesystem {
-		agg += m.MinSize
+		agg += m.MinSize.Uint64()
 	}
 	// This ensures that file system customization `size` is a multiple of
 	// sector size (512)

pkg/blueprint/customizations_test.go

@@ -3,9 +3,10 @@ package blueprint
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/osbuild/images/internal/common"
 	"github.com/osbuild/images/pkg/customizations/anaconda"
-	"github.com/stretchr/testify/assert"
 )
 
 func TestCheckAllowed(t *testing.T) {

pkg/blueprint/disk_customizations.go

@@ -9,12 +9,13 @@ import (
 	"slices"
 	"strings"
 
+	"github.com/osbuild/images/pkg/datasizes"
 	"github.com/osbuild/images/pkg/pathpolicy"
 )
 
 type DiskCustomization struct {
 	// TODO: Add partition table type: gpt or dos
-	MinSize    uint64                   `json:"minsize,omitempty" toml:"minsize,omitempty"`
+	MinSize    datasizes.Size           `json:"minsize,omitempty" toml:"minsize,omitempty"`
 	Partitions []PartitionCustomization `json:"partitions,omitempty" toml:"partitions,omitempty"`
 }
 
@@ -37,7 +38,7 @@ type PartitionCustomization struct {
 	// addition, certain mountpoints have required minimum sizes. See
 	// https://osbuild.org/docs/user-guide/partitioning for more details.
 	// (optional, defaults depend on payload and mountpoints).
-	MinSize uint64 `json:"minsize" toml:"minsize"`
+	MinSize datasizes.Size `json:"minsize" toml:"minsize"`
 
 	BtrfsVolumeCustomization
 
@@ -71,36 +72,11 @@ type LVCustomization struct {
 	Name string `json:"name,omitempty" toml:"name,omitempty"`
 
 	// Minimum size of the logical volume
-	MinSize uint64 `json:"minsize,omitempty" toml:"minsize,omitempty"`
+	MinSize datasizes.Size `json:"minsize,omitempty" toml:"minsize,omitempty"`
 
 	FilesystemTypedCustomization
 }
 
-// Custom JSON unmarshaller for LVCustomization for handling the conversion of
-// data sizes (minsize) expressed as strings to uint64.
-func (lv *LVCustomization) UnmarshalJSON(data []byte) error {
-	var lvAnySize struct {
-		Name    string `json:"name,omitempty" toml:"name,omitempty"`
-		MinSize any    `json:"minsize,omitempty" toml:"minsize,omitempty"`
-		FilesystemTypedCustomization
-	}
-	if err := json.Unmarshal(data, &lvAnySize); err != nil {
-		return err
-	}
-
-	lv.Name = lvAnySize.Name
-	lv.FilesystemTypedCustomization = lvAnySize.FilesystemTypedCustomization
-
-	if lvAnySize.MinSize != nil {
-		size, err := decodeSize(lvAnySize.MinSize)
-		if err != nil {
-			return err
-		}
-		lv.MinSize = size
-	}
-	return nil
-}
-
 // A btrfs volume consisting of one or more subvolumes.
 type BtrfsVolumeCustomization struct {
 	Subvolumes []BtrfsSubvolumeCustomization
@@ -124,8 +100,7 @@ type BtrfsSubvolumeCustomization struct {
 func (v *PartitionCustomization) UnmarshalJSON(data []byte) error {
 	errPrefix := "JSON unmarshal:"
 	var typeSniffer struct {
-		Type    string `json:"type"`
-		MinSize any    `json:"minsize"`
+		Type string `json:"type"`
 	}
 	if err := json.Unmarshal(data, &typeSniffer); err != nil {
 		return fmt.Errorf("%s %w", errPrefix, err)
@@ -155,14 +130,6 @@ func (v *PartitionCustomization) UnmarshalJSON(data []byte) error {
 
 	v.Type = partType
 
-	if typeSniffer.MinSize != nil {
-		minsize, err := decodeSize(typeSniffer.MinSize)
-		if err != nil {
-			return fmt.Errorf("%s error decoding minsize for partition: %w", errPrefix, err)
-		}
-		v.MinSize = minsize
-	}
-
 	return nil
 }
 
@@ -171,10 +138,10 @@ func (v *PartitionCustomization) UnmarshalJSON(data []byte) error {
 // the type is "plain", none of the fields for btrfs or lvm are used.
 func decodePlain(v *PartitionCustomization, data []byte) error {
 	var plain struct {
-		// Type and minsize are handled by the caller. These are added here to
+		// Type is handled by the caller. These are added here to
 		// satisfy "DisallowUnknownFields" when decoding.
-		Type    string `json:"type"`
-		MinSize any    `json:"minsize"`
+		Type    string         `json:"type"`
+		MinSize datasizes.Size `json:"minsize"`
 		FilesystemTypedCustomization
 	}
 
@@ -186,6 +153,7 @@ func decodePlain(v *PartitionCustomization, data []byte) error {
 	}
 
 	v.FilesystemTypedCustomization = plain.FilesystemTypedCustomization
+	v.MinSize = plain.MinSize
 	return nil
 }
 
@@ -194,10 +162,10 @@ func decodePlain(v *PartitionCustomization, data []byte) error {
 // the type is btrfs, none of the fields for plain or lvm are used.
 func decodeBtrfs(v *PartitionCustomization, data []byte) error {
 	var btrfs struct {
-		// Type and minsize are handled by the caller. These are added here to
+		// Type is handled by the caller. These are added here to
 		// satisfy "DisallowUnknownFields" when decoding.
-		Type    string `json:"type"`
-		MinSize any    `json:"minsize"`
+		Type    string         `json:"type"`
+		MinSize datasizes.Size `json:"minsize"`
 		BtrfsVolumeCustomization
 	}
 
@@ -209,6 +177,7 @@ func decodeBtrfs(v *PartitionCustomization, data []byte) error {
 	}
 
 	v.BtrfsVolumeCustomization = btrfs.BtrfsVolumeCustomization
+	v.MinSize = btrfs.MinSize
 	return nil
 }
 
@@ -217,10 +186,10 @@ func decodeBtrfs(v *PartitionCustomization, data []byte) error {
 // is lvm, none of the fields for plain or btrfs are used.
 func decodeLVM(v *PartitionCustomization, data []byte) error {
 	var vg struct {
-		// Type and minsize are handled by the caller. These are added here to
+		// Type handled by the caller. These are added here to
 		// satisfy "DisallowUnknownFields" when decoding.
-		Type    string `json:"type"`
-		MinSize any    `json:"minsize"`
+		Type    string         `json:"type"`
+		MinSize datasizes.Size `json:"minsize"`
 		VGCustomization
 	}
 
@@ -231,6 +200,7 @@ func decodeLVM(v *PartitionCustomization, data []byte) error {
 	}
 
 	v.VGCustomization = vg.VGCustomization
+	v.MinSize = vg.MinSize
 	return nil
 }
 
@@ -279,14 +249,6 @@ func (v *PartitionCustomization) UnmarshalTOML(data any) error {
 
 	v.Type = partType
 
-	if minsizeField, ok := d["minsize"]; ok {
-		minsize, err := decodeSize(minsizeField)
-		if err != nil {
-			return fmt.Errorf("%s error decoding minsize for partition: %w", errPrefix, err)
-		}
-		v.MinSize = minsize
-	}
-
 	return nil
 }
 

pkg/blueprint/disk_customizations_test.go

@@ -1264,7 +1264,7 @@ func TestPartitionCustomizationUnmarshalJSON(t *testing.T) {
 				"mountpoint": "/",
 				"fs_type": "xfs"
 			}`,
-			errorMsg: "JSON unmarshal: error decoding minsize for partition: cannot be negative",
+			errorMsg: "JSON unmarshal: error decoding partition with type \"plain\": error decoding JSON size: cannot be negative",
 		},
 		"wrong-type/btrfs-with-lvm": {
 			input: `{
@@ -1559,12 +1559,12 @@ func TestPartitionCustomizationUnmarshalTOML(t *testing.T) {
 			input:    `type = 5`,
 			errorMsg: `toml: line 0: TOML unmarshal: type must be a string, got "5" of type int64`,
 		},
-		"negative-size": {
+		"negative-size-2": {
 			input: `minsize = -10
 					mountpoint = "/"
 					fs_type = "xfs"
 					`,
-			errorMsg: "toml: line 0: TOML unmarshal: error decoding minsize for partition: cannot be negative",
+			errorMsg: "toml: line 0: TOML unmarshal: error decoding partition with type \"plain\": error decoding JSON size: cannot be negative",
 		},
 		"wrong-type/btrfs-with-lvm": {
 			input: `type = "btrfs"

pkg/blueprint/filesystem_customizations.go

@@ -10,50 +10,8 @@ import (
 )
 
 type FilesystemCustomization struct {
-	Mountpoint string `json:"mountpoint,omitempty" toml:"mountpoint,omitempty"`
-	MinSize    uint64 `json:"minsize,omitempty" toml:"minsize,omitempty"`
-}
-
-func (fsc *FilesystemCustomization) UnmarshalTOML(data interface{}) error {
-	d, ok := data.(map[string]interface{})
-	if !ok {
-		return fmt.Errorf("customizations.filesystem is not an object")
-	}
-
-	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"])
-	}
-	minSize, err := decodeSize(d["minsize"])
-	if err != nil {
-		return fmt.Errorf("TOML unmarshal: error decoding minsize value for mountpoint %q: %w", fsc.Mountpoint, err)
-	}
-	fsc.MinSize = minSize
-	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"])
-	}
-
-	minSize, err := decodeSize(d["minsize"])
-	if err != nil {
-		return fmt.Errorf("JSON unmarshal: error decoding minsize value for mountpoint %q: %w", fsc.Mountpoint, err)
-	}
-	fsc.MinSize = minSize
-	return nil
+	Mountpoint string         `json:"mountpoint,omitempty" toml:"mountpoint,omitempty"`
+	MinSize    datasizes.Size `json:"minsize,omitempty" toml:"minsize,omitempty"`
 }
 
 // CheckMountpointsPolicy checks if the mountpoints are allowed by the policy
@@ -72,26 +30,19 @@ func CheckMountpointsPolicy(mountpoints []FilesystemCustomization, mountpointAll
 	return nil
 }
 
-// decodeSize takes an integer or string representing a data size (with a data
-// suffix) and returns the uint64 representation.
-func decodeSize(size any) (uint64, error) {
-	switch s := size.(type) {
-	case string:
-		return datasizes.Parse(s)
-	case int64:
-		if s < 0 {
-			return 0, fmt.Errorf("cannot be negative")
-		}
-		return uint64(s), nil
-	case float64:
-		if s < 0 {
-			return 0, fmt.Errorf("cannot be negative")
+func (fsc *FilesystemCustomization) UnmarshalJSON(data []byte) error {
+	// this is only needed to generate nicer errors with a hint
+	// if the custom unmarshal for minsize failed (as encoding/json
+	// provides sadly no context), c.f.
+	// https://github.com/golang/go/issues/58655
+	type filesystemCustomization FilesystemCustomization
+	var fc filesystemCustomization
+	if err := json.Unmarshal(data, &fc); err != nil {
+		if fc.Mountpoint != "" {
+			return fmt.Errorf("JSON unmarshal: error decoding minsize value for mountpoint %q: %w", fc.Mountpoint, err)
 		}
-		// TODO: emit warning of possible truncation?
-		return uint64(s), nil
-	case uint64:
-		return s, nil
-	default:
-		return 0, fmt.Errorf("failed to convert value \"%v\" to number", size)
+		return err
 	}
+	*fsc = FilesystemCustomization(fc)
+	return nil
 }

pkg/blueprint/filesystem_customizations_test.go

@@ -47,19 +47,19 @@ func TestFilesystemCustomizationUnmarshalTOMLUnhappy(t *testing.T) {
 			name: "mountpoint not string",
 			input: `mountpoint = 42
 			minsize = 42`,
-			err: `toml: line 0: TOML unmarshal: mountpoint must be string, got "42" of type int64`,
+			err: `toml: line 1 (last key "mountpoint"): incompatible types: TOML value has type int64; destination has type string`,
 		},
 		{
 			name: "minsize nor string nor int",
 			input: `mountpoint="/"
 			minsize = true`,
-			err: `toml: line 0: TOML unmarshal: error decoding minsize value for mountpoint "/": failed to convert value "true" to number`,
+			err: `toml: line 2 (last key "minsize"): error decoding TOML size: failed to convert value "true" to number`,
 		},
 		{
 			name: "minsize not parseable",
 			input: `mountpoint="/"
 			minsize = "20 KG"`,
-			err: `toml: line 0: TOML unmarshal: error decoding minsize value for mountpoint "/": unknown data size units in string: 20 KG`,
+			err: `toml: line 2 (last key "minsize"): error decoding TOML size: unknown data size units in string: 20 KG`,
 		},
 	}
 
@@ -81,17 +81,17 @@ func TestFilesystemCustomizationUnmarshalJSONUnhappy(t *testing.T) {
 		{
 			name:  "mountpoint not string",
 			input: `{"mountpoint": 42, "minsize": 42}`,
-			err:   `JSON unmarshal: mountpoint must be string, got "42" of type float64`,
+			err:   `json: cannot unmarshal number into Go struct field filesystemCustomization.mountpoint of type string`,
 		},
 		{
 			name:  "minsize nor string nor int",
 			input: `{"mountpoint":"/", "minsize": true}`,
-			err:   `JSON unmarshal: error decoding minsize value for mountpoint "/": failed to convert value "true" to number`,
+			err:   `JSON unmarshal: error decoding minsize value for mountpoint "/": error decoding JSON size: failed to convert value "true" to number`,
 		},
 		{
 			name:  "minsize not parseable",
 			input: `{ "mountpoint": "/", "minsize": "20 KG"}`,
-			err:   `JSON unmarshal: error decoding minsize value for mountpoint "/": unknown data size units in string: 20 KG`,
+			err:   `JSON unmarshal: error decoding minsize value for mountpoint "/": error decoding JSON size: unknown data size units in string: 20 KG`,
 		},
 	}
 
@@ -115,7 +115,7 @@ func TestFilesystemCustomizationUnmarshalTOMLNotAnObject(t *testing.T) {
 			input: `
 [customizations]
 filesystem = ["hello"]`,
-			err: "toml: line 3 (last key \"customizations.filesystem\"): customizations.filesystem is not an object",
+			err: "toml: line 3 (last key \"customizations.filesystem\"): type mismatch for blueprint.FilesystemCustomization: expected table but found string",
 		},
 	}
 
@@ -144,3 +144,26 @@ path "/boot/" must be canonical`
 	err := blueprint.CheckMountpointsPolicy(mps, policy)
 	assert.EqualError(t, err, expectedErr)
 }
+
+func TestUnmarshalTOMLNoUndecoded(t *testing.T) {
+	testTOML := `
+[[customizations.filesystem]]
+mountpoint = "/foo"
+minsize = 1000
+`
+
+	var bp blueprint.Blueprint
+	meta, err := toml.Decode(testTOML, &bp)
+	assert.NoError(t, err)
+	assert.Equal(t, blueprint.Blueprint{
+		Customizations: &blueprint.Customizations{
+			Filesystem: []blueprint.FilesystemCustomization{
+				{
+					Mountpoint: "/foo",
+					MinSize:    1000,
+				},
+			},
+		},
+	}, bp)
+	assert.Equal(t, 0, len(meta.Undecoded()))
+}