Skip to content

Use mapstructure to (de-)serialize internal metadata; move metadata processing from specific stores to general place#2120

Open
felixfontein wants to merge 11 commits intogetsops:mainfrom
felixfontein:mapstructure
Open

Use mapstructure to (de-)serialize internal metadata; move metadata processing from specific stores to general place#2120
felixfontein wants to merge 11 commits intogetsops:mainfrom
felixfontein:mapstructure

Conversation

@felixfontein
Copy link
Copy Markdown
Contributor

Right now, metadata is handled quite strangely when flattened (by going through JSON), and all stores somehow have to deal with metadata. This PR cleans this up as follows:

  • Use mapstructure to convert to/from metadata and coerce types.
  • Convert metadata to/from sops.TreeBranch, and provide three options for handling flattening (none; keep sops toplevel map; flatten completely).
  • Properly handle errors during flattening and unflattening. Right now you can panic SOPS and overwrite values by playing around with flattened metadata.
  • Remove old public APIs for dealing with internal metadata and (un-)flattening.

While this is a breaking change, I think this is still OK:

  • The APIs that are broken are internal ones. To from our promise, they're not public and it's not a breaking change.
  • They should only be used by store implementations, which are all part of SOPS itself. This means that even though some other programs use parts of SOPS as a library that aren't public, it is unlikely that they directly use these parts of SOPS that are changed by this PR.

This is related to #1401, #1338, #1046, #1009, which were earlier attempts to introduce mapstructure.

@felixfontein felixfontein requested a review from a team March 23, 2026 21:52
@felixfontein felixfontein force-pushed the mapstructure branch 2 times, most recently from af435dc to 97fea11 Compare April 21, 2026 18:36
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Comment thread stores/metadata.go Outdated
Comment thread stores/metadata.go Outdated
Comment thread stores/metadata_test.go Outdated
},
},
sops.TreeItem{
Key: "agekey",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be

Suggested change
Key: "agekey",
Key: "age",

I believe?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2513d1c.

Comment thread stores/metadata_test.go Outdated
},
},
sops.TreeItem{
Key: "agekey",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be

Suggested change
Key: "agekey",
Key: "age",

I believe? Also not caught by tests, as I don't believe we check for it as expected output.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2513d1c.

Comment thread stores/stores.go

// DecodeNewLines replaces \\n with \n for all string values in the map.
// Used by config stores that do not handle multi-line values (ini, dotenv).
func DecodeNewLines(m map[string]interface{}) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not appear to be used anymore? Same for EncodeNewLines. Also wonder if because this is no longer called, we are not introducing regressions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the previous versions of the INI and DotEnv stores, these function were used to process the metadata - despite the same transformation being made afterwards (store) resp. before (load) in the plain file variant. (As these transformations are idempotent, this never got noticed before.)

I guess we can simply remove them now; the plain loader/storer of these stores already handles newlines. WDYT?

Comment thread stores/metadata.go Outdated
Comment thread stores/metadata.go Outdated
Comment thread stores/metadata.go Outdated
Comment thread stores/flatten.go Outdated
felixfontein and others added 2 commits April 21, 2026 22:07
Co-authored-by: Hidde Beydals <hiddeco@users.noreply.github.com>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
@felixfontein felixfontein requested a review from hiddeco April 21, 2026 20:19
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.

2 participants