-
Notifications
You must be signed in to change notification settings - Fork 70
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
IR-114: Adding support for OCI schema #255
Merged
openshift-merge-robot
merged 3 commits into
openshift:master
from
ricardomaraschini:oci-support
Jan 5, 2021
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
136 changes: 136 additions & 0 deletions
136
pkg/dockerregistry/server/manifesthandler/manifestocihandler.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,136 @@ | ||
package manifesthandler | ||
|
||
import ( | ||
"context" | ||
"time" | ||
|
||
"github.com/docker/distribution" | ||
dcontext "github.com/docker/distribution/context" | ||
"github.com/docker/distribution/manifest/ocischema" | ||
"github.com/opencontainers/go-digest" | ||
"k8s.io/apimachinery/pkg/util/wait" | ||
|
||
imageapiv1 "github.com/openshift/api/image/v1" | ||
imageapi "github.com/openshift/image-registry/pkg/origin-common/image/apis/image" | ||
) | ||
|
||
type manifestOCIHandler struct { | ||
blobStore distribution.BlobStore | ||
manifest *ocischema.DeserializedManifest | ||
cachedConfig []byte | ||
} | ||
|
||
var _ ManifestHandler = &manifestOCIHandler{} | ||
|
||
func (h *manifestOCIHandler) Config(ctx context.Context) ([]byte, error) { | ||
if h.cachedConfig == nil { | ||
blob, err := h.blobStore.Get(ctx, h.manifest.Config.Digest) | ||
if err != nil { | ||
dcontext.GetLogger(ctx).Errorf("failed to get manifest config: %v", err) | ||
return nil, err | ||
} | ||
h.cachedConfig = blob | ||
} | ||
|
||
return h.cachedConfig, nil | ||
} | ||
|
||
func (h *manifestOCIHandler) Digest() (digest.Digest, error) { | ||
_, p, err := h.manifest.Payload() | ||
if err != nil { | ||
return "", err | ||
} | ||
return digest.FromBytes(p), nil | ||
} | ||
|
||
func (h *manifestOCIHandler) Manifest() distribution.Manifest { | ||
return h.manifest | ||
} | ||
|
||
func (h *manifestOCIHandler) Layers(ctx context.Context) (string, []imageapiv1.ImageLayer, error) { | ||
layers := make([]imageapiv1.ImageLayer, len(h.manifest.Layers)) | ||
for i, layer := range h.manifest.Layers { | ||
layers[i].Name = layer.Digest.String() | ||
layers[i].LayerSize = layer.Size | ||
layers[i].MediaType = layer.MediaType | ||
} | ||
return imageapi.DockerImageLayersOrderAscending, layers, nil | ||
} | ||
|
||
func (h *manifestOCIHandler) Payload() (mediaType string, payload []byte, canonical []byte, err error) { | ||
mt, p, err := h.manifest.Payload() | ||
return mt, p, p, err | ||
} | ||
|
||
func (h *manifestOCIHandler) verifyLayer(ctx context.Context, fsLayer distribution.Descriptor) error { | ||
// https://bugzilla.redhat.com/show_bug.cgi?id=1745743 | ||
// AWS S3 (and potentially other object stores) only have eventual | ||
// consistency guarantees. Stat can fail here if an image layer was | ||
// recently pushed. In the event Stat returns `ErrBlobUnknown`, retry | ||
// up to 3 seconds. | ||
var desc distribution.Descriptor | ||
if err := wait.ExponentialBackoff( | ||
wait.Backoff{ | ||
Duration: 100 * time.Millisecond, | ||
Factor: 2, | ||
Steps: 6, | ||
}, | ||
func() (done bool, err error) { | ||
desc, err = h.blobStore.Stat(ctx, fsLayer.Digest) | ||
switch { | ||
case err == nil: | ||
return true, nil | ||
case err == distribution.ErrBlobUnknown: | ||
return false, nil | ||
default: | ||
return true, err | ||
} | ||
}, | ||
); err != nil { | ||
if err == wait.ErrWaitTimeout { | ||
return distribution.ErrBlobUnknown | ||
} | ||
return err | ||
} | ||
|
||
if fsLayer.Size != desc.Size { | ||
return ErrManifestBlobBadSize{ | ||
Digest: fsLayer.Digest, | ||
ActualSize: desc.Size, | ||
SizeInManifest: fsLayer.Size, | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (h *manifestOCIHandler) Verify(ctx context.Context, skipDependencyVerification bool) error { | ||
var errs distribution.ErrManifestVerification | ||
|
||
if skipDependencyVerification { | ||
return nil | ||
} | ||
|
||
// we want to verify that referenced blobs exist locally or accessible over | ||
// pullthroughBlobStore. The base image of this image can be remote repository | ||
// and since we use pullthroughBlobStore all the layer existence checks will be | ||
// successful. This means that the docker client will not attempt to send them | ||
// to us as it will assume that the registry has them. | ||
|
||
for _, fsLayer := range h.manifest.References() { | ||
if err := h.verifyLayer(ctx, fsLayer); err != nil { | ||
if err != distribution.ErrBlobUnknown { | ||
errs = append(errs, err) | ||
continue | ||
} | ||
|
||
// On error here, we always append unknown blob errors. | ||
errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest}) | ||
} | ||
} | ||
|
||
if len(errs) > 0 { | ||
return errs | ||
} | ||
return nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
package integration | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"net/url" | ||
"testing" | ||
"time" | ||
|
||
imageclientv1 "github.com/openshift/client-go/image/clientset/versioned/typed/image/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
||
"github.com/openshift/image-registry/pkg/testframework" | ||
"github.com/openshift/image-registry/pkg/testutil" | ||
) | ||
|
||
func TestOCIPush(t *testing.T) { | ||
ctx, cancel := context.WithTimeout(context.Background(), time.Minute) | ||
defer cancel() | ||
|
||
master := testframework.NewMaster(t) | ||
defer master.Close() | ||
registry := master.StartRegistry(t) | ||
defer registry.Close() | ||
|
||
namespace := "oci-integration-test" | ||
isname := "imagestream" | ||
testuser := master.CreateUser("testuser", "testp@ssw0rd") | ||
proj := master.CreateProject(namespace, testuser.Name) | ||
|
||
regURL, err := url.Parse(registry.BaseURL()) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
dgst, _, _, _, err := testutil.CreateAndUploadTestManifest( | ||
ctx, | ||
testutil.ManifestSchemaOCI, | ||
5, | ||
regURL, | ||
testutil.NewBasicCredentialStore(testuser.Name, testuser.Token), | ||
fmt.Sprintf("%s/%s", proj.Name, isname), | ||
"latest", | ||
) | ||
if err != nil { | ||
t.Errorf("error uploading manifest: %s", err) | ||
} | ||
|
||
imgcli := imageclientv1.NewForConfigOrDie(testuser.KubeConfig()) | ||
is, err := imgcli.ImageStreams(namespace).Get( | ||
ctx, isname, metav1.GetOptions{}) | ||
if err != nil { | ||
t.Errorf("error getting image stream: %s", err) | ||
} | ||
if len(is.Status.Tags) != 1 { | ||
t.Fatal("no tag found in image stream") | ||
} | ||
|
||
if is.Status.Tags[0].Items[0].Image != dgst.String() { | ||
t.Errorf( | ||
"expecting image digest %s, received %s instead", | ||
dgst, | ||
is.Status.Tags[0].Items[0].Image, | ||
) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's better to check that the image object has proper mediatype and information about the config blob. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. PTAL. |
||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the error message will be misleading if there are more than 1 tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. PTAL.