refactor: metrics configuration code (#5475)

* refactor: metrics configuration

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

cmd/cleanup-controller/main.go

@@ -41,17 +41,14 @@ const (
 func setupMetrics(logger logr.Logger, kubeClient kubernetes.Interface) (*metrics.MetricsConfig, context.CancelFunc, error) {
 	logger = logger.WithName("metrics")
 	logger.Info("setup metrics...", "otel", otel, "port", metricsPort, "collector", otelCollector, "creds", transportCreds)
-	metricsConfigData, err := config.NewMetricsConfigData(kubeClient)
-	if err != nil {
-		return nil, nil, err
-	}
+	metricsConfiguration := internal.GetMetricsConfiguration(logger, kubeClient)
 	metricsAddr := ":" + metricsPort
 	metricsConfig, metricsServerMux, metricsPusher, err := metrics.InitMetrics(
 		disableMetricsExport,
 		otel,
 		metricsAddr,
 		otelCollector,
-		metricsConfigData,
+		metricsConfiguration,
 		transportCreds,
 		kubeClient,
 		logging.WithName("metrics"),

cmd/internal/metrics.go

@@ -0,0 +1,15 @@
+package internal
+
+import (
+	"github.com/go-logr/logr"
+	"github.com/kyverno/kyverno/pkg/config"
+	"k8s.io/client-go/kubernetes"
+)
+
+func GetMetricsConfiguration(logger logr.Logger, client kubernetes.Interface) config.MetricsConfiguration {
+	logger = logger.WithName("metrics")
+	logger.Info("load metrics configuration...")
+	metricsConfiguration, err := config.NewMetricsConfiguration(client)
+	checkError(logger, err, "failed to load metrics configuration")
+	return metricsConfiguration
+}

cmd/kyverno/main.go

@@ -120,17 +120,14 @@ func parseFlags(config internal.Configuration) {
 func setupMetrics(logger logr.Logger, kubeClient kubernetes.Interface) (*metrics.MetricsConfig, context.CancelFunc, error) {
 	logger = logger.WithName("metrics")
 	logger.Info("setup metrics...", "otel", otel, "port", metricsPort, "collector", otelCollector, "creds", transportCreds)
-	metricsConfigData, err := config.NewMetricsConfigData(kubeClient)
-	if err != nil {
-		return nil, nil, err
-	}
+	metricsConfiguration := internal.GetMetricsConfiguration(logger, kubeClient)
 	metricsAddr := ":" + metricsPort
 	metricsConfig, metricsServerMux, metricsPusher, err := metrics.InitMetrics(
 		disableMetricsExport,
 		otel,
 		metricsAddr,
 		otelCollector,
-		metricsConfigData,
+		metricsConfiguration,
 		transportCreds,
 		kubeClient,
 		logging.WithName("metrics"),

pkg/config/config.go

@@ -133,7 +133,7 @@ type configuration struct {
 	generateSuccessEvents bool
 }
 
-// NewConfiguration ...
+// NewDefaultConfiguration ...
 func NewDefaultConfiguration() *configuration {
 	return &configuration{
 		excludeGroupRole: defaultExcludeGroupRole,

pkg/config/fake.go

@@ -1,21 +0,0 @@
-package config
-
-import "k8s.io/client-go/kubernetes"
-
-func NewFakeConfig() Configuration {
-	return &configuration{}
-}
-
-func NewFakeMetricsConfig(client kubernetes.Interface) *MetricsConfigData {
-	return &MetricsConfigData{
-		client: client,
-		cmName: "",
-		metricsConfig: MetricsConfig{
-			metricsRefreshInterval: 0,
-			namespaces: namespacesConfig{
-				IncludeNamespaces: []string{},
-				ExcludeNamespaces: []string{},
-			},
-		},
-	}
-}

pkg/config/metricsconfig.go

@@ -3,10 +3,11 @@ package config
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"os"
 	"time"
 
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 )
@@ -15,17 +16,16 @@ import (
 // that stores the information associated with Kyverno's metrics exposure
 const metricsConfigEnvVar string = "METRICS_CONFIG"
 
-// MetricsConfigData stores the metrics-related configuration
-type MetricsConfigData struct {
-	client        kubernetes.Interface
-	cmName        string
-	metricsConfig MetricsConfig
-}
-
 // MetricsConfig stores the config for metrics
-type MetricsConfig struct {
-	namespaces             namespacesConfig
-	metricsRefreshInterval time.Duration
+type MetricsConfiguration interface {
+	// GetExcludeNamespaces returns the namespaces to ignore for metrics exposure
+	GetExcludeNamespaces() []string
+	// GetIncludeNamespaces returns the namespaces to specifically consider for metrics exposure
+	GetIncludeNamespaces() []string
+	// GetMetricsRefreshInterval returns the refresh interval for the metrics
+	GetMetricsRefreshInterval() time.Duration
+	// namespaces             namespacesConfig
+	// metricsRefreshInterval time.Duration
 }
 
 type namespacesConfig struct {
@@ -33,74 +33,89 @@ type namespacesConfig struct {
 	ExcludeNamespaces []string `json:"exclude,omitempty"`
 }
 
+// metricsConfig stores the config for metrics
+type metricsConfig struct {
+	namespaces             namespacesConfig
+	metricsRefreshInterval time.Duration
+}
+
 // GetExcludeNamespaces returns the namespaces to ignore for metrics exposure
-func (mcd *MetricsConfigData) GetExcludeNamespaces() []string {
-	return mcd.metricsConfig.namespaces.ExcludeNamespaces
+func (mcd *metricsConfig) GetExcludeNamespaces() []string {
+	return mcd.namespaces.ExcludeNamespaces
 }
 
 // GetIncludeNamespaces returns the namespaces to specifically consider for metrics exposure
-func (mcd *MetricsConfigData) GetIncludeNamespaces() []string {
-	return mcd.metricsConfig.namespaces.IncludeNamespaces
+func (mcd *metricsConfig) GetIncludeNamespaces() []string {
+	return mcd.namespaces.IncludeNamespaces
 }
 
 // GetMetricsRefreshInterval returns the refresh interval for the metrics
-func (mcd *MetricsConfigData) GetMetricsRefreshInterval() time.Duration {
-	return mcd.metricsConfig.metricsRefreshInterval
+func (mcd *metricsConfig) GetMetricsRefreshInterval() time.Duration {
+	return mcd.metricsRefreshInterval
 }
 
-// GetMetricsConfigMapName returns the configmap name for the metric
-func (mcd *MetricsConfigData) GetMetricsConfigMapName() string {
-	return mcd.cmName
+func (cd *metricsConfig) load(cm *corev1.ConfigMap) {
+	logger := logger.WithValues("name", cm.Name, "namespace", cm.Namespace)
+	if cm.Data == nil {
+		return
+	}
+	// reset
+	cd.metricsRefreshInterval = 0
+	cd.namespaces = namespacesConfig{
+		IncludeNamespaces: []string{},
+		ExcludeNamespaces: []string{},
+	}
+	// load metricsRefreshInterval
+	metricsRefreshInterval, found := cm.Data["metricsRefreshInterval"]
+	if found {
+		metricsRefreshInterval, err := time.ParseDuration(metricsRefreshInterval)
+		if err != nil {
+			logger.Error(err, "failed to parse metricsRefreshInterval")
+		} else {
+			cd.metricsRefreshInterval = metricsRefreshInterval
+		}
+	}
+	// load namespaces
+	namespaces, ok := cm.Data["namespaces"]
+	if ok {
+		namespaces, err := parseIncludeExcludeNamespacesFromNamespacesConfig(namespaces)
+		if err != nil {
+			logger.Error(err, "failed to parse namespaces")
+		} else {
+			cd.namespaces = namespaces
+		}
+	}
 }
 
-// NewMetricsConfigData ...
-func NewMetricsConfigData(rclient kubernetes.Interface) (*MetricsConfigData, error) {
-	cmName := os.Getenv(metricsConfigEnvVar)
-
-	mcd := MetricsConfigData{
-		client: rclient,
-		cmName: cmName,
-		metricsConfig: MetricsConfig{
-			metricsRefreshInterval: 0,
-			namespaces: namespacesConfig{
-				IncludeNamespaces: []string{},
-				ExcludeNamespaces: []string{},
-			},
+// NewDefaultMetricsConfiguration ...
+func NewDefaultMetricsConfiguration() *metricsConfig {
+	return &metricsConfig{
+		metricsRefreshInterval: 0,
+		namespaces: namespacesConfig{
+			IncludeNamespaces: []string{},
+			ExcludeNamespaces: []string{},
 		},
 	}
+}
 
+// NewMetricsConfiguration ...
+func NewMetricsConfiguration(client kubernetes.Interface) (MetricsConfiguration, error) {
+	configuration := NewDefaultMetricsConfiguration()
+	cmName := os.Getenv(metricsConfigEnvVar)
 	if cmName != "" {
-		kyvernoNamespace := kyvernoNamespace
-		configMap, err := rclient.CoreV1().ConfigMaps(kyvernoNamespace).Get(context.TODO(), mcd.cmName, metav1.GetOptions{})
-		if err != nil {
-			return nil, fmt.Errorf("error occurred while fetching the metrics configmap at %s/%s: %w", kyvernoNamespace, mcd.cmName, err)
-		}
-		namespacesConfigStr, found := configMap.Data["namespaces"]
-		if found {
-			mcd.metricsConfig.namespaces.IncludeNamespaces, mcd.metricsConfig.namespaces.ExcludeNamespaces, err = parseIncludeExcludeNamespacesFromNamespacesConfig(namespacesConfigStr)
-			if err != nil {
-				return nil, fmt.Errorf("error occurred while parsing the 'namespaces' field of metrics config map: %w", err)
+		if cm, err := client.CoreV1().ConfigMaps(kyvernoNamespace).Get(context.TODO(), cmName, metav1.GetOptions{}); err != nil {
+			if !errors.IsNotFound(err) {
+				return nil, err
 			}
+		} else {
+			configuration.load(cm)
 		}
-		metricsRefreshInterval, found := configMap.Data["metricsRefreshInterval"]
-		if found {
-			mcd.metricsConfig.metricsRefreshInterval, err = time.ParseDuration(metricsRefreshInterval)
-			if err != nil {
-				return nil, fmt.Errorf("error occurred while parsing metricsRefreshInterval: %w", err)
-			}
-		}
-	} else {
-		logger.Info("ConfigMap name not defined in env:METRICS_CONFIG: loading default configuration")
 	}
-
-	return &mcd, nil
+	return configuration, nil
 }
 
-func parseIncludeExcludeNamespacesFromNamespacesConfig(jsonStr string) ([]string, []string, error) {
-	var namespacesConfigObject *namespacesConfig
+func parseIncludeExcludeNamespacesFromNamespacesConfig(jsonStr string) (namespacesConfig, error) {
+	var namespacesConfigObject namespacesConfig
 	err := json.Unmarshal([]byte(jsonStr), &namespacesConfigObject)
-	if err != nil {
-		return nil, nil, err
-	}
-	return namespacesConfigObject.IncludeNamespaces, namespacesConfigObject.ExcludeNamespaces, nil
+	return namespacesConfigObject, err
 }

pkg/metrics/fake.go

@@ -2,16 +2,14 @@ package metrics
 
 import (
 	"github.com/kyverno/kyverno/pkg/config"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 )
 
-func NewFakeMetricsConfig(client kubernetes.Interface) *MetricsConfig {
+func NewFakeMetricsConfig() *MetricsConfig {
 	mc := &MetricsConfig{
-		Config: config.NewFakeMetricsConfig(client),
+		Config: config.NewDefaultMetricsConfiguration(),
 		Log:    klog.NewKlogr(),
 	}
-
-	mc, _ = initializeMetrics(mc)
+	_ = mc.initializeMetrics()
 	return mc
 }

pkg/metrics/init.go

@@ -14,7 +14,7 @@ func InitMetrics(
 	otel string,
 	metricsAddr string,
 	otelCollector string,
-	metricsConfigData *config.MetricsConfigData,
+	metricsConfiguration config.MetricsConfiguration,
 	transportCreds string,
 	kubeClient kubernetes.Interface,
 	log logr.Logger,
@@ -23,11 +23,12 @@ func InitMetrics(
 	var metricsServerMux *http.ServeMux
 	var pusher *controller.Controller
 
-	metricsConfig := new(MetricsConfig)
-	metricsConfig.Log = log
-	metricsConfig.Config = metricsConfigData
+	metricsConfig := MetricsConfig{
+		Log:    log,
+		Config: metricsConfiguration,
+	}
 
-	metricsConfig, err = initializeMetrics(metricsConfig)
+	err = metricsConfig.initializeMetrics()
 	if err != nil {
 		log.Error(err, "Failed initializing metrics")
 		return nil, nil, nil, err
@@ -57,5 +58,5 @@ func InitMetrics(
 			}
 		}
 	}
-	return metricsConfig, metricsServerMux, pusher, nil
+	return &metricsConfig, metricsServerMux, pusher, nil
 }

pkg/metrics/metrics.go

@@ -42,7 +42,7 @@ type MetricsConfig struct {
 	clientQueriesMetric           syncint64.Counter
 
 	// config
-	Config *kconfig.MetricsConfigData
+	Config kconfig.MetricsConfiguration
 	Log    logr.Logger
 }
 
@@ -56,54 +56,54 @@ type MetricsConfigManager interface {
 	RecordClientQueries(clientQueryOperation ClientQueryOperation, clientType ClientType, resourceKind string, resourceNamespace string)
 }
 
-func initializeMetrics(m *MetricsConfig) (*MetricsConfig, error) {
+func (m *MetricsConfig) initializeMetrics() error {
 	var err error
 	meter := global.MeterProvider().Meter(meterName)
 
 	m.policyResultsMetric, err = meter.SyncInt64().Counter("kyverno_policy_results_total", instrument.WithDescription("can be used to track the results associated with the policies applied in the user’s cluster, at the level from rule to policy to admission requests"))
 	if err != nil {
 		m.Log.Error(err, "Failed to create instrument, kyverno_policy_results_total")
-		return nil, err
+		return err
 	}
 
 	m.policyChangesMetric, err = meter.SyncInt64().Counter("kyverno_policy_changes_total", instrument.WithDescription("can be used to track all the changes associated with the Kyverno policies present on the cluster such as creation, updates and deletions"))
 	if err != nil {
 		m.Log.Error(err, "Failed to create instrument, kyverno_policy_changes_total")
-		return nil, err
+		return err
 	}
 
 	m.admissionRequestsMetric, err = meter.SyncInt64().Counter("kyverno_admission_requests_total", instrument.WithDescription("can be used to track the number of admission requests encountered by Kyverno in the cluster"))
 	if err != nil {
 		m.Log.Error(err, "Failed to create instrument, kyverno_admission_requests_total")
-		return nil, err
+		return err
 	}
 
 	m.policyExecutionDurationMetric, err = meter.SyncFloat64().Histogram("kyverno_policy_execution_duration_seconds", instrument.WithDescription("can be used to track the latencies (in seconds) associated with the execution/processing of the individual rules under Kyverno policies whenever they evaluate incoming resource requests"))
 	if err != nil {
 		m.Log.Error(err, "Failed to create instrument, kyverno_policy_execution_duration_seconds")
-		return nil, err
+		return err
 	}
 
 	m.admissionReviewDurationMetric, err = meter.SyncFloat64().Histogram("kyverno_admission_review_duration_seconds", instrument.WithDescription("can be used to track the latencies (in seconds) associated with the entire individual admission review. For example, if an incoming request trigger, say, five policies, this metric will track the e2e latency associated with the execution of all those policies"))
 	if err != nil {
 		m.Log.Error(err, "Failed to create instrument, kyverno_admission_review_duration_seconds")
-		return nil, err
+		return err
 	}
 
 	// Register Async Callbacks
 	m.policyRuleInfoMetric, err = meter.AsyncFloat64().Gauge("kyverno_policy_rule_info_total", instrument.WithDescription("can be used to track the info of the rules or/and policies present in the cluster. 0 means the rule doesn't exist and has been deleted, 1 means the rule is currently existent in the cluster"))
 	if err != nil {
 		m.Log.Error(err, "Failed to create instrument, kyverno_policy_rule_info_total")
-		return nil, err
+		return err
 	}
 
 	m.clientQueriesMetric, err = meter.SyncInt64().Counter("kyverno_client_queries_total", instrument.WithDescription("can be used to track the number of client queries sent from Kyverno to the API-server"))
 	if err != nil {
 		m.Log.Error(err, "Failed to create instrument, kyverno_client_queries_total")
-		return nil, err
+		return err
 	}
 
-	return m, nil
+	return nil
 }
 
 func ShutDownController(ctx context.Context, pusher *controller.Controller) {

pkg/webhooks/resource/fake.go

@@ -20,7 +20,7 @@ import (
 
 func NewFakeHandlers(ctx context.Context, policyCache policycache.Cache) webhooks.ResourceHandlers {
 	client := fake.NewSimpleClientset()
-	metricsConfig := metrics.NewFakeMetricsConfig(client)
+	metricsConfig := metrics.NewFakeMetricsConfig()
 
 	informers := informers.NewSharedInformerFactory(client, 0)
 	informers.Start(ctx.Done())
@@ -30,7 +30,7 @@ func NewFakeHandlers(ctx context.Context, policyCache policycache.Cache) webhook
 	kyvernoInformers.Start(ctx.Done())
 
 	dclient := dclient.NewEmptyFakeClient()
-	configuration := config.NewFakeConfig()
+	configuration := config.NewDefaultConfiguration()
 	rbLister := informers.Rbac().V1().RoleBindings().Lister()
 	crbLister := informers.Rbac().V1().ClusterRoleBindings().Lister()
 	urLister := kyvernoInformers.Kyverno().V1beta1().UpdateRequests().Lister().UpdateRequests(config.KyvernoNamespace())