Skip to content

Commit

Permalink
fix: pass through burst and qps for auth.kubeclient (argoproj#12575)
Browse files Browse the repository at this point in the history 
Signed-off-by: shuangkun <[email protected]>
  • Loading branch information
@shuangkun @isubasinghe
shuangkun authored and isubasinghe committed Mar 12, 2024
1 parent 6b0d1ea commit c9e308f
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 13 deletions.
17 changes: 13 additions & 4 deletions server/auth/gatekeeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type Gatekeeper interface {
StreamServerInterceptor() grpc.StreamServerInterceptor
}

type ClientForAuthorization func(authorization string) (*rest.Config, *servertypes.Clients, error)
type ClientForAuthorization func(authorization string, config *rest.Config) (*rest.Config, *servertypes.Clients, error)

type gatekeeper struct {
Modes Modes
Expand Down Expand Up @@ -194,7 +194,7 @@ func (s gatekeeper) getClients(ctx context.Context, req interface{}) (*servertyp
}
switch mode {
case Client:
restConfig, clients, err := s.clientForAuthorization(authorization)
restConfig, clients, err := s.clientForAuthorization(authorization, s.restConfig)
if err != nil {
return nil, nil, status.Error(codes.Unauthenticated, err.Error())
}
Expand Down Expand Up @@ -286,7 +286,7 @@ func (s *gatekeeper) getClientsForServiceAccount(ctx context.Context, claims *ty
if err != nil {
return nil, err
}
_, clients, err := s.clientForAuthorization(authorization)
_, clients, err := s.clientForAuthorization(authorization, s.restConfig)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -337,11 +337,12 @@ func addClaimsLogFields(claims *types.Claims, fields log.Fields) log.Fields {
return fields
}

func DefaultClientForAuthorization(authorization string) (*rest.Config, *servertypes.Clients, error) {
func DefaultClientForAuthorization(authorization string, config *rest.Config) (*rest.Config, *servertypes.Clients, error) {
restConfig, err := kubeconfig.GetRestConfig(authorization)
if err != nil {
return nil, nil, fmt.Errorf("failed to create REST config: %w", err)
}
restConfig = mergeServerRestConfig(config, restConfig)
dynamicClient, err := dynamic.NewForConfig(restConfig)
if err != nil {
return nil, nil, fmt.Errorf("failure to create dynamic client: %w", err)
Expand Down Expand Up @@ -370,3 +371,11 @@ func DefaultClientForAuthorization(authorization string) (*rest.Config, *servert
Kubernetes: kubeClient,
}, nil
}

func mergeServerRestConfig(argoServerConfig *rest.Config, newConfig *rest.Config) *rest.Config {
newConfig.Burst = argoServerConfig.Burst
newConfig.QPS = argoServerConfig.QPS
newConfig.UserAgent = argoServerConfig.UserAgent
// TO DO: Merge other common configurations，such as RateLimiter.
return newConfig
}
18 changes: 9 additions & 9 deletions server/auth/gatekeeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func TestServer_GetWFClient(t *testing.T) {
)
resourceCache := cache.NewResourceCache(kubeClient, corev1.NamespaceAll)
resourceCache.Run(context.TODO().Done())
var clientForAuthorization ClientForAuthorization = func(authorization string) (*rest.Config, *servertypes.Clients, error) {
var clientForAuthorization ClientForAuthorization = func(authorization string, config *rest.Config) (*rest.Config, *servertypes.Clients, error) {
return &rest.Config{}, &servertypes.Clients{Workflow: &fakewfclientset.Clientset{}, Kubernetes: &kubefake.Clientset{}}, nil
}
clients := &servertypes.Clients{Workflow: wfClient, Kubernetes: kubeClient}
Expand Down Expand Up @@ -153,7 +153,7 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Claims: jwt.Claims{Subject: "my-sub"}}, nil)
ssoIf.On("IsRBACEnabled").Return(false)
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.Context(x("Bearer v2:whatever"))
if assert.NoError(t, err) {
Expand All @@ -172,7 +172,7 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.Context(x("Bearer v2:whatever"))
if assert.NoError(t, err) {
Expand All @@ -193,7 +193,7 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user1-ns"))
if assert.NoError(t, err) {
Expand All @@ -214,7 +214,7 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user1-ns"))
if assert.NoError(t, err) {
Expand All @@ -235,7 +235,7 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user2-ns"))
if assert.NoError(t, err) {
Expand All @@ -257,7 +257,7 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user3-ns"))
if assert.NoError(t, err) {
Expand All @@ -278,7 +278,7 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.Context(x("Bearer v2:whatever"))
if assert.NoError(t, err) {
Expand All @@ -291,7 +291,7 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
_, err := g.Context(x("Bearer v2:whatever"))
assert.EqualError(t, err, "rpc error: code = PermissionDenied desc = not allowed")
Expand Down

0 comments on commit c9e308f

Please sign in to comment.