Merge pull request #16353 from mfojtik/allow-read-signatures

Automatic merge from submit-queue (batch tested with PRs 15834, 16321, 16353, 15298, 15433)

use privileged  client in registry instead of user client when getting or creating signatures

@csrwng 

Fixes: #16349

pkg/dockerregistry/server/app.go

@@ -115,7 +115,11 @@ func NewApp(ctx context.Context, registryClient client.RegistryClient, dockerCon
 
 	// Registry extensions endpoint provides extra functionality to handle the image
 	// signatures.
-	RegisterSignatureHandler(dockerApp)
+	isImageClient, err := registryClient.Client()
+	if err != nil {
+		context.GetLogger(dockerApp).Fatalf("unable to get client for signatures: %v", err)
+	}
+	RegisterSignatureHandler(dockerApp, isImageClient)
 
 	// Registry extensions endpoint provides prometheus metrics.
 	if extraConfig.Metrics.Enabled {

pkg/dockerregistry/server/auth.go

@@ -333,6 +333,8 @@ func (ac *AccessController) Authorized(ctx context.Context, accessRecords ...reg
 				if err := verifyImageSignatureAccess(ctx, namespace, name, osClient); err != nil {
 					return nil, ac.wrapErr(ctx, err)
 				}
+			default:
+				return nil, ac.wrapErr(ctx, ErrUnsupportedAction)
 			}
 
 		case "metrics":

pkg/dockerregistry/server/repositoryconfig.go

@@ -15,6 +15,10 @@ const (
 	// DEPRECATED: Use the OPENSHIFT_DEFAULT_REGISTRY instead.
 	DockerRegistryURLEnvVar = "DOCKER_REGISTRY_URL"
 
+	// DockerRegistryURLEnvVarOption is an optional environment that overrides the
+	// DOCKER_REGISTRY_URL.
+	DockerRegistryURLEnvVarOption = "REGISTRY_MIDDLEWARE_REPOSITORY_OPENSHIFT_DOCKERREGISTRYURL"
+
 	// OpenShiftDefaultRegistry overrides the DockerRegistryURLEnvVar as in OpenShift the
 	// default registry URL is controller by this environment variable.
 	OpenShiftDefaultRegistryEnvVar = "OPENSHIFT_DEFAULT_REGISTRY"
@@ -74,6 +78,13 @@ func newRepositoryConfig(ctx context.Context, options map[string]interface{}) (r
 	} else {
 		context.GetLogger(ctx).Infof("DEPRECATED: %q is deprecated, use the %q instead", DockerRegistryURLEnvVar, OpenShiftDefaultRegistryEnvVar)
 	}
+	if len(rc.registryAddr) == 0 {
+		rc.registryAddr, err = getStringOption(DockerRegistryURLEnvVarOption, "dockerregistryurl", rc.registryAddr, options)
+		if err != nil {
+			return
+		}
+	}
+
 	// TODO: This is a fallback to assuming there is a service named 'docker-registry'. This
 	// might change in the future and we should make this configurable.
 	if len(rc.registryAddr) == 0 {

pkg/dockerregistry/server/signaturedispatcher.go

@@ -17,6 +17,7 @@ import (
 	"github.com/docker/distribution/registry/api/v2"
 	"github.com/docker/distribution/registry/handlers"
 
+	"github.com/openshift/origin/pkg/dockerregistry/server/client"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1"
 
@@ -60,18 +61,27 @@ var (
 )
 
 type signatureHandler struct {
-	ctx       *handlers.Context
-	reference imageapi.DockerImageReference
+	ctx           *handlers.Context
+	reference     imageapi.DockerImageReference
+	isImageClient client.ImageStreamImagesNamespacer
 }
 
-// SignatureDispatcher handles the GET and PUT requests for signature endpoint.
-func SignatureDispatcher(ctx *handlers.Context, r *http.Request) http.Handler {
-	signatureHandler := &signatureHandler{ctx: ctx}
-	signatureHandler.reference, _ = imageapi.ParseDockerImageReference(ctxu.GetStringValue(ctx, "vars.name") + "@" + ctxu.GetStringValue(ctx, "vars.digest"))
-
-	return gorillahandlers.MethodHandler{
-		"GET": http.HandlerFunc(signatureHandler.Get),
-		"PUT": http.HandlerFunc(signatureHandler.Put),
+// NewSignatureDispatcher provides a function that handles the GET and PUT
+// requests for signature endpoint.
+func NewSignatureDispatcher(isImageClient client.ImageStreamImagesNamespacer) func(*handlers.Context, *http.Request) http.Handler {
+	return func(ctx *handlers.Context, r *http.Request) http.Handler {
+		reference, _ := imageapi.ParseDockerImageReference(
+			ctxu.GetStringValue(ctx, "vars.name") + "@" + ctxu.GetStringValue(ctx, "vars.digest"),
+		)
+		signatureHandler := &signatureHandler{
+			ctx:           ctx,
+			isImageClient: isImageClient,
+			reference:     reference,
+		}
+		return gorillahandlers.MethodHandler{
+			"GET": http.HandlerFunc(signatureHandler.Get),
+			"PUT": http.HandlerFunc(signatureHandler.Put),
+		}
 	}
 }
 
@@ -142,18 +152,13 @@ func (s *signatureHandler) Get(w http.ResponseWriter, req *http.Request) {
 		s.handleError(s.ctx, v2.ErrorCodeNameInvalid.WithDetail("missing image name or image ID"), w)
 		return
 	}
-	client, ok := userClientFrom(s.ctx)
-	if !ok {
-		s.handleError(s.ctx, errcode.ErrorCodeUnknown.WithDetail("unable to get origin client"), w)
-		return
-	}
 
 	if len(s.reference.ID) == 0 {
 		s.handleError(s.ctx, v2.ErrorCodeNameInvalid.WithDetail("the image ID must be specified (sha256:<digest>"), w)
 		return
 	}
 
-	image, err := client.ImageStreamImages(s.reference.Namespace).Get(imageapi.MakeImageStreamImageName(s.reference.Name, s.reference.ID), metav1.GetOptions{})
+	image, err := s.isImageClient.ImageStreamImages(s.reference.Namespace).Get(imageapi.MakeImageStreamImageName(s.reference.Name, s.reference.ID), metav1.GetOptions{})
 	switch {
 	case err == nil:
 	case kapierrors.IsUnauthorized(err):

pkg/dockerregistry/server/signaturedispatcher_test.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"os"
 	"reflect"
 	"testing"
 
@@ -80,7 +79,7 @@ func TestSignatureGet(t *testing.T) {
 		},
 		Middleware: map[string][]configuration.Middleware{
 			"registry":   {{Name: "openshift"}},
-			"repository": {{Name: "openshift"}},
+			"repository": {{Name: "openshift", Options: configuration.Parameters{"dockerregistryurl": "localhost:5000"}}},
 			"storage":    {{Name: "openshift"}},
 		},
 	}, &registryconfig.Configuration{}, nil)
@@ -91,7 +90,6 @@ func TestSignatureGet(t *testing.T) {
 	if err != nil {
 		t.Fatalf("error parsing server url: %v", err)
 	}
-	os.Setenv("OPENSHIFT_DEFAULT_REGISTRY", serverURL.Host)
 
 	url := fmt.Sprintf("http://%s/extensions/v2/user/app/signatures/%s", serverURL.Host, testImage.Name)
 
@@ -186,7 +184,7 @@ func TestSignaturePut(t *testing.T) {
 		},
 		Middleware: map[string][]configuration.Middleware{
 			"registry":   {{Name: "openshift"}},
-			"repository": {{Name: "openshift"}},
+			"repository": {{Name: "openshift", Options: configuration.Parameters{"dockerregistryurl": "localhost:5000"}}},
 			"storage":    {{Name: "openshift"}},
 		},
 	}, &registryconfig.Configuration{}, nil)
@@ -197,7 +195,6 @@ func TestSignaturePut(t *testing.T) {
 	if err != nil {
 		t.Fatalf("error parsing server url: %v", err)
 	}
-	os.Setenv("OPENSHIFT_DEFAULT_REGISTRY", serverURL.Host)
 
 	signData, err := json.Marshal(testSignature)
 	if err != nil {

pkg/dockerregistry/server/signaturehandler.go

@@ -8,11 +8,12 @@ import (
 	"github.com/docker/distribution/registry/handlers"
 
 	"github.com/openshift/origin/pkg/dockerregistry/server/api"
+	"github.com/openshift/origin/pkg/dockerregistry/server/client"
 )
 
 // RegisterSignatureHandler registers the Docker image signature extension to Docker
 // registry.
-func RegisterSignatureHandler(app *handlers.App) {
+func RegisterSignatureHandler(app *handlers.App, isImageClient client.ImageStreamImagesNamespacer) {
 	extensionsRouter := app.NewRoute().PathPrefix(api.ExtensionsPrefix).Subrouter()
 	var (
 		getSignatureAccess = func(r *http.Request) []auth.Access {
@@ -40,13 +41,13 @@ func RegisterSignatureHandler(app *handlers.App) {
 	)
 	app.RegisterRoute(
 		extensionsRouter.Path(api.SignaturesPath).Methods("GET"),
-		SignatureDispatcher,
+		NewSignatureDispatcher(isImageClient),
 		handlers.NameRequired,
 		getSignatureAccess,
 	)
 	app.RegisterRoute(
 		extensionsRouter.Path(api.SignaturesPath).Methods("PUT"),
-		SignatureDispatcher,
+		NewSignatureDispatcher(isImageClient),
 		handlers.NameRequired,
 		putSignatureAccess,
 	)