From d08a854fcb16f03efbdc5ec6816048f8eda3b112 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 19 Nov 2024 12:35:08 +0100 Subject: [PATCH] blueprint: use new `datasizes.Size` for minsize in disk.Customizations This commit makes use of the new `datasizes.Size` type that gives us automatic json/toml unmarshal for string/int sizes. This allows us to remove some code (not that much but a bit). --- pkg/blueprint/filesystem_customizations.go | 18 +++++ .../filesystem_customizations_test.go | 18 ++--- pkg/blueprint/partitioning_customizations.go | 72 +++++-------------- .../partitioning_customizations_test.go | 6 +- 4 files changed, 47 insertions(+), 67 deletions(-) diff --git a/pkg/blueprint/filesystem_customizations.go b/pkg/blueprint/filesystem_customizations.go index 2988916804..fba50da529 100644 --- a/pkg/blueprint/filesystem_customizations.go +++ b/pkg/blueprint/filesystem_customizations.go @@ -1,6 +1,7 @@ package blueprint import ( + "encoding/json" "errors" "fmt" @@ -28,3 +29,20 @@ func CheckMountpointsPolicy(mountpoints []FilesystemCustomization, mountpointAll return nil } + +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) + } + return err + } + *fsc = FilesystemCustomization(fc) + return nil +} diff --git a/pkg/blueprint/filesystem_customizations_test.go b/pkg/blueprint/filesystem_customizations_test.go index ea44f60e04..224a279605 100644 --- a/pkg/blueprint/filesystem_customizations_test.go +++ b/pkg/blueprint/filesystem_customizations_test.go @@ -50,16 +50,16 @@ func TestFilesystemCustomizationUnmarshalTOMLUnhappy(t *testing.T) { err: `toml: line 1 (last key "mountpoint"): incompatible types: TOML value has type int64; destination has type string`, }, { - name: "misize nor string nor int", + name: "minsize nor string nor int", input: `mountpoint="/" minsize = true`, - err: `toml: line 2 (last key "minsize"): TOML unmarshal: error decoding size: 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: "misize not parseable", + name: "minsize not parseable", input: `mountpoint="/" minsize = "20 KG"`, - err: `toml: line 2 (last key "minsize"): TOML unmarshal: error decoding size: 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: cannot unmarshal number into Go struct field FilesystemCustomization.mountpoint of type string`, + err: `json: cannot unmarshal number into Go struct field filesystemCustomization.mountpoint of type string`, }, { - name: "misize nor string nor int", + name: "minsize nor string nor int", input: `{"mountpoint":"/", "minsize": true}`, - err: `JSON unmarshal: error decoding size: 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: "misize not parseable", + name: "minsize not parseable", input: `{ "mountpoint": "/", "minsize": "20 KG"}`, - err: `JSON unmarshal: error decoding size: 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`, }, } diff --git a/pkg/blueprint/partitioning_customizations.go b/pkg/blueprint/partitioning_customizations.go index 800063a2b9..8b8eb9a8ba 100644 --- a/pkg/blueprint/partitioning_customizations.go +++ b/pkg/blueprint/partitioning_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 } diff --git a/pkg/blueprint/partitioning_customizations_test.go b/pkg/blueprint/partitioning_customizations_test.go index 980190015b..c4ddd34497 100644 --- a/pkg/blueprint/partitioning_customizations_test.go +++ b/pkg/blueprint/partitioning_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"