Merge pull request openshift#15853 from mfojtik/public-pull

Automatic merge from submit-queue (batch tested with PRs 15853, 15916, 16017, 16027, 16043)

registry: report publicDockerImageRepository to image stream if configured

This PR introduces two new fields for the master config, `externalRegistryHostname` and `internalRegistryHostname`.

The first one will be used as a hostname for newly added `publicDockerImageRepository ` field and it will reveal a public Docker pull spec users should use to pull the image if the registry is exposed externally (via route, etc.).

The later is a replacement for `OPENSHIFT_DEFAULT_REGISTRY` environment variable (but that variable is still picked up to guarantee backward compatibility). If set it will override the `OPENSHIFT_DEFAULT_REGISTRY`.

pkg/cmd/server/admission/init.go

@@ -28,7 +28,7 @@ type PluginInitializer struct {
 	Informers                    kinternalinformers.SharedInformerFactory
 	ClusterResourceQuotaInformer quotainformer.ClusterResourceQuotaInformer
 	ClusterQuotaMapper           clusterquotamapping.ClusterQuotaMapper
-	DefaultRegistryFn            imageapi.DefaultRegistryFunc
+	RegistryHostnameRetriever    imageapi.RegistryHostnameRetriever
 	SecurityInformers            securityinformer.SharedInformerFactory
 	UserInformers                userinformer.SharedInformerFactory
 }
