Skip to content

buildflags: marshal attestations into json with extra attributes correctly#2937

Merged
crazy-max merged 1 commit intodocker:masterfrom
jsternberg:attests-json-marshal
Jan 22, 2025
Merged

buildflags: marshal attestations into json with extra attributes correctly#2937
crazy-max merged 1 commit intodocker:masterfrom
jsternberg:attests-json-marshal

Conversation

@jsternberg
Copy link
Copy Markdown
Collaborator

@jsternberg jsternberg commented Jan 21, 2025

MarshalJSON would not include the extra attributes because it iterated over the target map rather than the source map.

Also fixes JSON unmarshaling for SSH and secrets. The intention was to
unmarshal into the struct, but UnmarshalText takes priority over the
default struct unmarshaling so it didn't work as intended.

Tests have been added for all marshaling and unmarshaling methods.

Fixes #2936.

Copy link
Copy Markdown
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Tests?

…ectly

`MarshalJSON` would not include the extra attributes because it iterated
over the target map rather than the source map.

Also fixes JSON unmarshaling for SSH and secrets. The intention was to
unmarshal into the struct, but `UnmarshalText` takes priority over the
default struct unmarshaling so it didn't work as intended.

Tests have been added for all marshaling and unmarshaling methods.

Signed-off-by: Jonathan A. Sternberg <[email protected]>
@jsternberg
Copy link
Copy Markdown
Collaborator Author

Tests have now been added. I found an issue with the JSON unmarshaling for SSH and Secrets so those have also been fixed.

Not included in this PR is to support the string format for the JSON format. Currently, the JSON format requires the object syntax. It would be possible to allow a mixing like we do for the HCL syntax. If we want that, we should create an issue to track that work and it can be included in another PR.

@crazy-max crazy-max merged commit 3e0682f into docker:master Jan 22, 2025
@jsternberg jsternberg deleted the attests-json-marshal branch January 22, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Composable attributes for provenance attestation not correctly handled

3 participants