Skip to content

Commit 0ffb71e

Browse files
committed
registry: report publicDockerImageRepository to image stream if configured
1 parent 59247fe commit 0ffb71e

File tree

23 files changed

+193
-81
lines changed

23 files changed

+193
-81
lines changed

pkg/cmd/server/admission/init.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ type PluginInitializer struct {
2828
Informers kinternalinformers.SharedInformerFactory
2929
ClusterResourceQuotaInformer quotainformer.ClusterResourceQuotaInformer
3030
ClusterQuotaMapper clusterquotamapping.ClusterQuotaMapper
31-
DefaultRegistryFn imageapi.DefaultRegistryFunc
31+
RegistryHostnameRetriever imageapi.RegistryHostnameRetriever
3232
SecurityInformers securityinformer.SharedInformerFactory
3333
UserInformers userinformer.SharedInformerFactory
3434
}
@@ -70,7 +70,7 @@ func (i *PluginInitializer) Initialize(plugin admission.Interface) {
7070
wantsSecurityInformer.SetSecurityInformers(i.SecurityInformers)
7171
}
7272
if wantsDefaultRegistryFunc, ok := plugin.(WantsDefaultRegistryFunc); ok {
73-
wantsDefaultRegistryFunc.SetDefaultRegistryFunc(i.DefaultRegistryFn)
73+
wantsDefaultRegistryFunc.SetDefaultRegistryFunc(i.RegistryHostnameRetriever.InternalRegistryHostnameFn())
7474
}
7575
if wantsUserInformer, ok := plugin.(WantsUserInformer); ok {
7676
wantsUserInformer.SetUserInformer(i.UserInformers)

pkg/cmd/server/admission/types.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"github.com/openshift/origin/pkg/client"
1111
configapi "github.com/openshift/origin/pkg/cmd/server/api"
12-
imageapi "github.com/openshift/origin/pkg/image/apis/image"
1312
"github.com/openshift/origin/pkg/project/cache"
1413
"github.com/openshift/origin/pkg/quota/controller/clusterquotamapping"
1514
quotainformer "github.com/openshift/origin/pkg/quota/generated/informers/internalversion/quota/internalversion"
@@ -79,7 +78,7 @@ type WantsSecurityInformer interface {
7978
// WantsDefaultRegistryFunc should be implemented by admission plugins that need to know the default registry
8079
// address.
8180
type WantsDefaultRegistryFunc interface {
82-
SetDefaultRegistryFunc(imageapi.DefaultRegistryFunc)
81+
SetDefaultRegistryFunc(func() (string, bool))
8382
admission.Validator
8483
}
8584

pkg/cmd/server/api/types.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,15 @@ type ImagePolicyConfig struct {
519519
// this policy - typically only administrators or system integrations will have those
520520
// permissions.
521521
AllowedRegistriesForImport *AllowedRegistries
522+
// InternalRegistryHostname sets the hostname for the default internal Docker
523+
// Registry. This can be overriden by using OPENSHIFT_DEFAULT_REGISTRY
524+
// environment variable.
525+
InternalRegistryHostname string
526+
// ExternalRegistryHostname sets the hostname for the default external Docker
527+
// Registry. The external hostname should be set only when the registry is
528+
// exposed externally. The value is used in 'publicDockerImageRepository'
529+
// field in ImageStreams.
530+
ExternalRegistryHostname string
522531
}
523532

524533
// AllowedRegistries represents a list of registries allowed for the image import.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,8 @@ var map_ImagePolicyConfig = map[string]string{
339339
"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.",
340340
"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.",
341341
"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.",
342+
"internalRegistryHostname": "InternalRegistryHostname sets the hostname for the default internal Docker Registry. This can be overriden by using OPENSHIFT_DEFAULT_REGISTRY environment variable.",
343+
"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.",
342344
}
343345

344346
func (ImagePolicyConfig) SwaggerDoc() map[string]string {

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,15 @@ type ImagePolicyConfig struct {
377377
// this policy - typically only administrators or system integrations will have those
378378
// permissions.
379379
AllowedRegistriesForImport *AllowedRegistries `json:"allowedRegistriesForImport,omitempty"`
380+
// InternalRegistryHostname sets the hostname for the default internal Docker
381+
// Registry. This can be overriden by using OPENSHIFT_DEFAULT_REGISTRY
382+
// environment variable.
383+
InternalRegistryHostname string `json:"internalRegistryHostname,omitempty"`
384+
// ExternalRegistryHostname sets the hostname for the default external Docker
385+
// Registry. The external hostname should be set only when the registry is
386+
// exposed externally. The value is used in 'publicDockerImageRepository'
387+
// field in ImageStreams.
388+
ExternalRegistryHostname string `json:"externalRegistryHostname,omitempty"`
380389
}
381390

382391
// AllowedRegistries represents a list of registries allowed for the image import.

pkg/cmd/server/origin/master.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (c *MasterConfig) newOpenshiftAPIConfig(kubeAPIServerConfig apiserver.Confi
7373
RuleResolver: c.RuleResolver,
7474
SubjectLocator: c.SubjectLocator,
7575
LimitVerifier: c.LimitVerifier,
76-
RegistryNameFn: c.RegistryNameFn,
76+
RegistryHostnameRetriever: c.RegistryHostnameRetriever,
7777
AllowedRegistriesForImport: c.Options.ImagePolicyConfig.AllowedRegistriesForImport,
7878
MaxImagesBulkImportedPerRepository: c.Options.ImagePolicyConfig.MaxImagesBulkImportedPerRepository,
7979
RouteAllocator: c.RouteAllocator(),

pkg/cmd/server/origin/master_config.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
kapierrors "k8s.io/apimachinery/pkg/api/errors"
1414
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1515
"k8s.io/apimachinery/pkg/labels"
16-
"k8s.io/apimachinery/pkg/runtime"
1716
"k8s.io/apimachinery/pkg/runtime/schema"
1817
"k8s.io/apimachinery/pkg/util/sets"
1918
"k8s.io/apiserver/pkg/admission"
@@ -67,7 +66,6 @@ import (
6766
admissionregistry "github.com/openshift/origin/pkg/cmd/server/origin/admission"
6867
originrest "github.com/openshift/origin/pkg/cmd/server/origin/rest"
6968
"github.com/openshift/origin/pkg/cmd/util/pluginconfig"
70-
"github.com/openshift/origin/pkg/cmd/util/variable"
7169
imageadmission "github.com/openshift/origin/pkg/image/admission"
7270
imagepolicy "github.com/openshift/origin/pkg/image/admission/imagepolicy/api"
7371
imageapi "github.com/openshift/origin/pkg/image/apis/image"
@@ -121,9 +119,9 @@ type MasterConfig struct {
121119
// of both the origin config AND the kube config, so this spot makes more sense.
122120
KubeAdmissionControl admission.Interface
123121

124-
// RegistryNameFn retrieves the name of the integrated registry, or false if no such registry
122+
// RegistryHostnameRetriever retrieves the name of the integrated registry, or false if no such registry
125123
// is available.
126-
RegistryNameFn imageapi.DefaultRegistryFunc
124+
RegistryHostnameRetriever imageapi.RegistryHostnameRetriever
127125

128126
KubeletClientConfig *kubeletclient.KubeletClientConfig
129127

@@ -267,7 +265,7 @@ func BuildMasterConfig(options configapi.MasterConfig, informers InformerAccess)
267265
Informers: informers.GetInternalKubeInformers(),
268266
ClusterResourceQuotaInformer: informers.GetQuotaInformers().Quota().InternalVersion().ClusterResourceQuotas(),
269267
ClusterQuotaMapper: clusterQuotaMappingController.GetClusterQuotaMapper(),
270-
DefaultRegistryFn: imageapi.DefaultRegistryFunc(defaultRegistryFunc),
268+
RegistryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
271269
SecurityInformers: informers.GetSecurityInformers(),
272270
UserInformers: informers.GetUserInformers(),
273271
}
@@ -318,7 +316,7 @@ func BuildMasterConfig(options configapi.MasterConfig, informers InformerAccess)
318316
AdmissionControl: originAdmission,
319317
KubeAdmissionControl: kubeAdmission,
320318

321-
RegistryNameFn: imageapi.DefaultRegistryFunc(defaultRegistryFunc),
319+
RegistryHostnameRetriever: imageapi.DefaultRegistryHostnameRetriever(defaultRegistryFunc, options.ImagePolicyConfig.ExternalRegistryHostname, options.ImagePolicyConfig.InternalRegistryHostname),
322320

323321
KubeletClientConfig: kubeletClientConfig,
324322

pkg/cmd/server/origin/openshift_apiserver.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ type OpenshiftAPIConfig struct {
8282
RuleResolver rbacregistryvalidation.AuthorizationRuleResolver
8383
SubjectLocator authorizer.SubjectLocator
8484
LimitVerifier imageadmission.LimitVerifier
85-
// RegistryNameFn retrieves the name of the integrated registry, or false if no such registry
86-
// is available.
87-
RegistryNameFn imageapi.DefaultRegistryFunc
85+
// RegistryHostnameRetriever retrieves the internal and external hostname of
86+
// the integrated registry, or false if no such registry is available.
87+
RegistryHostnameRetriever imageapi.RegistryHostnameRetriever
8888
AllowedRegistriesForImport *configapi.AllowedRegistries
8989
MaxImagesBulkImportedPerRepository int
9090

@@ -143,8 +143,8 @@ func (c *OpenshiftAPIConfig) Validate() error {
143143
if c.LimitVerifier == nil {
144144
ret = append(ret, fmt.Errorf("LimitVerifier is required"))
145145
}
146-
if c.RegistryNameFn == nil {
147-
ret = append(ret, fmt.Errorf("RegistryNameFn is required"))
146+
if c.RegistryHostnameRetriever == nil {
147+
ret = append(ret, fmt.Errorf("RegistryHostnameRetriever is required"))
148148
}
149149
if c.RouteAllocator == nil {
150150
ret = append(ret, fmt.Errorf("RouteAllocator is required"))

pkg/cmd/server/origin/storage.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,12 +204,12 @@ func (c OpenshiftAPIConfig) GetRestStorage() (map[schema.GroupVersion]map[string
204204
imageRegistry := image.NewRegistry(imageStorage)
205205
imageSignatureStorage := imagesignature.NewREST(c.DeprecatedOpenshiftClient.Images())
206206
imageStreamSecretsStorage := imagesecret.NewREST(c.KubeClientInternal.Core())
207-
imageStreamStorage, imageStreamStatusStorage, internalImageStreamStorage, err := imagestreametcd.NewREST(c.GenericConfig.RESTOptionsGetter, c.RegistryNameFn, subjectAccessReviewRegistry, c.LimitVerifier)
207+
imageStreamStorage, imageStreamStatusStorage, internalImageStreamStorage, err := imagestreametcd.NewREST(c.GenericConfig.RESTOptionsGetter, c.RegistryHostnameRetriever, subjectAccessReviewRegistry, c.LimitVerifier)
208208
if err != nil {
209209
return nil, fmt.Errorf("error building REST storage: %v", err)
210210
}
211211
imageStreamRegistry := imagestream.NewRegistry(imageStreamStorage, imageStreamStatusStorage, internalImageStreamStorage)
212-
imageStreamMappingStorage := imagestreammapping.NewREST(imageRegistry, imageStreamRegistry, c.RegistryNameFn)
212+
imageStreamMappingStorage := imagestreammapping.NewREST(imageRegistry, imageStreamRegistry, c.RegistryHostnameRetriever)
213213
imageStreamTagStorage := imagestreamtag.NewREST(imageRegistry, imageStreamRegistry)
214214
importerCache, err := imageimporter.NewImageStreamLayerCache(imageimporter.DefaultImageStreamLayerCacheSize)
215215
if err != nil {
@@ -231,7 +231,7 @@ func (c OpenshiftAPIConfig) GetRestStorage() (map[schema.GroupVersion]map[string
231231
insecureImportTransport,
232232
importerDockerClientFn,
233233
c.AllowedRegistriesForImport,
234-
c.RegistryNameFn,
234+
c.RegistryHostnameRetriever,
235235
c.DeprecatedOpenshiftClient.SubjectAccessReviews())
236236
imageStreamImageStorage := imagestreamimage.NewREST(imageRegistry, imageStreamRegistry)
237237

pkg/image/admission/imagepolicy/imagepolicy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func newImagePolicyPlugin(parsed *api.ImagePolicyConfig) (*imagePolicyPlugin, er
108108
}, nil
109109
}
110110

111-
func (a *imagePolicyPlugin) SetDefaultRegistryFunc(fn imageapi.DefaultRegistryFunc) {
111+
func (a *imagePolicyPlugin) SetDefaultRegistryFunc(fn func() (string, bool)) {
112112
a.integratedRegistryMatcher.RegistryMatcher = rules.RegistryNameMatcher(fn)
113113
}
114114

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ type RegistryMatcher interface {
2323
Matches(name string) bool
2424
}
2525

26-
type RegistryNameMatcher imageapi.DefaultRegistryFunc
26+
type RegistryNameMatcher func() (string, bool)
2727

2828
func (m RegistryNameMatcher) Matches(name string) bool {
29-
current, ok := imageapi.DefaultRegistryFunc(m)()
29+
current, ok := m()
3030
if !ok {
3131
return false
3232
}

pkg/image/apis/image/helper.go

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,46 @@ var errNoRegistryURLPathAllowed = fmt.Errorf("no path after <host>[:<port>] is a
4545
var errNoRegistryURLQueryAllowed = fmt.Errorf("no query arguments are allowed after <host>[:<port>]")
4646
var errRegistryURLHostEmpty = fmt.Errorf("no host name specified")
4747

48-
// DefaultRegistry returns the default Docker registry (host or host:port), or false if it is not available.
49-
type DefaultRegistry interface {
50-
DefaultRegistry() (string, bool)
48+
// RegistryHostnameRetriever represents an interface for retrieving the hostname
49+
// of internal and external registry.
50+
type RegistryHostnameRetriever interface {
51+
InternalRegistryHostnameFn() func() (string, bool)
52+
ExternalRegistryHostnameFn() func() (string, bool)
5153
}
5254

53-
// DefaultRegistryFunc implements DefaultRegistry for a simple function.
54-
type DefaultRegistryFunc func() (string, bool)
55+
// DefaultRegistryHostnameRetriever is a default implementation of
56+
// RegistryHostnameRetriever.
57+
// The first argument is a function that lazy-loads the value of
58+
// OPENSHIFT_DEFAULT_REGISTRY environment variable which should be deprecated in
59+
// future.
60+
func DefaultRegistryHostnameRetriever(deprecatedDefaultFn func() (string, bool), external, internal string) RegistryHostnameRetriever {
61+
return &defaultRegistryHostnameRetriever{deprecatedDefaultFn: deprecatedDefaultFn, external: external, internal: internal}
62+
}
63+
64+
type defaultRegistryHostnameRetriever struct {
65+
deprecatedDefaultFn func() (string, bool)
66+
internal, external string
67+
}
68+
69+
// InternalRegistryHostnameFn returns a function that can be used to lazy-load
70+
// the internal Docker Registry hostname. If the master configuration propertly
71+
// InternalRegistryHostname is set, it will prefer that over the lazy-loaded
72+
// environment variable 'OPENSHIFT_DEFAULT_REGISTRY'.
73+
func (r *defaultRegistryHostnameRetriever) InternalRegistryHostnameFn() func() (string, bool) {
74+
if len(r.internal) > 0 {
75+
return func() (string, bool) { return r.internal, true }
76+
}
77+
if r.deprecatedDefaultFn == nil {
78+
return func() (string, bool) { return "", false }
79+
}
80+
return r.deprecatedDefaultFn
81+
}
5582

56-
// DefaultRegistry implements the DefaultRegistry interface for a function.
57-
func (fn DefaultRegistryFunc) DefaultRegistry() (string, bool) {
58-
return fn()
83+
// ExternalRegistryHostnameFn returns a function that can be used to retrieve an
84+
// external/public hostname of Docker Registry. External location can be
85+
// configured in master config using 'ExternalRegistryHostname' property.
86+
func (r *defaultRegistryHostnameRetriever) ExternalRegistryHostnameFn() func() (string, bool) {
87+
return func() (string, bool) { return r.external, len(r.external) > 0 }
5988
}
6089

6190
// ParseImageStreamImageName splits a string into its name component and ID component, and returns an error

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type REST struct {
2525
var _ rest.StandardStorage = &REST{}
2626

2727
// NewREST returns a new REST.
28-
func NewREST(optsGetter restoptions.Getter, defaultRegistry imageapi.DefaultRegistry, subjectAccessReviewRegistry subjectaccessreview.Registry, limitVerifier imageadmission.LimitVerifier) (*REST, *StatusREST, *InternalREST, error) {
28+
func NewREST(optsGetter restoptions.Getter, registryHostname imageapi.RegistryHostnameRetriever, subjectAccessReviewRegistry subjectaccessreview.Registry, limitVerifier imageadmission.LimitVerifier) (*REST, *StatusREST, *InternalREST, error) {
2929
store := registry.Store{
3030
Copier: kapi.Scheme,
3131
NewFunc: func() runtime.Object { return &imageapi.ImageStream{} },
@@ -39,7 +39,7 @@ func NewREST(optsGetter restoptions.Getter, defaultRegistry imageapi.DefaultRegi
3939
subjectAccessReviewRegistry: subjectAccessReviewRegistry,
4040
}
4141
// strategy must be able to load image streams across namespaces during tag verification
42-
strategy := imagestream.NewStrategy(defaultRegistry, subjectAccessReviewRegistry, limitVerifier, rest)
42+
strategy := imagestream.NewStrategy(registryHostname, subjectAccessReviewRegistry, limitVerifier, rest)
4343

4444
store.CreateStrategy = strategy
4545
store.UpdateStrategy = strategy

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ const (
2727
)
2828

2929
var (
30-
testDefaultRegistry = imageapi.DefaultRegistryFunc(func() (string, bool) { return "test", true })
31-
noDefaultRegistry = imageapi.DefaultRegistryFunc(func() (string, bool) { return "", false })
30+
testDefaultRegistry = func() (string, bool) { return "test", true }
31+
noDefaultRegistry = func() (string, bool) { return "", false }
3232
)
3333

3434
type fakeSubjectAccessReviewRegistry struct {
@@ -48,7 +48,8 @@ func (f *fakeSubjectAccessReviewRegistry) CreateSubjectAccessReview(ctx apireque
4848

4949
func newStorage(t *testing.T) (*REST, *StatusREST, *InternalREST, *etcdtesting.EtcdTestServer) {
5050
etcdStorage, server := registrytest.NewEtcdStorage(t, latest.Version.Group)
51-
imageStorage, statusStorage, internalStorage, err := NewREST(restoptions.NewSimpleGetter(etcdStorage), noDefaultRegistry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{})
51+
registry := imageapi.DefaultRegistryHostnameRetriever(noDefaultRegistry, "", "")
52+
imageStorage, statusStorage, internalStorage, err := NewREST(restoptions.NewSimpleGetter(etcdStorage), registry, &fakeSubjectAccessReviewRegistry{}, &testutil.FakeImageStreamLimitVerifier{})
5253
if err != nil {
5354
t.Fatal(err)
5455
}

0 commit comments

Comments
 (0)