@@ -70,7 +70,7 @@ func (i *PluginInitializer) Initialize(plugin admission.Interface) {
 		wantsSecurityInformer.SetSecurityInformers(i.SecurityInformers)
 	}
 	if wantsDefaultRegistryFunc, ok := plugin.(WantsDefaultRegistryFunc); ok {
-		wantsDefaultRegistryFunc.SetDefaultRegistryFunc(i.DefaultRegistryFn)
+		wantsDefaultRegistryFunc.SetDefaultRegistryFunc(i.RegistryHostnameRetriever.InternalRegistryHostname)
 	}
 	if wantsUserInformer, ok := plugin.(WantsUserInformer); ok {
 		wantsUserInformer.SetUserInformer(i.UserInformers)

pkg/cmd/server/admission/types.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/openshift/origin/pkg/client"
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
-	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	"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"
@@ -79,7 +78,7 @@ type WantsSecurityInformer interface {
 // WantsDefaultRegistryFunc should be implemented by admission plugins that need to know the default registry
 // address.
 type WantsDefaultRegistryFunc interface {
-	SetDefaultRegistryFunc(imageapi.DefaultRegistryFunc)
+	SetDefaultRegistryFunc(func() (string, bool))
 	admission.Validator
 }
 

pkg/cmd/server/api/types.go

@@ -514,6 +514,16 @@ type ImagePolicyConfig struct {
 	// this policy - typically only administrators or system integrations will have those
 	// permissions.
 	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
+	// is exposed externally. The value is used in 'publicDockerImageRepository'
+	// field in ImageStreams. The value must be in "hostname[:port]" format.
+	ExternalRegistryHostname string
 }
 
 // AllowedRegistries represents a list of registries allowed for the image import.

pkg/cmd/server/api/v1/swagger_doc.go

@@ -339,6 +339,8 @@ var map_ImagePolicyConfig = map[string]string{
 	"scheduledImageImportMinimumIntervalSeconds": "ScheduledImageImportMinimumIntervalSeconds is the minimum number of seconds that can elapse between when image streams scheduled for background import are checked against the upstream repository. The default value is 15 minutes.",
 	"maxScheduledImageImportsPerMinute":          "MaxScheduledImageImportsPerMinute is the maximum number of scheduled image streams that will be imported in the background per minute. The default value is 60. Set to -1 for unlimited.",
 	"allowedRegistriesForImport":                 "AllowedRegistriesForImport limits the docker registries that normal users may import images from. Set this list to the registries that you trust to contain valid Docker images and that you want applications to be able to import from. Users with permission to create Images or ImageStreamMappings via the API are not affected by this policy - typically only administrators or system integrations will have those permissions.",
+	"internalRegistryHostname":                   "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.",
+	"externalRegistryHostname":                   "ExternalRegistryHostname sets the hostname for the default external image registry. The external hostname should be set only when the image registry is exposed externally. The value is used in 'publicDockerImageRepository' field in ImageStreams. The value must be in \"hostname[:port]\" format.",
 }
 
 func (ImagePolicyConfig) SwaggerDoc() map[string]string {

pkg/cmd/server/api/v1/types.go

@@ -372,6 +372,16 @@ type ImagePolicyConfig struct {
 	// this policy - typically only administrators or system integrations will have those
 	// permissions.
 	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
+	// is exposed externally. The value is used in 'publicDockerImageRepository'
+	// field in ImageStreams. The value must be in "hostname[:port]" format.
+	ExternalRegistryHostname string `json:"externalRegistryHostname,omitempty"`
 }
 
 // AllowedRegistries represents a list of registries allowed for the image import.

pkg/cmd/server/origin/master.go

@@ -73,7 +73,7 @@ func (c *MasterConfig) newOpenshiftAPIConfig(kubeAPIServerConfig apiserver.Confi
 		RuleResolver:                       c.RuleResolver,
 		SubjectLocator:                     c.SubjectLocator,
 		LimitVerifier:                      c.LimitVerifier,
-		RegistryNameFn:                     c.RegistryNameFn,
+		RegistryHostnameRetriever:          c.RegistryHostnameRetriever,
 		AllowedRegistriesForImport:         c.Options.ImagePolicyConfig.AllowedRegistriesForImport,
 		MaxImagesBulkImportedPerRepository: c.Options.ImagePolicyConfig.MaxImagesBulkImportedPerRepository,
 		RouteAllocator:                     c.RouteAllocator(),

pkg/cmd/server/origin/master_config.go

@@ -118,9 +118,9 @@ type MasterConfig struct {
 	// of both the origin config AND the kube config, so this spot makes more sense.
 	KubeAdmissionControl admission.Interface
 
-	// RegistryNameFn retrieves the name of the integrated registry, or false if no such registry
+	// RegistryHostnameRetriever retrieves the name of the integrated registry, or false if no such registry
 	// is available.
-	RegistryNameFn imageapi.DefaultRegistryFunc
+	RegistryHostnameRetriever imageapi.RegistryHostnameRetriever
 
 	KubeletClientConfig *kubeletclient.KubeletClientConfig
 
@@ -264,7 +264,7 @@ func BuildMasterConfig(options configapi.MasterConfig, informers InformerAccess)
 		Informers:                    informers.GetInternalKubeInformers(),
 		ClusterResourceQuotaInformer: informers.GetQuotaInformers().Quota().InternalVersion().ClusterResourceQuotas(),
 		ClusterQuotaMapper:           clusterQuotaMappingController.GetClusterQuotaMapper(),
-		DefaultRegistryFn:            imageapi.DefaultRegistryFunc(defaultRegistryFunc),
+		RegistryHostnameRetriever:    imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
 		SecurityInformers:            informers.GetSecurityInformers(),
 		UserInformers:                informers.GetUserInformers(),
 	}
@@ -319,7 +319,7 @@ func BuildMasterConfig(options configapi.MasterConfig, informers InformerAccess)
 		AdmissionControl:     originAdmission,
 		KubeAdmissionControl: kubeAdmission,
 
-		RegistryNameFn: imageapi.DefaultRegistryFunc(defaultRegistryFunc),
+		RegistryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
 
 		KubeletClientConfig: kubeletClientConfig,
 

pkg/cmd/server/origin/openshift_apiserver.go

@@ -84,9 +84,9 @@ type OpenshiftAPIConfig struct {
 	RuleResolver   rbacregistryvalidation.AuthorizationRuleResolver
 	SubjectLocator authorizer.SubjectLocator
 	LimitVerifier  imageadmission.LimitVerifier
-	// RegistryNameFn retrieves the name of the integrated registry, or false if no such registry
-	// is available.
-	RegistryNameFn                     imageapi.DefaultRegistryFunc
+	// RegistryHostnameRetriever retrieves the internal and external hostname of
+	// the integrated registry, or false if no such registry is available.
+	RegistryHostnameRetriever          imageapi.RegistryHostnameRetriever
 	AllowedRegistriesForImport         *configapi.AllowedRegistries
 	MaxImagesBulkImportedPerRepository int
 
@@ -145,8 +145,8 @@ func (c *OpenshiftAPIConfig) Validate() error {
 	if c.LimitVerifier == nil {
 		ret = append(ret, fmt.Errorf("LimitVerifier is required"))
 	}
-	if c.RegistryNameFn == nil {
-		ret = append(ret, fmt.Errorf("RegistryNameFn is required"))
+	if c.RegistryHostnameRetriever == nil {
+		ret = append(ret, fmt.Errorf("RegistryHostnameRetriever is required"))
 	}
 	if c.RouteAllocator == nil {
 		ret = append(ret, fmt.Errorf("RouteAllocator is required"))

pkg/cmd/server/origin/storage.go

@@ -145,12 +145,12 @@ func (c OpenshiftAPIConfig) GetRestStorage() (map[schema.GroupVersion]map[string
 	imageRegistry := image.NewRegistry(imageStorage)
 	imageSignatureStorage := imagesignature.NewREST(c.DeprecatedOpenshiftClient.Images())
 	imageStreamSecretsStorage := imagesecret.NewREST(c.KubeClientInternal.Core())
-	imageStreamStorage, imageStreamStatusStorage, internalImageStreamStorage, err := imagestreametcd.NewREST(c.GenericConfig.RESTOptionsGetter, c.RegistryNameFn, subjectAccessReviewRegistry, c.LimitVerifier)
+	imageStreamStorage, imageStreamStatusStorage, internalImageStreamStorage, err := imagestreametcd.NewREST(c.GenericConfig.RESTOptionsGetter, c.RegistryHostnameRetriever, subjectAccessReviewRegistry, c.LimitVerifier)
 	if err != nil {
 		return nil, fmt.Errorf("error building REST storage: %v", err)
 	}
 	imageStreamRegistry := imagestream.NewRegistry(imageStreamStorage, imageStreamStatusStorage, internalImageStreamStorage)
-	imageStreamMappingStorage := imagestreammapping.NewREST(imageRegistry, imageStreamRegistry, c.RegistryNameFn)
+	imageStreamMappingStorage := imagestreammapping.NewREST(imageRegistry, imageStreamRegistry, c.RegistryHostnameRetriever)
 	imageStreamTagStorage := imagestreamtag.NewREST(imageRegistry, imageStreamRegistry)
 	importerCache, err := imageimporter.NewImageStreamLayerCache(imageimporter.DefaultImageStreamLayerCacheSize)
 	if err != nil {
@@ -172,7 +172,7 @@ func (c OpenshiftAPIConfig) GetRestStorage() (map[schema.GroupVersion]map[string
 		insecureImportTransport,
 		importerDockerClientFn,
 		c.AllowedRegistriesForImport,
-		c.RegistryNameFn,
+		c.RegistryHostnameRetriever,
 		c.DeprecatedOpenshiftClient.SubjectAccessReviews())
 	imageStreamImageStorage := imagestreamimage.NewREST(imageRegistry, imageStreamRegistry)
 

pkg/image/admission/imagepolicy/imagepolicy.go

@@ -108,7 +108,7 @@ func newImagePolicyPlugin(parsed *api.ImagePolicyConfig) (*imagePolicyPlugin, er
 	}, nil
 }
 
-func (a *imagePolicyPlugin) SetDefaultRegistryFunc(fn imageapi.DefaultRegistryFunc) {
+func (a *imagePolicyPlugin) SetDefaultRegistryFunc(fn func() (string, bool)) {
 	a.integratedRegistryMatcher.RegistryMatcher = rules.RegistryNameMatcher(fn)
 }
 

pkg/image/admission/imagepolicy/rules/rules.go

@@ -23,10 +23,10 @@ type RegistryMatcher interface {
 	Matches(name string) bool
 }
 
-type RegistryNameMatcher imageapi.DefaultRegistryFunc
+type RegistryNameMatcher func() (string, bool)
 
 func (m RegistryNameMatcher) Matches(name string) bool {
-	current, ok := imageapi.DefaultRegistryFunc(m)()
+	current, ok := m()
 	if !ok {
 		return false
 	}

pkg/image/apis/image/helper.go

@@ -45,17 +45,53 @@ var errNoRegistryURLPathAllowed = fmt.Errorf("no path after <host>[:<port>] is a
 var errNoRegistryURLQueryAllowed = fmt.Errorf("no query arguments are allowed after <host>[:<port>]")
 var errRegistryURLHostEmpty = fmt.Errorf("no host name specified")
 
-// DefaultRegistry returns the default Docker registry (host or host:port), or false if it is not available.
-type DefaultRegistry interface {
-	DefaultRegistry() (string, bool)
+// RegistryHostnameRetriever represents an interface for retrieving the hostname
+// of internal and external registry.
+type RegistryHostnameRetriever interface {
+	InternalRegistryHostname() (string, bool)
+	ExternalRegistryHostname() (string, bool)
 }
 
-// DefaultRegistryFunc implements DefaultRegistry for a simple function.
-type DefaultRegistryFunc func() (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
+}
 
-// DefaultRegistry implements the DefaultRegistry interface for a function.
-func (fn DefaultRegistryFunc) DefaultRegistry() (string, bool) {
-	return fn()
+// 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

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

@@ -25,7 +25,7 @@ type REST struct {
 var _ rest.StandardStorage = &REST{}
 
 // NewREST returns a new REST.
-func NewREST(optsGetter restoptions.Getter, defaultRegistry imageapi.DefaultRegistry, subjectAccessReviewRegistry subjectaccessreview.Registry, limitVerifier imageadmission.LimitVerifier) (*REST, *StatusREST, *InternalREST, error) {
+func NewREST(optsGetter restoptions.Getter, registryHostname imageapi.RegistryHostnameRetriever, subjectAccessReviewRegistry subjectaccessreview.Registry, limitVerifier imageadmission.LimitVerifier) (*REST, *StatusREST, *InternalREST, error) {
 	store := registry.Store{
 		Copier:                   kapi.Scheme,
 		NewFunc:                  func() runtime.Object { return &imageapi.ImageStream{} },
@@ -39,7 +39,7 @@ func NewREST(optsGetter restoptions.Getter, defaultRegistry imageapi.DefaultRegi
 		subjectAccessReviewRegistry: subjectAccessReviewRegistry,
 	}
 	// strategy must be able to load image streams across namespaces during tag verification
-	strategy := imagestream.NewStrategy(defaultRegistry, subjectAccessReviewRegistry, limitVerifier, rest)
+	strategy := imagestream.NewStrategy(registryHostname, subjectAccessReviewRegistry, limitVerifier, rest)
 
 	store.CreateStrategy = strategy
 	store.UpdateStrategy = strategy

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

@@ -27,8 +27,8 @@ const (
 )
 
 var (
-	testDefaultRegistry = imageapi.DefaultRegistryFunc(func() (string, bool) { return "test", true })
-	noDefaultRegistry   = imageapi.DefaultRegistryFunc(func() (string, bool) { return "", false })
+	testDefaultRegistry = func() (string, bool) { return "test", true }
+	noDefaultRegistry   = func() (string, bool) { return "", false }
 )
 
 type fakeSubjectAccessReviewRegistry struct {
@@ -48,7 +48,8 @@ func (f *fakeSubjectAccessReviewRegistry) CreateSubjectAccessReview(ctx apireque
 
 func newStorage(t *testing.T) (*REST, *StatusREST, *InternalREST, *etcdtesting.EtcdTestServer) {
 	etcdStorage, server := registrytest.NewEtcdStorage(t, latest.Version.Group)
-	imageStorage, statusStorage, internalStorage, err := NewREST(restoptions.NewSimpleGetter(etcdStorage), noDefaultRegistry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{})
+	registry := imageapi.DefaultRegistryHostnameRetriever(noDefaultRegistry, "", "")
+	imageStorage, statusStorage, internalStorage, err := NewREST(restoptions.NewSimpleGetter(etcdStorage), registry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{})
 	if err != nil {
 		t.Fatal(err)
 	}