clean up image registry hostname retrieval

Author: deads2k

pkg/cmd/server/admission/init.go

@@ -7,7 +7,7 @@ import (
 
 	userinformer "github.com/openshift/client-go/user/informers/externalversions"
 	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
-	imageapi "github.com/openshift/origin/pkg/image/apis/image"
+	"github.com/openshift/origin/pkg/image/apiserver/registryhostname"
 	"github.com/openshift/origin/pkg/project/cache"
 	"github.com/openshift/origin/pkg/quota/controller/clusterquotamapping"
 	quotainformer "github.com/openshift/origin/pkg/quota/generated/informers/internalversion/quota/internalversion"
@@ -21,7 +21,7 @@ type PluginInitializer struct {
 	RESTClientConfig             restclient.Config
 	ClusterResourceQuotaInformer quotainformer.ClusterResourceQuotaInformer
 	ClusterQuotaMapper           clusterquotamapping.ClusterQuotaMapper
-	RegistryHostnameRetriever    imageapi.RegistryHostnameRetriever
+	RegistryHostnameRetriever    registryhostname.RegistryHostnameRetriever
 	SecurityInformers            securityinformer.SharedInformerFactory
 	UserInformers                userinformer.SharedInformerFactory
 }

pkg/cmd/server/origin/admission/plugin_initializer.go

@@ -3,10 +3,8 @@ package admission
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
 
 	"k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/initializer"
@@ -28,15 +26,14 @@ import (
 	oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
 	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
 	kubernetes "github.com/openshift/origin/pkg/cmd/server/kubernetes/master"
-	imageapi "github.com/openshift/origin/pkg/image/apis/image"
+	"github.com/openshift/origin/pkg/image/apiserver/registryhostname"
 	imageinformer "github.com/openshift/origin/pkg/image/generated/informers/internalversion"
 	imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset"
 	projectcache "github.com/openshift/origin/pkg/project/cache"
 	"github.com/openshift/origin/pkg/quota/controller/clusterquotamapping"
 	quotainformer "github.com/openshift/origin/pkg/quota/generated/informers/internalversion"
 	"github.com/openshift/origin/pkg/quota/image"
 	securityinformer "github.com/openshift/origin/pkg/security/generated/informers/internalversion"
-	"github.com/openshift/origin/pkg/service"
 )
 
 type InformerAccess interface {
@@ -61,7 +58,7 @@ func NewPluginInitializer(
 	if err != nil {
 		return nil, err
 	}
-	kubeClientGoClientSet, err := kubeclientgoclient.NewForConfig(privilegedLoopbackConfig)
+	kubeClient, err := kubeclientgoclient.NewForConfig(privilegedLoopbackConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -80,11 +77,9 @@ func NewPluginInitializer(
 		quotaRegistry.Add(imageEvaluators[i])
 	}
 
-	defaultRegistry := env("OPENSHIFT_DEFAULT_REGISTRY", "${DOCKER_REGISTRY_SERVICE_HOST}:${DOCKER_REGISTRY_SERVICE_PORT}")
-	svcCache := service.NewServiceResolverCache(kubeInternalClient.Core().Services(metav1.NamespaceDefault).Get)
-	defaultRegistryFunc, err := svcCache.Defer(defaultRegistry)
+	registryHostnameRetriever, err := registryhostname.DefaultRegistryHostnameRetriever(privilegedLoopbackConfig, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname)
 	if err != nil {
-		return nil, fmt.Errorf("OPENSHIFT_DEFAULT_REGISTRY variable is invalid %q: %v", defaultRegistry, err)
+		return nil, err
 	}
 
 	// punch through layers to build this in order to get a string for a cloud provider file
@@ -104,7 +99,7 @@ func NewPluginInitializer(
 	}
 	// note: we are passing a combined quota registry here...
 	genericInitializer := initializer.New(
-		kubeClientGoClientSet,
+		kubeClient,
 		informers.GetKubernetesInformers(),
 		authorizer,
 		legacyscheme.Scheme,
@@ -145,19 +140,10 @@ func NewPluginInitializer(
 		RESTClientConfig:             *privilegedLoopbackConfig,
 		ClusterResourceQuotaInformer: informers.GetInternalOpenshiftQuotaInformers().Quota().InternalVersion().ClusterResourceQuotas(),
 		ClusterQuotaMapper:           clusterQuotaMappingController.GetClusterQuotaMapper(),
-		RegistryHostnameRetriever:    imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
+		RegistryHostnameRetriever:    registryHostnameRetriever,
 		SecurityInformers:            informers.GetInternalOpenshiftSecurityInformers(),
 		UserInformers:                informers.GetOpenshiftUserInformers(),
 	}
 
 	return admission.PluginInitializers{genericInitializer, webhookInitializer, kubePluginInitializer, openshiftPluginInitializer}, nil
 }
-
-// env returns an environment variable, or the defaultValue if it is not set.
-func env(key string, defaultValue string) string {
-	val := os.Getenv(key)
-	if len(val) == 0 {
-		return defaultValue
-	}
-	return val
-}

pkg/cmd/server/origin/master.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"os"
 	"strings"
 	"time"
 
@@ -462,15 +461,6 @@ func (c *MasterConfig) RouteAllocator() *routeallocationcontroller.RouteAllocati
 	return factory.Create(plugin)
 }
 
-// env returns an environment variable, or the defaultValue if it is not set.
-func env(key string, defaultValue string) string {
-	val := os.Getenv(key)
-	if len(val) == 0 {
-		return defaultValue
-	}
-	return val
-}
-
 func WithPatternPrefixHandler(handler http.Handler, patternHandler http.Handler, prefixes ...string) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		for _, p := range prefixes {

pkg/cmd/server/origin/master_config.go

@@ -9,7 +9,6 @@ import (
 	"k8s.io/client-go/restmapper"
 
 	kapierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
@@ -39,7 +38,6 @@ import (
 	kubernetes "github.com/openshift/origin/pkg/cmd/server/kubernetes/master"
 	originadmission "github.com/openshift/origin/pkg/cmd/server/origin/admission"
 	originrest "github.com/openshift/origin/pkg/cmd/server/origin/rest"
-	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	imageadmission "github.com/openshift/origin/pkg/image/apiserver/admission/limitrange"
 	imageinformer "github.com/openshift/origin/pkg/image/generated/informers/internalversion"
 	networkinformer "github.com/openshift/origin/pkg/network/generated/informers/internalversion"
@@ -51,8 +49,8 @@ import (
 	quotainformer "github.com/openshift/origin/pkg/quota/generated/informers/internalversion"
 	templateinformer "github.com/openshift/origin/pkg/template/generated/informers/internalversion"
 
+	"github.com/openshift/origin/pkg/image/apiserver/registryhostname"
 	securityinformer "github.com/openshift/origin/pkg/security/generated/informers/internalversion"
-	"github.com/openshift/origin/pkg/service"
 	"github.com/openshift/origin/pkg/util/restoptions"
 )
 
@@ -77,7 +75,7 @@ type MasterConfig struct {
 
 	// RegistryHostnameRetriever retrieves the name of the integrated registry, or false if no such registry
 	// is available.
-	RegistryHostnameRetriever imageapi.RegistryHostnameRetriever
+	RegistryHostnameRetriever registryhostname.RegistryHostnameRetriever
 
 	// PrivilegedLoopbackClientConfig is the client configuration used to call OpenShift APIs from system components
 	// To apply different access control to a system component, create a client config specifically for that component.
@@ -171,11 +169,9 @@ func BuildMasterConfig(
 		return nil, err
 	}
 
-	defaultRegistry := env("OPENSHIFT_DEFAULT_REGISTRY", "${DOCKER_REGISTRY_SERVICE_HOST}:${DOCKER_REGISTRY_SERVICE_PORT}")
-	svcCache := service.NewServiceResolverCache(kubeInternalClient.Core().Services(metav1.NamespaceDefault).Get)
-	defaultRegistryFunc, err := svcCache.Defer(defaultRegistry)
+	registryHostnameRetriever, err := registryhostname.DefaultRegistryHostnameRetriever(privilegedLoopbackConfig, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname)
 	if err != nil {
-		return nil, fmt.Errorf("OPENSHIFT_DEFAULT_REGISTRY variable is invalid %q: %v", defaultRegistry, err)
+		return nil, err
 	}
 
 	authenticator, authenticatorPostStartHooks, err := NewAuthenticator(options, privilegedLoopbackConfig, informers)
@@ -188,7 +184,7 @@ func BuildMasterConfig(
 		return nil, err
 	}
 	clusterQuotaMappingController := newClusterQuotaMappingController(informers)
-	discoveryClient := cacheddiscovery.NewMemCacheClient(kubeInternalClient.Discovery())
+	discoveryClient := cacheddiscovery.NewMemCacheClient(privilegedLoopbackKubeClientsetExternal.Discovery())
 	restMapper := restmapper.NewDeferredDiscoveryRESTMapper(discoveryClient)
 	admissionInitializer, err := originadmission.NewPluginInitializer(options, privilegedLoopbackConfig, informers, authorizer, projectCache, restMapper, clusterQuotaMappingController)
 	if err != nil {
@@ -246,7 +242,7 @@ func BuildMasterConfig(
 		ClusterQuotaMappingController: clusterQuotaMappingController,
 		RESTMapper:                    restMapper,
 
-		RegistryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
+		RegistryHostnameRetriever: registryHostnameRetriever,
 
 		PrivilegedLoopbackClientConfig:                *privilegedLoopbackConfig,
 		PrivilegedLoopbackKubernetesClientsetInternal: kubeInternalClient,

pkg/cmd/server/origin/openshift_apiserver.go

@@ -37,9 +37,9 @@ import (
 	buildapiserver "github.com/openshift/origin/pkg/build/apiserver"
 	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
 	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
-	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	imageapiserver "github.com/openshift/origin/pkg/image/apiserver"
 	imageadmission "github.com/openshift/origin/pkg/image/apiserver/admission/limitrange"
+	"github.com/openshift/origin/pkg/image/apiserver/registryhostname"
 	networkapiserver "github.com/openshift/origin/pkg/network/apiserver"
 	oauthapiserver "github.com/openshift/origin/pkg/oauth/apiserver"
 	projectapiserver "github.com/openshift/origin/pkg/project/apiserver"
@@ -92,7 +92,7 @@ type OpenshiftAPIExtraConfig struct {
 	LimitVerifier imageadmission.LimitVerifier
 	// RegistryHostnameRetriever retrieves the internal and external hostname of
 	// the integrated registry, or false if no such registry is available.
-	RegistryHostnameRetriever          imageapi.RegistryHostnameRetriever
+	RegistryHostnameRetriever          registryhostname.RegistryHostnameRetriever
 	AllowedRegistriesForImport         *configapi.AllowedRegistries
 	MaxImagesBulkImportedPerRepository int
 	AdditionalTrustedCA                []byte

pkg/image/apis/image/helper.go

@@ -33,10 +33,6 @@ const (
 
 	// TagReferenceAnnotationTagHidden indicates that a given TagReference is hidden from search results
 	TagReferenceAnnotationTagHidden = "hidden"
-
-	// ImportRegistryNotAllowed indicates that the image tag was not imported due to
-	// untrusted registry.
-	ImportRegistryNotAllowed = "registry is not allowed for import"
 )
 
 var errNoRegistryURLPathAllowed = errors.New("no path after <host>[:<port>] is allowed")
@@ -59,55 +55,6 @@ var ErrCrossImageStreamReference = errors.New("reference tag points to another i
 // ErrInvalidReference is an error when reference tag is invalid.
 var ErrInvalidReference = errors.New("reference tag is invalid")
 
-// RegistryHostnameRetriever represents an interface for retrieving the hostname
-// of internal and external registry.
-type RegistryHostnameRetriever interface {
-	InternalRegistryHostname() (string, bool)
-	ExternalRegistryHostname() (string, bool)
-}
-
-// DefaultRegistryHostnameRetriever is a default implementation of
-// RegistryHostnameRetriever.
-// The first argument is a function that lazy-loads the value of
-// OPENSHIFT_DEFAULT_REGISTRY environment variable which should be deprecated in
-// future.
-func DefaultRegistryHostnameRetriever(deprecatedDefaultRegistryEnvFn func() (string, bool), external, internal string) RegistryHostnameRetriever {
-	return &defaultRegistryHostnameRetriever{
-		deprecatedDefaultFn: deprecatedDefaultRegistryEnvFn,
-		externalHostname:    external,
-		internalHostname:    internal,
-	}
-}
-
-type defaultRegistryHostnameRetriever struct {
-	// deprecatedDefaultFn points to a function that will lazy-load the value of
-	// OPENSHIFT_DEFAULT_REGISTRY.
-	deprecatedDefaultFn func() (string, bool)
-	internalHostname    string
-	externalHostname    string
-}
-
-// InternalRegistryHostnameFn returns a function that can be used to lazy-load
-// the internal Docker Registry hostname. If the master configuration properly
-// InternalRegistryHostname is set, it will prefer that over the lazy-loaded
-// environment variable 'OPENSHIFT_DEFAULT_REGISTRY'.
-func (r *defaultRegistryHostnameRetriever) InternalRegistryHostname() (string, bool) {
-	if len(r.internalHostname) > 0 {
-		return r.internalHostname, true
-	}
-	if r.deprecatedDefaultFn != nil {
-		return r.deprecatedDefaultFn()
-	}
-	return "", false
-}
-
-// ExternalRegistryHostnameFn returns a function that can be used to retrieve an
-// external/public hostname of Docker Registry. External location can be
-// configured in master config using 'ExternalRegistryHostname' property.
-func (r *defaultRegistryHostnameRetriever) ExternalRegistryHostname() (string, bool) {
-	return r.externalHostname, len(r.externalHostname) > 0
-}
-
 // ParseImageStreamImageName splits a string into its name component and ID component, and returns an error
 // if the string is not in the right form.
 func ParseImageStreamImageName(input string) (name string, id string, err error) {

pkg/image/apis/image/validation/validation_test.go

@@ -1252,10 +1252,7 @@ func TestValidateISTUpdateWithWhitelister(t *testing.T) {
 				Tag:        tc.newTagRef,
 			}
 
-			hostnameFunc := imageapi.DefaultRegistryHostnameRetriever(func() (string, bool) {
-				return tc.registryURL, len(tc.registryURL) > 0
-			}, "", "")
-			whitelister, err := whitelist.NewRegistryWhitelister(*tc.whitelist, hostnameFunc)
+			whitelister, err := whitelist.NewRegistryWhitelister(*tc.whitelist, &simpleHostnameRetriever{registryURL: tc.registryURL})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1267,6 +1264,18 @@ func TestValidateISTUpdateWithWhitelister(t *testing.T) {
 	}
 }
 
+type simpleHostnameRetriever struct {
+	registryURL string
+}
+
+func (r *simpleHostnameRetriever) InternalRegistryHostname() (string, bool) {
+	return r.registryURL, len(r.registryURL) > 0
+}
+
+func (r *simpleHostnameRetriever) ExternalRegistryHostname() (string, bool) {
+	return "", false
+}
+
 func TestValidateRegistryAllowedForImport(t *testing.T) {
 	const fieldName = "fieldName"
 

pkg/image/apis/image/validation/whitelist/whitelister.go

@@ -49,6 +49,13 @@ type RegistryWhitelister interface {
 	Copy() RegistryWhitelister
 }
 
+// RegistryHostnameRetriever represents an interface for retrieving the hostname
+// of internal and external registry.
+type RegistryHostnameRetriever interface {
+	InternalRegistryHostname() (string, bool)
+	ExternalRegistryHostname() (string, bool)
+}
+
 type allowedHostPortGlobs struct {
 	host string
 	port string
@@ -57,7 +64,7 @@ type allowedHostPortGlobs struct {
 type registryWhitelister struct {
 	whitelist             []allowedHostPortGlobs
 	pullSpecs             sets.String
-	registryHostRetriever imageapi.RegistryHostnameRetriever
+	registryHostRetriever RegistryHostnameRetriever
 }
 
 var _ RegistryWhitelister = &registryWhitelister{}
@@ -66,7 +73,7 @@ var _ RegistryWhitelister = &registryWhitelister{}
 // list of allowed registries and the current domain name of the integrated Docker registry.
 func NewRegistryWhitelister(
 	whitelist serverapi.AllowedRegistries,
-	registryHostRetriever imageapi.RegistryHostnameRetriever,
+	registryHostRetriever RegistryHostnameRetriever,
 ) (RegistryWhitelister, error) {
 	errs := []error{}
 	rw := registryWhitelister{

pkg/image/apiserver/apiserver.go

@@ -22,7 +22,6 @@ import (
 	imageapiv1 "github.com/openshift/api/image/v1"
 	imageclientv1 "github.com/openshift/client-go/image/clientset/versioned"
 	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
-	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	"github.com/openshift/origin/pkg/image/apis/image/validation/whitelist"
 	imageadmission "github.com/openshift/origin/pkg/image/apiserver/admission/limitrange"
 	"github.com/openshift/origin/pkg/image/apiserver/registry/image"
@@ -35,6 +34,7 @@ import (
 	"github.com/openshift/origin/pkg/image/apiserver/registry/imagestreamimport"
 	"github.com/openshift/origin/pkg/image/apiserver/registry/imagestreammapping"
 	"github.com/openshift/origin/pkg/image/apiserver/registry/imagestreamtag"
+	"github.com/openshift/origin/pkg/image/apiserver/registryhostname"
 	imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset"
 	"github.com/openshift/origin/pkg/image/importer"
 	imageimporter "github.com/openshift/origin/pkg/image/importer"
@@ -44,7 +44,7 @@ import (
 type ExtraConfig struct {
 	KubeAPIServerClientConfig          *restclient.Config
 	LimitVerifier                      imageadmission.LimitVerifier
-	RegistryHostnameRetriever          imageapi.RegistryHostnameRetriever
+	RegistryHostnameRetriever          registryhostname.RegistryHostnameRetriever
 	AllowedRegistriesForImport         *configapi.AllowedRegistries
 	MaxImagesBulkImportedPerRepository int
 	AdditionalTrustedCA                []byte

pkg/image/apiserver/registry/imagestream/etcd/etcd.go

@@ -22,6 +22,7 @@ import (
 	"github.com/openshift/origin/pkg/image/apis/image/validation/whitelist"
 	imageadmission "github.com/openshift/origin/pkg/image/apiserver/admission/limitrange"
 	"github.com/openshift/origin/pkg/image/apiserver/registry/imagestream"
+	"github.com/openshift/origin/pkg/image/apiserver/registryhostname"
 	printersinternal "github.com/openshift/origin/pkg/printers/internalversion"
 	"github.com/openshift/origin/pkg/util/restoptions"
 )
@@ -48,7 +49,7 @@ func (r *REST) ShortNames() []string {
 // NewREST returns a new REST.
 func NewREST(
 	optsGetter restoptions.Getter,
-	registryHostname imageapi.RegistryHostnameRetriever,
+	registryHostname registryhostname.RegistryHostnameRetriever,
 	subjectAccessReviewRegistry authorizationclient.SubjectAccessReviewInterface,
 	limitVerifier imageadmission.LimitVerifier,
 	registryWhitelister whitelist.RegistryWhitelister,

pkg/image/apiserver/registry/imagestream/etcd/etcd_test.go

@@ -21,6 +21,7 @@ import (
 
 	// install all APIs
 	_ "github.com/openshift/origin/pkg/api/install"
+	"github.com/openshift/origin/pkg/image/apiserver/registryhostname"
 )
 
 const (
@@ -52,7 +53,7 @@ func (f *fakeSubjectAccessReviewRegistry) Create(subjectAccessReview *authorizat
 func newStorage(t *testing.T) (*REST, *StatusREST, *InternalREST, *etcdtesting.EtcdTestServer) {
 	server, etcdStorage := etcdtesting.NewUnsecuredEtcd3TestClientServer(t)
 	etcdStorage.Codec = legacyscheme.Codecs.LegacyCodec(schema.GroupVersion{Group: "image.openshift.io", Version: "v1"})
-	registry := imageapi.DefaultRegistryHostnameRetriever(noDefaultRegistry, "", "")
+	registry := registryhostname.TestingRegistryHostnameRetriever(noDefaultRegistry, "", "")
 	imageStorage, _, statusStorage, internalStorage, err := NewREST(
 		restoptions.NewSimpleGetter(etcdStorage),
 		registry,