deprecate OPENSHIFT_DEFAULT_REGISTRY environment variable

Author: mfojtik

pkg/cmd/server/apis/config/types.go

@@ -518,8 +518,6 @@ type ImagePolicyConfig struct {
 	AllowedRegistriesForImport *AllowedRegistries
 	// InternalRegistryHostname sets the hostname for the default internal image
 	// registry. The value must be in "hostname[:port]" format.
-	// For backward compatibility, users can still use OPENSHIFT_DEFAULT_REGISTRY
-	// environment variable but this setting overrides the environment variable.
 	InternalRegistryHostname string
 	// ExternalRegistryHostname sets the hostname for the default external image
 	// registry. The external hostname should be set only when the image registry

pkg/cmd/server/apis/config/v1/types.go

@@ -403,8 +403,6 @@ type ImagePolicyConfig struct {
 	AllowedRegistriesForImport *AllowedRegistries `json:"allowedRegistriesForImport,omitempty"`
 	// InternalRegistryHostname sets the hostname for the default internal image
 	// registry. The value must be in "hostname[:port]" format.
-	// For backward compatibility, users can still use OPENSHIFT_DEFAULT_REGISTRY
-	// environment variable but this setting overrides the environment variable.
 	InternalRegistryHostname string `json:"internalRegistryHostname,omitempty"`
 	// ExternalRegistryHostname sets the hostname for the default external image
 	// registry. The external hostname should be set only when the image registry

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

@@ -3,7 +3,6 @@ package admission
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
 
 	authorizationclient "github.com/openshift/client-go/authorization/clientset/versioned"
 	buildclient "github.com/openshift/client-go/build/clientset/versioned"
@@ -21,10 +20,8 @@ import (
 	quotaclient "github.com/openshift/origin/pkg/quota/generated/internalclientset"
 	"github.com/openshift/origin/pkg/quota/image"
 	securityinformer "github.com/openshift/origin/pkg/security/generated/informers/internalversion"
-	"github.com/openshift/origin/pkg/service"
 	templateclient "github.com/openshift/origin/pkg/template/generated/internalclientset"
 	"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"
@@ -104,13 +101,6 @@ 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)
-	if err != nil {
-		return nil, fmt.Errorf("OPENSHIFT_DEFAULT_REGISTRY variable is invalid %q: %v", defaultRegistry, err)
-	}
-
 	// punch through layers to build this in order to get a string for a cloud provider file
 	// TODO refactor us into a forward building flow with a side channel like this
 	kubeOptions, err := kubernetes.BuildKubeAPIserverOptions(options)
@@ -123,7 +113,7 @@ func NewPluginInitializer(
 		var err error
 		cloudConfig, err = ioutil.ReadFile(kubeOptions.CloudProvider.CloudConfigFile)
 		if err != nil {
-			return nil, fmt.Errorf("Error reading from cloud configuration file %s: %v", kubeOptions.CloudProvider.CloudConfigFile, err)
+			return nil, fmt.Errorf("error reading from cloud configuration file %s: %v", kubeOptions.CloudProvider.CloudConfigFile, err)
 		}
 	}
 	// note: we are passing a combined quota registry here...
@@ -177,19 +167,10 @@ func NewPluginInitializer(
 		Informers:                            informers.GetInternalKubeInformers(),
 		ClusterResourceQuotaInformer:         informers.GetQuotaInformers().Quota().InternalVersion().ClusterResourceQuotas(),
 		ClusterQuotaMapper:                   clusterQuotaMappingController.GetClusterQuotaMapper(),
-		RegistryHostnameRetriever:            imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
+		RegistryHostnameRetriever:            imageapi.DefaultRegistryHostnameRetriever(options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
 		SecurityInformers:                    informers.GetSecurityInformers(),
 		UserInformers:                        informers.GetUserInformers(),
 	}
 
 	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_config.go

@@ -11,7 +11,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"
@@ -54,7 +53,6 @@ import (
 	templateinformer "github.com/openshift/origin/pkg/template/generated/informers/internalversion"
 
 	securityinformer "github.com/openshift/origin/pkg/security/generated/informers/internalversion"
-	"github.com/openshift/origin/pkg/service"
 	"github.com/openshift/origin/pkg/util/restoptions"
 )
 
@@ -150,13 +148,6 @@ 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)
-	if err != nil {
-		return nil, fmt.Errorf("OPENSHIFT_DEFAULT_REGISTRY variable is invalid %q: %v", defaultRegistry, err)
-	}
-
 	authenticator, authenticatorPostStartHooks, err := NewAuthenticator(options, privilegedLoopbackConfig, informers)
 	if err != nil {
 		return nil, err
@@ -184,14 +175,14 @@ func BuildMasterConfig(
 		admission.DecoratorFunc(namespaceLabelDecorator.WithNamespaceLabelConditions),
 		admission.DecoratorFunc(admissionmetrics.WithControllerMetrics),
 	}
-	admission, err := originadmission.NewAdmissionChains(options, admissionInitializer, admissionDecorators)
+	admissionChains, err := originadmission.NewAdmissionChains(options, admissionInitializer, admissionDecorators)
 	if err != nil {
 		return nil, err
 	}
 
 	kubeAPIServerConfig, err := kubernetes.BuildKubernetesMasterConfig(
 		options,
-		admission,
+		admissionChains,
 		authenticator,
 		authorizer,
 	)
@@ -235,7 +226,7 @@ func BuildMasterConfig(
 		ClusterQuotaMappingController: clusterQuotaMappingController,
 		RESTMapper:                    restMapper,
 
-		RegistryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
+		RegistryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
 
 		PrivilegedLoopbackClientConfig:                *privilegedLoopbackConfig,
 		PrivilegedLoopbackKubernetesClientsetInternal: kubeInternalClient,

pkg/image/apis/image/helper.go

@@ -32,10 +32,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")
@@ -67,36 +63,24 @@ type RegistryHostnameRetriever interface {
 
 // 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 {
+func DefaultRegistryHostnameRetriever(external, internal string) RegistryHostnameRetriever {
 	return &defaultRegistryHostnameRetriever{
-		deprecatedDefaultFn: deprecatedDefaultRegistryEnvFn,
-		externalHostname:    external,
-		internalHostname:    internal,
+		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
+	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'.
+// the internal Docker Registry hostname.
 func (r *defaultRegistryHostnameRetriever) InternalRegistryHostname() (string, bool) {
 	if len(r.internalHostname) > 0 {
 		return r.internalHostname, true
 	}
-	if r.deprecatedDefaultFn != nil {
-		return r.deprecatedDefaultFn()
-	}
 	return "", false
 }
 
@@ -813,10 +797,6 @@ func PrioritizeTags(tags []string) {
 	}
 }
 
-func LabelForStream(stream *ImageStream) string {
-	return fmt.Sprintf("%s/%s", stream.Namespace, stream.Name)
-}
-
 // JoinImageSignatureName joins image name and custom signature name into one string with @ separator.
 func JoinImageSignatureName(imageName, signatureName string) (string, error) {
 	if len(imageName) == 0 {