Feat: Enable proxy-authorization in admin client#437
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #437 +/- ##
==========================================
+ Coverage 75.92% 78.39% +2.46%
==========================================
Files 18 18
Lines 1458 1296 -162
==========================================
- Hits 1107 1016 -91
+ Misses 294 217 -77
- Partials 57 63 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
…ture Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
…ials future Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
…future Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
fg91
force-pushed
the
fg91/feat/proxy-authorization
branch
from
4a9735b to
132dbb9
Compare
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
…efore Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
| return proxyTokenSource, nil | ||
| } | ||
|
|
||
| func MaterializeProxyAuthCredentials(ctx context.Context, cfg *Config, proxyCredentialsFuture *PerRPCCredentialsFuture) error { |
There was a problem hiding this comment.
Adapted from MaterializeAuthCredentials.
| return err | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Adapted from NewAuthInterceptor.
| token, err := c.Exchange(context.Background(), req.URL.Query().Get("code"), opts...) | ||
| ctx := context.WithValue(context.Background(), oauth2.HTTPClient, client) | ||
| token, err := c.Exchange(ctx, req.URL.Query().Get("code"), opts...) | ||
| if err != nil { |
There was a problem hiding this comment.
If using a proxy, we need to use the http client created in auth_interceptor.go to retrieve the token. So far, this client wasn't used.
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
|
|
||
| err := invoker(ctx, method, req, reply, cc, opts...) | ||
| if err != nil { | ||
| newErr := MaterializeProxyAuthCredentials(ctx, cfg, proxyCredentialsFuture) |
There was a problem hiding this comment.
The external command is called a single time per call of flytectl. After the call of the external command, the credentials future is initialized and can be reused for further grpc calls.
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
wild-endeavor
left a comment
wild-endeavor
left a comment
There was a problem hiding this comment.
looks okay by me. tested with existing aws clusters as well - client credentials, pkce, device flow all continue to work
|
I moved this PR to the monorepo: flyteorg/flyte#4189. |
3 participants
TL;DR
Part of an effort to integrate Flyte with GCP Identity Aware Proxy, see flyteorg/flyte#3965.
Allows the flyteidl admin client to sent a
"proxy-authorization"header with every request to flyteadmin. Tokens for this header are created using an external command which is configured via the newproxyCommandconfig entry.Same logic as was introduced in
flytekitin flyteorg/flytekit#1787.I replicated the existing logic in
MaterializeCredentialsandNewAuthInterceptorfor the second token. The external command is called a single time perflytectlcall.Type
Are all requirements met?
Complete description
See flyteorg/flyte#3965
Tracking Issue
Closes flyteorg/flyte#3965
Follow-up issue
NA