Change defaulting, add tests

Author: sbueringer

pkg/manager/internal.go

@@ -36,7 +36,8 @@ import (
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/authorization/authorizerfactory"
 	"k8s.io/apiserver/pkg/server/options"
-	"k8s.io/client-go/kubernetes"
+	authenticationv1 "k8s.io/client-go/kubernetes/typed/authentication/v1"
+	authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/leaderelection"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
@@ -92,15 +93,18 @@ type controllerManager struct {
 	// metricsListener is used to serve prometheus metrics
 	metricsListener net.Listener
 
-	// metricsInsecureServing enables insecure metrics serving.
-	// This means metrics will be served via http and without authentication and authorization.
-	metricsInsecureServing bool
+	// metricsSecureServing enables secure metrics serving.
+	// This means metrics will be served via https and with authentication and authorization.
+	metricsSecureServing bool
 
 	// metricsExtraHandlers contains extra handlers to register on http server that serves metrics.
 	metricsExtraHandlers map[string]http.Handler
 
-	// metricsKubeClient is the client used to authenticate and authorize requests to the metrics endpoint.
-	metricsKubeClient *kubernetes.Clientset
+	// metricsAuthenticationClient is the client used to authenticate requests to the metrics endpoint.
+	metricsAuthenticationClient authenticationv1.AuthenticationV1Interface
+
+	// metricsAuthorizationClient is the client used to authorize requests to the metrics endpoint.
+	metricsAuthorizationClient authorizationv1.AuthorizationV1Interface
 
 	// healthProbeListener is used to serve liveness probe
 	healthProbeListener net.Listener
@@ -318,9 +322,9 @@ func (cm *controllerManager) addMetricsServer() error {
 
 	log := cm.logger.WithValues("path", defaultMetricsEndpoint)
 
-	if !cm.metricsInsecureServing {
+	if cm.metricsSecureServing {
 		var err error
-		handler, err = withAuthenticationAndAuthorization(log, cm.metricsKubeClient, handler)
+		handler, err = withAuthenticationAndAuthorization(log, cm.metricsAuthenticationClient, cm.metricsAuthorizationClient, handler)
 		if err != nil {
 			return fmt.Errorf("failed to add metrics server: %w", err)
 		}
@@ -340,11 +344,11 @@ func (cm *controllerManager) addMetricsServer() error {
 	})
 }
 
-func withAuthenticationAndAuthorization(log logr.Logger, metricsKubeClient *kubernetes.Clientset, handler http.Handler) (http.Handler, error) {
+func withAuthenticationAndAuthorization(log logr.Logger, authenticationClient authenticationv1.AuthenticationV1Interface, authorizationClient authorizationv1.AuthorizationV1Interface, handler http.Handler) (http.Handler, error) {
 	authenticatorConfig := authenticatorfactory.DelegatingAuthenticatorConfig{
 		Anonymous:                false, // Require authentication.
 		CacheTTL:                 1 * time.Minute,
-		TokenAccessReviewClient:  metricsKubeClient.AuthenticationV1(),
+		TokenAccessReviewClient:  authenticationClient,
 		TokenAccessReviewTimeout: 10 * time.Second,
 		WebhookRetryBackoff:      options.DefaultAuthWebhookRetryBackoff(),
 	}
@@ -354,7 +358,7 @@ func withAuthenticationAndAuthorization(log logr.Logger, metricsKubeClient *kube
 	}
 
 	authorizerConfig := authorizerfactory.DelegatingAuthorizerConfig{
-		SubjectAccessReviewClient: metricsKubeClient.AuthorizationV1(),
+		SubjectAccessReviewClient: authorizationClient,
 		AllowCacheTTL:             5 * time.Minute,
 		DenyCacheTTL:              30 * time.Second,
 		WebhookRetryBackoff:       options.DefaultAuthWebhookRetryBackoff(),
@@ -392,13 +396,13 @@ func withAuthenticationAndAuthorization(log logr.Logger, metricsKubeClient *kube
 
 		authorized, reason, err := delegatingAuthorizer.Authorize(ctx, attributes)
 		if err != nil {
-			msg := fmt.Sprintf("Authorization for user %s failed", attributes.User)
+			msg := fmt.Sprintf("Authorization for user %s failed", attributes.User.GetName())
 			log.Error(err, fmt.Sprintf("%s: %s", msg, err))
 			http.Error(w, msg, http.StatusInternalServerError)
 			return
 		}
 		if authorized != authorizer.DecisionAllow {
-			msg := fmt.Sprintf("Authorization denied for user %s", attributes.User)
+			msg := fmt.Sprintf("Authorization denied for user %s", attributes.User.GetName())
 			log.V(4).Info(fmt.Sprintf("%s: %s", msg, reason))
 			http.Error(w, msg, http.StatusForbidden)
 			return

pkg/manager/manager.go

@@ -30,6 +30,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
+	authenticationv1 "k8s.io/client-go/kubernetes/typed/authentication/v1"
+	authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
 	"k8s.io/client-go/tools/record"
@@ -260,7 +262,14 @@ type Options struct {
 	// MetricsBindAddress is the TCP address that the controller should bind to
 	// for serving prometheus metrics.
 	// It can be set to "0" to disable the metrics serving.
-	// Per default metrics will be served via https and authenticated (via TokenReviews)
+	//
+	// Per default metrics will be served via http and without authentication and authorization.
+	// If MetricsSecureServing is enabled metrics will be served via https and authenticated (via TokenReviews)
+	// and authorized (via SubjectAccessReviews) with the kube-apiserver.
+	MetricsBindAddress string
+
+	// MetricsSecureServing enables secure metrics serving.
+	// This means metrics will be served via https and authenticated (via TokenReviews)
 	// and authorized (via SubjectAccessReviews) with the kube-apiserver.
 	//
 	// For the authentication and authorization the controller needs a ClusterRole
@@ -283,13 +292,7 @@ type Options struct {
 	//   - "/metrics"
 	//   verbs:
 	//   - get
-	// Alternatively MetricsInsecureServing can be set to true, then metrics will
-	// be served via http and authentication and authorization is skipped.
-	MetricsBindAddress string
-
-	// MetricsInsecureServing enables insecure metrics serving.
-	// This means metrics will be served via http and authentication and authorization is skipped.
-	MetricsInsecureServing bool
+	MetricsSecureServing bool
 
 	// HealthProbeBindAddress is the TCP address that the controller should bind to
 	// for serving health probes
@@ -383,7 +386,7 @@ type Options struct {
 	// Dependency injection for testing
 	newRecorderProvider    func(config *rest.Config, httpClient *http.Client, scheme *runtime.Scheme, logger logr.Logger, makeBroadcaster intrec.EventBroadcasterProducer) (*intrec.Provider, error)
 	newResourceLock        func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error)
-	newMetricsListener     func(addr string, insecureServing bool) (net.Listener, error)
+	newMetricsListener     func(addr string, secureServing bool) (net.Listener, error)
 	newHealthProbeListener func(addr string) (net.Listener, error)
 	newPprofListener       func(addr string) (net.Listener, error)
 }
@@ -480,18 +483,20 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		}
 	}
 
-	var metricsKubeClient *kubernetes.Clientset
-	if !options.MetricsInsecureServing {
-		var err error
-		metricsKubeClient, err = kubernetes.NewForConfigAndClient(config, cluster.GetHTTPClient())
+	var metricsAuthenticationClient authenticationv1.AuthenticationV1Interface
+	var metricsAuthorizationClient authorizationv1.AuthorizationV1Interface
+	if options.MetricsSecureServing {
+		metricsKubeClient, err := kubernetes.NewForConfigAndClient(config, cluster.GetHTTPClient())
 		if err != nil {
 			return nil, err
 		}
+		metricsAuthenticationClient = metricsKubeClient.AuthenticationV1()
+		metricsAuthorizationClient = metricsKubeClient.AuthorizationV1()
 	}
 
 	// Create the metrics listener. This will throw an error if the metrics bind
 	// address is invalid or already in use.
-	metricsListener, err := options.newMetricsListener(options.MetricsBindAddress, options.MetricsInsecureServing)
+	metricsListener, err := options.newMetricsListener(options.MetricsBindAddress, options.MetricsSecureServing)
 	if err != nil {
 		return nil, err
 	}
@@ -524,9 +529,10 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		recorderProvider:              recorderProvider,
 		resourceLock:                  resourceLock,
 		metricsListener:               metricsListener,
-		metricsInsecureServing:        options.MetricsInsecureServing,
+		metricsSecureServing:          options.MetricsSecureServing,
 		metricsExtraHandlers:          metricsExtraHandlers,
-		metricsKubeClient:             metricsKubeClient,
+		metricsAuthenticationClient:   metricsAuthenticationClient,
+		metricsAuthorizationClient:    metricsAuthorizationClient,
 		controllerConfig:              options.Controller,
 		logger:                        options.Logger,
 		elected:                       make(chan struct{}),