rhobs · openshift-merge-bot · Oct 11, 2024 · Sep 27, 2024
diff --git a/pkg/controllers/monitoring/monitoring-stack/components.go b/pkg/controllers/monitoring/monitoring-stack/components.go
@@ -1,6 +1,8 @@
 package monitoringstack
 
 import (
+	"fmt"
+	"path/filepath"
 	"reflect"
 
 	monv1 "github.com/rhobs/obo-prometheus-operator/pkg/apis/monitoring/v1"
@@ -19,6 +21,8 @@ const (
 	AdditionalScrapeConfigsSelfScrapeKey = "self-scrape-config"
 	PrometheusUserFSGroupID              = int64(65534)
 	AlertmanagerUserFSGroupID            = int64(65535)
+
+	prometheusSecretsMountPoint = "/etc/prometheus/secrets"
 )
 
 func stackComponentReconcilers(
@@ -256,7 +260,7 @@ func newPrometheus(
 				SafeTLSConfig: monv1.SafeTLSConfig{
 					ServerName: ptr.To(ms.Name + "-alertmanager"),
 				},
-				CAFile: "/etc/prometheus/secrets/" + caSecret.Name + "/" + caSecret.Key,
+				CAFile: filepath.Join(prometheusSecretsMountPoint, caSecret.Name, caSecret.Key),
 			}
 		}
 	}
@@ -390,30 +394,28 @@ func newThanosSidecarService(ms *stack.MonitoringStack, instanceSelectorKey stri
 }
 
 func newAdditionalScrapeConfigsSecret(ms *stack.MonitoringStack, name string) *corev1.Secret {
-	prometheusScheme := "http"
-	prometheusTLSConfig := ""
+	var (
+		prometheusScheme     = "http"
+		prometheusCAFile     string
+		prometheusServerName string
 
-	alertmanagerScheme := "http"
-	alertmanagerTLSConfig := ""
+		alertmanagerScheme     = "http"
+		alertmanagerCAFile     string
+		alertmanagerServerName string
+	)
 
 	if ms.Spec.PrometheusConfig.WebTLSConfig != nil {
 		promCASecret := ms.Spec.PrometheusConfig.WebTLSConfig.CertificateAuthority
 		prometheusScheme = "https"
-		prometheusTLSConfig = `
-  tls_config:
-    ca_file: /etc/prometheus/secrets/` + promCASecret.Name + `/` + promCASecret.Key + `
-    server_name: ` + ms.Name + `-prometheus
-`
+		prometheusCAFile = filepath.Join(prometheusSecretsMountPoint, promCASecret.Name, promCASecret.Key)
+		prometheusServerName = fmt.Sprintf("%s-prometheus", ms.Name)
 	}
 
 	if ms.Spec.AlertmanagerConfig.WebTLSConfig != nil {
 		amCASecret := ms.Spec.AlertmanagerConfig.WebTLSConfig.CertificateAuthority
 		alertmanagerScheme = "https"
-		alertmanagerTLSConfig = `
-  tls_config:
-    ca_file: /etc/prometheus/secrets/` + amCASecret.Name + `/` + amCASecret.Key + `
-    server_name: ` + ms.Name + `-alertmanager
-`
+		alertmanagerCAFile = filepath.Join(prometheusSecretsMountPoint, amCASecret.Name, amCASecret.Key)
+		alertmanagerServerName = fmt.Sprintf("%s-alertmanager", ms.Name)
 	}
 	return &corev1.Secret{
 		TypeMeta: metav1.TypeMeta{
@@ -425,15 +427,17 @@ func newAdditionalScrapeConfigsSecret(ms *stack.MonitoringStack, name string) *c
 			Namespace: ms.Namespace,
 		},
 		StringData: map[string]string{
-			AdditionalScrapeConfigsSelfScrapeKey: `
+			AdditionalScrapeConfigsSelfScrapeKey: fmt.Sprintf(`
 - job_name: prometheus-self
-  honor_labels: true
-  scheme: ` + prometheusScheme + prometheusTLSConfig + `
+  scheme: %s
+  tls_config:
+    ca_file: %q
+    server_name: %q
   relabel_configs:
   - action: keep
     source_labels:
     - __meta_kubernetes_service_label_app_kubernetes_io_name
-    regex: ` + ms.Name + `-prometheus
+    regex: %s
   - action: keep
     source_labels:
     - __meta_kubernetes_endpoint_port_name
@@ -456,19 +460,20 @@ func newAdditionalScrapeConfigsSecret(ms *stack.MonitoringStack, name string) *c
   - role: endpoints
     namespaces:
       names:
-      - ` + ms.Namespace + `
+      - %s
 - job_name: alertmanager-self
-  honor_timestamps: true
   scrape_interval: 30s
   scrape_timeout: 10s
   metrics_path: /metrics
-  scheme: ` + alertmanagerScheme + alertmanagerTLSConfig + `
-  follow_redirects: true
+  scheme: %s
+  tls_config:
+    ca_file: %q
+    server_name: %q
   relabel_configs:
   - source_labels:
     - __meta_kubernetes_service_label_app_kubernetes_io_name
     separator: ;
-    regex: ` + ms.Name + `-alertmanager
+    regex: %s
     replacement: $1
     action: keep
   - source_labels: [__meta_kubernetes_endpoint_port_name]
@@ -507,11 +512,20 @@ func newAdditionalScrapeConfigsSecret(ms *stack.MonitoringStack, name string) *c
     action: replace
   kubernetes_sd_configs:
   - role: endpoints
-    kubeconfig_file: ""
-    follow_redirects: true
     namespaces:
       names:
-      - ` + ms.Namespace,
+      - %s`,
+				prometheusScheme,
+				prometheusCAFile,
+				prometheusServerName,
+				fmt.Sprintf("%s-prometheus", ms.Name),
+				ms.Namespace,
+				alertmanagerScheme,
+				alertmanagerCAFile,
+				alertmanagerServerName,
+				fmt.Sprintf("%s-alertmanager", ms.Name),
+				ms.Namespace,
+			),
 		},
 	}
 }

