diff --git a/pkg/cmd/dockerregistry/dockerregistry.go b/pkg/cmd/dockerregistry/dockerregistry.go index b9ff54d5eaf5..a65b4832b6cc 100644 --- a/pkg/cmd/dockerregistry/dockerregistry.go +++ b/pkg/cmd/dockerregistry/dockerregistry.go @@ -141,13 +141,22 @@ func Execute(configFile io.Reader) { if err != nil { log.Fatalf("error parsing configuration file: %s", err) } + + err = Start(dockerConfig, extraConfig) + if err != nil { + log.Fatal(err) + } +} + +// Start runs the Docker registry. Start always returns a non-nil error. +func Start(dockerConfig *configuration.Configuration, extraConfig *registryconfig.Configuration) error { setDefaultMiddleware(dockerConfig) setDefaultLogParameters(dockerConfig) ctx := context.Background() - ctx, err = configureLogging(ctx, dockerConfig) + ctx, err := configureLogging(ctx, dockerConfig) if err != nil { - log.Fatalf("error configuring logger: %v", err) + return fmt.Errorf("error configuring logger: %v", err) } log.WithFields(versionFields()).Info("start registry") @@ -171,69 +180,64 @@ func Execute(configFile io.Reader) { if dockerConfig.HTTP.TLS.Certificate == "" { context.GetLogger(ctx).Infof("listening on %v", dockerConfig.HTTP.Addr) - if err := http.ListenAndServe(dockerConfig.HTTP.Addr, handler); err != nil { - context.GetLogger(ctx).Fatalln(err) + return http.ListenAndServe(dockerConfig.HTTP.Addr, handler) + } + + var ( + minVersion uint16 + cipherSuites []uint16 + ) + if s := os.Getenv("REGISTRY_HTTP_TLS_MINVERSION"); len(s) > 0 { + minVersion, err = crypto.TLSVersion(s) + if err != nil { + return fmt.Errorf("invalid TLS version %q specified in REGISTRY_HTTP_TLS_MINVERSION: %v (valid values are %q)", s, err, crypto.ValidTLSVersions()) } - } else { - var ( - minVersion uint16 - cipherSuites []uint16 - ) - if s := os.Getenv("REGISTRY_HTTP_TLS_MINVERSION"); len(s) > 0 { - minVersion, err = crypto.TLSVersion(s) + } + if s := os.Getenv("REGISTRY_HTTP_TLS_CIPHERSUITES"); len(s) > 0 { + for _, cipher := range strings.Split(s, ",") { + cipherSuite, err := crypto.CipherSuite(cipher) if err != nil { - context.GetLogger(ctx).Fatalln(fmt.Errorf("invalid TLS version %q specified in REGISTRY_HTTP_TLS_MINVERSION: %v (valid values are %q)", s, err, crypto.ValidTLSVersions())) - } - } - if s := os.Getenv("REGISTRY_HTTP_TLS_CIPHERSUITES"); len(s) > 0 { - for _, cipher := range strings.Split(s, ",") { - cipherSuite, err := crypto.CipherSuite(cipher) - if err != nil { - context.GetLogger(ctx).Fatalln(fmt.Errorf("invalid cipher suite %q specified in REGISTRY_HTTP_TLS_CIPHERSUITES: %v (valid suites are %q)", s, err, crypto.ValidCipherSuites())) - } - cipherSuites = append(cipherSuites, cipherSuite) + return fmt.Errorf("invalid cipher suite %q specified in REGISTRY_HTTP_TLS_CIPHERSUITES: %v (valid suites are %q)", s, err, crypto.ValidCipherSuites()) } + cipherSuites = append(cipherSuites, cipherSuite) } + } - tlsConf := crypto.SecureTLSConfig(&tls.Config{ - ClientAuth: tls.NoClientCert, - MinVersion: minVersion, - CipherSuites: cipherSuites, - }) - - if len(dockerConfig.HTTP.TLS.ClientCAs) != 0 { - pool := x509.NewCertPool() + tlsConf := crypto.SecureTLSConfig(&tls.Config{ + ClientAuth: tls.NoClientCert, + MinVersion: minVersion, + CipherSuites: cipherSuites, + }) - for _, ca := range dockerConfig.HTTP.TLS.ClientCAs { - caPem, err := ioutil.ReadFile(ca) - if err != nil { - context.GetLogger(ctx).Fatalln(err) - } + if len(dockerConfig.HTTP.TLS.ClientCAs) != 0 { + pool := x509.NewCertPool() - if ok := pool.AppendCertsFromPEM(caPem); !ok { - context.GetLogger(ctx).Fatalln(fmt.Errorf("Could not add CA to pool")) - } + for _, ca := range dockerConfig.HTTP.TLS.ClientCAs { + caPem, err := ioutil.ReadFile(ca) + if err != nil { + return err } - for _, subj := range pool.Subjects() { - context.GetLogger(ctx).Debugf("CA Subject: %s", string(subj)) + if ok := pool.AppendCertsFromPEM(caPem); !ok { + return fmt.Errorf("could not add CA to pool") } - - tlsConf.ClientAuth = tls.RequireAndVerifyClientCert - tlsConf.ClientCAs = pool } - context.GetLogger(ctx).Infof("listening on %v, tls", dockerConfig.HTTP.Addr) - server := &http.Server{ - Addr: dockerConfig.HTTP.Addr, - Handler: handler, - TLSConfig: tlsConf, + for _, subj := range pool.Subjects() { + context.GetLogger(ctx).Debugf("CA Subject: %s", string(subj)) } - if err := server.ListenAndServeTLS(dockerConfig.HTTP.TLS.Certificate, dockerConfig.HTTP.TLS.Key); err != nil { - context.GetLogger(ctx).Fatalln(err) - } + tlsConf.ClientAuth = tls.RequireAndVerifyClientCert + tlsConf.ClientCAs = pool + } + + context.GetLogger(ctx).Infof("listening on %v, tls", dockerConfig.HTTP.Addr) + server := &http.Server{ + Addr: dockerConfig.HTTP.Addr, + Handler: handler, + TLSConfig: tlsConf, } + return server.ListenAndServeTLS(dockerConfig.HTTP.TLS.Certificate, dockerConfig.HTTP.TLS.Key) } // configureLogging prepares the context with a logger using the diff --git a/pkg/dockerregistry/server/blobdescriptorservice_test.go b/pkg/dockerregistry/server/blobdescriptorservice_test.go index 4aa6c36c4e8f..b8713761e461 100644 --- a/pkg/dockerregistry/server/blobdescriptorservice_test.go +++ b/pkg/dockerregistry/server/blobdescriptorservice_test.go @@ -87,7 +87,7 @@ func TestBlobDescriptorServiceIsApplied(t *testing.T) { t.Fatalf("error parsing server url: %v", err) } - desc, _, err := registrytest.UploadRandomTestBlob(serverURL, nil, "user/app") + desc, _, err := registrytest.UploadRandomTestBlob(ctx, serverURL, nil, "user/app") if err != nil { t.Fatal(err) } diff --git a/pkg/dockerregistry/server/manifesthandler.go b/pkg/dockerregistry/server/manifesthandler.go index bf298e24b355..a18418b8a5bb 100644 --- a/pkg/dockerregistry/server/manifesthandler.go +++ b/pkg/dockerregistry/server/manifesthandler.go @@ -24,6 +24,9 @@ type ManifestHandler interface { // Manifest returns a deserialized manifest object. Manifest() distribution.Manifest + // Layers returns image layers and a value for the dockerLayersOrder annotation. + Layers(ctx context.Context) (order string, layers []imageapiv1.ImageLayer, err error) + // Payload returns manifest's media type, complete payload with signatures and canonical payload without // signatures or an error if the information could not be fetched. Payload() (mediaType string, payload []byte, canonical []byte, err error) diff --git a/pkg/dockerregistry/server/manifestschema1handler.go b/pkg/dockerregistry/server/manifestschema1handler.go index c821a4871f5b..a22d9ab2eb55 100644 --- a/pkg/dockerregistry/server/manifestschema1handler.go +++ b/pkg/dockerregistry/server/manifestschema1handler.go @@ -11,6 +11,9 @@ import ( "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/reference" "github.com/docker/libtrust" + + imageapi "github.com/openshift/origin/pkg/image/apis/image" + imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1" ) func unmarshalManifestSchema1(content []byte, signatures [][]byte) (distribution.Manifest, error) { @@ -41,8 +44,9 @@ func unmarshalManifestSchema1(content []byte, signatures [][]byte) (distribution } type manifestSchema1Handler struct { - repo *repository - manifest *schema1.SignedManifest + repo *repository + manifest *schema1.SignedManifest + blobsCache map[digest.Digest]distribution.Descriptor } var _ ManifestHandler = &manifestSchema1Handler{} @@ -59,6 +63,44 @@ func (h *manifestSchema1Handler) Manifest() distribution.Manifest { return h.manifest } +func (h *manifestSchema1Handler) statBlob(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { + desc, ok := h.blobsCache[dgst] + if ok { + return desc, nil + } + + desc, err := h.repo.Blobs(ctx).Stat(ctx, dgst) + if err != nil { + return desc, err + } + + if h.blobsCache == nil { + h.blobsCache = make(map[digest.Digest]distribution.Descriptor) + } + h.blobsCache[dgst] = desc + + return desc, nil +} + +func (h *manifestSchema1Handler) Layers(ctx context.Context) (string, []imageapiv1.ImageLayer, error) { + layers := make([]imageapiv1.ImageLayer, len(h.manifest.FSLayers)) + for i, fslayer := range h.manifest.FSLayers { + desc, err := h.statBlob(ctx, fslayer.BlobSum) + if err != nil { + return "", nil, err + } + + // In a schema1 manifest the layers are ordered from the youngest to + // the oldest. But we want to have layers in different order. + revidx := (len(h.manifest.FSLayers) - 1) - i // n-1, n-2, ..., 1, 0 + + layers[revidx].Name = fslayer.BlobSum.String() + layers[revidx].LayerSize = desc.Size + layers[revidx].MediaType = schema1.MediaTypeManifestLayer + } + return imageapi.DockerImageLayersOrderAscending, layers, nil +} + func (h *manifestSchema1Handler) Payload() (mediaType string, payload []byte, canonical []byte, err error) { mt, payload, err := h.manifest.Payload() return mt, payload, h.manifest.Canonical, err @@ -72,7 +114,6 @@ func (h *manifestSchema1Handler) Verify(ctx context.Context, skipDependencyVerif // 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. - repo := h.repo if len(path.Join(h.repo.config.registryAddr, h.manifest.Name)) > reference.NameTotalLengthMax { errs = append(errs, @@ -116,7 +157,7 @@ func (h *manifestSchema1Handler) Verify(ctx context.Context, skipDependencyVerif } for _, fsLayer := range h.manifest.References() { - _, err := repo.Blobs(ctx).Stat(ctx, fsLayer.Digest) + _, err := h.statBlob(ctx, fsLayer.Digest) if err != nil { if err != distribution.ErrBlobUnknown { errs = append(errs, err) diff --git a/pkg/dockerregistry/server/manifestschema2handler.go b/pkg/dockerregistry/server/manifestschema2handler.go index 18e45a9c7223..962fe4d767da 100644 --- a/pkg/dockerregistry/server/manifestschema2handler.go +++ b/pkg/dockerregistry/server/manifestschema2handler.go @@ -10,6 +10,9 @@ import ( "github.com/docker/distribution/context" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest/schema2" + + imageapi "github.com/openshift/origin/pkg/image/apis/image" + imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1" ) var ( @@ -65,11 +68,51 @@ func (h *manifestSchema2Handler) Manifest() distribution.Manifest { return h.manifest } +func (h *manifestSchema2Handler) 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 *manifestSchema2Handler) Payload() (mediaType string, payload []byte, canonical []byte, err error) { mt, p, err := h.manifest.Payload() return mt, p, p, err } +func (h *manifestSchema2Handler) verifyLayer(ctx context.Context, fsLayer distribution.Descriptor) error { + if fsLayer.MediaType == schema2.MediaTypeForeignLayer { + // Clients download this layer from an external URL, so do not check for + // its presense. + if len(fsLayer.URLs) == 0 { + return errMissingURL + } + return nil + } + + if len(fsLayer.URLs) != 0 { + return errUnexpectedURL + } + + desc, err := h.repo.Blobs(ctx).Stat(ctx, fsLayer.Digest) + if err != nil { + return err + } + + if fsLayer.Size != desc.Size { + return ErrManifestBlobBadSize{ + Digest: fsLayer.Digest, + ActualSize: desc.Size, + SizeInManifest: fsLayer.Size, + } + } + + return nil +} + func (h *manifestSchema2Handler) Verify(ctx context.Context, skipDependencyVerification bool) error { var errs distribution.ErrManifestVerification @@ -82,35 +125,9 @@ func (h *manifestSchema2Handler) Verify(ctx context.Context, skipDependencyVerif // 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. - repo := h.repo - - target := h.manifest.Target() - _, err := repo.Blobs(ctx).Stat(ctx, target.Digest) - if err != nil { - if err != distribution.ErrBlobUnknown { - errs = append(errs, err) - } - - // On error here, we always append unknown blob errors. - errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: target.Digest}) - } for _, fsLayer := range h.manifest.References() { - var err error - if fsLayer.MediaType != schema2.MediaTypeForeignLayer { - if len(fsLayer.URLs) == 0 { - _, err = repo.Blobs(ctx).Stat(ctx, fsLayer.Digest) - } else { - err = errUnexpectedURL - } - } else { - // Clients download this layer from an external URL, so do not check for - // its presense. - if len(fsLayer.URLs) == 0 { - err = errMissingURL - } - } - if err != nil { + if err := h.verifyLayer(ctx, fsLayer); err != nil { if err != distribution.ErrBlobUnknown { errs = append(errs, err) continue diff --git a/pkg/dockerregistry/server/manifestservice.go b/pkg/dockerregistry/server/manifestservice.go index 9297301486f8..355a0f021928 100644 --- a/pkg/dockerregistry/server/manifestservice.go +++ b/pkg/dockerregistry/server/manifestservice.go @@ -21,6 +21,20 @@ import ( quotautil "github.com/openshift/origin/pkg/quota/util" ) +// ErrManifestBlobBadSize is returned when the blob size in a manifest does +// not match the actual size. The docker/distribution does not check this and +// therefore does not provide an error for this. +type ErrManifestBlobBadSize struct { + Digest digest.Digest + ActualSize int64 + SizeInManifest int64 +} + +func (err ErrManifestBlobBadSize) Error() string { + return fmt.Sprintf("the blob %s has the size (%d) different from the one specified in the manifest (%d)", + err.Digest, err.ActualSize, err.SizeInManifest) +} + var _ distribution.ManifestService = &manifestService{} type manifestService struct { @@ -104,6 +118,7 @@ func (m *manifestService) Put(ctx context.Context, manifest distribution.Manifes if err != nil { return "", regapi.ErrorCodeManifestInvalid.WithDetail(err) } + mediaType, payload, _, err := mh.Payload() if err != nil { return "", regapi.ErrorCodeManifestInvalid.WithDetail(err) @@ -134,6 +149,11 @@ func (m *manifestService) Put(ctx context.Context, manifest distribution.Manifes return "", err } + layerOrder, layers, err := mh.Layers(ctx) + if err != nil { + return "", err + } + // Upload to openshift ism := imageapiv1.ImageStreamMapping{ ObjectMeta: metav1.ObjectMeta{ @@ -146,12 +166,14 @@ func (m *manifestService) Put(ctx context.Context, manifest distribution.Manifes Annotations: map[string]string{ imageapi.ManagedByOpenShiftAnnotation: "true", imageapi.ImageManifestBlobStoredAnnotation: "true", + imageapi.DockerImageLayersOrderAnnotation: layerOrder, }, }, DockerImageReference: fmt.Sprintf("%s/%s/%s@%s", m.repo.config.registryAddr, m.repo.namespace, m.repo.name, dgst.String()), DockerImageManifest: string(payload), DockerImageManifestMediaType: mediaType, DockerImageConfig: string(config), + DockerImageLayers: layers, }, } diff --git a/pkg/dockerregistry/server/manifestservice_test.go b/pkg/dockerregistry/server/manifestservice_test.go index 116d5b2f5be4..930322a8e1f1 100644 --- a/pkg/dockerregistry/server/manifestservice_test.go +++ b/pkg/dockerregistry/server/manifestservice_test.go @@ -151,6 +151,7 @@ func TestManifestServicePut(t *testing.T) { Manifest: schema2.Manifest{ Config: distribution.Descriptor{ Digest: "test:1", + Size: 2, }, }, } diff --git a/pkg/dockerregistry/server/pullthroughblobstore_test.go b/pkg/dockerregistry/server/pullthroughblobstore_test.go index fe70ec1086c9..24a17d765a79 100644 --- a/pkg/dockerregistry/server/pullthroughblobstore_test.go +++ b/pkg/dockerregistry/server/pullthroughblobstore_test.go @@ -9,7 +9,6 @@ import ( "net/url" "os" "strconv" - "strings" "testing" "time" @@ -23,6 +22,7 @@ import ( registrytest "github.com/openshift/origin/pkg/dockerregistry/testutil" imageapi "github.com/openshift/origin/pkg/image/apis/image" imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1" + "github.com/openshift/origin/pkg/image/importer" ) func TestPullthroughServeBlob(t *testing.T) { @@ -55,11 +55,11 @@ func TestPullthroughServeBlob(t *testing.T) { }) registrytest.AddImage(t, fos, testImage, namespace, name, "latest") - blob1Desc, blob1Content, err := registrytest.UploadRandomTestBlob(serverURL, nil, repoName) + blob1Desc, blob1Content, err := registrytest.UploadRandomTestBlob(backgroundCtx, serverURL, nil, repoName) if err != nil { t.Fatal(err) } - blob2Desc, blob2Content, err := registrytest.UploadRandomTestBlob(serverURL, nil, repoName) + blob2Desc, blob2Content, err := registrytest.UploadRandomTestBlob(backgroundCtx, serverURL, nil, repoName) if err != nil { t.Fatal(err) } @@ -205,154 +205,99 @@ func TestPullthroughServeBlob(t *testing.T) { } func TestPullthroughServeNotSeekableBlob(t *testing.T) { - namespace, name := "user", "app" - repoName := fmt.Sprintf("%s/%s", namespace, name) - installFakeAccessController(t) - setPassthroughBlobDescriptorServiceFactory() - - blob1Content, err := registrytest.CreateRandomTarFile() + repoName := "foorepo" + blob, err := registrytest.CreateRandomTarFile() if err != nil { t.Fatalf("unexpected error generating test layer file: %v", err) } + dgst := digest.FromBytes(blob) - dgst := digest.FromBytes(blob1Content) - blob1Storage := map[digest.Digest][]byte{ - dgst: blob1Content, - } - - // start regular HTTP server - remoteRegistryServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - t.Logf("External registry got %s %s", r.Method, r.URL.Path) + externalRegistry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Logf("external registry got %s %s", r.Method, r.URL.Path) w.Header().Set("Docker-Distribution-API-Version", "registry/2.0") switch r.URL.Path { case "/v2/": w.Write([]byte(`{}`)) - case "/v2/" + repoName + "/tags/list": - w.Write([]byte("{\"name\": \"" + repoName + "\", \"tags\": [\"latest\"]}")) - case "/v2/" + repoName + "/manifests/latest", "/v2/" + repoName + "/manifests/" + etcdDigest: + case "/v2/" + repoName + "/blobs/" + dgst.String(): + w.Header().Set("Content-Length", fmt.Sprintf("%d", len(blob))) + w.Header().Set("Docker-Content-Digest", dgst.String()) + if r.Method == "HEAD" { - w.Header().Set("Content-Length", fmt.Sprintf("%d", len(etcdManifest))) - w.Header().Set("Docker-Content-Digest", etcdDigest) w.WriteHeader(http.StatusOK) } else { - w.Write([]byte(etcdManifest)) + // We need to return any return code between 200 and 399, + // except 200 and 206 [1]. + // + // In this case the docker client library will make a not + // truly seekable response [2]. + // + // [1]: https://github.com/docker/distribution/blob/7484e51bf6af0d3b1a849644cdaced3cfcf13617/registry/client/transport/http_reader.go#L239 + // [2]: https://github.com/docker/distribution/blob/7484e51bf6af0d3b1a849644cdaced3cfcf13617/registry/client/transport/http_reader.go#L119-L121 + w.WriteHeader(http.StatusNonAuthoritativeInfo) + w.Write(blob) } default: - if strings.HasPrefix(r.URL.Path, "/v2/"+repoName+"/blobs/") { - for dgst, payload := range blob1Storage { - if r.URL.Path != "/v2/"+repoName+"/blobs/"+dgst.String() { - continue - } - w.Header().Set("Content-Length", fmt.Sprintf("%d", len(payload))) - if r.Method == "HEAD" { - w.Header().Set("Docker-Content-Digest", dgst.String()) - w.WriteHeader(http.StatusOK) - return - } else { - // Important! - // - // We need to return any return code between 200 and 399, expept 200 and 206. - // https://github.com/docker/distribution/blob/master/registry/client/transport/http_reader.go#L192 - // - // In this case the docker client library will make a not truly - // seekable response. - // https://github.com/docker/distribution/blob/master/registry/client/transport/http_reader.go#L239 - w.WriteHeader(http.StatusAccepted) - } - w.Write(payload) - return - } - } - t.Fatalf("unexpected request %s: %#v", r.URL.Path, r) + panic(fmt.Errorf("unexpected request: %#+v", r)) } })) + defer externalRegistry.Close() - serverURL, err := url.Parse(remoteRegistryServer.URL) + externalRegistryURL, err := url.Parse(externalRegistry.URL) if err != nil { - t.Fatalf("error parsing server url: %v", err) + t.Fatal("error parsing test server url:", err) } - os.Setenv("OPENSHIFT_DEFAULT_REGISTRY", serverURL.Host) - - testImage, err := registrytest.NewImageForManifest(repoName, registrytest.SampleImageManifestSchema1, "", false) - if err != nil { - t.Fatal(err) - } - testImage.DockerImageReference = fmt.Sprintf("%s/%s@%s", serverURL.Host, repoName, testImage.Name) ctx := context.Background() ctx = registrytest.WithTestLogger(ctx, t) - fos, imageClient := registrytest.NewFakeOpenShiftWithClient(ctx) - registrytest.AddImageStream(t, fos, namespace, name, map[string]string{ - imageapi.InsecureRepositoryAnnotation: "true", - }) - registrytest.AddImage(t, fos, testImage, namespace, name, "latest") - localBlobStore := newTestBlobStore(nil, nil) - - ctx = WithTestPassthroughToUpstream(ctx, false) - repo := newTestRepository(ctx, t, namespace, name, testRepositoryOptions{ - client: registryclient.NewFakeRegistryAPIClient(nil, imageClient), - enablePullThrough: true, - }) - ptbs := &pullthroughBlobStore{ - BlobStore: localBlobStore, - repo: repo, - } - - req, err := http.NewRequest("GET", fmt.Sprintf("http://example.org/v2/user/app/blobs/%s", dgst), nil) + retriever := importer.NewContext(http.DefaultTransport, http.DefaultTransport).WithCredentials(importer.NoCredentials) + repo, err := retriever.Repository(ctx, externalRegistryURL, repoName, true) if err != nil { - t.Fatalf("failed to create http request: %v", err) + t.Fatal(err) } - w := httptest.NewRecorder() - if _, err = ptbs.Stat(ctx, dgst); err != nil { - t.Fatalf("Stat returned unexpected error: %#+v", err) - } + repoBlobs := repo.Blobs(ctx) - if err = ptbs.ServeBlob(ctx, w, req, dgst); err != nil { - t.Fatalf("ServeBlob returned unexpected error: %#+v", err) + // Test that the reader is not seekable. + remoteBlob, err := repoBlobs.Open(ctx, dgst) + if err != nil { + t.Fatalf("failed to Open blob %s: %v", dgst, err) } + defer remoteBlob.Close() - if w.Code != http.StatusOK { - t.Fatalf(`unexpected StatusCode: %d (expected %d)`, w.Code, http.StatusOK) + if _, err := remoteBlob.Seek(0, os.SEEK_END); err == nil { + t.Fatal("expected non-seekable blob reader, but Seek(0, os.SEEK_END) succeed") } - clstr := w.Header().Get("Content-Length") - if cl, err := strconv.ParseInt(clstr, 10, 64); err != nil { - t.Fatalf(`unexpected Content-Length: %q (expected "%d")`, clstr, int64(len(blob1Content))) - } else { - if cl != int64(len(blob1Content)) { - t.Fatalf("Content-Length does not match expected size: %d != %d", cl, int64(len(blob1Content))) - } + // Test that the blob can be fetched. + ptbs := &pullthroughBlobStore{ + BlobStore: newTestBlobStore(nil, nil), + repo: &repository{ + remoteBlobGetter: repoBlobs, + }, + mirror: false, } - body := w.Body.Bytes() - if int64(len(body)) != int64(len(blob1Content)) { - t.Errorf("unexpected size of body: %d != %d", len(body), int64(len(blob1Content))) - } + req := httptest.NewRequest("GET", "/unused", nil) + w := httptest.NewRecorder() - if localBlobStore.bytesServed != 0 { - t.Fatalf("remote blob served locally") + if err = ptbs.ServeBlob(ctx, w, req, dgst); err != nil { + t.Fatalf("ServeBlob failed: %v", err) } - expectedLocalCalls := map[string]int{ - "Stat": 1, - "ServeBlob": 1, + if w.Code != http.StatusOK { + t.Errorf("unexpected status code: got %d, want %d", w.Code, http.StatusOK) } - for name, expCount := range expectedLocalCalls { - count := localBlobStore.calls[name] - if count != expCount { - t.Errorf("expected %d calls to method %s of local blob store, not %d", expCount, name, count) - } + clstr := w.Header().Get("Content-Length") + if cl, err := strconv.ParseInt(clstr, 10, 64); err != nil || cl != int64(len(blob)) { + t.Errorf("Content-Length does not match the expected size: got %s, want %d", clstr, len(blob)) } - for name, count := range localBlobStore.calls { - if _, exists := expectedLocalCalls[name]; !exists { - t.Errorf("expected no calls to method %s of local blob store, got %d", name, count) - } + if w.Body.Len() != len(blob) { + t.Errorf("unexpected size of body: got %d, want %d", w.Body.Len(), len(blob)) } } @@ -378,7 +323,7 @@ func TestPullthroughServeBlobInsecure(t *testing.T) { } m1dgst, m1canonical, m1cfg, m1manifest, err := registrytest.CreateAndUploadTestManifest( - registrytest.ManifestSchema2, 2, serverURL, nil, repo1Name, "foo") + backgroundCtx, registrytest.ManifestSchema2, 2, serverURL, nil, repo1Name, "foo") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -388,7 +333,7 @@ func TestPullthroughServeBlobInsecure(t *testing.T) { } t.Logf("m1dgst=%s, m1manifest: %s", m1dgst, m1canonical) m2dgst, m2canonical, m2cfg, m2manifest, err := registrytest.CreateAndUploadTestManifest( - registrytest.ManifestSchema2, 2, serverURL, nil, repo2Name, "bar") + backgroundCtx, registrytest.ManifestSchema2, 2, serverURL, nil, repo2Name, "bar") if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/dockerregistry/server/pullthroughmanifestservice_test.go b/pkg/dockerregistry/server/pullthroughmanifestservice_test.go index 9881d4a9d51b..f80c608581b7 100644 --- a/pkg/dockerregistry/server/pullthroughmanifestservice_test.go +++ b/pkg/dockerregistry/server/pullthroughmanifestservice_test.go @@ -79,7 +79,7 @@ func TestPullthroughManifests(t *testing.T) { } ms1dgst, ms1canonical, _, ms1manifest, err := registrytest.CreateAndUploadTestManifest( - registrytest.ManifestSchema1, 2, serverURL, nil, repoName, "schema1") + backgroundCtx, registrytest.ManifestSchema1, 2, serverURL, nil, repoName, "schema1") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -204,7 +204,7 @@ func TestPullthroughManifestInsecure(t *testing.T) { } ms1dgst, ms1canonical, _, ms1manifest, err := registrytest.CreateAndUploadTestManifest( - registrytest.ManifestSchema1, 2, serverURL, nil, repoName, "schema1") + backgroundCtx, registrytest.ManifestSchema1, 2, serverURL, nil, repoName, "schema1") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -214,7 +214,7 @@ func TestPullthroughManifestInsecure(t *testing.T) { } t.Logf("ms1dgst=%s, ms1manifest: %s", ms1dgst, ms1canonical) ms2dgst, ms2canonical, ms2config, ms2manifest, err := registrytest.CreateAndUploadTestManifest( - registrytest.ManifestSchema2, 2, serverURL, nil, repoName, "schema2") + backgroundCtx, registrytest.ManifestSchema2, 2, serverURL, nil, repoName, "schema2") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -357,59 +357,59 @@ func TestPullthroughManifestInsecure(t *testing.T) { }, }, } { - fos, imageClient := registrytest.NewFakeOpenShiftWithClient(backgroundCtx) + t.Run(tc.name, func(t *testing.T) { + fos, imageClient := registrytest.NewFakeOpenShiftWithClient(backgroundCtx) - tc.fakeOpenShiftInit(fos) + tc.fakeOpenShiftInit(fos) - localManifestService := newTestManifestService(repoName, tc.localData) + localManifestService := newTestManifestService(repoName, tc.localData) - ctx := WithTestPassthroughToUpstream(backgroundCtx, false) - repo := newTestRepository(ctx, t, namespace, repo, testRepositoryOptions{ - client: registryclient.NewFakeRegistryAPIClient(nil, imageClient), - enablePullThrough: true, - }) - ctx = withRepository(ctx, repo) + ctx := WithTestPassthroughToUpstream(backgroundCtx, false) + repo := newTestRepository(ctx, t, namespace, repo, testRepositoryOptions{ + client: registryclient.NewFakeRegistryAPIClient(nil, imageClient), + enablePullThrough: true, + }) + ctx = withRepository(ctx, repo) - ptms := &pullthroughManifestService{ - ManifestService: localManifestService, - repo: repo, - } - - manifestResult, err := ptms.Get(ctx, tc.manifestDigest) - switch err.(type) { - case nil: - if len(tc.expectedErrorString) > 0 { - t.Errorf("[%s] unexpected successful response", tc.name) - continue + ptms := &pullthroughManifestService{ + ManifestService: localManifestService, + repo: repo, } - default: - if len(tc.expectedErrorString) > 0 { - if !strings.Contains(err.Error(), tc.expectedErrorString) { - t.Fatalf("expected error string %q, got instead: %s (%#+v)", tc.expectedErrorString, err.Error(), err) + + manifestResult, err := ptms.Get(ctx, tc.manifestDigest) + switch err.(type) { + case nil: + if len(tc.expectedErrorString) > 0 { + t.Fatalf("unexpected successful response") } - break + default: + if len(tc.expectedErrorString) > 0 { + if !strings.Contains(err.Error(), tc.expectedErrorString) { + t.Fatalf("expected error string %q, got instead: %s (%#+v)", tc.expectedErrorString, err.Error(), err) + } + break + } + t.Fatalf("unexpected error: %#+v (%s)", err, err.Error()) } - t.Fatalf("[%s] unexpected error: %#+v (%s)", tc.name, err, err.Error()) - } - if tc.localData != nil { - if manifestResult != nil && manifestResult != tc.localData[tc.manifestDigest] { - t.Errorf("[%s] unexpected result returned", tc.name) - continue + if tc.localData != nil { + if manifestResult != nil && manifestResult != tc.localData[tc.manifestDigest] { + t.Fatalf("unexpected result returned") + } } - } - registrytest.AssertManifestsEqual(t, tc.name, manifestResult, tc.expectedManifest) + registrytest.AssertManifestsEqual(t, tc.name, manifestResult, tc.expectedManifest) - for name, count := range localManifestService.calls { - expectCount, exists := tc.expectedLocalCalls[name] - if !exists { - t.Errorf("[%s] expected no calls to method %s of local manifest service, got %d", tc.name, name, count) - } - if count != expectCount { - t.Errorf("[%s] unexpected number of calls to method %s of local manifest service, got %d", tc.name, name, count) + for name, count := range localManifestService.calls { + expectCount, exists := tc.expectedLocalCalls[name] + if !exists { + t.Errorf("expected no calls to method %s of local manifest service, got %d", name, count) + } + if count != expectCount { + t.Errorf("unexpected number of calls to method %s of local manifest service, got %d", name, count) + } } - } + }) } } diff --git a/pkg/dockerregistry/testutil/manifests.go b/pkg/dockerregistry/testutil/manifests.go index 42a48d8e277f..fbfec8d3c57a 100644 --- a/pkg/dockerregistry/testutil/manifests.go +++ b/pkg/dockerregistry/testutil/manifests.go @@ -3,7 +3,6 @@ package testutil import ( "encoding/json" "fmt" - "net/http" "net/url" "reflect" "testing" @@ -14,11 +13,7 @@ import ( "github.com/docker/distribution/manifest" "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" - "github.com/docker/distribution/reference" - distclient "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/auth" - "github.com/docker/distribution/registry/client/auth/challenge" - "github.com/docker/distribution/registry/client/transport" "github.com/docker/libtrust" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -31,13 +26,8 @@ import ( ) type ManifestSchemaVersion int -type LayerPayload []byte -type ConfigPayload []byte -type Payload struct { - Config ConfigPayload - Layers []LayerPayload -} +type ConfigPayload []byte const ( ManifestSchema1 ManifestSchemaVersion = 1 @@ -46,8 +36,7 @@ const ( // MakeSchema1Manifest constructs a schema 1 manifest from a given list of digests and returns // the digest of the manifest -// github.com/docker/distribution/testutil -func MakeSchema1Manifest(name, tag string, layers []distribution.Descriptor) (string, distribution.Manifest, error) { +func MakeSchema1Manifest(name, tag string, layers []distribution.Descriptor) (distribution.Manifest, error) { m := schema1.Manifest{ Versioned: manifest.Versioned{ SchemaVersion: 1, @@ -65,20 +54,20 @@ func MakeSchema1Manifest(name, tag string, layers []distribution.Descriptor) (st pk, err := libtrust.GenerateECP256PrivateKey() if err != nil { - return "", nil, fmt.Errorf("unexpected error generating private key: %v", err) + return nil, fmt.Errorf("unexpected error generating private key: %v", err) } signedManifest, err := schema1.Sign(&m, pk) if err != nil { - return "", nil, fmt.Errorf("error signing manifest: %v", err) + return nil, fmt.Errorf("error signing manifest: %v", err) } - return string(signedManifest.Canonical), signedManifest, nil + return signedManifest, nil } // MakeSchema2Manifest constructs a schema 2 manifest from a given list of digests and returns // the digest of the manifest -func MakeSchema2Manifest(config distribution.Descriptor, layers []distribution.Descriptor) (string, distribution.Manifest, error) { +func MakeSchema2Manifest(config distribution.Descriptor, layers []distribution.Descriptor) (distribution.Manifest, error) { m := schema2.Manifest{ Versioned: schema2.SchemaVersion, Config: config, @@ -93,37 +82,38 @@ func MakeSchema2Manifest(config distribution.Descriptor, layers []distribution.D manifest, err := schema2.FromStruct(m) if err != nil { - return "", nil, err - } - - _, payload, err := manifest.Payload() - if err != nil { - return "", nil, err + return nil, err } - return string(payload), manifest, nil + return manifest, nil } -func MakeRandomLayers(layerCount int) ([]distribution.Descriptor, []LayerPayload, error) { - var ( - layers []distribution.Descriptor - payloads []LayerPayload - ) - - for i := 0; i < layerCount; i++ { - content, err := CreateRandomTarFile() +// CanonicalManifest returns m in its canonical representation. +func CanonicalManifest(m distribution.Manifest) ([]byte, error) { + switch m := m.(type) { + case *schema1.SignedManifest: + return m.Canonical, nil + case *schema2.DeserializedManifest: + _, payload, err := m.Payload() if err != nil { - return layers, payloads, fmt.Errorf("unexpected error generating test layer file: %v", err) + return nil, err } + return payload, nil + } + return nil, fmt.Errorf("no canonical representation of %T: %#+v", m, m) +} - layers = append(layers, distribution.Descriptor{ - Digest: digest.FromBytes(content), - Size: int64(len(content)), - }) - payloads = append(payloads, LayerPayload(content)) +func MakeRandomLayer() ([]byte, distribution.Descriptor, error) { + content, err := CreateRandomTarFile() + if err != nil { + return nil, distribution.Descriptor{}, fmt.Errorf("failed to generate data for a random layer: %v", err) } - return layers, payloads, nil + return content, distribution.Descriptor{ + MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip", + Size: int64(len(content)), + Digest: digest.FromBytes(content), + }, nil } func MakeManifestConfig() (ConfigPayload, distribution.Descriptor, error) { @@ -141,42 +131,10 @@ func MakeManifestConfig() (ConfigPayload, distribution.Descriptor, error) { return jsonBytes, cfgDesc, nil } -func CreateRandomManifest(schemaVersion ManifestSchemaVersion, layerCount int) (string, distribution.Manifest, *Payload, error) { - var ( - rawManifest string - manifest distribution.Manifest - cfgDesc distribution.Descriptor - err error - ) - - layersDescs, layerPayloads, err := MakeRandomLayers(layerCount) - if err != nil { - return "", nil, nil, fmt.Errorf("cannot generate layers: %v", err) - } - - payload := &Payload{ - Layers: layerPayloads, - } - - switch schemaVersion { - case ManifestSchema1: - rawManifest, manifest, err = MakeSchema1Manifest("who", "cares", layersDescs) - case ManifestSchema2: - _, cfgDesc, err = MakeManifestConfig() - if err != nil { - return "", nil, nil, err - } - rawManifest, manifest, err = MakeSchema2Manifest(cfgDesc, layersDescs) - default: - return "", nil, nil, fmt.Errorf("unsupported manifest version %d", schemaVersion) - } - - return rawManifest, manifest, payload, err -} - // CreateUploadTestManifest generates a random manifest blob and uploads it to the given repository. For this // purpose, a given number of layers will be created and uploaded. func CreateAndUploadTestManifest( + ctx context.Context, schemaVersion ManifestSchemaVersion, layerCount int, serverURL *url.URL, @@ -188,16 +146,26 @@ func CreateAndUploadTestManifest( ) for i := 0; i < layerCount; i++ { - ds, _, err := UploadRandomTestBlob(serverURL, creds, repoName) + ds, _, err := UploadRandomTestBlob(ctx, serverURL, creds, repoName) if err != nil { return "", "", "", nil, fmt.Errorf("unexpected error generating test blob layer: %v", err) } layerDescriptors = append(layerDescriptors, ds) } + rt, err := NewTransport(serverURL.String(), repoName, creds) + if err != nil { + return "", "", "", nil, err + } + + repo, err := NewRepository(ctx, repoName, serverURL.String(), rt) + if err != nil { + return "", "", "", nil, err + } + switch schemaVersion { case ManifestSchema1: - canonical, manifest, err = MakeSchema1Manifest(repoName, tag, layerDescriptors) + manifest, err = MakeSchema1Manifest(repoName, tag, layerDescriptors) if err != nil { return "", "", "", nil, fmt.Errorf("failed to make manifest of schema 1: %v", err) } @@ -206,11 +174,11 @@ func CreateAndUploadTestManifest( if err != nil { return "", "", "", nil, err } - _, err = UploadBlob(cfgPayload, serverURL, creds, repoName) + err = UploadBlob(ctx, repo, cfgDesc, cfgPayload) if err != nil { return "", "", "", nil, fmt.Errorf("failed to upload manifest config of schema 2: %v", err) } - canonical, manifest, err = MakeSchema2Manifest(cfgDesc, layerDescriptors) + manifest, err = MakeSchema2Manifest(cfgDesc, layerDescriptors) if err != nil { return "", "", "", nil, fmt.Errorf("failed to make manifest schema 2: %v", err) } @@ -219,47 +187,18 @@ func CreateAndUploadTestManifest( return "", "", "", nil, fmt.Errorf("unsupported manifest version %d", schemaVersion) } - expectedDgst := digest.FromBytes([]byte(canonical)) - - ctx := context.Background() - ref, err := reference.ParseNamed(repoName) + canonicalBytes, err := CanonicalManifest(manifest) if err != nil { return "", "", "", nil, err } - var rt http.RoundTripper - if creds != nil { - challengeManager := challenge.NewSimpleManager() - _, err := ping(challengeManager, serverURL.String()+"/v2/", "") - if err != nil { - return "", "", "", nil, err - } - rt = transport.NewTransport( - nil, - auth.NewAuthorizer( - challengeManager, - auth.NewTokenHandler(nil, creds, repoName, "pull", "push"), - auth.NewBasicHandler(creds))) - } - repo, err := distclient.NewRepository(ctx, ref, serverURL.String(), rt) - if err != nil { - return "", "", "", nil, fmt.Errorf("failed to get repository %q: %v", repoName, err) - } + dgst = digest.FromBytes(canonicalBytes) - ms, err := repo.Manifests(ctx) - if err != nil { - return "", "", "", nil, err - } - dgst, err = ms.Put(ctx, manifest) - if err != nil { + if err := UploadManifest(ctx, repo, tag, manifest); err != nil { return "", "", "", nil, err } - if expectedDgst != dgst { - return "", "", "", nil, fmt.Errorf("registry server computed different digest for uploaded manifest than expected: %q != %q", dgst, expectedDgst) - } - - return dgst, canonical, manifestConfig, manifest, nil + return dgst, string(canonicalBytes), manifestConfig, manifest, nil } // AssertManifestsEqual compares two manifests and returns if they are equal. Signatures of manifest schema 1 diff --git a/pkg/dockerregistry/testutil/util.go b/pkg/dockerregistry/testutil/util.go index 1bf215644c25..373a5167f9cc 100644 --- a/pkg/dockerregistry/testutil/util.go +++ b/pkg/dockerregistry/testutil/util.go @@ -22,65 +22,107 @@ import ( imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1" ) -// UploadBlob uploads a blob with payload to the registry server located at -// serverURL. -func UploadBlob( - payload []byte, - serverURL *url.URL, - creds auth.CredentialStore, - repoName string, -) (distribution.Descriptor, error) { - // TODO(dmage): get the context from the caller - ctx := context.Background() +func NewTransport(baseURL string, repoName string, creds auth.CredentialStore) (http.RoundTripper, error) { + if creds == nil { + return nil, nil + } - ref, err := reference.ParseNamed(repoName) + challengeManager := challenge.NewSimpleManager() + + _, err := ping(challengeManager, baseURL+"/v2/", "") if err != nil { - return distribution.Descriptor{}, err + return nil, err } - var rt http.RoundTripper - if creds != nil { - challengeManager := challenge.NewSimpleManager() - _, err := ping(challengeManager, serverURL.String()+"/v2/", "") - if err != nil { - return distribution.Descriptor{}, err - } - rt = transport.NewTransport( - nil, - auth.NewAuthorizer( - challengeManager, - auth.NewTokenHandler(nil, creds, repoName, "pull", "push"), - auth.NewBasicHandler(creds))) - } + return transport.NewTransport( + nil, + auth.NewAuthorizer( + challengeManager, + auth.NewTokenHandler(nil, creds, repoName, "pull", "push"), + auth.NewBasicHandler(creds), + ), + ), nil +} - repo, err := distclient.NewRepository(ctx, ref, serverURL.String(), rt) +// NewRepository creates a new Repository for the given repository name, base URL and creds. +func NewRepository(ctx context.Context, repoName string, baseURL string, transport http.RoundTripper) (distribution.Repository, error) { + ref, err := reference.ParseNamed(repoName) if err != nil { - return distribution.Descriptor{}, fmt.Errorf("failed to get repository %q: %v", repoName, err) + return nil, err } + return distclient.NewRepository(ctx, ref, baseURL, transport) +} + +// UploadBlob uploads the blob with content to repo and verifies its digest. +func UploadBlob(ctx context.Context, repo distribution.Repository, desc distribution.Descriptor, content []byte) error { wr, err := repo.Blobs(ctx).Create(ctx) if err != nil { - return distribution.Descriptor{}, err + return fmt.Errorf("failed to create upload to %s: %v", repo.Named(), err) } - _, err = io.Copy(wr, bytes.NewReader(payload)) + _, err = io.Copy(wr, bytes.NewReader(content)) if err != nil { - return distribution.Descriptor{}, fmt.Errorf("unexpected error copying to upload: %v", err) + return fmt.Errorf("error uploading blob to %s: %v", repo.Named(), err) } - return wr.Commit(ctx, distribution.Descriptor{ - Digest: digest.FromBytes(payload), + uploadDesc, err := wr.Commit(ctx, distribution.Descriptor{ + Digest: digest.FromBytes(content), }) + if err != nil { + return fmt.Errorf("failed to complete upload to %s: %v", repo.Named(), err) + } + + // uploadDesc is checked here and is not returned, because it has invalid MediaType. + if uploadDesc.Digest != desc.Digest { + return fmt.Errorf("upload blob to %s failed: digest mismatch: got %s, want %s", repo.Named(), uploadDesc.Digest, desc.Digest) + } + + return nil +} + +// UploadManifest uploads manifest to repo and verifies its digest. +func UploadManifest(ctx context.Context, repo distribution.Repository, tag string, manifest distribution.Manifest) error { + canonical, err := CanonicalManifest(manifest) + if err != nil { + return err + } + + ms, err := repo.Manifests(ctx) + if err != nil { + return fmt.Errorf("failed to get manifest service for %s: %v", repo.Named(), err) + } + + dgst, err := ms.Put(ctx, manifest, distribution.WithTag(tag)) + if err != nil { + return fmt.Errorf("failed to upload manifest to %s: %v", repo.Named(), err) + } + + if expectedDgst := digest.FromBytes(canonical); dgst != expectedDgst { + return fmt.Errorf("upload manifest to %s failed: digest mismatch: got %s, want %s", repo.Named(), dgst, expectedDgst) + } + + return nil } // UploadRandomTestBlob generates a random tar file and uploads it to the given repository. -func UploadRandomTestBlob(serverURL *url.URL, creds auth.CredentialStore, repoName string) (distribution.Descriptor, []byte, error) { - payload, err := CreateRandomTarFile() +func UploadRandomTestBlob(ctx context.Context, serverURL *url.URL, creds auth.CredentialStore, repoName string) (distribution.Descriptor, []byte, error) { + payload, desc, err := MakeRandomLayer() if err != nil { return distribution.Descriptor{}, nil, fmt.Errorf("unexpected error generating test layer file: %v", err) } - desc, err := UploadBlob(payload, serverURL, creds, repoName) + rt, err := NewTransport(serverURL.String(), repoName, creds) + if err != nil { + return distribution.Descriptor{}, nil, err + } + + repo, err := NewRepository(ctx, repoName, serverURL.String(), rt) + if err != nil { + return distribution.Descriptor{}, nil, err + } + + err = UploadBlob(ctx, repo, desc, payload) if err != nil { return distribution.Descriptor{}, nil, fmt.Errorf("upload random test blob: %s", err) } @@ -90,9 +132,9 @@ func UploadRandomTestBlob(serverURL *url.URL, creds auth.CredentialStore, repoNa // CreateRandomTarFile creates a random tarfile and returns its content. // An error is returned if there is a problem generating valid content. -// Inspired by github.com/vendor/docker/distribution/testutil/tarfile.go. +// Inspired by github.com/docker/distribution/testutil/tarfile.go. func CreateRandomTarFile() ([]byte, error) { - nFiles := 2 + nFiles := 2 // random enough var target bytes.Buffer wr := tar.NewWriter(&target) @@ -138,7 +180,18 @@ func CreateRandomTarFile() ([]byte, error) { // CreateRandomImage creates an image with a random content. func CreateRandomImage(namespace, name string) (*imageapiv1.Image, error) { - _, manifest, _, err := CreateRandomManifest(ManifestSchema1, 3) + const layersCount = 3 + + layersDescs := make([]distribution.Descriptor, layersCount) + for i := range layersDescs { + _, desc, err := MakeRandomLayer() + if err != nil { + return nil, err + } + layersDescs[i] = desc + } + + manifest, err := MakeSchema1Manifest("unused-name", "unused-tag", layersDescs) if err != nil { return nil, err } @@ -148,17 +201,12 @@ func CreateRandomImage(namespace, name string) (*imageapiv1.Image, error) { return nil, err } - image, err := NewImageForManifest( + return NewImageForManifest( fmt.Sprintf("%s/%s", namespace, name), string(manifestSchema1), "", false, ) - if err != nil { - return nil, err - } - - return image, nil } const SampleImageManifestSchema1 = `{ diff --git a/pkg/image/util/helpers.go b/pkg/image/util/helpers.go index eae76e40cbf9..c50b68a2fa44 100644 --- a/pkg/image/util/helpers.go +++ b/pkg/image/util/helpers.go @@ -14,6 +14,58 @@ import ( imageapi "github.com/openshift/origin/pkg/image/apis/image" ) +func fillImageLayers(image *imageapi.Image, manifest imageapi.DockerImageManifest) error { + if len(image.DockerImageLayers) != 0 { + // DockerImageLayers is already filled by the registry. + return nil + } + + switch manifest.SchemaVersion { + case 1: + if len(manifest.History) != len(manifest.FSLayers) { + return fmt.Errorf("the image %s (%s) has mismatched history and fslayer cardinality (%d != %d)", image.Name, image.DockerImageReference, len(manifest.History), len(manifest.FSLayers)) + } + + image.DockerImageLayers = make([]imageapi.ImageLayer, len(manifest.FSLayers)) + for i, obj := range manifest.History { + layer := manifest.FSLayers[i] + + var size imageapi.DockerV1CompatibilityImageSize + if err := json.Unmarshal([]byte(obj.DockerV1Compatibility), &size); err != nil { + size.Size = 0 + } + + // reverse order of the layers: in schema1 manifests the + // first layer is the youngest (base layers are at the + // end), but we want to store layers in the Image resource + // in order from the oldest to the youngest. + revidx := (len(manifest.History) - 1) - i // n-1, n-2, ..., 1, 0 + + image.DockerImageLayers[revidx].Name = layer.DockerBlobSum + image.DockerImageLayers[revidx].LayerSize = size.Size + image.DockerImageLayers[revidx].MediaType = schema1.MediaTypeManifestLayer + } + case 2: + // The layer list is ordered starting from the base image (opposite order of schema1). + // So, we do not need to change the order of layers. + image.DockerImageLayers = make([]imageapi.ImageLayer, len(manifest.Layers)) + for i, layer := range manifest.Layers { + image.DockerImageLayers[i].Name = layer.Digest + image.DockerImageLayers[i].LayerSize = layer.Size + image.DockerImageLayers[i].MediaType = layer.MediaType + } + default: + return fmt.Errorf("unrecognized Docker image manifest schema %d for %q (%s)", manifest.SchemaVersion, image.Name, image.DockerImageReference) + } + + if image.Annotations == nil { + image.Annotations = map[string]string{} + } + image.Annotations[imageapi.DockerImageLayersOrderAnnotation] = imageapi.DockerImageLayersOrderAscending + + return nil +} + // ImageWithMetadata mutates the given image. It parses raw DockerImageManifest data stored in the image and // fills its DockerImageMetadata and other fields. func ImageWithMetadata(image *imageapi.Image) error { @@ -21,35 +73,30 @@ func ImageWithMetadata(image *imageapi.Image) error { return nil } + ReorderImageLayers(image) + if len(image.DockerImageLayers) > 0 && image.DockerImageMetadata.Size > 0 && len(image.DockerImageManifestMediaType) > 0 { glog.V(5).Infof("Image metadata already filled for %s", image.Name) - - ReorderImageLayers(image) - - // don't update image already filled return nil } - manifestData := image.DockerImageManifest - manifest := imageapi.DockerImageManifest{} - if err := json.Unmarshal([]byte(manifestData), &manifest); err != nil { + if err := json.Unmarshal([]byte(image.DockerImageManifest), &manifest); err != nil { return err } - if image.Annotations == nil { - image.Annotations = map[string]string{} + err := fillImageLayers(image, manifest) + if err != nil { + return err } switch manifest.SchemaVersion { - case 0: - // legacy config object case 1: image.DockerImageManifestMediaType = schema1.MediaTypeManifest if len(manifest.History) == 0 { - // should never have an empty history, but just in case... - return nil + // It should never have an empty history, but just in case. + return fmt.Errorf("the image %s (%s) has a schema 1 manifest, but it doesn't have history", image.Name, image.DockerImageReference) } v1Metadata := imageapi.DockerV1CompatibilityImage{} @@ -57,32 +104,6 @@ func ImageWithMetadata(image *imageapi.Image) error { return err } - image.DockerImageLayers = make([]imageapi.ImageLayer, len(manifest.FSLayers)) - for i, layer := range manifest.FSLayers { - image.DockerImageLayers[i].MediaType = schema1.MediaTypeManifestLayer - image.DockerImageLayers[i].Name = layer.DockerBlobSum - } - if len(manifest.History) == len(image.DockerImageLayers) { - // This code does not work for images converted from v2 to v1, since V1Compatibility does not - // contain size information in this case. - image.DockerImageLayers[0].LayerSize = v1Metadata.Size - var size = imageapi.DockerV1CompatibilityImageSize{} - for i, obj := range manifest.History[1:] { - size.Size = 0 - if err := json.Unmarshal([]byte(obj.DockerV1Compatibility), &size); err != nil { - continue - } - image.DockerImageLayers[i+1].LayerSize = size.Size - } - } else { - glog.V(4).Infof("Imported image has mismatched layer count and history count, not updating image metadata: %s", image.Name) - } - // reverse order of the layers for v1 (lowest = 0, highest = i) - for i, j := 0, len(image.DockerImageLayers)-1; i < j; i, j = i+1, j-1 { - image.DockerImageLayers[i], image.DockerImageLayers[j] = image.DockerImageLayers[j], image.DockerImageLayers[i] - } - image.Annotations[imageapi.DockerImageLayersOrderAnnotation] = imageapi.DockerImageLayersOrderAscending - image.DockerImageMetadata.ID = v1Metadata.ID image.DockerImageMetadata.Parent = v1Metadata.Parent image.DockerImageMetadata.Comment = v1Metadata.Comment @@ -93,41 +114,18 @@ func ImageWithMetadata(image *imageapi.Image) error { image.DockerImageMetadata.Author = v1Metadata.Author image.DockerImageMetadata.Config = v1Metadata.Config image.DockerImageMetadata.Architecture = v1Metadata.Architecture - if len(image.DockerImageLayers) > 0 { - size := int64(0) - layerSet := sets.NewString() - for _, layer := range image.DockerImageLayers { - if layerSet.Has(layer.Name) { - continue - } - layerSet.Insert(layer.Name) - size += layer.LayerSize - } - image.DockerImageMetadata.Size = size - } else { - image.DockerImageMetadata.Size = v1Metadata.Size - } case 2: image.DockerImageManifestMediaType = schema2.MediaTypeManifest if len(image.DockerImageConfig) == 0 { return fmt.Errorf("dockerImageConfig must not be empty for manifest schema 2") } + config := imageapi.DockerImageConfig{} if err := json.Unmarshal([]byte(image.DockerImageConfig), &config); err != nil { return fmt.Errorf("failed to parse dockerImageConfig: %v", err) } - // The layer list is ordered starting from the base image (opposite order of schema1). - // So, we do not need to change the order of layers. - image.DockerImageLayers = make([]imageapi.ImageLayer, len(manifest.Layers)) - for i, layer := range manifest.Layers { - image.DockerImageLayers[i].Name = layer.Digest - image.DockerImageLayers[i].LayerSize = layer.Size - image.DockerImageLayers[i].MediaType = layer.MediaType - } - image.Annotations[imageapi.DockerImageLayersOrderAnnotation] = imageapi.DockerImageLayersOrderAscending - image.DockerImageMetadata.ID = manifest.Config.Digest image.DockerImageMetadata.Parent = config.Parent image.DockerImageMetadata.Comment = config.Comment @@ -138,22 +136,25 @@ func ImageWithMetadata(image *imageapi.Image) error { image.DockerImageMetadata.Author = config.Author image.DockerImageMetadata.Config = config.Config image.DockerImageMetadata.Architecture = config.Architecture - image.DockerImageMetadata.Size = int64(len(image.DockerImageConfig)) - - layerSet := sets.NewString(image.DockerImageMetadata.ID) - if len(image.DockerImageLayers) > 0 { - for _, layer := range image.DockerImageLayers { - if layerSet.Has(layer.Name) { - continue - } - layerSet.Insert(layer.Name) - image.DockerImageMetadata.Size += layer.LayerSize - } - } default: return fmt.Errorf("unrecognized Docker image manifest schema %d for %q (%s)", manifest.SchemaVersion, image.Name, image.DockerImageReference) } + layerSet := sets.NewString() + if manifest.SchemaVersion == 2 { + layerSet.Insert(manifest.Config.Digest) + image.DockerImageMetadata.Size = int64(len(image.DockerImageConfig)) + } else { + image.DockerImageMetadata.Size = 0 + } + for _, layer := range image.DockerImageLayers { + if layerSet.Has(layer.Name) { + continue + } + layerSet.Insert(layer.Name) + image.DockerImageMetadata.Size += layer.LayerSize + } + return nil } @@ -167,7 +168,7 @@ func ReorderImageLayers(image *imageapi.Image) { layersOrder, ok := image.Annotations[imageapi.DockerImageLayersOrderAnnotation] if !ok { switch image.DockerImageManifestMediaType { - case schema1.MediaTypeManifest: + case schema1.MediaTypeManifest, schema1.MediaTypeSignedManifest: layersOrder = imageapi.DockerImageLayersOrderAscending case schema2.MediaTypeManifest: layersOrder = imageapi.DockerImageLayersOrderDescending diff --git a/pkg/image/util/helpers_test.go b/pkg/image/util/helpers_test.go index 851ce1a09daf..1ce386e2eeee 100644 --- a/pkg/image/util/helpers_test.go +++ b/pkg/image/util/helpers_test.go @@ -35,6 +35,7 @@ func TestImageWithMetadata(t *testing.T) { expectedImage: imageapi.Image{ DockerImageManifest: `{"name": "library/ubuntu", "tag": "latest"}`, }, + expectError: true, }, "error unmarshalling v1 compat": { image: imageapi.Image{ diff --git a/test/integration/dockerregistry_imagelayers_test.go b/test/integration/dockerregistry_imagelayers_test.go new file mode 100644 index 000000000000..73ec219f89c6 --- /dev/null +++ b/test/integration/dockerregistry_imagelayers_test.go @@ -0,0 +1,264 @@ +package integration + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "os" + "testing" + "time" + + "github.com/docker/distribution" + "github.com/docker/distribution/configuration" + "github.com/docker/distribution/context" + "github.com/docker/distribution/digest" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/openshift/origin/pkg/cmd/dockerregistry" + cmdutil "github.com/openshift/origin/pkg/cmd/util" + "github.com/openshift/origin/pkg/cmd/util/tokencmd" + registryconfig "github.com/openshift/origin/pkg/dockerregistry/server/configuration" + registrytest "github.com/openshift/origin/pkg/dockerregistry/testutil" + imageapi "github.com/openshift/origin/pkg/image/apis/image" + imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset" + testutil "github.com/openshift/origin/test/util" + testserver "github.com/openshift/origin/test/util/server" +) + +func StartTestRegistry() (string, error) { + registryAddr, err := testserver.FindAvailableBindAddress(10000, 29999) + if err != nil { + return "", fmt.Errorf("unable to find a bind address for the registry: %v", err) + } + + dockerConfig := &configuration.Configuration{ + Version: "0.1", + Storage: configuration.Storage{ + "inmemory": configuration.Parameters{}, + }, + Auth: configuration.Auth{ + "openshift": configuration.Parameters{}, + }, + Middleware: map[string][]configuration.Middleware{ + "registry": {{ + Name: "openshift", + }}, + "repository": {{ + Name: "openshift", + Options: configuration.Parameters{ + "dockerregistryurl": registryAddr, + "acceptschema2": true, + "pullthrough": true, + "enforcequota": false, + "projectcachettl": "1m", + "blobrepositorycachettl": "10m", + }, + }}, + "storage": {{ + Name: "openshift", + }}, + }, + } + dockerConfig.Log.Level = "debug" + dockerConfig.HTTP.Addr = registryAddr + + extraConfig := ®istryconfig.Configuration{} + + go func() { + err := dockerregistry.Start(dockerConfig, extraConfig) + panic(fmt.Errorf("failed to start the integrated registry: %v", err)) + }() + + return registryAddr, cmdutil.WaitForSuccessfulDial(false, "tcp", registryAddr, 100*time.Millisecond, 1*time.Second, 35) +} + +// uploadImageWithSchema2Manifest creates a random image with a schema 2 +// manifest and uploads it to the repository. +func uploadImageWithSchema2Manifest(ctx context.Context, repo distribution.Repository, tag string) error { + layers := make([]distribution.Descriptor, 3) + for i := range layers { + content, desc, err := registrytest.MakeRandomLayer() + if err != nil { + return fmt.Errorf("make random layer: %v", err) + } + + if err := registrytest.UploadBlob(ctx, repo, desc, content); err != nil { + return fmt.Errorf("upload random blob: %v", err) + } + + layers[i] = desc + } + + cfg := imageapi.DockerImageConfig{ + History: make([]imageapi.DockerConfigHistory, len(layers)), + RootFS: &imageapi.DockerConfigRootFS{ + DiffIDs: make([]string, len(layers)), + }, + } + + configContent, err := json.Marshal(&cfg) + if err != nil { + return fmt.Errorf("marshal image config: %v", err) + } + + config := distribution.Descriptor{ + Digest: digest.FromBytes(configContent), + Size: int64(len(configContent)), + } + + if err := registrytest.UploadBlob(ctx, repo, config, configContent); err != nil { + return fmt.Errorf("upload image config: %v", err) + } + + manifest, err := registrytest.MakeSchema2Manifest(config, layers) + if err != nil { + return fmt.Errorf("make schema 2 manifest: %v", err) + } + + if err := registrytest.UploadManifest(ctx, repo, tag, manifest); err != nil { + return fmt.Errorf("upload schema 2 manifest: %v", err) + } + + return nil +} + +// getSchema1Manifest simulates a client which supports only schema 1 +// manifests, fetches a manifest from a registry and returns it. +func getSchema1Manifest(transport http.RoundTripper, baseURL, repoName, tag string) (distribution.Manifest, error) { + c := &http.Client{ + Transport: transport, + } + + resp, err := c.Get(baseURL + "/v2/" + repoName + "/manifests/" + tag) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("read manifest %s:%s: %v", repoName, tag, err) + } + + m, _, err := distribution.UnmarshalManifest(resp.Header.Get("Content-Type"), body) + return m, err +} + +// TestRegistryImageLayers tests that the integrated registry handles schema 1 +// manifests and schema 2 manifests consistently and it produces similar Image +// resources for them. +// +// The test relies on ability of the registry to downconvert manifests. +func TestRegistryImageLayers(t *testing.T) { + masterConfig, clusterAdminKubeConfig, err := testserver.StartTestMasterAPI() + if err != nil { + t.Fatalf("start master: %v", err) + } + + defer testserver.CleanupMasterEtcd(t, masterConfig) + + clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig) + if err != nil { + t.Fatalf("get cluster admin client config: %v", err) + } + + namespace := testutil.Namespace() + imageStreamName := "test-imagelayers" + user := "admin" + + _, adminConfig, err := testserver.CreateNewProject(clusterAdminClientConfig, namespace, user) + if err != nil { + t.Fatalf("create namespace: %v", err) + } + + adminImageClient := imageclient.NewForConfigOrDie(adminConfig) + token, err := tokencmd.RequestToken(clusterAdminClientConfig, nil, user, "password") + if err != nil { + t.Fatalf("error requesting token: %v", err) + } + + os.Setenv("OPENSHIFT_CA_DATA", string(clusterAdminClientConfig.CAData)) + os.Setenv("OPENSHIFT_CERT_DATA", string(clusterAdminClientConfig.CertData)) + os.Setenv("OPENSHIFT_KEY_DATA", string(clusterAdminClientConfig.KeyData)) + os.Setenv("OPENSHIFT_MASTER", clusterAdminClientConfig.Host) + + registryAddr, err := StartTestRegistry() + if err != nil { + t.Fatalf("start registry: %v", err) + } + + creds := registrytest.NewBasicCredentialStore(user, token) + + baseURL := "http://" + registryAddr + repoName := fmt.Sprintf("%s/%s", namespace, imageStreamName) + + schema1Tag := "schema1" + schema2Tag := "schema2" + + transport, err := registrytest.NewTransport(baseURL, repoName, creds) + if err != nil { + t.Fatalf("get transport: %v", err) + } + + ctx := context.Background() + + repo, err := registrytest.NewRepository(ctx, repoName, baseURL, transport) + if err != nil { + t.Fatalf("get repository: %v", err) + } + + if err := uploadImageWithSchema2Manifest(ctx, repo, schema2Tag); err != nil { + t.Fatalf("upload image with schema 2 manifest: %v", err) + } + + // get the schema2 image's manifest downconverted to a schema 1 manifest + schema1Manifest, err := getSchema1Manifest(transport, baseURL, repoName, schema2Tag) + if err != nil { + t.Fatalf("get schema 1 manifest for image schema2: %v", err) + } + + if err := registrytest.UploadManifest(ctx, repo, schema1Tag, schema1Manifest); err != nil { + t.Fatalf("upload schema 1 manifest: %v", err) + } + + schema1ISTag, err := adminImageClient.ImageStreamTags(namespace).Get(imageStreamName+":"+schema1Tag, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get image stream tag %s:%s: %v", imageStreamName, schema1Tag, err) + } + + schema2ISTag, err := adminImageClient.ImageStreamTags(namespace).Get(imageStreamName+":"+schema2Tag, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get image stream tag %s:%s: %v", imageStreamName, schema1Tag, err) + } + + if schema1ISTag.Image.DockerImageManifestMediaType == schema2ISTag.Image.DockerImageManifestMediaType { + t.Errorf("expected different media types, but got %q", schema1ISTag.Image.DockerImageManifestMediaType) + } + + image1LayerOrder := schema1ISTag.Image.Annotations[imageapi.DockerImageLayersOrderAnnotation] + image2LayerOrder := schema2ISTag.Image.Annotations[imageapi.DockerImageLayersOrderAnnotation] + if image1LayerOrder != image2LayerOrder { + t.Errorf("the layer order annotations are different: schema1=%q, schema2=%q", image1LayerOrder, image2LayerOrder) + } else if image1LayerOrder == "" { + t.Errorf("the layer order annotation is empty or not present") + } + + image1Layers := schema1ISTag.Image.DockerImageLayers + image2Layers := schema2ISTag.Image.DockerImageLayers + if len(image1Layers) != len(image2Layers) { + t.Errorf("layers are different: schema1=%#+v, schema2=%#+v", image1Layers, image2Layers) + } else { + for i := range image1Layers { + if image1Layers[i].Name != image2Layers[i].Name { + t.Errorf("different names for the layer #%d: schema1=%#+v, schema2=%#+v", i, image1Layers[i], image2Layers[i]) + } + if image1Layers[i].LayerSize != image2Layers[i].LayerSize { + t.Errorf("different sizes for the layer #%d: schema1=%#+v, schema2=%#+v", i, image1Layers[i], image2Layers[i]) + } else if image1Layers[i].LayerSize <= 0 { + t.Errorf("unexpected size for the layer #%d: %d", i, image1Layers[i].LayerSize) + } + } + } +} diff --git a/test/integration/v2_docker_registry_test.go b/test/integration/v2_docker_registry_test.go index 40c5b04535c9..7f65db733409 100644 --- a/test/integration/v2_docker_registry_test.go +++ b/test/integration/v2_docker_registry_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/docker/distribution/context" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" "github.com/docker/distribution/manifest/schema1" @@ -278,7 +279,7 @@ middleware: func putManifest(name, user, token string) (digest.Digest, error) { creds := registryutil.NewBasicCredentialStore(user, token) - desc, _, err := registryutil.UploadRandomTestBlob(&url.URL{Host: "127.0.0.1:5000", Scheme: "http"}, creds, testutil.Namespace()+"/"+name) + desc, _, err := registryutil.UploadRandomTestBlob(context.Background(), &url.URL{Host: "127.0.0.1:5000", Scheme: "http"}, creds, testutil.Namespace()+"/"+name) if err != nil { return "", err }