Merge pull request #103 from legionus/alweays-pullthrough

pullthrough is not optional

Author: openshift-merge-robot

pkg/dockerregistry/server/blobdescriptorservice.go

@@ -52,7 +52,7 @@ func (bs *blobDescriptorService) Stat(ctx context.Context, dgst digest.Digest) (
 			// ensure it's referenced inside of corresponding image stream
 			if bs.repo.cache.ContainsRepository(dgst, bs.repo.imageStream.Reference()) {
 				context.GetLogger(ctx).Debugf("found cached blob %q in repository %s", dgst.String(), bs.repo.imageStream.Reference())
-			} else if image := bs.repo.imageStream.HasBlob(ctx, dgst, !bs.repo.app.config.Pullthrough.Enabled); image != nil {
+			} else if image := bs.repo.imageStream.HasBlob(ctx, dgst); image != nil {
 				// remember all the layers of matching image
 				RememberLayersOfImage(ctx, bs.repo.cache, image, bs.repo.imageStream.Reference())
 			} else {

pkg/dockerregistry/server/configuration/configuration.go

@@ -440,6 +440,12 @@ func migratePullthroughSection(cfg *Configuration, options configuration.Paramet
 	if err != nil {
 		err = fmt.Errorf("configuration error in openshift.pullthrough.mirror: %v", err)
 	}
+
+	if !cfg.Pullthrough.Enabled {
+		log.Warnf("pullthrough can't be disabled anymore")
+		cfg.Pullthrough.Enabled = true
+	}
+
 	return
 }
 

pkg/dockerregistry/server/registry_test.go

@@ -37,15 +37,14 @@ func newTestRegistry(
 	osClient registryclient.Interface,
 	storageDriver driver.StorageDriver,
 	blobrepositorycachettl time.Duration,
-	pullthrough bool,
 	useBlobDescriptorCacheProvider bool,
 ) (distribution.Namespace, error) {
 	cfg := &configuration.Configuration{
 		Server: &configuration.Server{
 			Addr: "localhost:5000",
 		},
 		Pullthrough: &configuration.Pullthrough{
-			Enabled: pullthrough,
+			Enabled: true,
 		},
 		Cache: &configuration.Cache{
 			BlobRepositoryTTL: blobrepositorycachettl,

pkg/dockerregistry/server/repository.go

@@ -76,14 +76,12 @@ func (app *App) Repository(ctx context.Context, repo distribution.Repository, cr
 		},
 	}
 
-	if app.config.Pullthrough.Enabled {
-		r.remoteBlobGetter = NewBlobGetterService(
-			r.imageStream,
-			r.imageStream.GetSecrets,
-			r.cache,
-			r.app.metrics,
-		)
-	}
+	r.remoteBlobGetter = NewBlobGetterService(
+		r.imageStream,
+		r.imageStream.GetSecrets,
+		r.cache,
+		r.app.metrics,
+	)
 
 	repo = distribution.Repository(r)
 	repo = r.app.metrics.Repository(repo, repo.Named().Name())
@@ -112,18 +110,16 @@ func (r *repository) Manifests(ctx context.Context, options ...distribution.Mani
 		acceptSchema2: r.app.config.Compatibility.AcceptSchema2,
 	}
 
-	if r.app.config.Pullthrough.Enabled {
-		ms = &pullthroughManifestService{
-			ManifestService: ms,
-			newLocalManifestService: func(ctx context.Context) (distribution.ManifestService, error) {
-				return r.Repository.Manifests(ctx, opts...)
-			},
-			imageStream:  r.imageStream,
-			cache:        r.cache,
-			mirror:       r.app.config.Pullthrough.Mirror,
-			registryAddr: r.app.config.Server.Addr,
-			metrics:      r.app.metrics,
-		}
+	ms = &pullthroughManifestService{
+		ManifestService: ms,
+		newLocalManifestService: func(ctx context.Context) (distribution.ManifestService, error) {
+			return r.Repository.Manifests(ctx, opts...)
+		},
+		imageStream:  r.imageStream,
+		cache:        r.cache,
+		mirror:       r.app.config.Pullthrough.Mirror,
+		registryAddr: r.app.config.Server.Addr,
+		metrics:      r.app.metrics,
 	}
 
 	ms = newPendingErrorsManifestService(ms, r)
@@ -147,15 +143,13 @@ func (r *repository) Blobs(ctx context.Context) distribution.BlobStore {
 		}
 	}
 
-	if r.app.config.Pullthrough.Enabled {
-		bs = &pullthroughBlobStore{
-			BlobStore: bs,
+	bs = &pullthroughBlobStore{
+		BlobStore: bs,
 
-			remoteBlobGetter:  r.remoteBlobGetter,
-			writeLimiter:      r.app.writeLimiter,
-			mirror:            r.app.config.Pullthrough.Mirror,
-			newLocalBlobStore: r.Repository.Blobs,
-		}
+		remoteBlobGetter:  r.remoteBlobGetter,
+		writeLimiter:      r.app.writeLimiter,
+		mirror:            r.app.config.Pullthrough.Mirror,
+		newLocalBlobStore: r.Repository.Blobs,
 	}
 
 	bs = newPendingErrorsBlobStore(bs, r)
@@ -172,9 +166,8 @@ func (r *repository) Tags(ctx context.Context) distribution.TagService {
 	ts := r.Repository.Tags(ctx)
 
 	ts = &tagService{
-		TagService:         ts,
-		imageStream:        r.imageStream,
-		pullthroughEnabled: r.app.config.Pullthrough.Enabled,
+		TagService:  ts,
+		imageStream: r.imageStream,
 	}
 
 	ts = newPendingErrorsTagService(ts, r)

pkg/dockerregistry/server/repository_test.go

@@ -99,7 +99,6 @@ func TestRepositoryBlobStat(t *testing.T) {
 		stat               string
 		images             []imageapiv1.Image
 		imageStreams       []imageapiv1.ImageStream
-		pullthrough        bool
 		skipAuth           bool
 		deferredErrors     deferredErrors
 		expectedDescriptor distribution.Descriptor
@@ -165,7 +164,6 @@ func TestRepositoryBlobStat(t *testing.T) {
 					},
 				},
 			},
-			pullthrough:        true,
 			expectedDescriptor: testNewDescriptorForLayer(testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1]),
 			expectedActions:    []clientAction{{"get", "imagestreams"}, {"get", "images"}},
 		},
@@ -205,7 +203,7 @@ func TestRepositoryBlobStat(t *testing.T) {
 				},
 			},
 			expectedError:   distribution.ErrBlobUnknown,
-			expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "images"}},
+			expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "images"}, {"list", "imagestreams"}},
 		},
 
 		{
@@ -232,7 +230,8 @@ func TestRepositoryBlobStat(t *testing.T) {
 					},
 				},
 			},
