Skip to content

Commit b897684

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #15624 from mfojtik/image-clients
Automatic merge from submit-queue (batch tested with PRs 15964, 15624, 15924) Replace legacy client in Docker Registry with external clientset This use the recent changes to codegen to provide missing clients for some of the image.openshift.io group resouces and then replace clients in registry with external clientsets. In order to pickup external clients I had to rework the registry code in some places to deal with external types. I failed to do that in some places and fall-backed to conversions, which I hate but it will require more plumbing in the origin code to get this right. Also I failed to use the external clientset for ImageSecrets as it is implemented as not-standard client returning `[]Secret` instead of `ImageSecretList` (so we can't generate client for that). Part of fixing: #15272
2 parents 51f1de6 + 9364e12 commit b897684

37 files changed

+1248
-572
lines changed

‎pkg/cmd/dockerregistry/dockerregistry.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
"github.com/openshift/origin/pkg/dockerregistry/server"
4848
"github.com/openshift/origin/pkg/dockerregistry/server/api"
4949
"github.com/openshift/origin/pkg/dockerregistry/server/audit"
50+
"github.com/openshift/origin/pkg/dockerregistry/server/client"
5051
registryconfig "github.com/openshift/origin/pkg/dockerregistry/server/configuration"
5152
"github.com/openshift/origin/pkg/dockerregistry/server/maxconnections"
5253
"github.com/openshift/origin/pkg/dockerregistry/server/prune"
@@ -94,7 +95,7 @@ func ExecutePruner(configFile io.Reader, dryRun bool) {
9495
}
9596
log.WithFields(versionFields()).Info(startPrune)
9697

97-
registryClient := server.NewRegistryClient(clientcmd.NewConfig().BindToFile())
98+
registryClient := client.NewRegistryClient(clientcmd.NewConfig().BindToFile())
9899

99100
storageDriver, err := factory.Create(config.Storage.Type(), config.Storage.Parameters())
100101
if err != nil {
@@ -153,7 +154,7 @@ func Execute(configFile io.Reader) {
153154
log.Fatalf("error configuring logger: %v", err)
154155
}
155156

156-
registryClient := server.NewRegistryClient(clientcmd.NewConfig().BindToFile())
157+
registryClient := client.NewRegistryClient(clientcmd.NewConfig().BindToFile())
157158
ctx = server.WithRegistryClient(ctx, registryClient)
158159

159160
readLimiter := newLimiter(extraConfig.Requests.Read)
@@ -171,8 +172,8 @@ func Execute(configFile io.Reader) {
171172
dockerConfig.Auth[server.OpenShiftAuth] = make(configuration.Parameters)
172173
}
173174
dockerConfig.Auth[server.OpenShiftAuth][server.AccessControllerOptionParams] = server.AccessControllerParams{
174-
Logger: context.GetLogger(ctx),
175-
SafeClientConfig: registryClient.SafeClientConfig(),
175+
Logger: context.GetLogger(ctx),
176+
RegistryClient: registryClient,
176177
}
177178
}
178179

‎pkg/dockerregistry/server/auth.go

Lines changed: 42 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,13 @@ import (
1212

1313
kerrors "k8s.io/apimachinery/pkg/api/errors"
1414
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15-
restclient "k8s.io/client-go/rest"
16-
kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
15+
authorizationapi "k8s.io/kubernetes/pkg/apis/authorization/v1"
1716

18-
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
19-
"github.com/openshift/origin/pkg/client"
20-
"github.com/openshift/origin/pkg/cmd/util/clientcmd"
2117
imageapi "github.com/openshift/origin/pkg/image/apis/image"
2218
"github.com/openshift/origin/pkg/util/httprequest"
2319

2420
"github.com/openshift/origin/pkg/dockerregistry/server/audit"
21+
"github.com/openshift/origin/pkg/dockerregistry/server/client"
2522
)
2623

2724
type deferredErrors map[string]error
@@ -51,40 +48,6 @@ const (
5148
AccessControllerOptionParams = "_params"
5249
)
5350

54-
// RegistryClient encapsulates getting access to the OpenShift API.
55-
type RegistryClient interface {
56-
// Clients return the authenticated clients to use with the server.
57-
Clients() (client.Interface, kcoreclient.CoreInterface, error)
58-
// SafeClientConfig returns a client config without authentication info.
59-
SafeClientConfig() restclient.Config
60-
}
61-
62-
// registryClient implements RegistryClient
63-
type registryClient struct {
64-
config *clientcmd.Config
65-
}
66-
67-
var _ RegistryClient = &registryClient{}
68-
69-
// NewRegistryClient creates a registry client.
70-
func NewRegistryClient(config *clientcmd.Config) RegistryClient {
71-
return &registryClient{config: config}
72-
}
73-
74-
// Client returns the authenticated client to use with the server.
75-
func (r *registryClient) Clients() (client.Interface, kcoreclient.CoreInterface, error) {
76-
oc, kc, err := r.config.Clients()
77-
if err != nil {
78-
return nil, nil, err
79-
}
80-
return oc, kc.Core(), err
81-
}
82-
83-
// SafeClientConfig returns a client config without authentication info.
84-
func (r *registryClient) SafeClientConfig() restclient.Config {
85-
return clientcmd.AnonymousClientConfig(r.config.OpenShiftConfig())
86-
}
87-
8851
func init() {
8952
registryauth.Register(OpenShiftAuth, registryauth.InitFunc(newAccessController))
9053
}
@@ -102,10 +65,10 @@ func WithUserInfoLogger(ctx context.Context, username, userid string) context.Co
10265
}
10366

