registry: report publicDockerImageRepository to image stream if confi…

…gured

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.InternalRegistryHostnameFn())
 	}
 	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

@@ -519,6 +519,15 @@ 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 Docker
+	// Registry. This can be overriden by using OPENSHIFT_DEFAULT_REGISTRY
+	// environment variable.
+	InternalRegistryHostname string
+	// ExternalRegistryHostname sets the hostname for the default external Docker
+	// Registry. The external hostname should be set only when the registry is
+	// exposed externally. The value is used in 'publicDockerImageRepository'
+	// field in ImageStreams.
+	ExternalRegistryHostname string
 }
 
 // AllowedRegistries represents a list of registries allowed for the image import.

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

@@ -377,6 +377,15 @@ 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 Docker
+	// Registry. This can be overriden by using OPENSHIFT_DEFAULT_REGISTRY
+	// environment variable.
+	InternalRegistryHostname string `json:"internalRegistryHostname"`
+	// ExternalRegistryHostname sets the hostname for the default external Docker
+	// Registry. The external hostname should be set only when the registry is
+	// exposed externally. The value is used in 'publicDockerImageRepository'
+	// field in ImageStreams.
+	ExternalRegistryHostname string `json:"externalRegistryHostname"`
 }
 
 // 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

@@ -141,9 +141,9 @@ type MasterConfig struct {
 
 	// ImageFor is a function that returns the appropriate image to use for a named component
 	ImageFor func(component string) string
-	// 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
 
 	// ExternalVersionCodec is the codec used when serializing annotations, which cannot be changed
 	// without all clients being aware of the new version.
@@ -318,7 +318,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(),
 	}
@@ -365,8 +365,8 @@ func BuildMasterConfig(options configapi.MasterConfig, informers InformerAccess)
 		AdmissionControl:     originAdmission,
 		KubeAdmissionControl: kubeAdmission,
 
-		ImageFor:       imageTemplate.ExpandOrDie,
-		RegistryNameFn: imageapi.DefaultRegistryFunc(defaultRegistryFunc),
+		ImageFor:                  imageTemplate.ExpandOrDie,
+		RegistryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, "", ""),
 
 		// TODO: migration of versions of resources stored in annotations must be sorted out
 		ExternalVersionCodec: kapi.Codecs.LegacyCodec(schema.GroupVersion{Group: "", Version: "v1"}),

pkg/cmd/server/origin/openshift_apiserver.go

@@ -83,9 +83,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
 
@@ -144,8 +144,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

