Stop caching encrypted layers in docker destination

[hshiina]

In a case where a non-encrypted images is committed to a registry after
an encrypted image is committed, this library may try to reuse
encrypted layers for the non-encrypted images if the images have common
layers. When the non-encrypted image is committed, there is no
information to recognize that cached layers are encrypted. As a result,
the non-encrypted image has encrypted layers unexpectedly in the
registry. This causes an error when the non-encrypted image is pulled
from the registry.

To avoid reusing encrypted layers for another image, this change stops
caching a layer if it is encrypted.

Signed-off-by: Hironori Shiina shiina.hironori@jp.fujitsu.com

[mtrmac]

Thanks for the PR.

Targeting RecordKnownLocation like this seems unlikely to be correct

• As a trivial matter, RecordKnownLocation is called elsewhere for other reasons; e.g. just copying an encrypted image from one registry to another can create a record, so this fix doesn’t work.
• Layer reuse can happen without a RecordKnownLocation at all.
• It’s awkward for the transport to deal with this; this should be handled in the transport-independent copy logic, somehow.
• Conceptually, the location record is not the issue at all; we want to avoid the reuse of encrypted layers when the original is not encrypted (or differently-encrypted); i.e. the issue is the digest/uncompressed-digest associations don’t record the information.

[mtrmac]

we want to avoid the reuse of encrypted layers when the original is not encrypted (or differently-encrypted); i.e. the issue is the digest/uncompressed-digest associations don’t record the information.

E.g. consider what happens when an already-encrypted image is copied to a registry (which does not trigger this code path at all); and then is copied decrypting it, into a docker-archive: destination; that might create a digest/uncompressed record as well, and a known location. Copying the same image back into the same repo could again trigger reuse.

I haven’t really thought about how to actually cleanly fix this yet, though, I’m afraid.

[mtrmac]

I haven’t really thought about how to actually cleanly fix this yet, though, I’m afraid.

#1533 is probably not the most accurate logic possible, but it at least seems to conservatively avoid the problematic layer reuse (after one-time manual removal of the blob info cache).

[mtrmac]

#1533 was merged, and is effectively a superset of this PR. Thanks!