From 709353cc6469a327fe9b4048ce7002bbcebe91f7 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 24 Sep 2024 15:01:30 +0200 Subject: [PATCH] blueprints: fix filesystem customizations to allow toml Undecoded() The current toml filesytem customizations will decode a `FilesystemCustomization` as a map[string]interface{}. This means that the underlying toml engine will not konw what keys inside the map are decoded and which are not decoded. This lead to the issue https://github.com/osbuild/bootc-image-builder/issues/655 in `bootc-image-builder` where we check if we have unencoded entries and if so error (in this case incorrectly). This commit fixes the issue by moving the toml decoding from the struct/map[string]interface{} to primitive types. It's a bit ugly as is (partly because of the limited go type system) but with that the tests pass and len(meta.Undecoded()) is the expected zero. I opened https://github.com/BurntSushi/toml/issues/425 to see if there is another way via a custom unmarshaler. --- pkg/blueprint/customizations.go | 2 +- pkg/blueprint/filesystem_customizations.go | 37 +++++++++++-------- .../filesystem_customizations_test.go | 16 ++++++++ 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/pkg/blueprint/customizations.go b/pkg/blueprint/customizations.go index 28ea76105..2771ad337 100644 --- a/pkg/blueprint/customizations.go +++ b/pkg/blueprint/customizations.go @@ -301,7 +301,7 @@ func (c *Customizations) GetFilesystemsMinSize() uint64 { } var agg uint64 for _, m := range c.Filesystem { - agg += m.MinSize + agg += uint64(m.MinSize) } // This ensures that file system customization `size` is a multiple of // sector size (512) diff --git a/pkg/blueprint/filesystem_customizations.go b/pkg/blueprint/filesystem_customizations.go index ad0ac1c49..2884abcfa 100644 --- a/pkg/blueprint/filesystem_customizations.go +++ b/pkg/blueprint/filesystem_customizations.go @@ -10,31 +10,36 @@ import ( ) type FilesystemCustomization struct { - Mountpoint string `json:"mountpoint,omitempty" toml:"mountpoint,omitempty"` - MinSize uint64 `json:"minsize,omitempty" toml:"minsize,omitempty"` + Mountpoint Mountpoint `json:"mountpoint,omitempty" toml:"mountpoint,omitempty"` + MinSize MinSize `json:"minsize,omitempty" toml:"minsize,omitempty"` } -func (fsc *FilesystemCustomization) UnmarshalTOML(data interface{}) error { - d, _ := data.(map[string]interface{}) +type Mountpoint string - switch d["mountpoint"].(type) { +func (mt *Mountpoint) UnmarshalTOML(d interface{}) error { + switch d.(type) { case string: - fsc.Mountpoint = d["mountpoint"].(string) + *mt = Mountpoint(d.(string)) default: - return fmt.Errorf("TOML unmarshal: mountpoint must be string, got %v of type %T", d["mountpoint"], d["mountpoint"]) + return fmt.Errorf("TOML unmarshal: mountpoint must be string, got %v of type %T", d, d) } + return nil +} - switch d["minsize"].(type) { +type MinSize uint64 + +func (ms *MinSize) UnmarshalTOML(d interface{}) error { + switch d.(type) { case int64: - fsc.MinSize = uint64(d["minsize"].(int64)) + *ms = MinSize(d.(int64)) case string: - minSize, err := common.DataSizeToUint64(d["minsize"].(string)) + minSize, err := common.DataSizeToUint64(d.(string)) if err != nil { return fmt.Errorf("TOML unmarshal: minsize is not valid filesystem size (%w)", err) } - fsc.MinSize = minSize + *ms = MinSize(minSize) default: - return fmt.Errorf("TOML unmarshal: minsize must be integer or string, got %v of type %T", d["minsize"], d["minsize"]) + return fmt.Errorf("TOML unmarshal: minsize must be integer or string, got %v of type %T", d, d) } return nil @@ -49,7 +54,7 @@ func (fsc *FilesystemCustomization) UnmarshalJSON(data []byte) error { switch d["mountpoint"].(type) { case string: - fsc.Mountpoint = d["mountpoint"].(string) + fsc.Mountpoint = Mountpoint(d["mountpoint"].(string)) default: return fmt.Errorf("JSON unmarshal: mountpoint must be string, got %v of type %T", d["mountpoint"], d["mountpoint"]) } @@ -57,13 +62,13 @@ func (fsc *FilesystemCustomization) UnmarshalJSON(data []byte) error { // The JSON specification only mentions float64 and Go defaults to it: https://go.dev/blog/json switch d["minsize"].(type) { case float64: - fsc.MinSize = uint64(d["minsize"].(float64)) + fsc.MinSize = MinSize(d["minsize"].(float64)) case string: minSize, err := common.DataSizeToUint64(d["minsize"].(string)) if err != nil { return fmt.Errorf("JSON unmarshal: minsize is not valid filesystem size (%w)", err) } - fsc.MinSize = minSize + fsc.MinSize = MinSize(minSize) default: return fmt.Errorf("JSON unmarshal: minsize must be float64 number or string, got %v of type %T", d["minsize"], d["minsize"]) } @@ -75,7 +80,7 @@ func (fsc *FilesystemCustomization) UnmarshalJSON(data []byte) error { func CheckMountpointsPolicy(mountpoints []FilesystemCustomization, mountpointAllowList *pathpolicy.PathPolicies) error { var errs []error for _, m := range mountpoints { - if err := mountpointAllowList.Check(m.Mountpoint); err != nil { + if err := mountpointAllowList.Check(string(m.Mountpoint)); err != nil { errs = append(errs, err) } } diff --git a/pkg/blueprint/filesystem_customizations_test.go b/pkg/blueprint/filesystem_customizations_test.go index 7bdc19518..50bea820b 100644 --- a/pkg/blueprint/filesystem_customizations_test.go +++ b/pkg/blueprint/filesystem_customizations_test.go @@ -120,3 +120,19 @@ path "/boot/" must be canonical` err := blueprint.CheckMountpointsPolicy(mps, policy) assert.EqualError(t, err, expectedErr) } + +func TestReadConfigNoUndecoded(t *testing.T) { + testTOML := ` +mountpoint = "/foo" +minsize = 1000 +` + + var fsc blueprint.FilesystemCustomization + meta, err := toml.Decode(testTOML, &fsc) + assert.NoError(t, err) + assert.Equal(t, blueprint.FilesystemCustomization{ + Mountpoint: "/foo", + MinSize: 1000, + }, fsc) + assert.Equal(t, 0, len(meta.Undecoded())) +}