Merge pull request #2148 from njhale/fix/auth

fix(packageserver): add tolerant delegating auth config

Author: openshift-merge-robot

pkg/package-server/server/server.go

@@ -9,6 +9,7 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	"k8s.io/apimachinery/pkg/util/wait"
 	genericserver "k8s.io/apiserver/pkg/server"
 	genericoptions "k8s.io/apiserver/pkg/server/options"
 	"k8s.io/client-go/informers"
@@ -52,7 +53,6 @@ func NewCommandStartPackageServer(ctx context.Context, defaults *PackageServerOp
 }
 
 type PackageServerOptions struct {
-	// RecommendedOptions *genericoptions.RecommendedOptions
 	SecureServing  *genericoptions.SecureServingOptionsWithLoopback
 	Authentication *genericoptions.DelegatingAuthenticationOptions
 	Authorization  *genericoptions.DelegatingAuthorizationOptions
@@ -77,7 +77,6 @@ type PackageServerOptions struct {
 
 func NewPackageServerOptions(out, errOut io.Writer) *PackageServerOptions {
 	o := &PackageServerOptions{
-
 		SecureServing:  genericoptions.NewSecureServingOptions().WithLoopback(),
 		Authentication: genericoptions.NewDelegatingAuthenticationOptions(),
 		Authorization:  genericoptions.NewDelegatingAuthorizationOptions(),
@@ -96,29 +95,82 @@ func NewPackageServerOptions(out, errOut io.Writer) *PackageServerOptions {
 }
 
 // Config returns config for the PackageServerOptions.
-func (o *PackageServerOptions) Config() (*apiserver.Config, error) {
+func (o *PackageServerOptions) Config(ctx context.Context) (*apiserver.Config, error) {
 	if err := o.SecureServing.MaybeDefaultWithSelfSignedCerts("localhost", nil, []net.IP{net.ParseIP("127.0.0.1")}); err != nil {
 		return nil, fmt.Errorf("error creating self-signed certificates: %v", err)
 	}
 
-	serverConfig := genericserver.NewConfig(genericpackageserver.Codecs)
-	if err := o.SecureServing.ApplyTo(&serverConfig.SecureServing, &serverConfig.LoopbackClientConfig); err != nil {
+	config := genericserver.NewConfig(genericpackageserver.Codecs)
+	if err := o.SecureServing.ApplyTo(&config.SecureServing, &config.LoopbackClientConfig); err != nil {
 		return nil, err
 	}
 
-	if !o.DisableAuthForTesting {
-		if err := o.Authentication.ApplyTo(&serverConfig.Authentication, serverConfig.SecureServing, nil); err != nil {
-			return nil, err
+	serverConfig := &apiserver.Config{
+		GenericConfig:  config,
+		ProviderConfig: genericpackageserver.ProviderConfig{},
+	}
+
+	if o.DisableAuthForTesting {
+		return serverConfig, nil
+	}
+
+	// See https://github.com/openshift/library-go/blob/7a65fdb398e28782ee1650959a5e0419121e97ae/pkg/config/serving/server.go#L61-L63 for details on
+	// the following auth/z config
+	pollCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+	defer cancel()
+
+	authenticationOptions := genericoptions.NewDelegatingAuthenticationOptions()
+	authenticationOptions.RemoteKubeConfigFile = o.Kubeconfig
+
+	// The platform generally uses 30s for /metrics scraping, avoid API request for every other /metrics request to the component
+	authenticationOptions.CacheTTL = 35 * time.Second
+
+	// In some cases the API server can return connection refused when getting the "extension-apiserver-authentication" config map
+	var lastApplyErr error
+	err := wait.PollImmediateUntil(1*time.Second, func() (done bool, err error) {
+		lastApplyErr := authenticationOptions.ApplyTo(&config.Authentication, config.SecureServing, config.OpenAPIConfig)
+		if lastApplyErr != nil {
+			log.WithError(lastApplyErr).Warn("Error initializing delegating authentication (will retry)")
+			return false, nil
 		}
-		if err := o.Authorization.ApplyTo(&serverConfig.Authorization); err != nil {
-			return nil, err
+		return true, nil
+	}, pollCtx.Done())
+
+	if err != nil {
+		return nil, lastApplyErr
+	}
+
+	if err := o.Authentication.ApplyTo(&config.Authentication, config.SecureServing, nil); err != nil {
+		return nil, err
+	}
+
+	authorizationOptions := genericoptions.NewDelegatingAuthorizationOptions().
+		WithAlwaysAllowPaths("/healthz", "/readyz", "/livez"). // This allows the kubelet to always get health and readiness without causing an access check
+		WithAlwaysAllowGroups("system:masters")                // in a kube cluster, system:masters can take any action, so there is no need to ask for an authz check
+	authenticationOptions.RemoteKubeConfigFile = o.Kubeconfig
+
+	// The platform generally uses 30s for /metrics scraping, avoid API request for every other /metrics request to the component
+	authorizationOptions.AllowCacheTTL = 35 * time.Second
+
+	// In some cases the API server can return connection refused when getting the "extension-apiserver-authentication" config map
+	err = wait.PollImmediateUntil(1*time.Second, func() (done bool, err error) {
+		lastApplyErr = authorizationOptions.ApplyTo(&config.Authorization)
+		if lastApplyErr != nil {
+			log.WithError(lastApplyErr).Warn("Error initializing delegating authorization (will retry)")
+			return false, nil
 		}
+		return true, nil
+	}, pollCtx.Done())
+
+	if err != nil {
+		return nil, lastApplyErr
 	}
 
-	return &apiserver.Config{
-		GenericConfig:  serverConfig,
-		ProviderConfig: genericpackageserver.ProviderConfig{},
-	}, nil
+	if err := o.Authorization.ApplyTo(&config.Authorization); err != nil {
+		return nil, err
+	}
+
+	return serverConfig, nil
 }
 
 // Run starts a new packageserver for the PackageServerOptions.
@@ -127,14 +179,14 @@ func (o *PackageServerOptions) Run(ctx context.Context) error {
 		log.SetLevel(log.DebugLevel)
 	}
 
-	// grab the config for the API server
-	config, err := o.Config()
+	// Grab the config for the API server
+	config, err := o.Config(ctx)
 	if err != nil {
 		return err
 	}
 	config.GenericConfig.EnableMetrics = true
 
-	// set up the client config
+	// Set up the client config
 	var clientConfig *rest.Config
 	if len(o.Kubeconfig) > 0 {
 		loadingRules := &clientcmd.ClientConfigLoadingRules{ExplicitPath: o.Kubeconfig}
@@ -169,7 +221,7 @@ func (o *PackageServerOptions) Run(ctx context.Context) error {
 	}
 	config.ProviderConfig.Provider = sourceProvider
 
-	// we should never need to resync, since we're not worried about missing events,
+	// We should never need to resync, since we're not worried about missing events,
 	// and resync is actually for regular interval-based reconciliation these days,
 	// so set the default resync interval to 0
 	informerFactory := informers.NewSharedInformerFactory(kubeClient, 0)