-			expectedError: distribution.ErrBlobUnknown,
+			expectedError:   distribution.ErrBlobUnknown,
+			expectedActions: []clientAction{{"get", "imagestreams"}, {"list", "imagestreams"}},
 		},
 
 		{
@@ -259,7 +258,8 @@ func TestRepositoryBlobStat(t *testing.T) {
 					},
 				},
 			},
-			expectedError: distribution.ErrBlobUnknown,
+			expectedError:   distribution.ErrBlobUnknown,
+			expectedActions: []clientAction{{"get", "imagestreams"}, {"list", "imagestreams"}},
 		},
 
 		{
@@ -312,7 +312,7 @@ func TestRepositoryBlobStat(t *testing.T) {
 				}
 			}
 
-			reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, cfg.Cache.BlobRepositoryTTL, tc.pullthrough, true)
+			reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, cfg.Cache.BlobRepositoryTTL, true)
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
@@ -374,7 +374,7 @@ func TestRepositoryBlobStatCacheEviction(t *testing.T) {
 	testutil.AddImageStream(t, fos, "nm", "is", nil)
 	testutil.AddImage(t, fos, testImage, "nm", "is", "latest")
 
-	reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, blobRepoCacheTTL, false, false)
+	reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, blobRepoCacheTTL, false)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}

pkg/dockerregistry/server/tagservice.go

@@ -11,8 +11,7 @@ import (
 type tagService struct {
 	distribution.TagService
 
-	imageStream        imagestream.ImageStream
-	pullthroughEnabled bool
+	imageStream imagestream.ImageStream
 }
 
 func (t tagService) Get(ctx context.Context, tag string) (distribution.Descriptor, error) {
@@ -35,17 +34,6 @@ func (t tagService) Get(ctx context.Context, tag string) (distribution.Descripto
 		return distribution.Descriptor{}, distribution.ErrTagUnknown{Tag: tag}
 	}
 
-	if !t.pullthroughEnabled {
-		image, err := t.imageStream.GetImageOfImageStream(ctx, dgst)
-		if err != nil {
-			return distribution.Descriptor{}, err
-		}
-
-		if !imagestream.IsImageManaged(image) {
-			return distribution.Descriptor{}, distribution.ErrTagUnknown{Tag: tag}
-		}
-	}
-
 	return distribution.Descriptor{Digest: dgst}, nil
 }
 
@@ -64,30 +52,10 @@ func (t tagService) All(ctx context.Context) ([]string, error) {
 	}
 
 	tagList := []string{}
-	managedImages := make(map[string]bool)
-	for tag, dgst := range tags {
-		if t.pullthroughEnabled {
-			tagList = append(tagList, tag)
-			continue
-		}
-
-		managed, found := managedImages[dgst.String()]
-		if !found {
-			image, err := t.imageStream.GetImageOfImageStream(ctx, dgst)
-			if err != nil {
-				context.GetLogger(ctx).Errorf("unable to get image %s %s: %v", t.imageStream.Reference(), dgst.String(), err)
-				continue
-			}
-			managed = imagestream.IsImageManaged(image)
-			managedImages[dgst.String()] = managed
-		}
-
-		if !managed {
-			continue
-		}
-
+	for tag := range tags {
 		tagList = append(tagList, tag)
 	}
+
 	return tagList, nil
 }
 
@@ -106,34 +74,14 @@ func (t tagService) Lookup(ctx context.Context, desc distribution.Descriptor) ([
 	}
 
 	tagList := []string{}
-	managedImages := make(map[string]bool)
 	for tag, dgst := range tags {
 		if dgst != desc.Digest {
 			continue
 		}
 
-		if t.pullthroughEnabled {
-			tagList = append(tagList, tag)
-			continue
-		}
-
-		managed, found := managedImages[dgst.String()]
-		if !found {
-			image, err := t.imageStream.GetImageOfImageStream(ctx, dgst)
-			if err != nil {
-				context.GetLogger(ctx).Errorf("unable to get image %s %s: %v", t.imageStream.Reference(), dgst.String(), err)
-				continue
-			}
-			managed = imagestream.IsImageManaged(image)
-			managedImages[dgst.String()] = managed
-		}
-
-		if !managed {
-			continue
-		}
-
 		tagList = append(tagList, tag)
 	}
+
 	return tagList, nil
 }