Merge pull request #423 from nckturner/dont-pass-metrics-obj

Don't pass metrics around

cmd/aws-iam-authenticator/server.go

@@ -23,9 +23,11 @@ import (
 
 	"k8s.io/sample-controller/pkg/signals"
 	"sigs.k8s.io/aws-iam-authenticator/pkg/mapper"
+	"sigs.k8s.io/aws-iam-authenticator/pkg/metrics"
 	"sigs.k8s.io/aws-iam-authenticator/pkg/server"
 
 	"github.com/aws/aws-sdk-go/aws/endpoints"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -46,7 +48,7 @@ var serverCmd = &cobra.Command{
 	Long:  ``,
 	Run: func(cmd *cobra.Command, args []string) {
 		var err error
-
+		metrics.InitMetrics(prometheus.DefaultRegisterer)
 		stopCh := signals.SetupSignalHandler()
 
 		cfg, err := getConfig()

pkg/mapper/configmap/configmap.go

@@ -31,10 +31,9 @@ type MapStore struct {
 	// Used as set.
 	awsAccounts map[string]interface{}
 	configMap   v1.ConfigMapInterface
-	metrics     metrics.Metrics
 }
 
-func New(masterURL, kubeConfig string, authenticatorMetrics metrics.Metrics) (*MapStore, error) {
+func New(masterURL, kubeConfig string) (*MapStore, error) {
 	clientconfig, err := clientcmd.BuildConfigFromFlags(masterURL, kubeConfig)
 	if err != nil {
 		return nil, err
@@ -46,7 +45,6 @@ func New(masterURL, kubeConfig string, authenticatorMetrics metrics.Metrics) (*M
 
 	ms := MapStore{}
 	ms.configMap = clientset.CoreV1().ConfigMaps("kube-system")
-	ms.metrics = authenticatorMetrics
 	return &ms, nil
 }
 
@@ -65,7 +63,7 @@ func (ms *MapStore) startLoadConfigMap(stopCh <-chan struct{}) {
 				})
 				if err != nil {
 					logrus.Errorf("Unable to re-establish watch: %v, sleeping for 5 seconds.", err)
-					ms.metrics.ConfigMapWatchFailures.Inc()
+					metrics.Get().ConfigMapWatchFailures.Inc()
 					time.Sleep(5 * time.Second)
 					continue
 				}

pkg/mapper/configmap/configmap_test.go

@@ -5,7 +5,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/prometheus/client_golang/prometheus"
 	core_v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/watch"
@@ -14,7 +13,6 @@ import (
 	"k8s.io/client-go/kubernetes/typed/core/v1/fake"
 	k8stesting "k8s.io/client-go/testing"
 	"sigs.k8s.io/aws-iam-authenticator/pkg/config"
-	"sigs.k8s.io/aws-iam-authenticator/pkg/metrics"
 )
 
 var testUser = config.UserMapping{Username: "matlan", Groups: []string{"system:master", "dev"}}
@@ -25,7 +23,6 @@ func makeStore() MapStore {
 		users:       make(map[string]config.UserMapping),
 		roles:       make(map[string]config.RoleMapping),
 		awsAccounts: make(map[string]interface{}),
-		metrics:     metrics.CreateMetrics(prometheus.NewRegistry()),
 	}
 	ms.users["matt"] = testUser
 	ms.roles["instance"] = testRole
@@ -41,7 +38,6 @@ func makeStoreWClient() (MapStore, *fake.FakeConfigMaps) {
 		users:     make(map[string]config.UserMapping),
 		roles:     make(map[string]config.RoleMapping),
 		configMap: v1.ConfigMapInterface(fakeConfigMaps),
-		metrics:   metrics.CreateMetrics(prometheus.NewRegistry()),
 	}
 	return ms, fakeConfigMaps
 }

pkg/mapper/configmap/mapper.go

@@ -5,7 +5,6 @@ import (
 
 	"sigs.k8s.io/aws-iam-authenticator/pkg/config"
 	"sigs.k8s.io/aws-iam-authenticator/pkg/mapper"
-	"sigs.k8s.io/aws-iam-authenticator/pkg/metrics"
 )
 
 type ConfigMapMapper struct {
@@ -14,8 +13,8 @@ type ConfigMapMapper struct {
 
 var _ mapper.Mapper = &ConfigMapMapper{}
 
-func NewConfigMapMapper(cfg config.Config, authenticatorMetrics metrics.Metrics) (*ConfigMapMapper, error) {
-	ms, err := New(cfg.Master, cfg.Kubeconfig, authenticatorMetrics)
+func NewConfigMapMapper(cfg config.Config) (*ConfigMapMapper, error) {
+	ms, err := New(cfg.Master, cfg.Kubeconfig)
 	if err != nil {
 		return nil, err
 	}

pkg/mapper/configmap/yaml_test.go

@@ -9,7 +9,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/prometheus/client_golang/prometheus"
 	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -18,7 +17,6 @@ import (
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/aws-iam-authenticator/pkg/config"
-	"sigs.k8s.io/aws-iam-authenticator/pkg/metrics"
 )
 
 var log = logrus.New()
@@ -107,9 +105,7 @@ func TestConfigMap(t *testing.T) {
 			}
 
 			cs := fake.NewSimpleClientset()
-			ms := MapStore{
-				metrics: metrics.CreateMetrics(prometheus.NewRegistry()),
-			}
+			ms := MapStore{}
 			ms.configMap = cs.CoreV1().ConfigMaps("kube-system")
 
 			stopCh := make(chan struct{})

pkg/metrics/metrics.go

@@ -14,6 +14,16 @@ const (
 	Success   = "success"
 )
 
+var authenticatorMetrics Metrics
+
+func InitMetrics(registerer prometheus.Registerer) {
+	authenticatorMetrics = CreateMetrics(registerer)
+}
+
+func Get() Metrics {
+	return authenticatorMetrics
+}
+
 // Metrics are handles to the collectors for prometheus for the various metrics we are tracking.
 type Metrics struct {
 	ConfigMapWatchFailures prometheus.Counter

pkg/server/server.go

@@ -36,7 +36,6 @@ import (
 	"sigs.k8s.io/aws-iam-authenticator/pkg/token"
 
 	awsarn "github.com/aws/aws-sdk-go/aws/arn"
-	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/sirupsen/logrus"
 	authenticationv1beta1 "k8s.io/api/authentication/v1beta1"
@@ -65,7 +64,6 @@ var (
 type handler struct {
 	http.ServeMux
 	verifier         token.Verifier
-	metrics          metrics.Metrics
 	ec2Provider      ec2provider.EC2Provider
 	clusterID        string
 	mappers          []mapper.Mapper
@@ -78,10 +76,7 @@ func New(cfg config.Config, stopCh <-chan struct{}) *Server {
 		Config: cfg,
 	}
 
-	authenticatorMetrics := metrics.CreateMetrics(prometheus.DefaultRegisterer)
-	c.metrics = authenticatorMetrics
-
-	mappers, err := BuildMapperChain(cfg, authenticatorMetrics)
+	mappers, err := BuildMapperChain(cfg)
 	if err != nil {
 		logrus.Fatalf("failed to build mapper chain: %v", err)
 	}
@@ -140,7 +135,7 @@ func New(cfg config.Config, stopCh <-chan struct{}) *Server {
 	logrus.Infof("reconfigure your apiserver with `--authentication-token-webhook-config-file=%s` to enable (assuming default hostPath mounts)", c.GenerateKubeconfigPath)
 	c.httpServer = http.Server{
 		ErrorLog: log.New(errLog, "", 0),
-		Handler:  c.getHandler(authenticatorMetrics, mappers, c.EC2DescribeInstancesQps, c.EC2DescribeInstancesBurst),
+		Handler:  c.getHandler(mappers, c.EC2DescribeInstancesQps, c.EC2DescribeInstancesBurst),
 	}
 	c.listener = listener
 	return c
@@ -164,7 +159,7 @@ type healthzHandler struct{}
 func (m *healthzHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	fmt.Fprintf(w, "ok")
 }
-func (c *Server) getHandler(authenticatorMetrics metrics.Metrics, mappers []mapper.Mapper, ec2DescribeQps int, ec2DescribeBurst int) *handler {
+func (c *Server) getHandler(mappers []mapper.Mapper, ec2DescribeQps int, ec2DescribeBurst int) *handler {
 	if c.ServerEC2DescribeInstancesRoleARN != "" {
 		_, err := awsarn.Parse(c.ServerEC2DescribeInstancesRoleARN)
 		if err != nil {
@@ -174,7 +169,6 @@ func (c *Server) getHandler(authenticatorMetrics metrics.Metrics, mappers []mapp
 
 	h := &handler{
 		verifier:         token.NewVerifier(c.ClusterID, c.PartitionID),
-		metrics:          authenticatorMetrics,
 		ec2Provider:      ec2provider.New(c.ServerEC2DescribeInstancesRoleARN, ec2DescribeQps, ec2DescribeBurst),
 		clusterID:        c.ClusterID,
 		mappers:          mappers,
@@ -191,7 +185,7 @@ func (c *Server) getHandler(authenticatorMetrics metrics.Metrics, mappers []mapp
 	return h
 }
 
-func BuildMapperChain(cfg config.Config, authenticatorMetrics metrics.Metrics) ([]mapper.Mapper, error) {
+func BuildMapperChain(cfg config.Config) ([]mapper.Mapper, error) {
 	modes := cfg.BackendMode
 	mappers := []mapper.Mapper{}
 	for _, mode := range modes {
@@ -207,7 +201,7 @@ func BuildMapperChain(cfg config.Config, authenticatorMetrics metrics.Metrics) (
 		case mapper.ModeConfigMap:
 			fallthrough
 		case mapper.ModeEKSConfigMap:
-			configMapMapper, err := configmap.NewConfigMapMapper(cfg, authenticatorMetrics)
+			configMapMapper, err := configmap.NewConfigMapMapper(cfg)
 			if err != nil {
 				return nil, fmt.Errorf("backend-mode %q creation failed: %v", mode, err)
 			}
@@ -249,13 +243,13 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
 	if req.Method != http.MethodPost {
 		log.Error("unexpected request method")
 		http.Error(w, "expected POST", http.StatusMethodNotAllowed)
-		h.metrics.Latency.WithLabelValues(metrics.Malformed).Observe(duration(start))
+		metrics.Get().Latency.WithLabelValues(metrics.Malformed).Observe(duration(start))
 		return
 	}
 	if req.Body == nil {
 		log.Error("empty request body")
 		http.Error(w, "expected a request body", http.StatusBadRequest)
-		h.metrics.Latency.WithLabelValues(metrics.Malformed).Observe(duration(start))
+		metrics.Get().Latency.WithLabelValues(metrics.Malformed).Observe(duration(start))
 		return
 	}
 	defer req.Body.Close()
@@ -264,7 +258,7 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
 	if err := json.NewDecoder(req.Body).Decode(&tokenReview); err != nil {
 		log.WithError(err).Error("could not parse request body")
 		http.Error(w, "expected a request body to be a TokenReview", http.StatusBadRequest)
-		h.metrics.Latency.WithLabelValues(metrics.Malformed).Observe(duration(start))
+		metrics.Get().Latency.WithLabelValues(metrics.Malformed).Observe(duration(start))
 		return
 	}
 
@@ -277,9 +271,9 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
 	identity, err := h.verifier.Verify(tokenReview.Spec.Token)
 	if err != nil {
 		if _, ok := err.(token.STSError); ok {
-			h.metrics.Latency.WithLabelValues(metrics.STSError).Observe(duration(start))
+			metrics.Get().Latency.WithLabelValues(metrics.STSError).Observe(duration(start))
 		} else {
-			h.metrics.Latency.WithLabelValues(metrics.Invalid).Observe(duration(start))
+			metrics.Get().Latency.WithLabelValues(metrics.Invalid).Observe(duration(start))
 		}
 		log.WithError(err).Warn("access denied")
 		w.WriteHeader(http.StatusForbidden)
@@ -302,7 +296,7 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
 
 	username, groups, err := h.doMapping(identity)
 	if err != nil {
-		h.metrics.Latency.WithLabelValues(metrics.Unknown).Observe(duration(start))
+		metrics.Get().Latency.WithLabelValues(metrics.Unknown).Observe(duration(start))
 		log.WithError(err).Warn("access denied")
 		w.WriteHeader(http.StatusForbidden)
 		w.Write(tokenReviewDenyJSON)
@@ -321,7 +315,7 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
 		"uid":      uid,
 		"groups":   groups,
 	}).Info("access granted")
-	h.metrics.Latency.WithLabelValues(metrics.Success).Observe(duration(start))
+	metrics.Get().Latency.WithLabelValues(metrics.Success).Observe(duration(start))
 	w.WriteHeader(http.StatusOK)
 
 	userExtra := map[string]authenticationv1beta1.ExtraValue{}

pkg/server/server_test.go

@@ -102,9 +102,9 @@ func newIAMIdentityMapping(arn, canonicalARN, username string, groups []string)
 }
 
 func setup(verifier token.Verifier) *handler {
+	metrics.InitMetrics(prometheus.NewRegistry())
 	return &handler{
 		verifier: verifier,
-		metrics:  metrics.CreateMetrics(prometheus.NewRegistry()),
 	}
 }
 

pkg/server/types.go

@@ -21,7 +21,6 @@ import (
 	"net/http"
 
 	"sigs.k8s.io/aws-iam-authenticator/pkg/config"
-	"sigs.k8s.io/aws-iam-authenticator/pkg/metrics"
 )
 
 // Server for the authentication webhook.
@@ -30,5 +29,4 @@ type Server struct {
 	config.Config
 	httpServer http.Server
 	listener   net.Listener
-	metrics    metrics.Metrics
 }

tests/integration/go.mod

@@ -4,6 +4,7 @@ go 1.16
 
 require (
 	github.com/aws/aws-sdk-go v1.38.49
+	github.com/prometheus/client_golang v1.11.0 // indirect
 	github.com/sirupsen/logrus v1.8.1
 	k8s.io/api v0.22.1
 	k8s.io/apimachinery v0.22.1

tests/integration/testutils/testserver.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws/endpoints"
+	"github.com/prometheus/client_golang/prometheus"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
 	client "k8s.io/client-go/kubernetes"
@@ -20,6 +21,7 @@ import (
 
 	"sigs.k8s.io/aws-iam-authenticator/pkg/config"
 	"sigs.k8s.io/aws-iam-authenticator/pkg/mapper"
+	"sigs.k8s.io/aws-iam-authenticator/pkg/metrics"
 	"sigs.k8s.io/aws-iam-authenticator/pkg/server"
 )
 
@@ -39,6 +41,8 @@ type AuthenticatorTestFrameworkSetup struct {
 }
 
 func StartAuthenticatorTestFramework(t *testing.T, stopCh <-chan struct{}, setup AuthenticatorTestFrameworkSetup) (client.Interface, client.Interface) {
+	metrics.InitMetrics(prometheus.NewRegistry())
+
 	cfg, err := testConfig(t, setup)
 	if err != nil {
 		t.Fatal(err)