Revert "Always register the metadata endpoint (#392)" (#403)

This reverts commit f04414e.
flyteorg · Apr 12, 2022 · 33dd675 · 33dd675
1 parent 3d092c7
commit 33dd675
Show file tree

Hide file tree

Showing 9 changed files with 26 additions and 54 deletions.
diff --git a/auth/authzserver/metadata_provider.go b/auth/authzserver/metadata_provider.go
@@ -7,8 +7,6 @@ import (
 	"net/url"
 	"strings"
 
-	"github.com/flyteorg/flyteadmin/pkg/config"
-
 	"github.com/flyteorg/flyteadmin/auth"
 
 	authConfig "github.com/flyteorg/flyteadmin/auth/config"
@@ -17,11 +15,10 @@ import (
 )
 
 type OAuth2MetadataProvider struct {
-	cfg       *authConfig.Config
-	serverCfg *config.ServerConfig
+	cfg *authConfig.Config
 }
 
-// AuthFuncOverride overrides auth func to enforce anonymous access on the implemented APIs
+// Override auth func to enforce anonymous access on the implemented APIs
 // Ref: https://github.com/grpc-ecosystem/go-grpc-middleware/blob/master/auth/auth.go#L31
 func (s OAuth2MetadataProvider) AuthFuncOverride(ctx context.Context, fullMethodName string) (context.Context, error) {
 	return ctx, nil
@@ -93,13 +90,11 @@ func (s OAuth2MetadataProvider) GetPublicClientConfig(context.Context, *service.
 		RedirectUri:              s.cfg.AppAuth.ThirdParty.FlyteClientConfig.RedirectURI,
 		Scopes:                   s.cfg.AppAuth.ThirdParty.FlyteClientConfig.Scopes,
 		AuthorizationMetadataKey: s.cfg.GrpcAuthorizationHeader,
-		ServiceHttpEndpoint:      s.serverCfg.ServiceHTTPEndpoint.String(),
 	}, nil
 }
 
-func NewService(serverCfg *config.ServerConfig, config *authConfig.Config) OAuth2MetadataProvider {
+func NewService(config *authConfig.Config) OAuth2MetadataProvider {
 	return OAuth2MetadataProvider{
-		cfg:       config,
-		serverCfg: serverCfg,
+		cfg: config,
 	}
 }
diff --git a/auth/authzserver/metadata_provider_test.go b/auth/authzserver/metadata_provider_test.go
@@ -8,17 +8,16 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/flyteorg/flyteadmin/pkg/config"
-
-	stdConfig "github.com/flyteorg/flytestdlib/config"
+	config2 "github.com/flyteorg/flytestdlib/config"
 
+	"github.com/flyteorg/flyteadmin/auth/config"
 	authConfig "github.com/flyteorg/flyteadmin/auth/config"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/service"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestOAuth2MetadataProvider_FlyteClient(t *testing.T) {
-	provider := NewService(&config.ServerConfig{}, &authConfig.Config{
+	provider := NewService(&authConfig.Config{
 		AppAuth: authConfig.OAuth2Options{
 			ThirdParty: authConfig.ThirdPartyConfigOptions{
 				FlyteClientConfig: authConfig.FlyteClientConfig{
@@ -40,8 +39,8 @@ func TestOAuth2MetadataProvider_FlyteClient(t *testing.T) {
 
 func TestOAuth2MetadataProvider_OAuth2Metadata(t *testing.T) {
 	t.Run("Self AuthServer", func(t *testing.T) {
-		provider := NewService(&config.ServerConfig{}, &authConfig.Config{
-			AuthorizedURIs: []stdConfig.URL{{URL: *authConfig.MustParseURL("https://issuer/")}},
+		provider := NewService(&authConfig.Config{
+			AuthorizedURIs: []config2.URL{{URL: *config.MustParseURL("https://issuer/")}},
 		})
 
 		ctx := context.Background()
@@ -75,12 +74,12 @@ func TestOAuth2MetadataProvider_OAuth2Metadata(t *testing.T) {
 	http.DefaultClient = s.Client()
 
 	t.Run("External AuthServer", func(t *testing.T) {
-		provider := NewService(&config.ServerConfig{}, &authConfig.Config{
-			AuthorizedURIs: []stdConfig.URL{{URL: *authConfig.MustParseURL("https://issuer/")}},
+		provider := NewService(&authConfig.Config{
+			AuthorizedURIs: []config2.URL{{URL: *config.MustParseURL("https://issuer/")}},
 			AppAuth: authConfig.OAuth2Options{
 				AuthServerType: authConfig.AuthorizationServerTypeExternal,
 				ExternalAuthServer: authConfig.ExternalAuthorizationServer{
-					BaseURL: stdConfig.URL{URL: *authConfig.MustParseURL(s.URL)},
+					BaseURL: config2.URL{URL: *config.MustParseURL(s.URL)},
 				},
 			},
 		})
@@ -92,14 +91,14 @@ func TestOAuth2MetadataProvider_OAuth2Metadata(t *testing.T) {
 	})
 
 	t.Run("External AuthServer fallback url", func(t *testing.T) {
-		provider := NewService(&config.ServerConfig{}, &authConfig.Config{
-			AuthorizedURIs: []stdConfig.URL{{URL: *authConfig.MustParseURL("https://issuer/")}},
+		provider := NewService(&authConfig.Config{
+			AuthorizedURIs: []config2.URL{{URL: *config.MustParseURL("https://issuer/")}},
 			AppAuth: authConfig.OAuth2Options{
 				AuthServerType: authConfig.AuthorizationServerTypeExternal,
 			},
 			UserAuth: authConfig.UserAuthConfig{
 				OpenID: authConfig.OpenIDOptions{
-					BaseURL: stdConfig.URL{URL: *authConfig.MustParseURL(s.URL)},
+					BaseURL: config2.URL{URL: *config.MustParseURL(s.URL)},
 				},
 			},
 		})

diff --git a/auth/handlers.go b/auth/handlers.go
@@ -54,9 +54,8 @@ func RegisterHandlers(ctx context.Context, handler interfaces.HandlerRegisterer,
 	handler.HandleFunc("/logout", GetLogoutEndpointHandler(ctx, authCtx))
 }
 
-// RefreshTokensIfExists looks for access token and refresh token, if both are present and the access token is expired,
-// then attempt to refresh. Otherwise do nothing and proceed to the next handler. If successfully refreshed, proceed to
-// the landing page.
+// Look for access token and refresh token, if both are present and the access token is expired, then attempt to
+// refresh. Otherwise do nothing and proceed to the next handler. If successfully refreshed, proceed to the landing page.
 func RefreshTokensIfExists(ctx context.Context, authCtx interfaces.AuthenticationContext, authHandler http.HandlerFunc) http.HandlerFunc {
 
 	return func(writer http.ResponseWriter, request *http.Request) {

diff --git a/go.mod b/go.mod
@@ -13,7 +13,7 @@ require (
 	github.com/cloudevents/sdk-go/v2 v2.8.0
 	github.com/coreos/go-oidc v2.2.1+incompatible
 	github.com/evanphx/json-patch v4.9.0+incompatible
-	github.com/flyteorg/flyteidl v0.24.18
+	github.com/flyteorg/flyteidl v0.24.17
 	github.com/flyteorg/flyteplugins v0.10.16
 	github.com/flyteorg/flytepropeller v0.16.36
 	github.com/flyteorg/flytestdlib v0.4.23

diff --git a/go.sum b/go.sum
@@ -369,8 +369,8 @@ github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8S
 github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/flyteorg/flyteidl v0.23.0/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
 github.com/flyteorg/flyteidl v0.24.0/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
-github.com/flyteorg/flyteidl v0.24.18 h1:5HMlc11OcSzRHR5VRRAac2gH6yO2XFcQ3fWlcl9tj+k=
-github.com/flyteorg/flyteidl v0.24.18/go.mod h1:vHSugApgS3hRITIafzQDU8DZD/W8wFRfFcgaFU35Dww=
+github.com/flyteorg/flyteidl v0.24.17 h1:Xx70bJbuQGyvS8uAyU4AN74rot6KnzJ9r/L9gcCdEsU=
+github.com/flyteorg/flyteidl v0.24.17/go.mod h1:vHSugApgS3hRITIafzQDU8DZD/W8wFRfFcgaFU35Dww=
 github.com/flyteorg/flyteplugins v0.10.16 h1:rwNI2MACPbcST2O6CEUsNW6bccz7ZLni0GiY3orevfw=
 github.com/flyteorg/flyteplugins v0.10.16/go.mod h1:YBWV8QnFakDJfLyua8pYddiWqszAqseBKIJPNMERlos=
 github.com/flyteorg/flytepropeller v0.16.36 h1:5uE8JsutrPVyLVrRJ8BgvhZUOmTBFkEkn5xmIOo21nU=

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -26,12 +26,6 @@ type ServerConfig struct {
 	DeprecatedThirdPartyConfig authConfig.ThirdPartyConfigOptions `json:"thirdPartyConfig" pflag:",Deprecated please use auth.appAuth.thirdPartyConfig instead."`
 
 	DataProxy DataProxyConfig `json:"dataProxy" pflag:",Defines data proxy configuration."`
-
-	// ServiceHTTPEndpoint allows specifying the http endpoint this admin instance is accessible on. This is useful
-	// for when there is no ingress setup or when the service is serving http and grpc over two different ports.
-	// Setting it here allows gRPC clients to retrieve the http endpoint to be able to present it to end-users (e.g.
-	// open up the browser to the workflow overview page)
-	ServiceHTTPEndpoint config.URL `json:"serviceHttpEndpoint" pflag:",Defines the http endpoint the service is accessible at."`
 }
 
 type DataProxyConfig struct {

diff --git a/pkg/config/serverconfig_flags.go b/pkg/config/serverconfig_flags.go
diff --git a/pkg/config/serverconfig_flags_test.go b/pkg/config/serverconfig_flags_test.go
diff --git a/pkg/server/service.go b/pkg/server/service.go
@@ -72,7 +72,7 @@ func blanketAuthorization(ctx context.Context, req interface{}, _ *grpc.UnarySer
 
 // Creates a new gRPC Server with all the configuration
 func newGRPCServer(ctx context.Context, pluginRegistry *plugins.Registry, cfg *config.ServerConfig,
-	storageCfg *storage.Config, authCfg *authConfig.Config, authCtx interfaces.AuthenticationContext,
+	storageCfg *storage.Config, authCtx interfaces.AuthenticationContext,
 	scope promutils.Scope, opts ...grpc.ServerOption) (*grpc.Server, error) {
 	// Not yet implemented for streaming
 	var chainedUnaryInterceptors grpc.UnaryServerInterceptor
@@ -107,8 +107,8 @@ func newGRPCServer(ctx context.Context, pluginRegistry *plugins.Registry, cfg *c
 
 	configuration := runtime2.NewConfigurationProvider()
 	service.RegisterAdminServiceServer(grpcServer, adminservice.NewAdminServer(ctx, pluginRegistry, configuration, cfg.KubeConfig, cfg.Master, dataStorageClient, scope.NewSubScope("admin")))
-	service.RegisterAuthMetadataServiceServer(grpcServer, authzserver.NewService(cfg, authCfg))
 	if cfg.Security.UseAuth {
+		service.RegisterAuthMetadataServiceServer(grpcServer, authCtx.AuthMetadataService())
 		service.RegisterIdentityServiceServer(grpcServer, authCtx.IdentityService())
 	}
 
@@ -243,7 +243,7 @@ func serveGatewayInsecure(ctx context.Context, pluginRegistry *plugins.Registry,
 			}
 		}
 
-		oauth2MetadataProvider := authzserver.NewService(cfg, authCfg)
+		oauth2MetadataProvider := authzserver.NewService(authCfg)
 		oidcUserInfoProvider := auth.NewUserInfoProvider()
 
 		authCtx, err = auth.NewAuthenticationContext(ctx, sm, oauth2Provider, oauth2ResourceServer, oauth2MetadataProvider, oidcUserInfoProvider, authCfg)
@@ -253,7 +253,7 @@ func serveGatewayInsecure(ctx context.Context, pluginRegistry *plugins.Registry,
 		}
 	}
 
-	grpcServer, err := newGRPCServer(ctx, pluginRegistry, cfg, storageConfig, authCfg, authCtx, scope)
+	grpcServer, err := newGRPCServer(ctx, pluginRegistry, cfg, storageConfig, authCtx, scope)
 	if err != nil {
 		return fmt.Errorf("failed to create a newGRPCServer. Error: %w", err)
 	}
@@ -346,7 +346,7 @@ func serveGatewaySecure(ctx context.Context, pluginRegistry *plugins.Registry, c
 			}
 		}
 
-		oauth2MetadataProvider := authzserver.NewService(cfg, authCfg)
+		oauth2MetadataProvider := authzserver.NewService(authCfg)
 		oidcUserInfoProvider := auth.NewUserInfoProvider()
 
 		authCtx, err = auth.NewAuthenticationContext(ctx, sm, oauth2Provider, oauth2ResourceServer, oauth2MetadataProvider, oidcUserInfoProvider, authCfg)
@@ -356,7 +356,7 @@ func serveGatewaySecure(ctx context.Context, pluginRegistry *plugins.Registry, c
 		}
 	}
 
-	grpcServer, err := newGRPCServer(ctx, pluginRegistry, cfg, storageCfg, authCfg, authCtx, scope, grpc.Creds(credentials.NewServerTLSFromCert(cert)))
+	grpcServer, err := newGRPCServer(ctx, pluginRegistry, cfg, storageCfg, authCtx, scope, grpc.Creds(credentials.NewServerTLSFromCert(cert)))
 	if err != nil {
 		return fmt.Errorf("failed to create a newGRPCServer. Error: %w", err)
 	}