diff --git a/pkg/controllers/monitoring/monitoring-stack/components_test.go b/pkg/controllers/monitoring/monitoring-stack/components_test.go
@@ -6,12 +6,15 @@ import (
 	monv1 "github.com/rhobs/obo-prometheus-operator/pkg/apis/monitoring/v1"
 	v1 "github.com/rhobs/obo-prometheus-operator/pkg/apis/monitoring/v1"
 	"gotest.tools/v3/assert"
+	"gotest.tools/v3/golden"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	stack "github.com/rhobs/observability-operator/pkg/apis/monitoring/v1alpha1"
 )
 
 func TestStorageSpec(t *testing.T) {
-
 	validPVCSpec := &corev1.PersistentVolumeClaimSpec{
 		AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
 		Resources: corev1.VolumeResourceRequirements{
@@ -42,3 +45,71 @@ func TestStorageSpec(t *testing.T) {
 		assert.DeepEqual(t, tc.expected, actual)
 	}
 }
+
+func TestNewAdditionalScrapeConfigsSecret(t *testing.T) {
+	for _, tc := range []struct {
+		name       string
+		spec       stack.MonitoringStackSpec
+		goldenFile string
+	}{
+		{
+			name: "no-tls",
+			spec: stack.MonitoringStackSpec{
+				PrometheusConfig:   &stack.PrometheusConfig{},
+				AlertmanagerConfig: stack.AlertmanagerConfig{},
+			},
+			goldenFile: "no-tls",
+		},
+		{
+			name: "with-tls",
+			spec: stack.MonitoringStackSpec{
+				PrometheusConfig: &stack.PrometheusConfig{
+					WebTLSConfig: &stack.WebTLSConfig{
+						PrivateKey: stack.SecretKeySelector{
+							Name: "prometheus-tls",
+							Key:  "key.pem",
+						},
+						Certificate: stack.SecretKeySelector{
+							Name: "prometheus-tls",
+							Key:  "cert.pem",
+						},
+						CertificateAuthority: stack.SecretKeySelector{
+							Name: "prometheus-tls",
+							Key:  "ca.pem",
+						},
+					},
+				},
+				AlertmanagerConfig: stack.AlertmanagerConfig{
+					WebTLSConfig: &stack.WebTLSConfig{
+						PrivateKey: stack.SecretKeySelector{
+							Name: "alertmanager-tls",
+							Key:  "key.pem",
+						},
+						Certificate: stack.SecretKeySelector{
+							Name: "alertmanager-tls",
+							Key:  "cert.pem",
+						},
+						CertificateAuthority: stack.SecretKeySelector{
+							Name: "alertmanager-tls",
+							Key:  "ca.pem",
+						},
+					},
+				},
+			},
+			goldenFile: "tls",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ms := stack.MonitoringStack{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "ms-" + tc.name,
+					Namespace: "ns-" + tc.name,
+				},
+				Spec: tc.spec,
+			}
+			s := newAdditionalScrapeConfigsSecret(&ms, tc.name)
+			assert.Equal(t, s.Name, tc.name)
+			golden.Assert(t, s.StringData[AdditionalScrapeConfigsSelfScrapeKey], tc.goldenFile)
+		})
+	}
+}
diff --git a/pkg/controllers/monitoring/monitoring-stack/testdata/no-tls b/pkg/controllers/monitoring/monitoring-stack/testdata/no-tls
@@ -0,0 +1,88 @@
+
+- job_name: prometheus-self
+  scheme: http
+  tls_config:
+    ca_file: ""
+    server_name: ""
+  relabel_configs:
+  - action: keep
+    source_labels:
+    - __meta_kubernetes_service_label_app_kubernetes_io_name
+    regex: ms-no-tls-prometheus
+  - action: keep
+    source_labels:
+    - __meta_kubernetes_endpoint_port_name
+    regex: web
+  - source_labels:
+    - __meta_kubernetes_namespace
+    target_label: namespace
+  - source_labels:
+    - __meta_kubernetes_service_name
+    target_label: service
+  - source_labels:
+    - __meta_kubernetes_pod_name
+    target_label: pod
+  - source_labels:
+    - __meta_kubernetes_pod_container_name
+    target_label: container
+  - target_label: endpoint
+    replacement: web
+  kubernetes_sd_configs:
+  - role: endpoints
+    namespaces:
+      names:
+      - ns-no-tls
+- job_name: alertmanager-self
+  scrape_interval: 30s
+  scrape_timeout: 10s
+  metrics_path: /metrics
+  scheme: http
+  tls_config:
+    ca_file: ""
+    server_name: ""
+  relabel_configs:
+  - source_labels:
+    - __meta_kubernetes_service_label_app_kubernetes_io_name
+    separator: ;
+    regex: ms-no-tls-alertmanager
+    replacement: $1
+    action: keep
+  - source_labels: [__meta_kubernetes_endpoint_port_name]
+    separator: ;
+    regex: web
+    replacement: $1
+    action: keep
+  - source_labels: [__meta_kubernetes_namespace]
+    separator: ;
+    regex: (.*)
+    target_label: namespace
+    replacement: $1
+    action: replace
+  - source_labels: [__meta_kubernetes_service_name]
+    separator: ;
+    regex: (.*)
+    target_label: service
+    replacement: $1
+    action: replace
+  - source_labels: [__meta_kubernetes_pod_name]
+    separator: ;
+    regex: (.*)
+    target_label: pod
+    replacement: $1
+    action: replace
+  - source_labels: [__meta_kubernetes_pod_container_name]
+    separator: ;
+    regex: (.*)
+    target_label: container
+    replacement: $1
+    action: replace
+  - separator: ;
+    regex: (.*)
+    target_label: endpoint
+    replacement: web
+    action: replace
+  kubernetes_sd_configs:
+  - role: endpoints
+    namespaces:
+      names:
+      - ns-no-tls