Merge pull request #163 from dmage/externalreg

prevent external hostname pullthrough + bump digest cache size
openshift · Feb 23, 2019 · 75a1fbe · 75a1fbe
2 parents 8b97ae1 + 7d6da2d
commit 75a1fbe
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 12 deletions.
diff --git a/pkg/dockerregistry/server/app.go b/pkg/dockerregistry/server/app.go
@@ -24,7 +24,7 @@ import (
 
 const (
 	// Default values
-	defaultDescriptorCacheSize         = 4096
+	defaultDescriptorCacheSize         = 6 * 4096 // total cache size ends up ~16mb
 	defaultDigestToRepositoryCacheSize = 2048
 	defaultPaginationCacheSize         = 1024
 )

diff --git a/pkg/imagestream/identifycandidaterepositories.go b/pkg/imagestream/identifycandidaterepositories.go
@@ -37,12 +37,21 @@ func (by *byInsecureFlag) Less(i, j int) bool {
 	return !by.specs[i].Insecure
 }
 
+func stringListContains(list []string, val string) bool {
+	for _, x := range list {
+		if x == val {
+			return true
+		}
+	}
+	return false
+}
+
 // identifyCandidateRepositories returns a list of remote repository names sorted from the best candidate to
 // the worst and a map of remote repositories referenced by this image stream. The best candidate is a secure
 // one. The worst allows for insecure transport.
 func identifyCandidateRepositories(
 	is *imageapiv1.ImageStream,
-	localRegistry string,
+	localRegistry []string,
 	primary bool,
 ) ([]string, map[string]ImagePullthroughSpec) {
 	insecureByDefault := false
@@ -76,7 +85,7 @@ func identifyCandidateRepositories(
 			}
 			// skip anything that matches the innate registry
 			// TODO: there may be a better way to make this determination
-			if len(localRegistry) != 0 && localRegistry == ref.Registry {
+			if stringListContains(localRegistry, ref.Registry) {
 				continue
 			}
 			ref = ref.DockerClientDefaults()

diff --git a/pkg/imagestream/identifycandidaterepositories_test.go b/pkg/imagestream/identifycandidaterepositories_test.go
@@ -218,7 +218,7 @@ func TestIdentifyCandidateRepositories(t *testing.T) {
 			},
 		},
 	} {
-		repositories, search := identifyCandidateRepositories(tc.is, tc.localRegistry, tc.primary)
+		repositories, search := identifyCandidateRepositories(tc.is, []string{tc.localRegistry}, tc.primary)
 
 		if !reflect.DeepEqual(repositories, tc.expectedRepositories) {
 			if len(repositories) != 0 || len(tc.expectedRepositories) != 0 {

diff --git a/pkg/imagestream/imagestream.go b/pkg/imagestream/imagestream.go
@@ -241,21 +241,31 @@ func (is *imageStream) Exists(ctx context.Context) (bool, *rerrors.Error) {
 	return true, nil
 }
 
-func (is *imageStream) localRegistry(ctx context.Context) (string, *rerrors.Error) {
+func (is *imageStream) localRegistry(ctx context.Context) ([]string, *rerrors.Error) {
 	stream, rErr := is.imageStreamGetter.get()
 	if rErr != nil {
-		return "", convertImageStreamGetterError(rErr, fmt.Sprintf("localRegistry: failed to get image stream %s", is.Reference()))
+		return nil, convertImageStreamGetterError(rErr, fmt.Sprintf("localRegistry: failed to get image stream %s", is.Reference()))
 	}
 
+	var localNames []string
+
 	local, err := imageapi.ParseDockerImageReference(stream.Status.DockerImageRepository)
 	if err != nil {
-		return "", rerrors.NewError(
-			ErrImageStreamUnknownErrorCode,
-			fmt.Sprintf("localRegistry: unable to parse reference %q", stream.Status.DockerImageRepository),
-			err,
-		)
+		dcontext.GetLogger(ctx).Warnf("localRegistry: unable to parse dockerImageRepository %q", stream.Status.DockerImageRepository)
+	}
+	if len(local.Registry) != 0 {
+		localNames = append(localNames, local.Registry)
 	}
-	return local.Registry, nil
+
+	public, err := imageapi.ParseDockerImageReference(stream.Status.PublicDockerImageRepository)
+	if err != nil {
+		dcontext.GetLogger(ctx).Warnf("localRegistry: unable to parse publicDockerImageRepository %q", stream.Status.DockerImageRepository)
+	}
+	if len(public.Registry) != 0 {
+		localNames = append(localNames, public.Registry)
+	}
+
+	return localNames, nil
 }
 
 func (is *imageStream) IdentifyCandidateRepositories(ctx context.Context, primary bool) ([]string, map[string]ImagePullthroughSpec, *rerrors.Error) {