Remove global DefaultRegistryClient

Signed-off-by: Oleg Bulatov <[email protected]>

pkg/cmd/dockerregistry/dockerregistry.go

@@ -39,6 +39,7 @@ import (
 	"strings"
 
 	"github.com/openshift/origin/pkg/cmd/server/crypto"
+	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
 	"github.com/openshift/origin/pkg/dockerregistry/server"
 	"github.com/openshift/origin/pkg/dockerregistry/server/audit"
 )
@@ -57,11 +58,18 @@ func Execute(configFile io.Reader) {
 	if err != nil {
 		log.Fatalf("error configuring logger: %v", err)
 	}
+
+	registryClient := server.NewRegistryClient(clientcmd.NewConfig().BindToFile())
+	ctx = server.WithRegistryClient(ctx, registryClient)
+
 	log.Infof("version=%s", version.Version)
 	// inject a logger into the uuid library. warns us if there is a problem
 	// with uuid generation under low entropy.
 	uuid.Loggerf = context.GetLogger(ctx).Warnf
 
+	// pass context into the auth middleware
+	config.Auth[server.OpenShiftAuth]["_context"] = ctx
+
 	app := handlers.NewApp(ctx, config)
 
 	// Add a token handling endpoint
@@ -70,7 +78,7 @@ func Execute(configFile io.Reader) {
 		if err != nil {
 			log.Fatalf("error setting up token auth: %s", err)
 		}
-		err = app.NewRoute().Methods("GET").PathPrefix(tokenRealm.Path).Handler(server.NewTokenHandler(ctx, server.DefaultRegistryClient)).GetError()
+		err = app.NewRoute().Methods("GET").PathPrefix(tokenRealm.Path).Handler(server.NewTokenHandler(ctx, registryClient)).GetError()
 		if err != nil {
 			log.Fatalf("error setting up token endpoint at %q: %v", tokenRealm.Path, err)
 		}

pkg/dockerregistry/server/auth.go

@@ -55,9 +55,6 @@ type RegistryClient interface {
 	SafeClientConfig() restclient.Config
 }
 
-// DefaultRegistryClient is exposed for testing the registry with fake client.
-var DefaultRegistryClient = NewRegistryClient(clientcmd.NewConfig().BindToFile())
-
 // registryClient implements RegistryClient
 type registryClient struct {
 	config *clientcmd.Config
@@ -201,6 +198,11 @@ func TokenRealm(options map[string]interface{}) (*url.URL, error) {
 func newAccessController(options map[string]interface{}) (registryauth.AccessController, error) {
 	log.Info("Using Origin Auth handler")
 
+	ctx, ok := options["_context"].(context.Context)
+	if !ok {
+		return nil, fmt.Errorf("no context provided to Origin Auth handler")
+	}
+
 	realm, err := getStringOption("", RealmKey, "origin", options)
 	if err != nil {
 		return nil, err
@@ -214,7 +216,7 @@ func newAccessController(options map[string]interface{}) (registryauth.AccessCon
 	ac := &AccessController{
 		realm:      realm,
 		tokenRealm: tokenRealm,
-		config:     DefaultRegistryClient.SafeClientConfig(),
+		config:     RegistryClientFrom(ctx).SafeClientConfig(),
 	}
 
 	if audit, ok := options["audit"]; ok {

pkg/dockerregistry/server/auth_test.go

@@ -391,20 +391,20 @@ func TestAccessController(t *testing.T) {
 		if len(test.bearerToken) > 0 {
 			req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", test.bearerToken))
 		}
-		ctx := context.WithValue(context.Background(), "http.request", req)
 
 		server, actions := simulateOpenShiftMaster(test.openshiftResponses)
-		DefaultRegistryClient = NewRegistryClient(&clientcmd.Config{
+		options["_context"] = WithRegistryClient(context.Background(), NewRegistryClient(&clientcmd.Config{
 			CommonConfig: restclient.Config{
 				Host:     server.URL,
 				Insecure: true,
 			},
 			SkipEnv: true,
-		})
+		}))
 		accessController, err := newAccessController(options)
 		if err != nil {
 			t.Fatal(err)
 		}
+		ctx := context.WithValue(context.Background(), "http.request", req)
 		authCtx, err := accessController.Authorized(ctx, test.access...)
 		server.Close()
 

pkg/dockerregistry/server/blobdescriptorservice_test.go

@@ -47,8 +47,6 @@ func GetTestPassThroughToUpstream(ctx context.Context) bool {
 // It relies on the fact that blobDescriptorService requires higher levels to set repository object on given
 // context. If the object isn't given, its method will err out.
 func TestBlobDescriptorServiceIsApplied(t *testing.T) {
-	ctx := context.Background()
-
 	// don't do any authorization check
 	installFakeAccessController(t)
 	m := fakeBlobDescriptorService(t)
@@ -64,14 +62,8 @@ func TestBlobDescriptorServiceIsApplied(t *testing.T) {
 	client.AddReactor("get", "imagestreams", imagetest.GetFakeImageStreamGetHandler(t, *testImageStream))
 	client.AddReactor("get", "images", registrytest.GetFakeImageGetHandler(t, *testImage))
 
-	// TODO: get rid of those nasty global vars
-	backupRegistryClient := DefaultRegistryClient
-	DefaultRegistryClient = makeFakeRegistryClient(client, fake.NewSimpleClientset())
-	defer func() {
-		// set it back once this test finishes to make other unit tests working
-		DefaultRegistryClient = backupRegistryClient
-	}()
-
+	ctx := context.Background()
+	ctx = WithRegistryClient(ctx, makeFakeRegistryClient(client, fake.NewSimpleClientset()))
 	app := handlers.NewApp(ctx, &configuration.Configuration{
 		Loglevel: "debug",
 		Auth: map[string]configuration.Parameters{

pkg/dockerregistry/server/context.go

@@ -0,0 +1,15 @@
+package server
+
+import "context"
+
+var registryClientContextKey struct{}
+
+// RegistryClientFrom returns the registry client stored in ctx, if present. It will panic otherwise.
+func RegistryClientFrom(ctx context.Context) RegistryClient {
+	return ctx.Value(registryClientContextKey).(RegistryClient)
+}
+
+// WithRegistryClient creates a new context with provided registry client.
+func WithRegistryClient(ctx context.Context, client RegistryClient) context.Context {
+	return context.WithValue(ctx, registryClientContextKey, client)
+}

pkg/dockerregistry/server/pullthroughblobstore_test.go

@@ -20,8 +20,6 @@ import (
 	"github.com/docker/distribution/manifest/schema1"
 	_ "github.com/docker/distribution/registry/storage/driver/inmemory"
 
-	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
-
 	"github.com/openshift/origin/pkg/client/testclient"
 	registrytest "github.com/openshift/origin/pkg/dockerregistry/testutil"
 	imagetest "github.com/openshift/origin/pkg/image/admission/testutil"
@@ -42,14 +40,6 @@ func TestPullthroughServeBlob(t *testing.T) {
 	client := &testclient.Fake{}
 	client.AddReactor("get", "images", registrytest.GetFakeImageGetHandler(t, *testImage))
 
-	// TODO: get rid of those nasty global vars
-	backupRegistryClient := DefaultRegistryClient
-	DefaultRegistryClient = makeFakeRegistryClient(client, fake.NewSimpleClientset())
-	defer func() {
-		// set it back once this test finishes to make other unit tests working again
-		DefaultRegistryClient = backupRegistryClient
-	}()
-
 	remoteRegistryServer := createTestRegistryServer(t, context.Background())
 	defer remoteRegistryServer.Close()
 
@@ -524,14 +514,6 @@ func TestPullthroughServeBlobInsecure(t *testing.T) {
 	} {
 		client := &testclient.Fake{}
 
-		// TODO: get rid of those nasty global vars
-		backupRegistryClient := DefaultRegistryClient
-		DefaultRegistryClient = makeFakeRegistryClient(client, fake.NewSimpleClientset())
-		defer func() {
-			// set it back once this test finishes to make other unit tests working again
-			DefaultRegistryClient = backupRegistryClient
-		}()
-
 		tc.imageStreamInit(client)
 
 		localBlobStore := newTestBlobStore(tc.localBlobs)

pkg/dockerregistry/server/pullthroughmanifestservice_test.go

@@ -17,8 +17,6 @@ import (
 	"github.com/docker/distribution/registry/handlers"
 	_ "github.com/docker/distribution/registry/storage/driver/inmemory"
 
-	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
-
 	"github.com/openshift/origin/pkg/client/testclient"
 	registrytest "github.com/openshift/origin/pkg/dockerregistry/testutil"
 	imageapi "github.com/openshift/origin/pkg/image/api"
@@ -64,14 +62,6 @@ func TestPullthroughManifests(t *testing.T) {
 	log.SetLevel(log.DebugLevel)
 	client := &testclient.Fake{}
 
-	// TODO: get rid of those nasty global vars
-	backupRegistryClient := DefaultRegistryClient
-	DefaultRegistryClient = makeFakeRegistryClient(client, fake.NewSimpleClientset())
-	defer func() {
-		// set it back once this test finishes to make other unit tests working again
-		DefaultRegistryClient = backupRegistryClient
-	}()
-
 	installFakeAccessController(t)
 	setPassthroughBlobDescriptorServiceFactory()
 
@@ -381,14 +371,6 @@ func TestPullthroughManifestInsecure(t *testing.T) {
 	} {
 		client := &testclient.Fake{}
 
-		// TODO: get rid of those nasty global vars
-		backupRegistryClient := DefaultRegistryClient
-		DefaultRegistryClient = makeFakeRegistryClient(client, fake.NewSimpleClientset())
-		defer func() {
-			// set it back once this test finishes to make other unit tests working again
-			DefaultRegistryClient = backupRegistryClient
-		}()
-
 		tc.imageStreamInit(client)
 
 		localManifestService := newTestManifestService(repoName, tc.localData)

pkg/dockerregistry/server/repositorymiddleware.go

@@ -92,7 +92,7 @@ func init() {
 	cachedLayers = cache
 
 	// load the client when the middleware is initialized, which allows test code to change
-	// DefaultRegistryClient before starting a registry.
+	// the registry client before starting a registry.
 	repomw.Register("openshift",
 		func(ctx context.Context, repo distribution.Repository, options map[string]interface{}) (distribution.Repository, error) {
 			if dockerRegistry == nil {
@@ -103,7 +103,7 @@ func init() {
 				panic(fmt.Sprintf("Configuration error: OpenShift storage driver middleware not activated"))
 			}
 
-			registryOSClient, kClient, errClients := DefaultRegistryClient.Clients()
+			registryOSClient, kClient, errClients := RegistryClientFrom(ctx).Clients()
 			if errClients != nil {
 				return nil, errClients
 			}

pkg/dockerregistry/server/signaturedispatcher_test.go

@@ -30,15 +30,6 @@ import (
 
 func TestSignatureGet(t *testing.T) {
 	client := &testclient.Fake{}
-	// TODO: get rid of those nasty global vars
-	backupRegistryClient := DefaultRegistryClient
-	DefaultRegistryClient = makeFakeRegistryClient(client, fake.NewSimpleClientset())
-	defer func() {
-		// set it back once this test finishes to make other unit tests working again
-		DefaultRegistryClient = backupRegistryClient
-	}()
-
-	ctx := WithUserClient(context.Background(), client)
 
 	installFakeAccessController(t)
 
@@ -68,6 +59,9 @@ func TestSignatureGet(t *testing.T) {
 
 	client.AddReactor("get", "imagestreamimages", registrytest.GetFakeImageStreamImageGetHandler(t, testImageStream, *testImage))
 
+	ctx := context.Background()
+	ctx = WithRegistryClient(ctx, makeFakeRegistryClient(client, fake.NewSimpleClientset()))
+	ctx = WithUserClient(ctx, client)
 	registryApp := handlers.NewApp(ctx, &configuration.Configuration{
 		Loglevel: "debug",
 		Auth: map[string]configuration.Parameters{
@@ -143,15 +137,6 @@ func TestSignatureGet(t *testing.T) {
 
 func TestSignaturePut(t *testing.T) {
 	client := &testclient.Fake{}
-	// TODO: get rid of those nasty global vars
-	backupRegistryClient := DefaultRegistryClient
-	DefaultRegistryClient = makeFakeRegistryClient(client, fake.NewSimpleClientset())
-	defer func() {
-		// set it back once this test finishes to make other unit tests working again
-		DefaultRegistryClient = backupRegistryClient
-	}()
-
-	ctx := WithUserClient(context.Background(), client)
 
 	installFakeAccessController(t)
 
@@ -172,6 +157,9 @@ func TestSignaturePut(t *testing.T) {
 		return true, sign, nil
 	})
 
+	ctx := context.Background()
+	ctx = WithRegistryClient(ctx, makeFakeRegistryClient(client, fake.NewSimpleClientset()))
+	ctx = WithUserClient(ctx, client)
 	registryApp := handlers.NewApp(ctx, &configuration.Configuration{
 		Loglevel: "debug",
 		Auth: map[string]configuration.Parameters{