Merge pull request #1303 from hashicorp/prometheus-tls

Add annotations for enabling TLS on Envoy Prometheus endpoint

CHANGELOG.md

@@ -5,6 +5,10 @@ FEATURES:
   * Add support for secret watchers on the Peering Acceptor and Peering Dialer controllers. [[GH-1284](https://github.com/hashicorp/consul-k8s/pull/1284)]
   * Add support for version annotation on the Peering Acceptor and Peering Dialer controllers. [[GH-1302](https://github.com/hashicorp/consul-k8s/pull/1302)]
 
+IMPROVEMENTS:
+* Control Plane
+  * Added annotations `consul.hashicorp.com/prometheus-ca-file`, `consul.hashicorp.com/prometheus-ca-path`, `consul.hashicorp.com/prometheus-cert-file`, and `consul.hashicorp.com/prometheus-key-file` for configuring TLS scraping on Prometheus metrics endpoints for Envoy sidecars. To enable, set the cert and key file annotations along with one of the ca file/path annotations. [[GH-1303](https://github.com/hashicorp/consul-k8s/pull/1303)]
+
 ## 0.45.0 (June 17, 2022)
 FEATURES:
 * [Experimental] Cluster Peering: Support Consul cluster peering, which allows service connectivity between two independent clusters.

charts/consul/templates/client-daemonset.yaml

@@ -1,6 +1,5 @@
 {{- if .Values.global.imageK8s }}{{ fail "global.imageK8s is not a valid key, use global.imageK8S (note the capital 'S')" }}{{ end -}}
 {{- if (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }}
-{{- if (and (and .Values.global.tls.enabled .Values.global.tls.httpsOnly) (and .Values.global.metrics.enabled .Values.global.metrics.enableAgentMetrics))}}{{ fail "global.metrics.enableAgentMetrics cannot be enabled if TLS (HTTPS only) is enabled" }}{{ end -}}
 {{- $serverEnabled := (or (and (ne (.Values.server.enabled | toString) "-") .Values.server.enabled) (and (eq (.Values.server.enabled | toString) "-") .Values.global.enabled)) -}}
 {{- if (and .Values.global.adminPartitions.enabled $serverEnabled (ne .Values.global.adminPartitions.name "default"))}}{{ fail "global.adminPartitions.name has to be \"default\" in the server cluster" }}{{ end -}}
 {{- if (and (not .Values.global.secretsBackend.vault.consulClientRole) .Values.global.secretsBackend.vault.enabled) }}{{ fail "global.secretsBackend.vault.consulClientRole must be provided if global.secretsBackend.vault.enabled=true." }}{{ end -}}

charts/consul/templates/server-statefulset.yaml

@@ -5,7 +5,6 @@
 {{- if and .Values.server.serverCert.secretName (not .Values.global.tls.caCert.secretName) }}{{ fail "If server.serverCert.secretName is provided, global.tls.caCert must also be provided" }}{{ end }}
 {{- if .Values.server.disableFsGroupSecurityContext }}{{ fail "server.disableFsGroupSecurityContext has been removed. Please use global.openshift.enabled instead." }}{{ end }}
 {{- if .Values.server.bootstrapExpect }}{{ if lt (int .Values.server.bootstrapExpect) (int .Values.server.replicas) }}{{ fail "server.bootstrapExpect cannot be less than server.replicas" }}{{ end }}{{ end }}
-{{- if (and (and .Values.global.tls.enabled .Values.global.tls.httpsOnly) (and .Values.global.metrics.enabled .Values.global.metrics.enableAgentMetrics))}}{{ fail "global.metrics.enableAgentMetrics cannot be enabled if TLS (HTTPS only) is enabled" }}{{ end -}}
 {{- if (and .Values.global.gossipEncryption.secretName (not .Values.global.gossipEncryption.secretKey)) }}{{fail "gossipEncryption.secretKey and secretName must both be specified." }}{{ end -}}
 {{- if (and (not .Values.global.gossipEncryption.secretName) .Values.global.gossipEncryption.secretKey) }}{{fail "gossipEncryption.secretKey and secretName must both be specified." }}{{ end -}}
 {{- if (and .Values.global.secretsBackend.vault.enabled (not .Values.global.secretsBackend.vault.consulServerRole)) }}{{ fail "global.secretsBackend.vault.consulServerRole must be provided if global.secretsBackend.vault.enabled=true." }}{{ end -}}

charts/consul/test/unit/client-daemonset.bats

@@ -528,20 +528,6 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "client/DaemonSet: when global.metrics.enableAgentMetrics=true, global.tls.enabled=true and global.tls.httpsOnly=true, fail" {
-  cd `chart_dir`
-  run helm template \
-      -s templates/client-daemonset.yaml  \
-      --set 'global.metrics.enabled=true'  \
-      --set 'global.metrics.enableAgentMetrics=true'  \
-      --set 'global.tls.enabled=true'  \
-      --set 'global.tls.httpsOnly=true'  \
-      .
-
-  [ "$status" -eq 1 ]
-  [[ "$output" =~ "global.metrics.enableAgentMetrics cannot be enabled if TLS (HTTPS only) is enabled" ]]
-}
-
 #--------------------------------------------------------------------
 # config-configmap
 

charts/consul/test/unit/server-statefulset.bats

@@ -648,20 +648,6 @@ load _helpers
   [ "${actual}" = "/v1/agent/metrics" ]
 }
 
-@test "server/StatefulSet: when global.metrics.enableAgentMetrics=true, global.tls.enabled=true and global.tls.httpsOnly=true, fail" {
-  cd `chart_dir`
-  run helm template \
-      -s templates/server-statefulset.yaml  \
-      --set 'global.metrics.enabled=true'  \
-      --set 'global.metrics.enableAgentMetrics=true'  \
-      --set 'global.tls.enabled=true'  \
-      --set 'global.tls.httpsOnly=true'  \
-      .
-
-  [ "$status" -eq 1 ]
-  [[ "$output" =~ "global.metrics.enableAgentMetrics cannot be enabled if TLS (HTTPS only) is enabled" ]]
-}
-
 #--------------------------------------------------------------------
 # config-configmap
 

control-plane/connect-inject/annotations.go

@@ -100,6 +100,12 @@ const (
 	annotationServiceMetricsPort   = "consul.hashicorp.com/service-metrics-port"
 	annotationServiceMetricsPath   = "consul.hashicorp.com/service-metrics-path"
 
+	// annotations for configuring TLS for Prometheus.
+	annotationPrometheusCAFile   = "consul.hashicorp.com/prometheus-ca-file"
+	annotationPrometheusCAPath   = "consul.hashicorp.com/prometheus-ca-path"
+	annotationPrometheusCertFile = "consul.hashicorp.com/prometheus-cert-file"
+	annotationPrometheusKeyFile  = "consul.hashicorp.com/prometheus-key-file"
+
 	// annotationEnvoyExtraArgs is a space-separated list of arguments to be passed to the
 	// envoy binary. See list of args here: https://www.envoyproxy.io/docs/envoy/latest/operations/cli
 	// e.g. consul.hashicorp.com/envoy-extra-args: "--log-level debug --disable-hot-restart"

control-plane/connect-inject/container_init.go

@@ -47,6 +47,11 @@ type initContainerCommandData struct {
 	PrometheusScrapePath string
 	// PrometheusBackendPort configures where the listener on Envoy will point to.
 	PrometheusBackendPort string
+	// The file paths to use for configuring TLS on the Prometheus metrics endpoint.
+	PrometheusCAFile   string
+	PrometheusCAPath   string
+	PrometheusCertFile string
+	PrometheusKeyFile  string
 	// EnvoyUID is the Linux user id that will be used when tproxy is enabled.
 	EnvoyUID int
 
@@ -214,6 +219,32 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod,
 		data.PrometheusScrapePath = prometheusScrapePath
 		data.PrometheusBackendPort = mergedMetricsPort
 	}
+	// Pull the TLS config from the relevant annotations.
+	if raw, ok := pod.Annotations[annotationPrometheusCAFile]; ok && raw != "" {
+		data.PrometheusCAFile = raw
+	}
+	if raw, ok := pod.Annotations[annotationPrometheusCAPath]; ok && raw != "" {
+		data.PrometheusCAPath = raw
+	}
+	if raw, ok := pod.Annotations[annotationPrometheusCertFile]; ok && raw != "" {
+		data.PrometheusCertFile = raw
+	}
+	if raw, ok := pod.Annotations[annotationPrometheusKeyFile]; ok && raw != "" {
+		data.PrometheusKeyFile = raw
+	}
+
+	// Validate required Prometheus TLS config is present if set.
+	if data.PrometheusCertFile != "" || data.PrometheusKeyFile != "" || data.PrometheusCAFile != "" || data.PrometheusCAPath != "" {
+		if data.PrometheusCAFile == "" && data.PrometheusCAPath == "" {
+			return corev1.Container{}, fmt.Errorf("Must set one of %q or %q when providing prometheus TLS config", annotationPrometheusCAFile, annotationPrometheusCAPath)
+		}
+		if data.PrometheusCertFile == "" {
+			return corev1.Container{}, fmt.Errorf("Must set %q when providing prometheus TLS config", annotationPrometheusCertFile)
+		}
+		if data.PrometheusKeyFile == "" {
+			return corev1.Container{}, fmt.Errorf("Must set %q when providing prometheus TLS config", annotationPrometheusKeyFile)
+		}
+	}
 
 	// Render the command
 	var buf bytes.Buffer
@@ -408,6 +439,18 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
   {{- if .PrometheusBackendPort }}
   -prometheus-backend-port="{{ .PrometheusBackendPort }}" \
   {{- end }}
+  {{- if .PrometheusCAFile }}
+  -prometheus-ca-file="{{ .PrometheusCAFile }}" \
+  {{- end }}
+  {{- if .PrometheusCAPath }}
+  -prometheus-ca-path="{{ .PrometheusCAPath }}" \
+  {{- end }}
+  {{- if .PrometheusCertFile }}
+  -prometheus-cert-file="{{ .PrometheusCertFile }}" \
+  {{- end }}
+  {{- if .PrometheusKeyFile }}
+  -prometheus-key-file="{{ .PrometheusKeyFile }}" \
+  {{- end }}
   {{- if .AuthMethod }}
   {{- if .MultiPort }}
   -token-file="/consul/connect-inject/acl-token-{{ .ServiceName }}" \

control-plane/connect-inject/container_init_test.go

@@ -49,6 +49,7 @@ func TestHandlerContainerInit(t *testing.T) {
 		Webhook MeshWebhook
 		Cmd     string // Strings.Contains test
 		CmdNot  string // Not contains
+		ErrStr  string // Error contains
 	}{
 		// The first test checks the whole template. Subsequent tests check
 		// the parts that change.
@@ -70,6 +71,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
   -proxy-id="$(cat /consul/connect-inject/proxyid)" \
   -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`,
 			"",
+			"",
 		},
 
 		{
@@ -99,6 +101,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
   -service-name="web" \
 `,
 			"",
+			"",
 		},
 		{
 			"When running the merged metrics server, configures consul connect envoy command",
@@ -116,6 +119,10 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
 				pod.Annotations[annotationServiceMetricsPort] = "1234"
 				pod.Annotations[annotationPrometheusScrapePort] = "22222"
 				pod.Annotations[annotationPrometheusScrapePath] = "/scrape-path"
+				pod.Annotations[annotationPrometheusCAFile] = "/certs/ca.crt"
+				pod.Annotations[annotationPrometheusCAPath] = "/certs/ca/"
+				pod.Annotations[annotationPrometheusCertFile] = "/certs/server.crt"
+				pod.Annotations[annotationPrometheusKeyFile] = "/certs/key.pem"
 				return pod
 			},
 			MeshWebhook{
@@ -126,8 +133,73 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
   -proxy-id="$(cat /consul/connect-inject/proxyid)" \
   -prometheus-scrape-path="/scrape-path" \
   -prometheus-backend-port="20100" \
+  -prometheus-ca-file="/certs/ca.crt" \
+  -prometheus-ca-path="/certs/ca/" \
+  -prometheus-cert-file="/certs/server.crt" \
+  -prometheus-key-file="/certs/key.pem" \
   -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`,
 			"",
+			"",
+		},
+		{
+			"When providing Prometheus TLS config, missing CA gives an error",
+			func(pod *corev1.Pod) *corev1.Pod {
+				pod.Annotations[annotationService] = "web"
+				pod.Annotations[annotationEnableMetrics] = "true"
+				pod.Annotations[annotationEnableMetricsMerging] = "true"
+				pod.Annotations[annotationMergedMetricsPort] = "20100"
+				pod.Annotations[annotationPrometheusScrapePort] = "22222"
+				pod.Annotations[annotationPrometheusScrapePath] = "/scrape-path"
+				pod.Annotations[annotationPrometheusCertFile] = "/certs/server.crt"
+				pod.Annotations[annotationPrometheusKeyFile] = "/certs/key.pem"
+				return pod
+			},
+			MeshWebhook{
+				ConsulAPITimeout: 5 * time.Second,
+			},
+			"",
+			"",
+			fmt.Sprintf("Must set one of %q or %q", annotationPrometheusCAFile, annotationPrometheusCAPath),
+		},
+		{
+			"When providing Prometheus TLS config, missing cert gives an error",
+			func(pod *corev1.Pod) *corev1.Pod {
+				pod.Annotations[annotationService] = "web"
+				pod.Annotations[annotationEnableMetrics] = "true"
+				pod.Annotations[annotationEnableMetricsMerging] = "true"
+				pod.Annotations[annotationMergedMetricsPort] = "20100"
+				pod.Annotations[annotationPrometheusScrapePort] = "22222"
+				pod.Annotations[annotationPrometheusScrapePath] = "/scrape-path"
+				pod.Annotations[annotationPrometheusCAFile] = "/certs/ca.crt"
+				pod.Annotations[annotationPrometheusKeyFile] = "/certs/key.pem"
+				return pod
+			},
+			MeshWebhook{
+				ConsulAPITimeout: 5 * time.Second,
+			},
+			"",
+			"",
+			fmt.Sprintf("Must set %q", annotationPrometheusCertFile),
+		},
+		{
+			"When providing Prometheus TLS config, missing key gives an error",
+			func(pod *corev1.Pod) *corev1.Pod {
+				pod.Annotations[annotationService] = "web"
+				pod.Annotations[annotationEnableMetrics] = "true"
+				pod.Annotations[annotationEnableMetricsMerging] = "true"
+				pod.Annotations[annotationMergedMetricsPort] = "20100"
+				pod.Annotations[annotationPrometheusScrapePort] = "22222"
+				pod.Annotations[annotationPrometheusScrapePath] = "/scrape-path"
+				pod.Annotations[annotationPrometheusCAPath] = "/certs/ca/"
+				pod.Annotations[annotationPrometheusCertFile] = "/certs/server.crt"
+				return pod
+			},
+			MeshWebhook{
+				ConsulAPITimeout: 5 * time.Second,
+			},
+			"",
+			"",
+			fmt.Sprintf("Must set %q", annotationPrometheusKeyFile),
 		},
 	}
 
@@ -138,7 +210,11 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
 			h := tt.Webhook
 			pod := *tt.Pod(minimal())
 			container, err := h.containerInit(testNS, pod, multiPortInfo{})
-			require.NoError(err)
+			if tt.ErrStr == "" {
+				require.NoError(err)
+			} else {
+				require.Contains(err.Error(), tt.ErrStr)
+			}
 			actual := strings.Join(container.Command, " ")
 			require.Contains(actual, tt.Cmd)
 			if tt.CmdNot != "" {