10467
type AccessController struct {
105-
realm string
106-
tokenRealm *url.URL
107-
config restclient.Config
108-
auditLog bool
68+
realm string
69+
tokenRealm *url.URL
70+
registryClient client.RegistryClient
71+
auditLog bool
10972
}
11073

11174
var _ registryauth.AccessController = &AccessController{}
@@ -170,8 +133,8 @@ func TokenRealm(options map[string]interface{}) (*url.URL, error) {
170133

171134
// AccessControllerParams is the parameters for newAccessController
172135
type AccessControllerParams struct {
173-
Logger context.Logger
174-
SafeClientConfig restclient.Config
136+
Logger context.Logger
137+
RegistryClient client.RegistryClient
175138
}
176139

177140
func newAccessController(options map[string]interface{}) (registryauth.AccessController, error) {
@@ -193,9 +156,9 @@ func newAccessController(options map[string]interface{}) (registryauth.AccessCon
193156
}
194157

195158
ac := &AccessController{
196-
realm: realm,
197-
tokenRealm: tokenRealm,
198-
config: params.SafeClientConfig,
159+
realm: realm,
160+
tokenRealm: tokenRealm,
161+
registryClient: params.RegistryClient,
199162
}
200163

201164
if audit, ok := options["audit"]; ok {
@@ -305,9 +268,7 @@ func (ac *AccessController) Authorized(ctx context.Context, accessRecords ...reg
305268
return nil, ac.wrapErr(ctx, err)
306269
}
307270

308-
copied := ac.config
309-
copied.BearerToken = bearerToken
310-
osClient, err := client.New(&copied)
271+
osClient, err := ac.registryClient.ClientFromToken(bearerToken)
311272
if err != nil {
312273
return nil, ac.wrapErr(ctx, err)
313274
}
@@ -476,8 +437,8 @@ func getOpenShiftAPIToken(ctx context.Context, req *http.Request) (string, error
476437
return token, nil
477438
}
478439

479-
func verifyOpenShiftUser(ctx context.Context, client client.UsersInterface) (string, string, error) {
480-
userInfo, err := client.Users().Get("~", metav1.GetOptions{})
440+
func verifyOpenShiftUser(ctx context.Context, c client.UsersInterfacer) (string, string, error) {
441+
userInfo, err := c.Users().Get("~", metav1.GetOptions{})
481442
if err != nil {
482443
context.GetLogger(ctx).Errorf("Get user failed with error: %s", err)
483444
if kerrors.IsUnauthorized(err) || kerrors.IsForbidden(err) {
@@ -489,17 +450,19 @@ func verifyOpenShiftUser(ctx context.Context, client client.UsersInterface) (str
489450
return userInfo.GetName(), string(userInfo.GetUID()), nil
490451
}
491452

492-
func verifyWithSAR(ctx context.Context, resource, namespace, name, verb string, client client.LocalSubjectAccessReviewsNamespacer) error {
493-
sar := authorizationapi.LocalSubjectAccessReview{
494-
Action: authorizationapi.Action{
495-
Verb: verb,
496-
Group: imageapi.GroupName,
497-
Resource: resource,
498-
ResourceName: name,
453+
func verifyWithSAR(ctx context.Context, resource, namespace, name, verb string, c client.SelfSubjectAccessReviewsNamespacer) error {
454+
sar := authorizationapi.SelfSubjectAccessReview{
455+
Spec: authorizationapi.SelfSubjectAccessReviewSpec{
456+
ResourceAttributes: &authorizationapi.ResourceAttributes{
457+
Namespace: namespace,
458+
Verb: verb,
459+
Group: imageapi.GroupName,
460+
Resource: resource,
461+
Name: name,
462+
},
499463
},
500464
}
501-
response, err := client.LocalSubjectAccessReviews(namespace).Create(&sar)
502-
465+
response, err := c.SelfSubjectAccessReviews().Create(&sar)
503466
if err != nil {
504467
context.GetLogger(ctx).Errorf("OpenShift client error: %s", err)
505468
if kerrors.IsUnauthorized(err) || kerrors.IsForbidden(err) {
@@ -508,40 +471,42 @@ func verifyWithSAR(ctx context.Context, resource, namespace, name, verb string,
508471
return err
509472
}
510473

511-
if !response.Allowed {
512-
context.GetLogger(ctx).Errorf("OpenShift access denied: %s", response.Reason)
474+
if !response.Status.Allowed {
475+
context.GetLogger(ctx).Errorf("OpenShift access denied: %s", response.Status.Reason)
513476
return ErrOpenShiftAccessDenied
514477
}
515478

516479
return nil
517480
}
518481

519-
func verifyImageStreamAccess(ctx context.Context, namespace, imageRepo, verb string, client client.LocalSubjectAccessReviewsNamespacer) error {
520-
return verifyWithSAR(ctx, "imagestreams/layers", namespace, imageRepo, verb, client)
482+
func verifyImageStreamAccess(ctx context.Context, namespace, imageRepo, verb string, c client.SelfSubjectAccessReviewsNamespacer) error {
483+
return verifyWithSAR(ctx, "imagestreams/layers", namespace, imageRepo, verb, c)
521484
}
522485

523-
func verifyImageSignatureAccess(ctx context.Context, namespace, imageRepo string, client client.LocalSubjectAccessReviewsNamespacer) error {
524-
return verifyWithSAR(ctx, "imagesignatures", namespace, imageRepo, "create", client)
486+
func verifyImageSignatureAccess(ctx context.Context, namespace, imageRepo string, c client.SelfSubjectAccessReviewsNamespacer) error {
487+
return verifyWithSAR(ctx, "imagesignatures", namespace, imageRepo, "create", c)
525488
}
526489

527-
func verifyPruneAccess(ctx context.Context, client client.SubjectAccessReviews) error {
528-
sar := authorizationapi.SubjectAccessReview{
529-
Action: authorizationapi.Action{
530-
Verb: "delete",
531-
Group: imageapi.GroupName,
532-
Resource: "images",
490+
func verifyPruneAccess(ctx context.Context, c client.SelfSubjectAccessReviewsNamespacer) error {
491+
sar := authorizationapi.SelfSubjectAccessReview{
492+
Spec: authorizationapi.SelfSubjectAccessReviewSpec{
493+
ResourceAttributes: &authorizationapi.ResourceAttributes{
494+
Verb: "delete",
495+
Group: imageapi.GroupName,
496+
Resource: "images",
497+
},
533498
},
534499
}
535-
response, err := client.SubjectAccessReviews().Create(&sar)
500+
response, err := c.SelfSubjectAccessReviews().Create(&sar)
536501
if err != nil {
537502
context.GetLogger(ctx).Errorf("OpenShift client error: %s", err)
538503
if kerrors.IsUnauthorized(err) || kerrors.IsForbidden(err) {
539504
return ErrOpenShiftAccessDenied
540505
}
541506
return err
542507
}
543-
if !response.Allowed {
544-
context.GetLogger(ctx).Errorf("OpenShift access denied: %s", response.Reason)
508+
if !response.Status.Allowed {
509+
context.GetLogger(ctx).Errorf("OpenShift access denied: %s", response.Status.Reason)
545510
return ErrOpenShiftAccessDenied
546511
}
547512
return nil

0 commit comments

Comments
 (0)