Skip to content

Commit

Permalink
Allow flyteadmin to start even if OIDC is unavailable (improve flytea…
Browse files Browse the repository at this point in the history
…dmin startup resiliency)

Signed-off-by: ddl-rliu <[email protected]>
  • Loading branch information
ddl-rliu committed Aug 28, 2024
1 parent f075b34 commit 080a4cf
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 33 deletions.
85 changes: 53 additions & 32 deletions flyteadmin/auth/auth_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ type Context struct {
httpClient *http.Client
}

func (c Context) OAuth2Provider() interfaces.OAuth2Provider {
func (c *Context) OAuth2Provider() interfaces.OAuth2Provider {

Check warning on line 53 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L53

Added line #L53 was not covered by tests
return c.oauth2Provider
}

func (c Context) OAuth2ClientConfig(requestURL *url.URL) *oauth2.Config {
func (c *Context) OAuth2ClientConfig(requestURL *url.URL) *oauth2.Config {
if requestURL == nil || strings.HasPrefix(c.oauth2Client.RedirectURL, requestURL.ResolveReference(rootRelativeURL).String()) {
return c.oauth2Client
}
Expand All @@ -68,64 +68,86 @@ func (c Context) OAuth2ClientConfig(requestURL *url.URL) *oauth2.Config {
}
}

func (c Context) OidcProvider() *oidc.Provider {
func (c *Context) OidcProvider() *oidc.Provider {
if c.oidcProvider == nil {
logger.Warn(context.Background(), "OIDC provider was not initialized. Attempting to re-initialize OIDC provider...")
err := c.InitOIDC()
if err != nil {
logger.Warn(context.Background(), err)

Check warning on line 76 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L71-L76

Added lines #L71 - L76 were not covered by tests
}
}
return c.oidcProvider
}

func (c Context) CookieManager() interfaces.CookieHandler {
func (c *Context) InitOIDC() error {
var err error

Check warning on line 83 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L82-L83

Added lines #L82 - L83 were not covered by tests
// Construct an oidc Provider, which needs its own http Client.
oidcCtx := oidc.ClientContext(context.Background(), c.httpClient)
baseURL := c.options.UserAuth.OpenID.BaseURL.String()

Check warning on line 86 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L85-L86

Added lines #L85 - L86 were not covered by tests

c.oidcProvider, err = oidc.NewProvider(oidcCtx, baseURL)
if err != nil {
return errors.Wrapf(ErrauthCtx, err, "Error creating oidc provider w/ issuer [%v]", baseURL)

Check warning on line 90 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L88-L90

Added lines #L88 - L90 were not covered by tests
}

c.oauth2Client.Endpoint = c.oidcProvider.Endpoint()
return nil

Check warning on line 94 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L93-L94

Added lines #L93 - L94 were not covered by tests
}

func (c *Context) CookieManager() interfaces.CookieHandler {

Check warning on line 97 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L97

Added line #L97 was not covered by tests
return c.cookieManager
}

func (c Context) Options() *config.Config {
func (c *Context) Options() *config.Config {

Check warning on line 101 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L101

Added line #L101 was not covered by tests
return c.options
}

func (c Context) GetUserInfoURL() *url.URL {
func (c *Context) GetUserInfoURL() *url.URL {

Check warning on line 105 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L105

Added line #L105 was not covered by tests
return c.userInfoURL
}

func (c Context) GetHTTPClient() *http.Client {
func (c *Context) GetHTTPClient() *http.Client {

Check warning on line 109 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L109

Added line #L109 was not covered by tests
return c.httpClient
}

func (c Context) GetOAuth2MetadataURL() *url.URL {
func (c *Context) GetOAuth2MetadataURL() *url.URL {

Check warning on line 113 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L113

Added line #L113 was not covered by tests
return c.oauth2MetadataURL
}

func (c Context) GetOIdCMetadataURL() *url.URL {
func (c *Context) GetOIdCMetadataURL() *url.URL {

Check warning on line 117 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L117

Added line #L117 was not covered by tests
return c.oidcMetadataURL
}

func (c Context) AuthMetadataService() service.AuthMetadataServiceServer {
func (c *Context) AuthMetadataService() service.AuthMetadataServiceServer {

Check warning on line 121 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L121

Added line #L121 was not covered by tests
return c.authServiceImpl
}

func (c Context) IdentityService() service.IdentityServiceServer {
func (c *Context) IdentityService() service.IdentityServiceServer {

Check warning on line 125 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L125

Added line #L125 was not covered by tests
return c.identityServiceIml
}

func (c Context) OAuth2ResourceServer() interfaces.OAuth2ResourceServer {
func (c *Context) OAuth2ResourceServer() interfaces.OAuth2ResourceServer {

Check warning on line 129 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L129

Added line #L129 was not covered by tests
return c.oauth2ResourceServer
}
func NewAuthenticationContext(ctx context.Context, sm core.SecretManager, oauth2Provider interfaces.OAuth2Provider,
oauth2ResourceServer interfaces.OAuth2ResourceServer, authMetadataService service.AuthMetadataServiceServer,
identityService service.IdentityServiceServer, options *config.Config) (Context, error) {
identityService service.IdentityServiceServer, options *config.Config) (*Context, error) {

Check warning on line 134 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L134

Added line #L134 was not covered by tests

// Construct the cookie manager object.
hashKeyBase64, err := sm.Get(ctx, options.UserAuth.CookieHashKeySecretName)
if err != nil {
return Context{}, errors.Wrapf(ErrConfigFileRead, err, "Could not read hash key file")
return &Context{}, errors.Wrapf(ErrConfigFileRead, err, "Could not read hash key file")

Check warning on line 139 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L139

Added line #L139 was not covered by tests
}

blockKeyBase64, err := sm.Get(ctx, options.UserAuth.CookieBlockKeySecretName)
if err != nil {
return Context{}, errors.Wrapf(ErrConfigFileRead, err, "Could not read hash key file")
return &Context{}, errors.Wrapf(ErrConfigFileRead, err, "Could not read hash key file")

Check warning on line 144 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L144

Added line #L144 was not covered by tests
}

cookieManager, err := NewCookieManager(ctx, hashKeyBase64, blockKeyBase64, options.UserAuth.CookieSetting)
if err != nil {
logger.Errorf(ctx, "Error creating cookie manager %s", err)
return Context{}, errors.Wrapf(ErrauthCtx, err, "Error creating cookie manager")
return &Context{}, errors.Wrapf(ErrauthCtx, err, "Error creating cookie manager")

Check warning on line 150 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L150

Added line #L150 was not covered by tests
}

// Construct an http client for interacting with the IDP if necessary.
Expand All @@ -138,34 +160,26 @@ func NewAuthenticationContext(ctx context.Context, sm core.SecretManager, oauth2
httpClient.Transport = &http.Transport{Proxy: http.ProxyURL(&options.UserAuth.HTTPProxyURL.URL)}
}

// Construct an oidc Provider, which needs its own http Client.
oidcCtx := oidc.ClientContext(ctx, httpClient)
baseURL := options.UserAuth.OpenID.BaseURL.String()
provider, err := oidc.NewProvider(oidcCtx, baseURL)
if err != nil {
return Context{}, errors.Wrapf(ErrauthCtx, err, "Error creating oidc provider w/ issuer [%v]", baseURL)
}

// Construct the golang OAuth2 library's own internal configuration object from this package's config
oauth2Config, err := GetOAuth2ClientConfig(ctx, options.UserAuth.OpenID, provider.Endpoint(), sm)
oauth2Config, err := GetOAuth2ClientConfig(ctx, options.UserAuth.OpenID, sm)

Check warning on line 164 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L164

Added line #L164 was not covered by tests
if err != nil {
return Context{}, errors.Wrapf(ErrauthCtx, err, "Error creating OAuth2 library configuration")
return &Context{}, errors.Wrapf(ErrauthCtx, err, "Error creating OAuth2 library configuration")

Check warning on line 166 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L166

Added line #L166 was not covered by tests
}

logger.Infof(ctx, "Base IDP URL is %s", options.UserAuth.OpenID.BaseURL)

oauth2MetadataURL, err := url.Parse(OAuth2MetadataEndpoint)
if err != nil {
logger.Errorf(ctx, "Error parsing oauth2 metadata URL %s", err)
return Context{}, errors.Wrapf(ErrauthCtx, err, "Error parsing metadata URL")
return &Context{}, errors.Wrapf(ErrauthCtx, err, "Error parsing metadata URL")

Check warning on line 174 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L174

Added line #L174 was not covered by tests
}

logger.Infof(ctx, "Metadata endpoint is %s", oauth2MetadataURL)

oidcMetadataURL, err := url.Parse(OIdCMetadataEndpoint)
if err != nil {
logger.Errorf(ctx, "Error parsing oidc metadata URL %s", err)
return Context{}, errors.Wrapf(ErrauthCtx, err, "Error parsing metadata URL")
return &Context{}, errors.Wrapf(ErrauthCtx, err, "Error parsing metadata URL")

Check warning on line 182 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L182

Added line #L182 was not covered by tests
}

logger.Infof(ctx, "Metadata endpoint is %s", oidcMetadataURL)
Expand All @@ -175,7 +189,6 @@ func NewAuthenticationContext(ctx context.Context, sm core.SecretManager, oauth2
oidcMetadataURL: oidcMetadataURL,
oauth2MetadataURL: oauth2MetadataURL,
oauth2Client: &oauth2Config,
oidcProvider: provider,
httpClient: httpClient,
cookieManager: cookieManager,
oauth2Provider: oauth2Provider,
Expand All @@ -185,11 +198,20 @@ func NewAuthenticationContext(ctx context.Context, sm core.SecretManager, oauth2
authCtx.authServiceImpl = authMetadataService
authCtx.identityServiceIml = identityService

return authCtx, nil
err = authCtx.InitOIDC()
if err != nil {
if authCtx.options.UserAuth.OpenID.OnlyStartIfOIDCIsAvailable {
return &Context{}, errors.Wrapf(ErrauthCtx, err, "Error initializing OIDC")
} else {
logger.Warn(ctx, err)

Check warning on line 206 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L201-L206

Added lines #L201 - L206 were not covered by tests
}
}

return &authCtx, nil

Check warning on line 210 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L210

Added line #L210 was not covered by tests
}

// This creates a oauth2 library config object, with values from the Flyte Admin config
func GetOAuth2ClientConfig(ctx context.Context, options config.OpenIDOptions, providerEndpoints oauth2.Endpoint, sm core.SecretManager) (cfg oauth2.Config, err error) {
func GetOAuth2ClientConfig(ctx context.Context, options config.OpenIDOptions, sm core.SecretManager) (cfg oauth2.Config, err error) {

Check warning on line 214 in flyteadmin/auth/auth_context.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/auth/auth_context.go#L214

Added line #L214 was not covered by tests
var secret string
if len(options.DeprecatedClientSecretFile) > 0 {
secretBytes, err := ioutil.ReadFile(options.DeprecatedClientSecretFile)
Expand All @@ -212,6 +234,5 @@ func GetOAuth2ClientConfig(ctx context.Context, options config.OpenIDOptions, pr
ClientID: options.ClientID,
ClientSecret: secret,
Scopes: options.Scopes,
Endpoint: providerEndpoints,
}, nil
}
2 changes: 1 addition & 1 deletion flyteadmin/auth/authzserver/initialize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
func TestRegisterHandlers(t *testing.T) {
t.Run("No OAuth2 Provider, no registration required", func(t *testing.T) {
registerer := &mocks.HandlerRegisterer{}
RegisterHandlers(registerer, auth.Context{})
RegisterHandlers(registerer, &auth.Context{})
})

t.Run("Register 4 endpoints", func(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions flyteadmin/auth/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ var (
"openid",
"profile",
},
OnlyStartIfOIDCIsAvailable: true,
},
CookieSetting: CookieSettings{
Domain: "",
Expand Down Expand Up @@ -271,6 +272,9 @@ type OpenIDOptions struct {
// be supported by any OIdC server. Refer to https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims for
// a complete list. Other providers might support additional scopes that you can define in a config.
Scopes []string `json:"scopes"`

// Blocks flyte from starting up until the OIDC provider is healthy and available
OnlyStartIfOIDCIsAvailable bool `json:"onlyStartIfOIDCIsAvailable"`
}

func GetConfig() *Config {
Expand Down

0 comments on commit 080a4cf

Please sign in to comment.