Skip to content

Commit

Permalink
fix transfer OCI index (#683)
Browse files Browse the repository at this point in the history
## Description

The transfer of OCI indix manifests accidently rewrites the manifest,
even if it has not been changed during a transfer step.

## What type of PR is this? (check all applicable)

- [ ] 🍕 Feature
- [ ] 🐛 Bug Fix
- [ ] 📝 Documentation Update
- [ ] 🎨 Style
- [ ] 🧑‍💻 Code Refactor
- [ ] 🔥 Performance Improvements
- [ ] ✅ Test
- [ ] 🤖 Build
- [ ] 🔁 CI
- [ ] 📦 Chore (Release)
- [ ] ⏩ Revert

## Related Tickets & Documents

<!-- 
Please use this format link issue numbers: Fixes #123

https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->
- Related Issue # (issue)
- Closes # (issue)
- Fixes # (issue)
> Remove if not applicable

## Screenshots

<!-- Visual changes require screenshots -->


## Added tests?

- [ ] 👍 yes
- [ ] 🙅 no, because they aren't needed
- [ ] 🙋 no, because I need help
- [ ] Separate ticket for tests # (issue/pr)

Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. Please also list any relevant details
for your test configuration


## Added to documentation?

- [ ] 📜 README.md
- [ ] 🙅 no documentation needed

## Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream
modules
  • Loading branch information
mandelsoft authored Mar 5, 2024
1 parent f3416ae commit e4d14a1
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 8 deletions.
54 changes: 54 additions & 0 deletions pkg/contexts/oci/cpi/mod.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Open Component Model contributors.
//
// SPDX-License-Identifier: Apache-2.0

package cpi

import (
"github.com/opencontainers/go-digest"

"github.com/open-component-model/ocm/pkg/common/accessobj"
"github.com/open-component-model/ocm/pkg/contexts/oci/artdesc"
)

type _Artifact = artdesc.Artifact

type modifiedArtifact struct {
state accessobj.State
*_Artifact
}

var _ Artifact = (*modifiedArtifact)(nil)

// NewArtifact provides a copy of the given artifact,
// which will keep track of the original serialization.
// This one is used as long as the artifact is unchanged.
// (the returned underlying implementation of the Artifact interface
// might differ from the original one).
func NewArtifact(art Artifact) (Artifact, error) {
blob, err := art.Blob()
if err != nil {
return nil, err
}
state, err := accessobj.NewBlobStateForBlob(accessobj.ACC_WRITABLE, blob, NewArtifactStateHandler())
if err != nil {
return nil, err
}

return &modifiedArtifact{
_Artifact: state.GetState().(*_Artifact),
state: state,
}, nil
}

func (a *modifiedArtifact) Blob() (BlobAccess, error) {
return a.state.GetBlob()
}

func (a *modifiedArtifact) Digest() digest.Digest {
blob, _ := a.Blob()
if blob != nil {
return blob.Digest()
}
return ""
}
15 changes: 15 additions & 0 deletions pkg/contexts/oci/ociutils/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package ociutils

import (
"archive/tar"
"crypto/sha256"
"errors"
"fmt"
"io"
Expand All @@ -16,6 +17,7 @@ import (
"github.com/open-component-model/ocm/pkg/common/compression"
"github.com/open-component-model/ocm/pkg/contexts/oci/artdesc"
"github.com/open-component-model/ocm/pkg/contexts/oci/cpi"
"github.com/open-component-model/ocm/pkg/signing"
"github.com/open-component-model/ocm/pkg/utils"
)

Expand All @@ -39,6 +41,7 @@ func PrintManifest(pr common.Printer, m cpi.ManifestAccess, listFiles bool) {
pr.Printf("descriptor: invalid: %s\n", err)
} else {
pr.Printf("descriptor: %s\n", string(data))
pr.Printf("digest : %s\n", HashData(data))
}
man := m.GetDescriptor()
pr.Printf("config:\n")
Expand Down Expand Up @@ -116,6 +119,13 @@ func PrintLayer(pr common.Printer, blob blobaccess.BlobAccess, listFiles bool) {
}

func PrintIndex(pr common.Printer, i cpi.IndexAccess, listFiles bool) {
data, err := blobaccess.BlobData(i.Blob())
if err != nil {
pr.Printf("descriptor: invalid: %s\n", err)
} else {
pr.Printf("descriptor: %s\n", string(data))
pr.Printf("digest : %s\n", HashData(data))
}
printAnnotations(pr, i.GetDescriptor().Annotations)
pr.Printf("manifests:\n")
for _, l := range i.GetDescriptor().Manifests {
Expand Down Expand Up @@ -164,3 +174,8 @@ func printAnnotations(pr common.Printer, annos map[string]string) {
}
}
}

func HashData(data []byte) string {
s, _ := signing.Hash(sha256.New(), data)
return s
}
28 changes: 21 additions & 7 deletions pkg/contexts/oci/transfer/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
package transfer

import (
"slices"

"github.com/opencontainers/go-digest"

"github.com/open-component-model/ocm/pkg/contexts/oci/cpi"
Expand Down Expand Up @@ -47,13 +45,29 @@ func TransferIndexWithFilter(art cpi.IndexAccess, set cpi.ArtifactSink, filter f
var finalize finalizer.Finalizer
defer finalize.FinalizeWithErrorPropagation(&err)

index := *art.GetDescriptor()
index.Manifests = slices.Clone(index.Manifests)
// provide a local copy of the inbound artifact (index)
// which keeps track of the original serialized form,
// which is used if the manifest is left unchanged after
// filtering.
modified, err := cpi.NewArtifact(art)
if err != nil {
return nil, err
}

// don't add this (index) object later.
// it implements the same interface, but would
// provide a new serialization, which might differ
// from the original one even if it is unchanged,
// which would change the digest.
index, err := modified.Index()
if err != nil {
return nil, err
}

ign := 0
for i, l := range art.GetDescriptor().Manifests {
loop := finalize.Nested()
logging.Logger().Debug("indexed manifest", "digest", "digest", l.Digest, "size", l.Size)
logging.Logger().Debug("indexed manifest", "digest", l.Digest, "size", l.Size)
art, err := art.GetArtifact(l.Digest)
if err != nil {
return nil, errors.Wrapf(err, "getting indexed artifact %s", l.Digest)
Expand Down Expand Up @@ -90,11 +104,11 @@ func TransferIndexWithFilter(art cpi.IndexAccess, set cpi.ArtifactSink, filter f
}
}

_, err = set.AddArtifact(&index, tags...)
_, err = set.AddArtifact(modified, tags...)
if err != nil {
return nil, errors.Wrapf(err, "transferring index artifact")
}
return generics.Pointer(index.Digest()), err
return generics.Pointer(modified.Digest()), err
}

func TransferManifest(art cpi.ManifestAccess, set cpi.ArtifactSink, tags ...string) (err error) {
Expand Down
78 changes: 78 additions & 0 deletions pkg/contexts/ocm/blobhandler/handlers/oci/ocirepo/blobhandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,84 @@ func NewArtifactHandler(base BaseFunction) cpi.BlobHandler {
return &artifactHandler{blobHandler{base}}
}

func (b *artifactHandler) CheckBlob(blob cpi.BlobAccess, artType, hint string, global cpi.AccessSpec, ctx cpi.StorageContext) (bool, bool, error) {
mediaType := blob.MimeType()

if !artdesc.IsOCIMediaType(mediaType) || (!strings.HasSuffix(mediaType, "+tar") && !strings.HasSuffix(mediaType, "+tar+gzip")) {
return false, false, nil
}

log := cpi.BlobHandlerLogger(ctx.GetContext())

values := []interface{}{
"arttype", artType,
"mediatype", mediaType,
"hint", hint,
}

var art oci.ArtifactAccess
var err error
var finalizer Finalizer
defer finalizer.Finalize()

var namespace oci.NamespaceAccess
var version string
var name string
var tag string

ocictx, ok := ctx.(*storagecontext.StorageContext)
if !ok {
return false, false, fmt.Errorf("failed to assert type %T to storagecontext.StorageContext", ctx)
}
if hint == "" {
namespace = ocictx.Namespace
} else {
prefix := cpi.RepositoryPrefix(ctx.TargetComponentRepository().GetSpecification())
i := strings.LastIndex(hint, "@")
if i >= 0 {
hint = hint[:i] // remove digest
}
i = strings.LastIndex(hint, ":")
if i > 0 {
version = hint[i:]
tag = version[1:] // remove colon
name = hint[:i]
} else {
name = hint
}

hash := mapocirepoattr.Get(ctx.GetContext())
if hash.Prefix != nil {
prefix = *hash.Prefix
}
orig := name
mapped := hash.Map(name)
name = path.Join(prefix, mapped)
if mapped == orig {
log.Debug("namespace derived from hint",
generics.AppendedSlice[any](values, "namespace", name),
)
} else {
log.Debug("mapped namespace derived from hint",
generics.AppendedSlice[any](values, "namespace", name),
)
}

namespace, err = ocictx.Repository.LookupNamespace(name)
if err != nil {
return false, false, err
}
defer namespace.Close()
}

ok, err = namespace.HasArtifact(string(art.Digest()))
if ok {
return true, true, err
}
ok, err = namespace.HasArtifact(tag)
return ok, true, err
}

func (b *artifactHandler) StoreBlob(blob cpi.BlobAccess, artType, hint string, global cpi.AccessSpec, ctx cpi.StorageContext) (cpi.AccessSpec, error) {
mediaType := blob.MimeType()

Expand Down
2 changes: 1 addition & 1 deletion pkg/docker/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp

if resp.StatusCode > 299 {
if resp.StatusCode == http.StatusNotFound {
log.G(ctxWithLogger).Info("trying next host - response was http.StatusNotFound")
// log.G(ctxWithLogger).Info("trying next host - response was http.StatusNotFound")
continue
}
if resp.StatusCode > 399 {
Expand Down

0 comments on commit e4d14a1

Please sign in to comment.