Merge pull request #5856 from simonpasquier/fix-select-scrape-configs

fix: reject invalid ScrapeConfigs
2026-02-05 15:46:31 +01:00 · 2023-08-29 12:09:26 +02:00
parent 3541047506 3eba45726a
commit dc98ff6139
3 changed files with 162 additions and 54 deletions
--- a/pkg/assets/store.go
+++ b/pkg/assets/store.go
@@ -242,7 +242,7 @@ func (s *Store) AddSafeAuthorizationCredentials(ctx context.Context, namespace s

 	err := s.addToken(ctx, namespace, auth.Credentials, key)
 	if err != nil {
-		return errors.Wrapf(err, "failed to get authorization token of type %s", auth.Type)
+		return errors.Wrapf(err, "failed to get authorization token of type %q", auth.Type)
 	}
 	return nil
 }
@@ -258,7 +258,7 @@ func (s *Store) AddAuthorizationCredentials(ctx context.Context, namespace strin

 	err := s.addToken(ctx, namespace, auth.Credentials, key)
 	if err != nil {
-		return errors.Wrapf(err, "failed to get authorization token of type %s", auth.Type)
+		return errors.Wrapf(err, "failed to get authorization token of type %q", auth.Type)
 	}
 	return nil
 }
--- a/pkg/prometheus/resource_selector.go
+++ b/pkg/prometheus/resource_selector.go
@@ -658,7 +658,7 @@ func (rs *ResourceSelector) SelectScrapeConfigs(ctx context.Context, listFn List
 			level.Warn(rs.l).Log(
 				"msg", "skipping scrapeconfig",
 				"error", err.Error(),
-				"scrapeconfig", sc,
+				"scrapeconfig", scName,
 				"namespace", objMeta.GetNamespace(),
 				"prometheus", objMeta.GetName(),
 			)
@@ -686,25 +686,6 @@ func (rs *ResourceSelector) SelectScrapeConfigs(ctx context.Context, listFn List
 			continue
 		}

-		for i, config := range sc.Spec.HTTPSDConfigs {
-			configKey := fmt.Sprintf("scrapeconfig/%s/%s/httpsdconfig/%d", sc.GetNamespace(), sc.GetName(), i)
-			if err = rs.store.AddBasicAuth(ctx, sc.GetNamespace(), config.BasicAuth, configKey); err != nil {
-				rejectFn(sc, err)
-				continue
-			}
-
-			configAuthKey := fmt.Sprintf("scrapeconfig/auth/%s/%s/httpsdconfig/%d", sc.GetNamespace(), sc.GetName(), i)
-			if err = rs.store.AddSafeAuthorizationCredentials(ctx, sc.GetNamespace(), config.Authorization, configAuthKey); err != nil {
-				rejectFn(sc, err)
-				continue
-			}
-
-			if err = rs.store.AddSafeTLSConfig(ctx, sc.GetNamespace(), config.TLSConfig); err != nil {
-				rejectFn(sc, err)
-				continue
-			}
-		}
-
 		var scrapeInterval, scrapeTimeout monitoringv1.Duration = "", ""
 		if sc.Spec.ScrapeInterval != nil {
 			scrapeInterval = *sc.Spec.ScrapeInterval
@@ -724,37 +705,14 @@ func (rs *ResourceSelector) SelectScrapeConfigs(ctx context.Context, listFn List
 			continue
 		}

-		for i, config := range sc.Spec.ConsulSDConfigs {
-			configKey := fmt.Sprintf("scrapeconfig/%s/%s/consulsdconfig/%d", sc.GetNamespace(), sc.GetName(), i)
-			if err = rs.store.AddBasicAuth(ctx, sc.GetNamespace(), config.BasicAuth, configKey); err != nil {
-				rejectFn(sc, err)
-				continue
-			}
+		if err = rs.validateHTTPSDConfigs(ctx, sc); err != nil {
+			rejectFn(sc, fmt.Errorf("httpSDConfigs: %w", err))
+			continue
+		}

-			configAuthKey := fmt.Sprintf("scrapeconfig/auth/%s/%s/consulsdconfig/%d", sc.GetNamespace(), sc.GetName(), i)
-			if err = rs.store.AddSafeAuthorizationCredentials(ctx, sc.GetNamespace(), config.Authorization, configAuthKey); err != nil {
-				rejectFn(sc, err)
-				continue
-			}
-
-			if err = rs.store.AddSafeTLSConfig(ctx, sc.GetNamespace(), config.TLSConfig); err != nil {
-				rejectFn(sc, err)
-				continue
-			}
-
-			if _, err = rs.store.GetSecretKey(ctx, sc.GetNamespace(), *config.TokenRef); err != nil {
-				rejectFn(sc, err)
-				continue
-			}
-
-			for _, v := range config.ProxyConnectHeader {
-				_, err := rs.store.GetSecretKey(context.Background(), sc.GetNamespace(), v)
-
-				if err != nil {
-					rejectFn(sc, err)
-					continue
-				}
-			}
+		if err = rs.validateConsulSDConfigs(ctx, sc); err != nil {
+			rejectFn(sc, fmt.Errorf("consulSDConfigs: %w", err))
+			continue
 		}

 		res[scName] = sc
@@ -773,3 +731,52 @@ func (rs *ResourceSelector) SelectScrapeConfigs(ctx context.Context, listFn List

 	return res, nil
 }
+
+func (rs *ResourceSelector) validateConsulSDConfigs(ctx context.Context, sc *monitoringv1alpha1.ScrapeConfig) error {
+	for i, config := range sc.Spec.ConsulSDConfigs {
+		configKey := fmt.Sprintf("scrapeconfig/%s/%s/consulsdconfig/%d", sc.GetNamespace(), sc.GetName(), i)
+		if err := rs.store.AddBasicAuth(ctx, sc.GetNamespace(), config.BasicAuth, configKey); err != nil {
+			return fmt.Errorf("[%d]: %w", i, err)
+		}
+
+		configAuthKey := fmt.Sprintf("scrapeconfig/auth/%s/%s/consulsdconfig/%d", sc.GetNamespace(), sc.GetName(), i)
+		if err := rs.store.AddSafeAuthorizationCredentials(ctx, sc.GetNamespace(), config.Authorization, configAuthKey); err != nil {
+			return fmt.Errorf("[%d]: %w", i, err)
+		}
+
+		if err := rs.store.AddSafeTLSConfig(ctx, sc.GetNamespace(), config.TLSConfig); err != nil {
+			return fmt.Errorf("[%d]: %w", i, err)
+		}
+
+		if _, err := rs.store.GetSecretKey(ctx, sc.GetNamespace(), *config.TokenRef); err != nil {
+			return fmt.Errorf("[%d]: %w", i, err)
+		}
+
+		for k, v := range config.ProxyConnectHeader {
+			if _, err := rs.store.GetSecretKey(context.Background(), sc.GetNamespace(), v); err != nil {
+				return fmt.Errorf("[%d]: header[%s]: %w", i, k, err)
+			}
+		}
+	}
+	return nil
+}
+
+func (rs *ResourceSelector) validateHTTPSDConfigs(ctx context.Context, sc *monitoringv1alpha1.ScrapeConfig) error {
+	for i, config := range sc.Spec.HTTPSDConfigs {
+		configKey := fmt.Sprintf("scrapeconfig/%s/%s/httpsdconfig/%d", sc.GetNamespace(), sc.GetName(), i)
+		if err := rs.store.AddBasicAuth(ctx, sc.GetNamespace(), config.BasicAuth, configKey); err != nil {
+			return fmt.Errorf("[%d]: %w", i, err)
+		}
+
+		configAuthKey := fmt.Sprintf("scrapeconfig/auth/%s/%s/httpsdconfig/%d", sc.GetNamespace(), sc.GetName(), i)
+		if err := rs.store.AddSafeAuthorizationCredentials(ctx, sc.GetNamespace(), config.Authorization, configAuthKey); err != nil {
+			return fmt.Errorf("[%d]: %w", i, err)
+		}
+
+		if err := rs.store.AddSafeTLSConfig(ctx, sc.GetNamespace(), config.TLSConfig); err != nil {
+			return fmt.Errorf("[%d]: %w", i, err)
+		}
+	}
+
+	return nil
+}
--- a/pkg/prometheus/resource_selector_test.go
+++ b/pkg/prometheus/resource_selector_test.go
@@ -24,12 +24,15 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/prometheus/model/relabel"
 	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/cache"

 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	monitoringv1alpha1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1alpha1"
+	"github.com/prometheus-operator/prometheus-operator/pkg/assets"
 	"github.com/prometheus-operator/prometheus-operator/pkg/operator"
 )

@@ -950,12 +953,110 @@ func TestSelectScrapeConfigs(t *testing.T) {
 			},
 			selected: false,
 		},
+		{
+			scenario: "HTTP SD config with valid secret ref",
+			updateSpec: func(sc *monitoringv1alpha1.ScrapeConfigSpec) {
+				sc.HTTPSDConfigs = []monitoringv1alpha1.HTTPSDConfig{
+					{
+						URL: "http://example.com",
+						Authorization: &monitoringv1.SafeAuthorization{
+							Credentials: &v1.SecretKeySelector{
+								LocalObjectReference: v1.LocalObjectReference{
+									Name: "secret",
+								},
+								Key: "key1",
+							},
+						},
+					},
+				}
+			},
+			selected: true,
+		},
+		{
+			scenario: "HTTP SD config with invalid secret ref",
+			updateSpec: func(sc *monitoringv1alpha1.ScrapeConfigSpec) {
+				sc.HTTPSDConfigs = []monitoringv1alpha1.HTTPSDConfig{
+					{
+						URL: "http://example.com",
+						Authorization: &monitoringv1.SafeAuthorization{
+							Credentials: &v1.SecretKeySelector{
+								LocalObjectReference: v1.LocalObjectReference{
+									Name: "wrong",
+								},
+								Key: "key1",
+							},
+						},
+					},
+				}
+			},
+			selected: false,
+		},
+		{
+			scenario: "Consul SD config with valid secret ref",
+			updateSpec: func(sc *monitoringv1alpha1.ScrapeConfigSpec) {
+				sc.ConsulSDConfigs = []monitoringv1alpha1.ConsulSDConfig{
+					{
+						Server: "example.com",
+						TokenRef: &v1.SecretKeySelector{
+							LocalObjectReference: v1.LocalObjectReference{
+								Name: "secret",
+							},
+							Key: "key1",
+						},
+					},
+				}
+			},
+			selected: true,
+		},
+		{
+			scenario: "Consul SD config with invalid secret ref",
+			updateSpec: func(sc *monitoringv1alpha1.ScrapeConfigSpec) {
+				sc.ConsulSDConfigs = []monitoringv1alpha1.ConsulSDConfig{
+					{
+						Server: "example.com",
+						TokenRef: &v1.SecretKeySelector{
+							LocalObjectReference: v1.LocalObjectReference{
+								Name: "wrong",
+							},
+							Key: "key1",
+						},
+					},
+				}
+			},
+			selected: false,
+		},
 	} {
 		t.Run(tc.scenario, func(t *testing.T) {
+			cs := fake.NewSimpleClientset(
+				&v1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "secret",
+						Namespace: "test",
+					},
+					Data: map[string][]byte{
+						"key1": []byte("val1"),
+					},
+				},
+				&v1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "configmap",
+						Namespace: "test",
+					},
+					Data: map[string]string{
+						"key1": "val1",
+					},
+				},
+			)
+
 			rs := NewResourceSelector(
 				newLogger(),
-				&monitoringv1.Prometheus{},
-				nil,
+				&monitoringv1.Prometheus{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test",
+						Namespace: "test",
+					},
+				},
+				assets.NewStore(cs.CoreV1(), cs.CoreV1()),
 				nil,
 				operator.NewMetrics(prometheus.NewPedanticRegistry()),
 			)