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 <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
…ture Signed-off-by: Fabio Grätz <[email protected]>
…ials future Signed-off-by: Fabio Grätz <[email protected]>
…future Signed-off-by: Fabio Grätz <[email protected]>
fg91
force-pushed
the
fg91/feat/proxy-authorization
branch
from
4a9735b
to
132dbb9
Compare
Signed-off-by: Fabio Grätz <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
…efore Signed-off-by: Fabio Grätz <[email protected]>
| return proxyTokenSource, nil | ||
| } | ||
|
|
||
| func MaterializeProxyAuthCredentials(ctx context.Context, cfg *Config, proxyCredentialsFuture *PerRPCCredentialsFuture) error { |
There was a problem hiding this comment.
Adapted from MaterializeAuthCredentials.
| @@ -102,3 +156,18 @@ func NewAuthInterceptor(cfg *Config, tokenCache cache.TokenCache, credentialsFut | |||
| return err | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Adapted from NewAuthInterceptor.
| @@ -43,7 +43,8 @@ func getAuthServerCallbackHandler(c *oauth.Config, codeVerifier string, tokenCha | |||
| var opts []oauth2.AuthCodeOption | |||
| opts = append(opts, oauth2.SetAuthURLParam("code_verifier", codeVerifier)) | |||
|
|
|||
| 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 <[email protected]>
|
|
||
| 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 <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
|
I moved this PR to the monorepo: flyteorg/flyte#4189. |
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