diff --git a/storage/storage_dest.go b/storage/storage_dest.go index c28becf84..4f58b9e94 100644 --- a/storage/storage_dest.go +++ b/storage/storage_dest.go @@ -893,12 +893,9 @@ func (s *storageImageDestination) commitLayer(index int, info addedLayerInfo, si return false, fmt.Errorf("we have blob %q, but don't know its layer ID", info.digest.String()) } } - // The layerID refers either to the DiffID or the digest of the TOC. - layerIDComponent, layerIDComponentStandalone := trusted.singleLayerIDComponent() - id := layerIDComponent - if !layerIDComponentStandalone || parentLayer != "" { - id = digest.Canonical.FromString(parentLayer + "+" + layerIDComponent).Encoded() - } + + id := layerID(parentLayer, trusted) + if layer, err2 := s.imageRef.transport.store.Layer(id); layer != nil && err2 == nil { // There's already a layer that should have the right contents, just reuse it. s.indexToStorageID[index] = layer.ID @@ -916,13 +913,21 @@ func (s *storageImageDestination) commitLayer(index int, info addedLayerInfo, si return false, nil } -// singleLayerIDComponent returns a single layer’s the input to computing a layer (chain) ID, -// and an indication whether the input already has the shape of a layer ID. -func (trusted trustedLayerIdentityData) singleLayerIDComponent() (string, bool) { +// layerID computes a layer (“chain”) ID for (a possibly-empty parentLayer, trusted) +func layerID(parentLayer string, trusted trustedLayerIdentityData) string { + var layerIDComponent string if trusted.layerIdentifiedByTOC { - return "@TOC=" + trusted.tocDigest.Encoded(), false // "@" is not a valid start of a digest.Digest, so this is unambiguous. + // "@" is not a valid start of a digest.Digest.Encoded(), so this is unambiguous with the !layerIdentifiedByTOC case. + // But we _must_ hash this below to get a Digest.Encoded()-formatted value. + layerIDComponent = "@TOC=" + trusted.tocDigest.Encoded() + } else { + layerIDComponent = trusted.diffID.Encoded() // This looks like chain IDs, and it uses the traditional value. + } + id := layerIDComponent + if trusted.layerIdentifiedByTOC || parentLayer != "" { + id = digest.Canonical.FromString(parentLayer + "+" + layerIDComponent).Encoded() } - return trusted.diffID.Encoded(), true // This looks like chain IDs, and it uses the traditional value. + return id } // createNewLayer creates a new layer newLayerID for (index, layerDigest) on top of parentLayer (which may be ""). diff --git a/storage/storage_dest_test.go b/storage/storage_dest_test.go new file mode 100644 index 000000000..93b60e690 --- /dev/null +++ b/storage/storage_dest_test.go @@ -0,0 +1,105 @@ +package storage + +import ( + "testing" + + "github.com/opencontainers/go-digest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLayerID(t *testing.T) { + blobDigest, err := digest.Parse("sha256:0000000000000000000000000000000000000000000000000000000000000000") + require.NoError(t, err) + + for _, c := range []struct { + parentID string + identifiedByTOC bool + diffID string + tocDigest string + expected string + }{ + { + parentID: "", + identifiedByTOC: false, + diffID: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + tocDigest: "", + expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }, + { + parentID: "", + identifiedByTOC: false, + diffID: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + tocDigest: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + }, + { + parentID: "", + identifiedByTOC: true, + diffID: "", + tocDigest: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + expected: "07f60ddaf18a3d1fa18a71bf40f0b9889b473e26555d6fffdfbd72ba6a59469e", + }, + { + parentID: "", + identifiedByTOC: true, + diffID: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + tocDigest: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + expected: "07f60ddaf18a3d1fa18a71bf40f0b9889b473e26555d6fffdfbd72ba6a59469e", + }, + { + parentID: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + identifiedByTOC: false, + diffID: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + tocDigest: "", + expected: "76f79efda453922cda1cecb6ec9e7cf9d86ea968c6dd199d4030dd00078a1686", + }, + { + parentID: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + identifiedByTOC: false, + diffID: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + tocDigest: "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + expected: "76f79efda453922cda1cecb6ec9e7cf9d86ea968c6dd199d4030dd00078a1686", + }, + { + parentID: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + identifiedByTOC: true, + diffID: "", + tocDigest: "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + expected: "468becc3d25ee862f81fd728d229a2b2487cfc9b3e6cf3a4d0af8c3fdde0e7a9", + }, + { + parentID: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + identifiedByTOC: true, + diffID: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + tocDigest: "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + expected: "468becc3d25ee862f81fd728d229a2b2487cfc9b3e6cf3a4d0af8c3fdde0e7a9", + }, + } { + var diffID, tocDigest digest.Digest + if c.diffID != "" { + diffID, err = digest.Parse(c.diffID) + require.NoError(t, err) + } + if c.tocDigest != "" { + tocDigest, err = digest.Parse(c.tocDigest) + require.NoError(t, err) + } + + res := layerID(c.parentID, trustedLayerIdentityData{ + layerIdentifiedByTOC: c.identifiedByTOC, + diffID: diffID, + tocDigest: tocDigest, + blobDigest: "", + }) + assert.Equal(t, c.expected, res) + // blobDigest does not affect the layer ID + res = layerID(c.parentID, trustedLayerIdentityData{ + layerIdentifiedByTOC: c.identifiedByTOC, + diffID: diffID, + tocDigest: tocDigest, + blobDigest: blobDigest, + }) + assert.Equal(t, c.expected, res) + } +}