From af7cd9bbb2e33c5c38d266a1f8671f89dfcba67b Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 22 Nov 2024 12:20:40 +0100 Subject: [PATCH] blueprint: decouple FilesystemCustomization from marshaling This commit decouples the marshaling representation of the `FilesystemCustomization` from the actual struct. This means that we can reuse custom unmarshaling via datasizes.Size and also have our external represenation "pure". If we continue using this pattern is also means that we can rename fields in the marshaling and provide compatbility easily. Thanks to Thozza, c.f. https://github.com/osbuild/images/pull/1049#issuecomment-2493261642 and https://github.com/osbuild/images/pull/983 --- pkg/blueprint/filesystem_customizations.go | 56 +++++++++---------- .../filesystem_customizations_test.go | 14 ++--- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/pkg/blueprint/filesystem_customizations.go b/pkg/blueprint/filesystem_customizations.go index e783930052..7f9d1c1918 100644 --- a/pkg/blueprint/filesystem_customizations.go +++ b/pkg/blueprint/filesystem_customizations.go @@ -10,49 +10,43 @@ import ( ) type FilesystemCustomization struct { - Mountpoint string `json:"mountpoint,omitempty" toml:"mountpoint,omitempty"` - MinSize uint64 `json:"minsize,omitempty" toml:"minsize,omitempty"` + Mountpoint string + MinSize uint64 } -func (fsc *FilesystemCustomization) UnmarshalTOML(data interface{}) error { - d, ok := data.(map[string]interface{}) - if !ok { - return fmt.Errorf("customizations.filesystem is not an object") - } +type filesystemCustomizationMarshaling struct { + Mountpoint string `json:"mountpoint,omitempty" toml:"mountpoint,omitempty"` + MinSize datasizes.Size `json:"minsize,omitempty" toml:"minsize,omitempty"` +} - 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"]) +func (fsc *FilesystemCustomization) UnmarshalTOML(data any) error { + // This is the most efficient way to reuse code when unmarshaling + // structs in toml, it leaks json errors which is a bit sad but + // because the toml unmarshaler gives us not "[]byte" but an + // already pre-processed "any" we cannot just unmarshal into our + // "fooMarshaling" struct and reuse the result so we resort to + // this workaround (but toml will go away long term anyway). + dataJSON, err := json.Marshal(data) if err != nil { - return fmt.Errorf("TOML unmarshal: error decoding minsize value for mountpoint %q: %w", fsc.Mountpoint, err) + return fmt.Errorf("error unmarshaling TOML data %v: %w", data, err) + } + if err := fsc.UnmarshalJSON(dataJSON); err != nil { + return fmt.Errorf("error decoding TOML %v: %w", data, err) } - fsc.MinSize = minSize return nil } func (fsc *FilesystemCustomization) UnmarshalJSON(data []byte) error { - var v interface{} - if err := json.Unmarshal(data, &v); err != nil { + var fc filesystemCustomizationMarshaling + if err := json.Unmarshal(data, &fc); err != nil { + if fc.Mountpoint != "" { + return fmt.Errorf("error decoding minsize value for mountpoint %q: %w", fc.Mountpoint, err) + } return err } - d, _ := v.(map[string]interface{}) + fsc.Mountpoint = fc.Mountpoint + fsc.MinSize = fc.MinSize.Uint64() - 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 } diff --git a/pkg/blueprint/filesystem_customizations_test.go b/pkg/blueprint/filesystem_customizations_test.go index fbb85f0817..0c12f6cb68 100644 --- a/pkg/blueprint/filesystem_customizations_test.go +++ b/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 0: error decoding TOML map[minsize:42 mountpoint:42]: json: cannot unmarshal number into Go struct field filesystemCustomizationMarshaling.mountpoint of 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 0: error decoding TOML map[minsize:true mountpoint:/]: error decoding 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 0: error decoding TOML map[minsize:20 KG mountpoint:/]: error decoding 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 filesystemCustomizationMarshaling.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: `error decoding minsize value for mountpoint "/": error decoding 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: `error decoding minsize value for mountpoint "/": error decoding 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"): error decoding TOML hello: json: cannot unmarshal string into Go value of type blueprint.filesystemCustomizationMarshaling`, }, }