diff --git a/pkg/cmd/server/admission/init.go b/pkg/cmd/server/admission/init.go index 6bff73888942..11e9720a1c07 100644 --- a/pkg/cmd/server/admission/init.go +++ b/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) diff --git a/pkg/cmd/server/admission/types.go b/pkg/cmd/server/admission/types.go index 24046a1812f4..5c2912329ece 100644 --- a/pkg/cmd/server/admission/types.go +++ b/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 } diff --git a/pkg/cmd/server/api/types.go b/pkg/cmd/server/api/types.go index 7b17e5919f34..92d643a1121a 100644 --- a/pkg/cmd/server/api/types.go +++ b/pkg/cmd/server/api/types.go @@ -514,6 +514,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. The value should be in "hostname[:port]" format. + 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. The value should be in "hostname[:port]" format. + ExternalRegistryHostname string } // AllowedRegistries represents a list of registries allowed for the image import. diff --git a/pkg/cmd/server/api/v1/swagger_doc.go b/pkg/cmd/server/api/v1/swagger_doc.go index e20df2dd039d..01baa3943270 100644 --- a/pkg/cmd/server/api/v1/swagger_doc.go +++ b/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 Docker Registry. This can be overriden by using OPENSHIFT_DEFAULT_REGISTRY environment variable. The value should be in \"hostname[:port]\" format.", + "externalRegistryHostname": "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. The value should be in \"hostname[:port]\" format.", } func (ImagePolicyConfig) SwaggerDoc() map[string]string { diff --git a/pkg/cmd/server/api/v1/types.go b/pkg/cmd/server/api/v1/types.go index 647a0f3b7d2a..15e75112d90e 100644 --- a/pkg/cmd/server/api/v1/types.go +++ b/pkg/cmd/server/api/v1/types.go @@ -372,6 +372,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. The value should be in "hostname[:port]" format. + InternalRegistryHostname string `json:"internalRegistryHostname,omitempty"` + // 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. The value should be in "hostname[:port]" format. + ExternalRegistryHostname string `json:"externalRegistryHostname,omitempty"` } // AllowedRegistries represents a list of registries allowed for the image import. diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index 4c6c9693b860..de384aa14ed0 100644 --- a/pkg/cmd/server/origin/master.go +++ b/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(), diff --git a/pkg/cmd/server/origin/master_config.go b/pkg/cmd/server/origin/master_config.go index 737b662a592f..da98dfb9740a 100644 --- a/pkg/cmd/server/origin/master_config.go +++ b/pkg/cmd/server/origin/master_config.go @@ -119,9 +119,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 @@ -265,7 +265,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(), } @@ -316,7 +316,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, diff --git a/pkg/cmd/server/origin/openshift_apiserver.go b/pkg/cmd/server/origin/openshift_apiserver.go index 83bd6ae368fe..dfb43aee4b78 100644 --- a/pkg/cmd/server/origin/openshift_apiserver.go +++ b/pkg/cmd/server/origin/openshift_apiserver.go @@ -82,9 +82,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 @@ -143,8 +143,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")) diff --git a/pkg/cmd/server/origin/storage.go b/pkg/cmd/server/origin/storage.go index 686755babfbf..e8290b9bce28 100644 --- a/pkg/cmd/server/origin/storage.go +++ b/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) diff --git a/pkg/image/admission/imagepolicy/imagepolicy.go b/pkg/image/admission/imagepolicy/imagepolicy.go index 8a3108e296d4..3408ebd0dc27 100644 --- a/pkg/image/admission/imagepolicy/imagepolicy.go +++ b/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) } diff --git a/pkg/image/admission/imagepolicy/rules/rules.go b/pkg/image/admission/imagepolicy/rules/rules.go index ba3ffefedcd2..66f10f32fee8 100644 --- a/pkg/image/admission/imagepolicy/rules/rules.go +++ b/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 } diff --git a/pkg/image/apis/image/helper.go b/pkg/image/apis/image/helper.go index b334a2b10d32..945d0162cfa3 100644 --- a/pkg/image/apis/image/helper.go +++ b/pkg/image/apis/image/helper.go @@ -45,17 +45,53 @@ var errNoRegistryURLPathAllowed = fmt.Errorf("no path after [:] is a var errNoRegistryURLQueryAllowed = fmt.Errorf("no query arguments are allowed after [:]") 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 propertly +// 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 diff --git a/pkg/image/registry/imagestream/etcd/etcd.go b/pkg/image/registry/imagestream/etcd/etcd.go index 6596aaf64fb9..3e855db04ae5 100644 --- a/pkg/image/registry/imagestream/etcd/etcd.go +++ b/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 diff --git a/pkg/image/registry/imagestream/etcd/etcd_test.go b/pkg/image/registry/imagestream/etcd/etcd_test.go index d0d3e8f2637c..1e54811afe1b 100644 --- a/pkg/image/registry/imagestream/etcd/etcd_test.go +++ b/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) } diff --git a/pkg/image/registry/imagestream/strategy.go b/pkg/image/registry/imagestream/strategy.go index 237a3ac0130b..6aefb1ebe362 100644 --- a/pkg/image/registry/imagestream/strategy.go +++ b/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 // //. func (s Strategy) dockerImageRepository(stream *imageapi.ImageStream) string { - registry, ok := s.defaultRegistry.DefaultRegistry() + registry, ok := s.registryHostnameRetriever.InternalRegistryHostname() 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.ExternalRegistryHostname() + if !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.InternalRegistryHostname() 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)) diff --git a/pkg/image/registry/imagestream/strategy_test.go b/pkg/image/registry/imagestream/strategy_test.go index e45758d74c9e..3d1fc29b5b59 100644 --- a/pkg/image/registry/imagestream/strategy_test.go +++ b/pkg/image/registry/imagestream/strategy_test.go @@ -73,6 +73,47 @@ func (f *fakeSubjectAccessReviewRegistry) CreateSubjectAccessReview(ctx apireque return &authorizationapi.SubjectAccessReviewResponse{Allowed: f.allow}, f.err } +func TestPublicDockerImageRepository(t *testing.T) { + tests := map[string]struct { + stream *imageapi.ImageStream + expected string + publicRegistry string + }{ + "public registry is not set": { + stream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "somerepo", + }, + Spec: imageapi.ImageStreamSpec{ + DockerImageRepository: "a/b", + }, + }, + publicRegistry: "", + expected: "", + }, + "public registry is set": { + stream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "somerepo", + }, + Spec: imageapi.ImageStreamSpec{ + DockerImageRepository: "a/b", + }, + }, + publicRegistry: "registry-default.external.url", + expected: "registry-default.external.url/somerepo", + }, + } + + for testName, test := range tests { + strategy := NewStrategy(imageapi.DefaultRegistryHostnameRetriever(nil, test.publicRegistry, ""), &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}, nil) + value := strategy.publicDockerImageRepository(test.stream) + if e, a := test.expected, value; e != a { + t.Errorf("%s: expected %q, got %q", testName, e, a) + } + } +} + func TestDockerImageRepository(t *testing.T) { tests := map[string]struct { stream *imageapi.ImageStream @@ -135,7 +176,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 +560,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,9 +1122,10 @@ func TestTagsChanged(t *testing.T) { previousStream = nil } + fakeRegistry := &fakeDefaultRegistry{} s := &Strategy{ - defaultRegistry: &fakeDefaultRegistry{}, - imageStreamGetter: &fakeImageStreamGetter{test.otherStream}, + registryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(fakeRegistry.DefaultRegistry, "", ""), + imageStreamGetter: &fakeImageStreamGetter{test.otherStream}, } err := s.tagsChanged(previousStream, stream) if len(err) > 0 { diff --git a/pkg/image/registry/imagestreamimage/rest_test.go b/pkg/image/registry/imagestreamimage/rest_test.go index c4b309d0e4fd..4b5b3de8b708 100644 --- a/pkg/image/registry/imagestreamimage/rest_test.go +++ b/pkg/image/registry/imagestreamimage/rest_test.go @@ -28,7 +28,7 @@ import ( _ "github.com/openshift/origin/pkg/api/install" ) -var testDefaultRegistry = imageapi.DefaultRegistryFunc(func() (string, bool) { return "defaultregistry:5000", true }) +var testDefaultRegistry = func() (string, bool) { return "defaultregistry:5000", true } type fakeSubjectAccessReviewRegistry struct { } @@ -47,7 +47,8 @@ func setup(t *testing.T) (etcd.KV, *etcdtesting.EtcdTestServer, *REST) { if err != nil { t.Fatal(err) } - imageStreamStorage, imageStreamStatus, internalStorage, err := imagestreametcd.NewREST(restoptions.NewSimpleGetter(etcdStorage), testDefaultRegistry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}) + defaultRegistry := imageapi.DefaultRegistryHostnameRetriever(testDefaultRegistry, "", "") + imageStreamStorage, imageStreamStatus, internalStorage, err := imagestreametcd.NewREST(restoptions.NewSimpleGetter(etcdStorage), defaultRegistry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}) if err != nil { t.Fatal(err) } diff --git a/pkg/image/registry/imagestreamimport/rest.go b/pkg/image/registry/imagestreamimport/rest.go index 566a9c44f270..bf090e76d8bb 100644 --- a/pkg/image/registry/imagestreamimport/rest.go +++ b/pkg/image/registry/imagestreamimport/rest.go @@ -61,7 +61,7 @@ func NewREST(importFn ImporterFunc, streams imagestream.Registry, internalStream transport, insecureTransport http.RoundTripper, clientFn ImporterDockerRegistryFunc, allowedImportRegistries *serverapi.AllowedRegistries, - registryFn imageapi.DefaultRegistryFunc, + registryFn imageapi.RegistryHostnameRetriever, sarClient client.SubjectAccessReviewInterface, ) *REST { return &REST{ diff --git a/pkg/image/registry/imagestreamimport/strategy.go b/pkg/image/registry/imagestreamimport/strategy.go index d9f9b82b86af..2d71736fbc2e 100644 --- a/pkg/image/registry/imagestreamimport/strategy.go +++ b/pkg/image/registry/imagestreamimport/strategy.go @@ -15,15 +15,15 @@ import ( // strategy implements behavior for ImageStreamImports. type strategy struct { runtime.ObjectTyper - allowedRegistries *serverapi.AllowedRegistries - registryFn imageapi.DefaultRegistryFunc + allowedRegistries *serverapi.AllowedRegistries + registryHostRetriever imageapi.RegistryHostnameRetriever } -func NewStrategy(registries *serverapi.AllowedRegistries, registryFn imageapi.DefaultRegistryFunc) *strategy { +func NewStrategy(registries *serverapi.AllowedRegistries, registry imageapi.RegistryHostnameRetriever) *strategy { return &strategy{ - ObjectTyper: kapi.Scheme, - allowedRegistries: registries, - registryFn: registryFn, + ObjectTyper: kapi.Scheme, + allowedRegistries: registries, + registryHostRetriever: registry, } } @@ -44,9 +44,7 @@ func (s *strategy) ValidateAllowedRegistries(isi *imageapi.ImageStreamImport) fi return errs } allowedRegistries := *s.allowedRegistries - // FIXME: The registryFn won't return the registry location until the registry service - // is created. This should be switched to use registry DNS instead of lazy-loading. - if localRegistry, ok := s.registryFn(); ok { + if localRegistry, ok := s.registryHostRetriever.InternalRegistryHostname(); ok { allowedRegistries = append([]configapi.RegistryLocation{{DomainName: localRegistry}}, allowedRegistries...) } validate := func(path *field.Path, name string, insecure bool) field.ErrorList { diff --git a/pkg/image/registry/imagestreammapping/rest.go b/pkg/image/registry/imagestreammapping/rest.go index 46bd7bf54f34..00487c819fbd 100644 --- a/pkg/image/registry/imagestreammapping/rest.go +++ b/pkg/image/registry/imagestreammapping/rest.go @@ -34,11 +34,11 @@ type REST struct { var _ rest.Creater = &REST{} // NewREST returns a new REST. -func NewREST(imageRegistry image.Registry, imageStreamRegistry imagestream.Registry, defaultRegistry imageapi.DefaultRegistry) *REST { +func NewREST(imageRegistry image.Registry, imageStreamRegistry imagestream.Registry, registry imageapi.RegistryHostnameRetriever) *REST { return &REST{ imageRegistry: imageRegistry, imageStreamRegistry: imageStreamRegistry, - strategy: NewStrategy(defaultRegistry), + strategy: NewStrategy(registry), } } diff --git a/pkg/image/registry/imagestreammapping/rest_test.go b/pkg/image/registry/imagestreammapping/rest_test.go index 86d2a92eac03..8302bee297dc 100644 --- a/pkg/image/registry/imagestreammapping/rest_test.go +++ b/pkg/image/registry/imagestreammapping/rest_test.go @@ -38,7 +38,7 @@ import ( const testDefaultRegistryURL = "defaultregistry:5000" -var testDefaultRegistry = imageapi.DefaultRegistryFunc(func() (string, bool) { return testDefaultRegistryURL, true }) +var testDefaultRegistry = func() (string, bool) { return testDefaultRegistryURL, true } type fakeSubjectAccessReviewRegistry struct { } @@ -57,7 +57,8 @@ func setup(t *testing.T) (etcd.KV, *etcdtesting.EtcdTestServer, *REST) { if err != nil { t.Fatal(err) } - imageStreamStorage, imageStreamStatus, internalStorage, err := imagestreametcd.NewREST(restoptions.NewSimpleGetter(etcdStorage), testDefaultRegistry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}) + registry := imageapi.DefaultRegistryHostnameRetriever(testDefaultRegistry, "", "") + imageStreamStorage, imageStreamStatus, internalStorage, err := imagestreametcd.NewREST(restoptions.NewSimpleGetter(etcdStorage), registry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}) if err != nil { t.Fatal(err) } @@ -65,7 +66,7 @@ func setup(t *testing.T) (etcd.KV, *etcdtesting.EtcdTestServer, *REST) { imageRegistry := image.NewRegistry(imageStorage) imageStreamRegistry := imagestream.NewRegistry(imageStreamStorage, imageStreamStatus, internalStorage) - storage := NewREST(imageRegistry, imageStreamRegistry, testDefaultRegistry) + storage := NewREST(imageRegistry, imageStreamRegistry, registry) return etcdClient, server, storage } @@ -588,8 +589,9 @@ func TestTrackingTags(t *testing.T) { // TestCreateRetryUnrecoverable ensures that an attempt to create a mapping // using failing registry update calls will return an error. func TestCreateRetryUnrecoverable(t *testing.T) { + registry := imageapi.DefaultRegistryHostnameRetriever(nil, "", testDefaultRegistryURL) rest := &REST{ - strategy: NewStrategy(testDefaultRegistry), + strategy: NewStrategy(registry), imageRegistry: &fakeImageRegistry{ createImage: func(ctx apirequest.Context, image *imageapi.Image) error { return nil @@ -621,9 +623,10 @@ func TestCreateRetryUnrecoverable(t *testing.T) { // that result in resource conflicts that do NOT include tag diffs causes the // create to be retried successfully. func TestCreateRetryConflictNoTagDiff(t *testing.T) { + registry := imageapi.DefaultRegistryHostnameRetriever(nil, "", testDefaultRegistryURL) firstUpdate := true rest := &REST{ - strategy: NewStrategy(testDefaultRegistry), + strategy: NewStrategy(registry), imageRegistry: &fakeImageRegistry{ createImage: func(ctx apirequest.Context, image *imageapi.Image) error { return nil @@ -666,7 +669,7 @@ func TestCreateRetryConflictTagDiff(t *testing.T) { firstGet := true firstUpdate := true rest := &REST{ - strategy: NewStrategy(testDefaultRegistry), + strategy: NewStrategy(imageapi.DefaultRegistryHostnameRetriever(nil, "", testDefaultRegistryURL)), imageRegistry: &fakeImageRegistry{ createImage: func(ctx apirequest.Context, image *imageapi.Image) error { return nil diff --git a/pkg/image/registry/imagestreammapping/strategy.go b/pkg/image/registry/imagestreammapping/strategy.go index d3611f341965..43d4d11e3200 100644 --- a/pkg/image/registry/imagestreammapping/strategy.go +++ b/pkg/image/registry/imagestreammapping/strategy.go @@ -16,16 +16,16 @@ type Strategy struct { runtime.ObjectTyper names.NameGenerator - defaultRegistry imageapi.DefaultRegistry + registryHostRetriever imageapi.RegistryHostnameRetriever } // Strategy is the default logic that applies when creating ImageStreamMapping // objects via the REST API. -func NewStrategy(defaultRegistry imageapi.DefaultRegistry) Strategy { +func NewStrategy(registryHost imageapi.RegistryHostnameRetriever) Strategy { return Strategy{ - kapi.Scheme, - names.SimpleNameGenerator, - defaultRegistry, + ObjectTyper: kapi.Scheme, + NameGenerator: names.SimpleNameGenerator, + registryHostRetriever: registryHost, } } @@ -38,7 +38,7 @@ func (s Strategy) NamespaceScoped() bool { func (s Strategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) { ism := obj.(*imageapi.ImageStreamMapping) if len(ism.Image.DockerImageReference) == 0 { - internalRegistry, ok := s.defaultRegistry.DefaultRegistry() + internalRegistry, ok := s.registryHostRetriever.InternalRegistryHostname() if ok { ism.Image.DockerImageReference = imageapi.DockerImageReference{ Registry: internalRegistry, diff --git a/pkg/image/registry/imagestreamtag/rest_test.go b/pkg/image/registry/imagestreamtag/rest_test.go index 5045112044dc..66fe22733abc 100644 --- a/pkg/image/registry/imagestreamtag/rest_test.go +++ b/pkg/image/registry/imagestreamtag/rest_test.go @@ -32,7 +32,7 @@ import ( _ "github.com/openshift/origin/pkg/api/install" ) -var testDefaultRegistry = imageapi.DefaultRegistryFunc(func() (string, bool) { return "defaultregistry:5000", true }) +var testDefaultRegistry = func() (string, bool) { return "defaultregistry:5000", true } type fakeSubjectAccessReviewRegistry struct { } @@ -72,7 +72,8 @@ func setup(t *testing.T) (etcd.KV, *etcdtesting.EtcdTestServer, *REST) { if err != nil { t.Fatal(err) } - imageStreamStorage, imageStreamStatus, internalStorage, err := imagestreametcd.NewREST(restoptions.NewSimpleGetter(etcdStorage), testDefaultRegistry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}) + registry := imageapi.DefaultRegistryHostnameRetriever(testDefaultRegistry, "", "") + imageStreamStorage, imageStreamStatus, internalStorage, err := imagestreametcd.NewREST(restoptions.NewSimpleGetter(etcdStorage), registry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{}) if err != nil { t.Fatal(err) }