Merge pull request #2604 from machine424/nope

OCPBUGS-18282: prevent use of reserved labels keys in Prometheus externalLabels

Author: openshift-merge-bot[bot]

Documentation/api.md

@@ -424,7 +424,7 @@ The `PrometheusK8sConfig` resource defines settings for the Prometheus component
 | -------- | ---- | ----------- |
 | additionalAlertmanagerConfigs | [][AdditionalAlertmanagerConfig](#additionalalertmanagerconfig) | Configures additional Alertmanager instances that receive alerts from the Prometheus component. By default, no additional Alertmanager instances are configured. |
 | enforcedBodySizeLimit | string | Enforces a body size limit for Prometheus scraped metrics. If a scraped target's body response is larger than the limit, the scrape will fail. The following values are valid: an empty value to specify no limit, a numeric value in Prometheus size format (such as `64MB`), or the string `automatic`, which indicates that the limit will be automatically calculated based on cluster capacity. The default value is empty, which indicates no limit. |
-| externalLabels | map[string]string | Defines labels to be added to any time series or alerts when communicating with external systems such as federation, remote storage, and Alertmanager. By default, no labels are added. |
+| externalLabels | ExternalLabels | Defines labels to be added to any time series or alerts when communicating with external systems such as federation, remote storage, and Alertmanager. By default, no labels are added. |
 | logLevel | string | Defines the log level setting for Prometheus. The possible values are: `error`, `warn`, `info`, and `debug`. The default value is `info`. |
 | nodeSelector | map[string]string | Defines the nodes on which the pods are scheduled. |
 | queryLogFile | string | Specifies the file to which PromQL queries are logged. This setting can be either a filename, in which case the queries are saved to an `emptyDir` volume at `/var/log/prometheus`, or a full path to a location where an `emptyDir` volume will be mounted and the queries saved. Writing to `/dev/stderr`, `/dev/stdout` or `/dev/null` is supported, but writing to any other `/dev/` path is not supported. Relative paths are also not supported. By default, PromQL queries are not logged. |
@@ -493,7 +493,7 @@ The `PrometheusRestrictedConfig` resource defines the settings for the Prometheu
 | enforcedLabelValueLengthLimit | *uint64 | Specifies a per-scrape limit on the length of a label value for a sample. If the length of a label value exceeds this limit after metric relabeling, the entire scrape is treated as failed. The default value is `0`, which means that no limit is set. |
 | enforcedSampleLimit | *uint64 | Specifies a global limit on the number of scraped samples that will be accepted. This setting overrides the `SampleLimit` value set in any user-defined `ServiceMonitor` or `PodMonitor` object if the value is greater than `enforcedTargetLimit`. Administrators can use this setting to keep the overall number of samples under control. The default value is `0`, which means that no limit is set. |
 | enforcedTargetLimit | *uint64 | Specifies a global limit on the number of scraped targets. This setting overrides the `TargetLimit` value set in any user-defined `ServiceMonitor` or `PodMonitor` object if the value is greater than `enforcedSampleLimit`. Administrators can use this setting to keep the overall number of targets under control. The default value is `0`. |
-| externalLabels | map[string]string | Defines labels to be added to any time series or alerts when communicating with external systems such as federation, remote storage, and Alertmanager. By default, no labels are added. |
+| externalLabels | ExternalLabels | Defines labels to be added to any time series or alerts when communicating with external systems such as federation, remote storage, and Alertmanager. By default, no labels are added. |
 | logLevel | string | Defines the log level setting for Prometheus. The possible values are `error`, `warn`, `info`, and `debug`. The default setting is `info`. |
 | nodeSelector | map[string]string | Defines the nodes on which the pods are scheduled. |
 | queryLogFile | string | Specifies the file to which PromQL queries are logged. This setting can be either a filename, in which case the queries are saved to an `emptyDir` volume at `/var/log/prometheus`, or a full path to a location where an `emptyDir` volume will be mounted and the queries saved. Writing to `/dev/stderr`, `/dev/stdout` or `/dev/null` is supported, but writing to any other `/dev/` path is not supported. Relative paths are also not supported. By default, PromQL queries are not logged. |

Documentation/openshiftdocs/modules/prometheusk8sconfig.adoc

@@ -22,7 +22,7 @@ Appears in: link:clustermonitoringconfiguration.adoc[ClusterMonitoringConfigurat
 
 |enforcedBodySizeLimit|string|Enforces a body size limit for Prometheus scraped metrics. If a scraped target's body response is larger than the limit, the scrape will fail. The following values are valid: an empty value to specify no limit, a numeric value in Prometheus size format (such as `64MB`), or the string `automatic`, which indicates that the limit will be automatically calculated based on cluster capacity. The default value is empty, which indicates no limit.
 
-|externalLabels|map[string]string|Defines labels to be added to any time series or alerts when communicating with external systems such as federation, remote storage, and Alertmanager. By default, no labels are added.
+|externalLabels|ExternalLabels|Defines labels to be added to any time series or alerts when communicating with external systems such as federation, remote storage, and Alertmanager. By default, no labels are added.
 
 |logLevel|string|Defines the log level setting for Prometheus. The possible values are: `error`, `warn`, `info`, and `debug`. The default value is `info`.
 

Documentation/openshiftdocs/modules/prometheusrestrictedconfig.adoc

@@ -34,7 +34,7 @@ Appears in: link:userworkloadconfiguration.adoc[UserWorkloadConfiguration]
 
 |enforcedTargetLimit|*uint64|Specifies a global limit on the number of scraped targets. This setting overrides the `TargetLimit` value set in any user-defined `ServiceMonitor` or `PodMonitor` object if the value is greater than `enforcedSampleLimit`. Administrators can use this setting to keep the overall number of targets under control. The default value is `0`.
 
-|externalLabels|map[string]string|Defines labels to be added to any time series or alerts when communicating with external systems such as federation, remote storage, and Alertmanager. By default, no labels are added.
+|externalLabels|ExternalLabels|Defines labels to be added to any time series or alerts when communicating with external systems such as federation, remote storage, and Alertmanager. By default, no labels are added.
 
 |logLevel|string|Defines the log level setting for Prometheus. The possible values are `error`, `warn`, `info`, and `debug`. The default setting is `info`.
 

pkg/manifests/config.go

@@ -54,6 +54,7 @@ const (
 )
 
 var errAlertmanagerV1NotSupported = errors.New("alertmanager's apiVersion=v1 is no longer supported, v2 has been available since Alertmanager 0.16.0")
+var reservedPrometheusExternalLabels = []string{"prometheus", "prometheus_replica", "cluster", "managed_cluster"}
 
 type Config struct {
 	Images                               *Images `json:"-"`
@@ -756,3 +757,17 @@ func NewDefaultUserWorkloadMonitoringConfig() *UserWorkloadConfiguration {
 	u.applyDefaults()
 	return u
 }
+
+func (lb *ExternalLabels) UnmarshalJSON(data []byte) error {
+	var v map[string]string
+	if err := json.Unmarshal(data, &v); err != nil {
+		return err
+	}
+	for _, r := range reservedPrometheusExternalLabels {
+		if _, ok := v[r]; ok {
+			return fmt.Errorf("reserved keys %v cannot be set as external labels", reservedPrometheusExternalLabels)
+		}
+	}
+	*lb = v
+	return nil
+}

pkg/manifests/manifests_test.go

@@ -1665,7 +1665,10 @@ func TestPrometheusUserWorkloadConfiguration(t *testing.T) {
     whenUnsatisfiable: DoNotSchedule
     labelSelector:
       matchLabels:
-        foo: bar`)
+        foo: bar
+  externalLabels:
+    foo: bar
+    oof: rab`)
 
 	c.UserWorkloadConfiguration = uwc
 	if err != nil {
@@ -1724,6 +1727,8 @@ func TestPrometheusUserWorkloadConfiguration(t *testing.T) {
 	require.Len(t, p.Spec.ScrapeClasses, 1)
 	require.Equal(t, p.Spec.ScrapeClasses[0].Default, ptr.To(true))
 	require.Equal(t, p.Spec.ScrapeClasses[0].FallbackScrapeProtocol, ptr.To(monv1.PrometheusText1_0_0))
+
+	require.Equal(t, p.Spec.ExternalLabels, map[string]string{"foo": "bar", "oof": "rab"})
 }
 
 func TestPrometheusQueryLogFileConfig(t *testing.T) {
@@ -5205,3 +5210,76 @@ func TestSetTLSSecurityConfiguration(t *testing.T) {
 		})
 	}
 }
+
+func TestPromConfigurationExternalLabels(t *testing.T) {
+	for _, p := range []struct {
+		promFieldName string
+		parser        func(string) error
+	}{
+		// platform and UWM
+		{promFieldName: "prometheusK8s", parser: func(s string) error {
+			_, err := NewConfigFromString(s, false)
+			return err
+		}},
+		{promFieldName: "prometheus", parser: func(s string) error {
+			_, err := NewUserConfigFromString(s)
+			return err
+		}},
+	} {
+		t.Run(p.promFieldName, func(t *testing.T) {
+			for _, tc := range []struct {
+				name       string
+				template   string
+				shouldFail bool
+			}{
+				{
+					name: "all reserved labels",
+					template: `%s:
+  externalLabels:
+    prometheus: foo
+    prometheus_replica: bar
+`,
+					shouldFail: true,
+				},
+				{
+					name: "no reserved",
+					template: `%s:
+  externalLabels:
+    foo: bar
+    bar: foo
+`,
+				},
+				{
+					name: "one reserved",
+					template: `%s:
+  externalLabels:
+    cluster: bar
+    bar: foo
+`,
+					shouldFail: true,
+				},
+				{
+					name: "one rempty reserved",
+					template: `%s:
+  externalLabels:
+    managed_cluster:
+    bar: foo
+`,
+					shouldFail: true,
+				},
+			} {
+				t.Run(tc.name, func(t *testing.T) {
+					config := fmt.Sprintf(tc.template, p.promFieldName)
+					err := p.parser(config)
+					if tc.shouldFail {
+						require.Error(t, err)
+						// Ensure we’re testing what we intend.
+						require.ErrorContains(t, err, "cannot be set as external labels")
+						return
+					}
+					require.NoError(t, err)
+				})
+			}
+		})
+	}
+}

pkg/manifests/types.go

@@ -21,6 +21,7 @@ import (
 
 type CollectionProfile string
 type CollectionProfiles []CollectionProfile
+type ExternalLabels map[string]string
 
 const (
 	FullCollectionProfile    = "full"
@@ -195,7 +196,7 @@ type PrometheusK8sConfig struct {
 	// Defines labels to be added to any time series or alerts when
 	// communicating with external systems such as federation, remote storage,
 	// and Alertmanager. By default, no labels are added.
-	ExternalLabels map[string]string `json:"externalLabels,omitempty"`
+	ExternalLabels ExternalLabels `json:"externalLabels,omitempty"`
 	// Defines the log level setting for Prometheus.
 	// The possible values are: `error`, `warn`, `info`, and `debug`.
 	// The default value is `info`.
@@ -659,7 +660,7 @@ type PrometheusRestrictedConfig struct {
 	// communicating with external systems such as federation, remote storage,
 	// and Alertmanager.
 	// By default, no labels are added.
-	ExternalLabels map[string]string `json:"externalLabels,omitempty"`
+	ExternalLabels ExternalLabels `json:"externalLabels,omitempty"`
 	// Defines the log level setting for Prometheus.
 	// The possible values are `error`, `warn`, `info`, and `debug`.
 	// The default setting is `info`.