-
Notifications
You must be signed in to change notification settings - Fork 373
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
Refactor blob copying and compression #1579
Commits on Jun 15, 2022
-
Move imageCopier.copyBlobFromStream into a new copy/blob.go
The over-300-line-long function would benefit from splitting up, and copy/copy.go has grown entirely too long. As a first step, move it to a new copy/blob.go. Only moves unmodified code, should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 84115a5 - Browse repository at this point
Copy the full SHA 84115a5View commit details -
... so that the file reads from top-level callers down to implementation details. Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 345b1c0 - Browse repository at this point
Copy the full SHA 345b1c0View commit details -
Rename the srcStream parameter to srcReader
We will use srcStream for another purpose. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 52c06a9 - Browse repository at this point
Copy the full SHA 52c06a9View commit details -
Introduce sourceStream, use it for the first pipeline stage
The purpose of sourceStream is to make it a bit easier to split the blob copy pipeline stages into separate functions; they will update a *sourceStream, which we can pass a single parameter. For now, build it and use it to initialize a digestingReader; then update other code that should consume the current pipeline data. NOTE: That is a judgement call, it is not always obvious what data was intended to be used and what is a bug. As a geneeral principle, use srcInfo (the original unmodified data) in user-visible error reports, notably using the original digest. Otherwise, lean towards using srcStream.info. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 54c4a9e - Browse repository at this point
Copy the full SHA 54c4a9eView commit details -
Use stream.reader instead of destStream
One part of moving to use stream throughout the pipeline. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for acdd036 - Browse repository at this point
Copy the full SHA acdd036View commit details -
Use stream.info throughout. Should not change behavior. (In some cases that's dubious, we are currently losing data like annotations compression/decompression. This commit doesn't change that, only adds a FIXME? to eventually review.) Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 2ee1587 - Browse repository at this point
Copy the full SHA 2ee1587View commit details -
Move the OCI blob decryption pipeline step into copier.blobPipelineDe…
…cryptionStep Only moves the code with minimal necessary adjustments, should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 11c0fe0 - Browse repository at this point
Copy the full SHA 11c0fe0View commit details -
Beautify blobPipelineDecryptionStep a bit
Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 338d64d - Browse repository at this point
Copy the full SHA 338d64dView commit details -
This smaller function can use simpler identifiers. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 5472fa9 - Browse repository at this point
Copy the full SHA 5472fa9View commit details -
Beautify the DecryptLayer usage a bit
Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 651679a - Browse repository at this point
Copy the full SHA 651679aView commit details -
Move the OCI blob encryption pipeline step into copier.blobPipelineEn…
…cryptionStep Only moves the code with minimal necessary adjustments, should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 8dbbe7c - Browse repository at this point
Copy the full SHA 8dbbe7cView commit details -
Simplify blobPipelineEncryptionStep
Make the "not encrypting" case truly special, and then we don't need extra variables across the function. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 014c23d - Browse repository at this point
Copy the full SHA 014c23dView commit details -
Exit from updateCryptoOperationAndAnnotations early if not encrypting
Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for e8c3881 - Browse repository at this point
Copy the full SHA e8c3881View commit details -
Beautify blobPipelineEncryptionStep
Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for d6fccbd - Browse repository at this point
Copy the full SHA d6fccbdView commit details -
Rename copy/encrypt.go to copy/encryption.go
It contains decompression code as well. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 0f854d0 - Browse repository at this point
Copy the full SHA 0f854d0View commit details -
Move the compression detection step into blobPipelineDetectCompressio…
…nStep Only moves the code with minimal necessary adjustments, should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for f0b0bf4 - Browse repository at this point
Copy the full SHA f0b0bf4View commit details -
Beautify blobPipelineDetectCompressionStep
We can use shorter variable names in this short function now. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for bf130bc - Browse repository at this point
Copy the full SHA bf130bcView commit details -
Move the compression annotation update closer to the other compressio…
…n edits Only moves code, should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 003d340 - Browse repository at this point
Copy the full SHA 003d340View commit details -
Move copier.compressGoroutine to compression.go
Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 6af0c82 - Browse repository at this point
Copy the full SHA 6af0c82View commit details -
Move the compression/decompression step to blobPipelineCompressionStep
Only moves the code with minimal necessary adjustments, should not change behavior. Those adjustments are not completely trivial in this case: we could just do a "defer stream.Close()" in copyBlobFromStream, now we need a separate closers field in blobPipelineCompressionStepData. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for e83976e - Browse repository at this point
Copy the full SHA e83976eView commit details -
Rename detectedCompession to detected
The compression meaning is clear in context here. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 00a66fe - Browse repository at this point
Copy the full SHA 00a66feView commit details -
Rename compresisonOperation to operation
Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 23a031f - Browse repository at this point
Copy the full SHA 23a031fView commit details -
Rename uploadCompressionFormat to uploadedAlgorithm
Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 78fd34d - Browse repository at this point
Copy the full SHA 78fd34dView commit details -
Rename uploadCompressorName to uploadedCompressorName
Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for ce4d066 - Browse repository at this point
Copy the full SHA ce4d066View commit details -
Rename compressionMetadata to uploadedAnnotations.
Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 68e0d9d - Browse repository at this point
Copy the full SHA 68e0d9dView commit details -
Compute srcCompressorName already in blobPipelineDetectCompressionStep
Except for the encrypted content case, we already have the data available, and this will allow us to just pass around a *blobPipelineDetectCompressionStepData without an extra parameter for srcCompressorName. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 4f0548a - Browse repository at this point
Copy the full SHA 4f0548aView commit details -
Return a fresh &bpCompressionStepData in each case
That's actually shorter because in most cases we just use a field initializer instead of setting a variable; and we don't need that variable in the first place. For now, do just the minimal edit, we will clean this up a bit later. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for f93ebce - Browse repository at this point
Copy the full SHA f93ebceView commit details -
Move uploadedAnnotations inside the individual cases.
Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 7a81dce - Browse repository at this point
Copy the full SHA 7a81dceView commit details -
Move uploadedAlgorithm inside the individual cases.
Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 302967d - Browse repository at this point
Copy the full SHA 302967dView commit details -
Clean up the control flow of blobPipelineCompressionStep
Don't use else after each step that returns early. The function nows looks like a set of ~special cases, with the general fallback of doing nothing. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for c39fe5b - Browse repository at this point
Copy the full SHA c39fe5bView commit details -
Rename s to decompressed in the re-compress case
to avoid ambiguity. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for b6bb69b - Browse repository at this point
Copy the full SHA b6bb69bView commit details -
Factor out copier.compressedStream from copier.blobPipelineCompressio…
…nStep ... so that we only implement the pipe handling once. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for f70a390 - Browse repository at this point
Copy the full SHA f70a390View commit details -
Split blobPipelineCompressionStep into individual functions
The logic is a simple sequence of condition + decision, which is fairly regullar, and doesn't have to be in a single function. So, split it up. Eliminate the recently-introduced closers array again, in most cases. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 73bf1ae - Browse repository at this point
Copy the full SHA 73bf1aeView commit details -
Eliminate the closers array in bpcRecompressCompressed
... now that it only could effectively apply to a single element. Shoud not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 20f0638 - Browse repository at this point
Copy the full SHA 20f0638View commit details -
Use a loop for the alternatives in blobPipelineCompressionStep
The alternatives are, mostly, things to _try_; we can typically fall back to just using the original. So, just use a loop to take advantage of the regular structure. Signed-off-by: Miloslav Trmač <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 2756d70 - Browse repository at this point
Copy the full SHA 2756d70View commit details