@@ -204,12 +204,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 {
@@ -231,7 +231,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,42 @@ 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 {
+	InternalRegistryHostnameFn() func() (string, bool)
+	ExternalRegistryHostnameFn() func() (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.
+func DefaultRegistryHostnameRetriever(defaultFn func() (string, bool), external, internal string) RegistryHostnameRetriever {
+	return &defaultRegistryHostnameRetriever{defaultFn: defaultFn, external: external, internal: internal}
+}
+
+type defaultRegistryHostnameRetriever struct {
+	defaultFn          func() (string, bool)
+	internal, external string
+}
+
+// InternalRegistryHostnameFn returns a function that can be used to lazy-load
+// the internal Docker Registry hostname. If the master configuration propertly
+// InternalRegistryHostname is set, it will prefer that over the lazy-loaded
+// environment variable 'OPENSHIFT_DEFAULT_REGISTRY'.
+func (r *defaultRegistryHostnameRetriever) InternalRegistryHostnameFn() func() (string, bool) {
+	if len(r.internal) > 0 {
+		return func() (string, bool) { return r.internal, true }
+	}
+	return r.defaultFn
+}
 
-// 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) ExternalRegistryHostnameFn() func() (string, bool) {
+	return func() (string, bool) { return r.external, true }
 }
 
 // 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 {

pkg/image/registry/imagestream/strategy.go

@@ -34,22 +34,22 @@ type ResourceGetter interface {
 type Strategy struct {
 	runtime.ObjectTyper
 	names.NameGenerator
-	defaultRegistry   imageapi.DefaultRegistry
-	tagVerifier       *TagVerifier
-	limitVerifier     imageadmission.LimitVerifier
-	imageStreamGetter ResourceGetter
+	registryHostnameRetriever imageapi.RegistryHostnameRetriever
+	tagVerifier               *TagVerifier
+	limitVerifier             imageadmission.LimitVerifier
+	imageStreamGetter         ResourceGetter
 }
 
 // NewStrategy is the default logic that applies when creating and updating
 // ImageStream objects via the REST API.
-func NewStrategy(defaultRegistry imageapi.DefaultRegistry, subjectAccessReviewClient subjectaccessreview.Registry, limitVerifier imageadmission.LimitVerifier, imageStreamGetter ResourceGetter) Strategy {
+func NewStrategy(registryHostname imageapi.RegistryHostnameRetriever, subjectAccessReviewClient subjectaccessreview.Registry, limitVerifier imageadmission.LimitVerifier, imageStreamGetter ResourceGetter) Strategy {
 	return Strategy{
-		ObjectTyper:       kapi.Scheme,
-		NameGenerator:     names.SimpleNameGenerator,
-		defaultRegistry:   defaultRegistry,
-		tagVerifier:       &TagVerifier{subjectAccessReviewClient},
-		limitVerifier:     limitVerifier,
-		imageStreamGetter: imageStreamGetter,
+		ObjectTyper:               kapi.Scheme,
+		NameGenerator:             names.SimpleNameGenerator,
+		registryHostnameRetriever: registryHostname,
+		tagVerifier:               &TagVerifier{subjectAccessReviewClient},
+		limitVerifier:             limitVerifier,
+		imageStreamGetter:         imageStreamGetter,
 	}
 }
 
@@ -117,7 +117,7 @@ func (Strategy) AllowUnconditionalUpdate() bool {
 // if a default registry exists, the value returned is of the form
 // <default registry>/<namespace>/<stream name>.
 func (s Strategy) dockerImageRepository(stream *imageapi.ImageStream) string {
-	registry, ok := s.defaultRegistry.DefaultRegistry()
+	registry, ok := s.registryHostnameRetriever.InternalRegistryHostnameFn()()
 	if !ok {
 		return stream.Spec.DockerImageRepository
 	}
@@ -133,6 +133,23 @@ func (s Strategy) dockerImageRepository(stream *imageapi.ImageStream) string {
 	return ref.String()
 }
 
+// publicDockerImageRepository determines the public location of given image
+// stream. If the ExternalRegistryHostname is set in the master config, the
+// value of this property is used as a hostname part for the docker image
+// reference.
+func (s Strategy) publicDockerImageRepository(stream *imageapi.ImageStream) string {
+	externalHostname, ok := s.registryHostnameRetriever.ExternalRegistryHostnameFn()()
+	if len(externalHostname) == 0 || !ok {
+		return ""
+	}
+	ref := imageapi.DockerImageReference{
+		Registry:  externalHostname,
+		Namespace: stream.Namespace,
+		Name:      stream.Name,
+	}
+	return ref.String()
+}
+
 func parseFromReference(stream *imageapi.ImageStream, from *kapi.ObjectReference) (string, string, error) {
 	splitChar := ""
 	refType := ""
@@ -164,7 +181,7 @@ func parseFromReference(stream *imageapi.ImageStream, from *kapi.ObjectReference
 // tagsChanged updates stream.Status.Tags based on the old and new image stream.
 // if the old stream is nil, all tags are considered additions.
 func (s Strategy) tagsChanged(old, stream *imageapi.ImageStream) field.ErrorList {
-	internalRegistry, hasInternalRegistry := s.defaultRegistry.DefaultRegistry()
+	internalRegistry, hasInternalRegistry := s.registryHostnameRetriever.InternalRegistryHostnameFn()()
 
 	var errs field.ErrorList
 
@@ -522,10 +539,12 @@ func (s Strategy) Decorate(obj runtime.Object) error {
 	switch t := obj.(type) {
 	case *imageapi.ImageStream:
 		t.Status.DockerImageRepository = s.dockerImageRepository(t)
+		t.Status.PublicDockerImageRepository = s.publicDockerImageRepository(t)
 	case *imageapi.ImageStreamList:
 		for i := range t.Items {
 			is := &t.Items[i]
 			is.Status.DockerImageRepository = s.dockerImageRepository(is)
+			is.Status.PublicDockerImageRepository = s.publicDockerImageRepository(is)
 		}
 	default:
 		return kerrors.NewBadRequest(fmt.Sprintf("not an ImageStream nor ImageStreamList: %v", obj))

pkg/image/registry/imagestream/strategy_test.go

@@ -135,7 +135,8 @@ func TestDockerImageRepository(t *testing.T) {
 	}
 
 	for testName, test := range tests {
-		strategy := NewStrategy(&fakeDefaultRegistry{test.defaultRegistry}, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}, nil)
+		fakeRegistry := &fakeDefaultRegistry{test.defaultRegistry}
+		strategy := NewStrategy(imageapi.DefaultRegistryHostnameRetriever(fakeRegistry.DefaultRegistry, "", ""), &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}, nil)
 		value := strategy.dockerImageRepository(test.stream)
 		if e, a := test.expected, value; e != a {
 			t.Errorf("%s: expected %q, got %q", testName, e, a)
@@ -518,13 +519,13 @@ func TestLimitVerifier(t *testing.T) {
 			allow: true,
 		}
 		tagVerifier := &TagVerifier{sar}
-
+		fakeRegistry := &fakeDefaultRegistry{}
 		s := &Strategy{
 			tagVerifier: tagVerifier,
 			limitVerifier: &testutil.FakeImageStreamLimitVerifier{
 				ImageStreamEvaluator: tc.isEvaluator,
 			},
-			defaultRegistry: &fakeDefaultRegistry{},
+			registryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(fakeRegistry.DefaultRegistry, "", ""),
 		}
 
 		ctx := apirequest.WithUser(apirequest.NewDefaultContext(), &fakeUser{})
@@ -1080,8 +1081,9 @@ func TestTagsChanged(t *testing.T) {
 			previousStream = nil
 		}
 
+		fakeRegistry := &fakeDefaultRegistry{}
 		s := &Strategy{
-			defaultRegistry:   &fakeDefaultRegistry{},
+			defaultRegistry:   imageapi.DefaultRegistryHostnameRetriever(fakeRegistry.DefaultRegistry, "", ""),
 			imageStreamGetter: &fakeImageStreamGetter{test.otherStream},
 		}
 		err := s.tagsChanged(previousStream, stream)