Refactor blob copying and compression

[mtrmac]

This is the last refactor PR for #1574 , I promise.

copyBlobFromStream has grown into a 300-line function carrying a lot of variables, and it’s become increasingly untenable to add more logic to it, even if that logic is conceptually quite local. It’s well past the time it should have been split up. I have hoped to do that after unit tests exist, sadly that’s not panned out.

Major themes:

• Move copyBlobFromStream and closest helpers to copy/blob.go.
• Use a much more regular structure for the copy pipeline, with the stream processing pipeline maintained as a sourceStream. The copy pipeline is still not just ignorant plumbing, it carries around all the state (e.g. how encryption and decryption interact, and how the outcome of modifying the stream needs to be reflected in the updated blob descriptor); but it’s much more encapsulated now:
  • Actually modifying the stream (e.g. compression) happens in various blobPipeline…Step functions; those functions return blobPipeline…StepData structures. The top-level pipeline, and the following pipeline stops, thus still have access to the same data as before, but with the …StepData wrappers, it’s much fewer variables to track, and the step interdependencies are clearly visible.
  • Other changes (e.g. updates to the descriptor or the blob info cache) are now methods on the …StepData objects, allowing more encapsulation. The top-level code is not completely ignorant, it e.g. it needs to be aware of how the various edits interact (like the inability to compress and decompress at the same time).
• Encryption/decryption are now in copy/encryption.go, mostly unchanged.
• The compression/decompression step, as the largest pipeline element, ends up in copy/compression.go. The broad outline is the same, but it’s split even further:
  • Every case (compress, decompress, re-compress, …) is now a separate function, which checks whether that case is applicable/possible, and if so, performs it and returns the right metadata.
  • Ultimately — this is the goal: we can build more complex logic for the “applicable/possible” conditions, like “is the requested compression format supported in the relevant manifest schema”?, without cluttering the top-level of copyBlobFromStream with extra variables.

Various corner cases of the copy pipeline are somewhat dubious, and that’s been made maybe more visible by these changes (e.g. how we don’t immediately update MediaType on encryption/decryption, but make checks based on its value anyway; how some decisions are done on the original data even if we have decrypted it in the meantime; and more). For the most part, this PR leans towards not changing those things, to decrease risk of destabilizing the codebase.

See the individual commits for details. I’m afraid this is large, and with minimal test coverage. Ultimately, I think this is a good longer-term direction, but I could accept an opinion that this refactoring is currently not worth it and we should just live with making copyBlobFromStream uglier for some(?) more time.

[vrothberg]

Wooooow, I need to grab a coffee before reviewing 😄

[vrothberg]

Very nice work, @mtrmac.

Thank you for splitting up in smaller commits. Surgical precision.

LGTM