refactor(newSecretBasedPromClientController): move prometheus config validation under newSecretBasedPromClientController

2026-08-23 22:46:35 +00:00 · 2026-02-04 17:14:58 +01:00
parent 964df4ce95
commit a91a02cadc
2 changed files with 105 additions and 44 deletions
@@ -130,7 +130,14 @@ func newInClusterPromClientController(prometheusClient promapi.Client, prometheu
 	}
 }

-func newSecretBasedPromClientController(prometheusClient promapi.Client, prometheusConfig *api.Prometheus) *secretBasedPromClientController {
+func newSecretBasedPromClientController(prometheusClient promapi.Client, prometheusConfig *api.Prometheus) (*secretBasedPromClientController, error) {
+	if prometheusConfig == nil || prometheusConfig.AuthToken == nil || prometheusConfig.AuthToken.SecretReference == nil {
+		return nil, fmt.Errorf("prometheus metrics source configuration is missing authentication token secret")
+	}
+	authTokenSecret := prometheusConfig.AuthToken.SecretReference
+	if authTokenSecret.Name == "" || authTokenSecret.Namespace == "" {
+		return nil, fmt.Errorf("prometheus metrics source configuration is missing authentication token secret")
+	}
 	ctrl := &secretBasedPromClientController{
 		promClient:             prometheusClient,
 		queue:                  workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "descheduler"}),
@@ -138,7 +145,7 @@ func newSecretBasedPromClientController(prometheusClient promapi.Client, prometh
 		createPrometheusClient: client.CreatePrometheusClient,
 	}

-	return ctrl
+	return ctrl, nil
 }

 func (d *inClusterPromClientController) prometheusClient() promapi.Client {
@@ -183,21 +190,12 @@ func setupPrometheusProvider(ctrl *secretBasedPromClientController, namespacedSh
 	if ctrl == nil {
 		return nil
 	}
-	prometheusConfig := ctrl.prometheusConfig
-	if prometheusConfig == nil {
-		return fmt.Errorf("prometheus configuration is missing")
-	}
-	if prometheusConfig.AuthToken != nil {
-		authTokenSecret := prometheusConfig.AuthToken.SecretReference
-		if authTokenSecret == nil || authTokenSecret.Namespace == "" {
-			return fmt.Errorf("prometheus metrics source configuration is missing authentication token secret")
-		}
-		if namespacedSharedInformerFactory == nil {
-			return fmt.Errorf("namespacedSharedInformerFactory not configured")
-		}
-		namespacedSharedInformerFactory.Core().V1().Secrets().Informer().AddEventHandler(ctrl.eventHandler())
-		ctrl.namespacedSecretsLister = namespacedSharedInformerFactory.Core().V1().Secrets().Lister().Secrets(authTokenSecret.Namespace)
+	if namespacedSharedInformerFactory == nil {
+		return fmt.Errorf("namespacedSharedInformerFactory not configured")
 	}
+	authTokenSecret := ctrl.prometheusConfig.AuthToken.SecretReference
+	namespacedSharedInformerFactory.Core().V1().Secrets().Informer().AddEventHandler(ctrl.eventHandler())
+	ctrl.namespacedSecretsLister = namespacedSharedInformerFactory.Core().V1().Secrets().Lister().Secrets(authTokenSecret.Namespace)
 	return nil
 }

@@ -259,7 +257,11 @@ func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedu
 	if prometheusConfig != nil && prometheusConfig.URL != "" {
 		if configureSecretPromClientReconciler(prometheusConfig) {
 			// Secret-based mode
-			desch.secretBasedPromClientCtrl = newSecretBasedPromClientController(rs.PrometheusClient, prometheusConfig)
+			ctrl, err := newSecretBasedPromClientController(rs.PrometheusClient, prometheusConfig)
+			if err != nil {
+				return nil, err
+			}
+			desch.secretBasedPromClientCtrl = ctrl
 		} else {
 			// In-cluster mode
 			desch.inClusterPromClientCtrl = newInClusterPromClientController(rs.PrometheusClient, prometheusConfig)
@@ -351,9 +353,6 @@ func (d *secretBasedPromClientController) sync() error {
 	defer d.mu.Unlock()

 	prometheusConfig := d.prometheusConfig
-	if prometheusConfig == nil || prometheusConfig.AuthToken == nil || prometheusConfig.AuthToken.SecretReference == nil {
-		return fmt.Errorf("prometheus metrics source configuration is missing authentication token secret")
-	}
 	ns := prometheusConfig.AuthToken.SecretReference.Namespace
 	name := prometheusConfig.AuthToken.SecretReference.Name
 	secretObj, err := d.namespacedSecretsLister.Get(name)
@@ -1823,7 +1823,7 @@ type promClientControllerTestSetup struct {
 	namespace                 string
 }

-func setupPromClientControllerTest(ctx context.Context, t *testing.T, objects []runtime.Object, prometheusConfig *api.Prometheus, setNamespacedSharedInformerFactory bool) *promClientControllerTestSetup {
+func setupPromClientControllerTest(ctx context.Context, t *testing.T, objects []runtime.Object, prometheusConfig *api.Prometheus, setNamespacedSharedInformerFactory bool) (*promClientControllerTestSetup, error) {
 	fakeClient := fakeclientset.NewSimpleClientset(objects...)

 	namespace := "default"
@@ -1839,7 +1839,10 @@ func setupPromClientControllerTest(ctx context.Context, t *testing.T, objects []
 		Prometheus: prometheusConfig,
 	}})

-	ctrl := newSecretBasedPromClientController(nil, prometheusConfig)
+	ctrl, err := newSecretBasedPromClientController(nil, prometheusConfig)
+	if err != nil {
+		return nil, err
+	}
 	if setNamespacedSharedInformerFactory {
 		if err := setupPrometheusProvider(ctrl, namespacedInformerFactory); err != nil {
 			t.Fatal(err)
@@ -1865,7 +1868,7 @@ func setupPromClientControllerTest(ctx context.Context, t *testing.T, objects []
 		metricsProviders:          metricsProviders,
 		ctrl:                      ctrl,
 		namespace:                 namespace,
-	}
+	}, nil
 }

 func newPrometheusConfig() *api.Prometheus {
@@ -1882,11 +1885,10 @@ func newPrometheusConfig() *api.Prometheus {

 func TestPromClientControllerSync_InvalidConfig(t *testing.T) {
 	testCases := []struct {
-		name                          string
-		objects                       []runtime.Object
-		prometheusConfig              *api.Prometheus
-		expectedErr                   error
-		setupPromClientControllerTest bool
+		name             string
+		objects          []runtime.Object
+		prometheusConfig *api.Prometheus
+		expectedErr      error
 	}{
 		{
 			name:             "empty prometheus config",
@@ -1919,20 +1921,28 @@ func TestPromClientControllerSync_InvalidConfig(t *testing.T) {
 			expectedErr: fmt.Errorf("prometheus metrics source configuration is missing authentication token secret"),
 		},
 		{
-			name:                          "secret exists but empty token",
-			objects:                       []runtime.Object{newPrometheusAuthSecret(withToken(""))},
-			prometheusConfig:              newPrometheusConfig(),
-			setupPromClientControllerTest: true,
-			expectedErr:                   fmt.Errorf("prometheus authentication token secret missing \"prometheusAuthToken\" data or empty"),
+			name: "missing secret reference name",
+			prometheusConfig: &api.Prometheus{
+				URL: prometheusURL,
+				AuthToken: &api.AuthToken{
+					SecretReference: &api.SecretReference{
+						Namespace: "kube-system",
+					},
+				},
+			},
+			expectedErr: fmt.Errorf("prometheus metrics source configuration is missing authentication token secret"),
 		},
 		{
-			name: "secret exists but missing token key",
-			objects: []runtime.Object{newPrometheusAuthSecret(func(s *v1.Secret) {
-				s.Data = map[string][]byte{}
-			})},
-			prometheusConfig:              newPrometheusConfig(),
-			setupPromClientControllerTest: true,
-			expectedErr:                   fmt.Errorf("prometheus authentication token secret missing \"prometheusAuthToken\" data or empty"),
+			name: "missing secret reference namespace",
+			prometheusConfig: &api.Prometheus{
+				URL: prometheusURL,
+				AuthToken: &api.AuthToken{
+					SecretReference: &api.SecretReference{
+						Name:      "prom-token",
+					},
+				},
+			},
+			expectedErr: fmt.Errorf("prometheus metrics source configuration is missing authentication token secret"),
 		},
 	}

@@ -1940,10 +1950,56 @@ func TestPromClientControllerSync_InvalidConfig(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.TODO())
 			defer cancel()
-			setup := setupPromClientControllerTest(ctx, t, tc.objects, tc.prometheusConfig, tc.setupPromClientControllerTest)
+			_, err := setupPromClientControllerTest(ctx, t, tc.objects, tc.prometheusConfig, false)
+
+			// Verify error expectations
+			if tc.expectedErr != nil {
+				if err == nil {
+					t.Errorf("Expected error %q but got none", tc.expectedErr)
+				} else if err.Error() != tc.expectedErr.Error() {
+					t.Errorf("Expected error %q but got %q", tc.expectedErr, err.Error())
+				}
+			} else {
+				t.Errorf("Expected an error, got none")
+			}
+		})
+	}
+}
+
+func TestPromClientControllerSync_InvalidSecret(t *testing.T) {
+	testCases := []struct {
+		name             string
+		objects          []runtime.Object
+		prometheusConfig *api.Prometheus
+		expectedErr      error
+	}{
+		{
+			name:             "secret exists but empty token",
+			objects:          []runtime.Object{newPrometheusAuthSecret(withToken(""))},
+			prometheusConfig: newPrometheusConfig(),
+			expectedErr:      fmt.Errorf("prometheus authentication token secret missing \"prometheusAuthToken\" data or empty"),
+		},
+		{
+			name: "secret exists but missing token key",
+			objects: []runtime.Object{newPrometheusAuthSecret(func(s *v1.Secret) {
+				s.Data = map[string][]byte{}
+			})},
+			prometheusConfig: newPrometheusConfig(),
+			expectedErr:      fmt.Errorf("prometheus authentication token secret missing \"prometheusAuthToken\" data or empty"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx, cancel := context.WithCancel(context.TODO())
+			defer cancel()
+			setup, err := setupPromClientControllerTest(ctx, t, tc.objects, tc.prometheusConfig, true)
+			if err != nil {
+				t.Fatal(err)
+			}

 			// Call sync
-			err := setup.ctrl.sync()
+			err = setup.ctrl.sync()

 			// Verify error expectations
 			if tc.expectedErr != nil {
@@ -2021,7 +2077,10 @@ func TestPromClientControllerSync_ClientCreation(t *testing.T) {
 				{
 					name: "running with prom reconciler directly",
 					setupFn: func(ctx context.Context, t *testing.T, objects []runtime.Object) *secretBasedPromClientController {
-						setup := setupPromClientControllerTest(ctx, t, objects, newPrometheusConfig(), true)
+						setup, err := setupPromClientControllerTest(ctx, t, objects, newPrometheusConfig(), true)
+						if err != nil {
+							t.Fatal(err)
+						}
 						return setup.ctrl
 					},
 				},
@@ -2186,7 +2245,10 @@ func TestPromClientControllerSync_EventHandler(t *testing.T) {
 		{
 			name: "running with prom reconciler directly",
 			init: func(t *testing.T, ctx context.Context) (ctrl *secretBasedPromClientController, fakeClient *fakeclientset.Clientset) {
-				setup := setupPromClientControllerTest(ctx, t, nil, newPrometheusConfig(), true)
+				setup, err := setupPromClientControllerTest(ctx, t, nil, newPrometheusConfig(), true)
+				if err != nil {
+					t.Fatal(err)
+				}

 				// Start the reconciler to process queue items
 				go setup.ctrl.runAuthenticationSecretReconciler(ctx)