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

Serialize using mapstructure conversion instead of JSON marshalling. #1338

Closed
wants to merge 0 commits into from
Closed

Conversation

slewsys
Copy link

@slewsys slewsys commented Nov 3, 2023

These changes remove the JSON encoding/decoding steps that are performed during the serialization of ini and dotenv files. This roundtrip loses type information during the transformation which causes values to be incorrectly converted to the JSON marshaller defaults (int becomes float64, bool becomes string, etc, etc). In place of this JSON encoding, the mapstructure library allows for a direct conversion between the Metadata struct and
map[string]interface{} needed to leverage the stores.Flatten and stores.Unflatten functions.

In addition this adds mapstructure tags to the metadata structures to allow backwards compatibility with the JSON encoding.

Resolves #879 & resolves #857

This is a re-submission of @acastle's #1009 with mapstructure updated to v1.5.0 and unused imports removed applied to getsops/sops/main HEAD.

@Ph0tonic
Copy link
Contributor

Ph0tonic commented Nov 3, 2023

Hi,
Small tip, if you want this issue to close 2 issues automatically, you need to repeat resolves just before the second issue number, like :
Resolves #879 & resolves #857

@slewsys
Copy link
Author

slewsys commented Nov 4, 2023

Hi, Small tip, if you want this issue close to issues automatically, you need to repeat resolves just before the second issue number, like : Resolves #879 & resolves #857

Nice. The commit message actually belongs to @acastle, so hopefully we can both benefit! Thank you.

@devstein
Copy link
Contributor

@slewsys Apologies for the slow review time. Thank you for making this PR! I agree using mapstructure is a better solution.

Can you please

  1. Rebase to sign off on previous commits (See SO example)
  2. Update the branch with the latest

cc @acastle from the original PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants