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

blueprints: fix filesystem customizations to allow toml Undecoded() #951

Closed
wants to merge 1 commit into from

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented 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/bootc-image-builder that referenced this pull request Sep 24, 2024
This commit partially reverts PT#549 to unblock filesystem
customizations in bib.

This is a short term fix and we should revert and do something
smarter like osbuild/images#951 or see
if we can do better in the toml unmarshaling. But to unblock
toml customizations this is a (IMHO) reasonable first step.

Closes: osbuild#655
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 mvo5 force-pushed the toml-undecoded-only-primitive-types branch from 709353c to f55bad7 Compare September 24, 2024 14:27
@mvo5
Copy link
Contributor Author

mvo5 commented Sep 24, 2024

I submited an alternative idea upstream BurntSushi/toml#426

mvo5 added a commit to mvo5/bootc-image-builder that referenced this pull request Sep 25, 2024
This commit partially reverts PT#549 to unblock filesystem
customizations in bib.

This is a short term fix and we should revert and do something
smarter like osbuild/images#951 or see
if we can do better in the toml unmarshaling. But to unblock
toml customizations this is a (IMHO) reasonable first step.

Closes: osbuild#655
github-merge-queue bot pushed a commit to osbuild/bootc-image-builder that referenced this pull request Sep 25, 2024
This commit partially reverts PT#549 to unblock filesystem
customizations in bib.

This is a short term fix and we should revert and do something
smarter like osbuild/images#951 or see
if we can do better in the toml unmarshaling. But to unblock
toml customizations this is a (IMHO) reasonable first step.

Closes: #655
Copy link

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@mvo5
Copy link
Contributor Author

mvo5 commented Nov 18, 2024

Closing in favor of #1049

@mvo5 mvo5 closed this Nov 18, 2024
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

Successfully merging this pull request may close these issues.

1 participant