Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom UnmarshalTOML() decoded keys left in meta.Undecoded() #425

Open
mvo5 opened this issue Sep 24, 2024 · 0 comments
Open

Custom UnmarshalTOML() decoded keys left in meta.Undecoded() #425

mvo5 opened this issue Sep 24, 2024 · 0 comments

Comments

@mvo5
Copy link

mvo5 commented Sep 24, 2024

First, thanks your amazing library!

It looks like when doing a custom UnmarshalTOML() for a struct this will leave the keys decoded there in meta.Undecoded().

Here is a minimal test case:

type CustomStruct struct {
	Foo string `json:"foo"`
}

func (cs *CustomStruct) UnmarshalTOML(data interface{}) error {
	d, _ := data.(map[string]interface{})
	cs.Foo = d["foo"].(string)
	return nil
}

func TestDecodeCustomStruct(t *testing.T) {
	var cs CustomStruct
	meta, err := Decode(`
		foo = "bar"
	`, &cs)
	if err != nil {
		t.Fatalf("Decode failed: %s", err)
	}

	if cs.Foo != "bar" {
		t.Errorf("\nhave:\n%v\nwant:\n%v\n", cs.Foo, "bar")
	}
	if len(meta.Undecoded()) > 0 {
		t.Errorf("\ncustom decode leaves unencoded fields: %v\n", meta.Undecoded())
	}
}

and the output:

$ go test -run TestDecodeCustomStruct
--- FAIL: TestDecodeCustomStruct (0.00s)
    decode_test.go:1174: 
        custom decode leaves unencoded fields: [foo]

Our use-case is very similar [0], we want to dynamically handle int or string sizes in the toml.

I think we can fix this particular issue via moving decoding to primitive types and not structs
but it would be nice to have a way in UnmarshalTOML() for complex types to signal to the
MetaData object that certain keys are decoded (or maybe this exists and I just couldn't
figure it out)?

[0] https://github.com/osbuild/images/blob/main/pkg/blueprint/filesystem_customizations.go#L27

mvo5 added a commit to mvo5/images that referenced this issue Sep 24, 2024
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
osbuild/bootc-image-builder#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 BurntSushi/toml#425 to see if
there is another way via a custom unmarshaler.
mvo5 added a commit to mvo5/toml that referenced this issue Sep 24, 2024
This commit adds a failing test for the scenario decribed in issue
BurntSushi#425
mvo5 added a commit to mvo5/images that referenced this issue Sep 24, 2024
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
osbuild/bootc-image-builder#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 BurntSushi/toml#425 to see if
there is another way via a custom unmarshaler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant