feat(encryption) [13/N] Write encrypted manifest files#2821
Conversation
xanderbailey
changed the title
|
@blackmwk I think this is ready for a review when you have the time! Thanks! |
…/write_encrypted_manifest_files
blackmwk
left a comment
blackmwk
left a comment
There was a problem hiding this comment.
Thanks @xanderbailey for this pr, just one minor comment to add comments for the tracking issue.
| .expect("new snapshot should carry the appended data manifest"); | ||
|
|
||
| // The manifest list entry must carry decodable key metadata. | ||
| let key_metadata_bytes = manifest_file |
There was a problem hiding this comment.
Please create an issue to create a ManifestReader(similar to ManifestWriter) so that we don't need to do this everytime.
There was a problem hiding this comment.
Good idea - filed #2881 to track adding a ManifestReader symmetric to ManifestWriter so we don't repeat the read/decrypt/parse dance. Left a TODO(#2881) in the test.
There was a problem hiding this comment.
I'll prep a PR for this now actually.
There was a problem hiding this comment.
I've actually remove this part of the test as it wasn't actually adding anything. But have opened #2884 for the manifest reader.
|
CI failing fixed in #2885 |
2 participants
Which issue does this PR close?
What changes are included in this PR?
Today it is possible to read encrypted manifest files but not write them.
#2568 adds the foundations of what's needed to be able to do these writes but it wasn't wired into the actual write path.
Are these changes tested?
Yes - added round-trip write path test.