From 52a98bfccb900d80f9df998269cc50b231ee5197 Mon Sep 17 00:00:00 2001 From: Kyle Schochenmaier Date: Wed, 29 Jun 2022 17:16:35 -0500 Subject: [PATCH 1/4] add envoy user volumes via annotations --- control-plane/connect-inject/annotations.go | 4 +++ control-plane/connect-inject/envoy_sidecar.go | 11 ++++++++ .../connect-inject/envoy_sidecar_test.go | 26 +++++++++++++++++++ control-plane/connect-inject/mesh_webhook.go | 10 +++++++ 4 files changed, 51 insertions(+) diff --git a/control-plane/connect-inject/annotations.go b/control-plane/connect-inject/annotations.go index 31a4dc7f01..5c8545ee23 100644 --- a/control-plane/connect-inject/annotations.go +++ b/control-plane/connect-inject/annotations.go @@ -86,6 +86,10 @@ const ( annotationConsulSidecarMemoryLimit = "consul.hashicorp.com/consul-sidecar-memory-limit" annotationConsulSidecarMemoryRequest = "consul.hashicorp.com/consul-sidecar-memory-request" + // annotations for sidecar volumes. + annotationConsulSidecarUserVolume = "consul.hashicorp.com/consul-sidecar-user-volume" + annotationConsulSidecarUserVolumeMount = "consul.hashicorp.com/consul-sidecar-user-volume-mount" + // annotations for sidecar concurrency. annotationEnvoyProxyConcurrency = "consul.hashicorp.com/consul-envoy-proxy-concurrency" diff --git a/control-plane/connect-inject/envoy_sidecar.go b/control-plane/connect-inject/envoy_sidecar.go index 77cc7f47bc..c50fbac5ff 100644 --- a/control-plane/connect-inject/envoy_sidecar.go +++ b/control-plane/connect-inject/envoy_sidecar.go @@ -1,6 +1,7 @@ package connectinject import ( + "encoding/json" "fmt" "strconv" "strings" @@ -48,6 +49,16 @@ func (w *MeshWebhook) envoySidecar(namespace corev1.Namespace, pod corev1.Pod, m Command: cmd, } + // Add any extra Envoy VolumeMounts. + if _, ok := pod.Annotations[annotationConsulSidecarUserVolumeMount]; ok { + var volumeMount []corev1.VolumeMount + err := json.Unmarshal([]byte(pod.Annotations[annotationConsulSidecarUserVolumeMount]), &volumeMount) + if err != nil { + return corev1.Container{}, err + } + container.VolumeMounts = append(container.VolumeMounts, volumeMount...) + } + tproxyEnabled, err := transparentProxyEnabled(namespace, pod, w.EnableTransparentProxy) if err != nil { return corev1.Container{}, err diff --git a/control-plane/connect-inject/envoy_sidecar_test.go b/control-plane/connect-inject/envoy_sidecar_test.go index 7727db0fd4..4bd76d3b05 100644 --- a/control-plane/connect-inject/envoy_sidecar_test.go +++ b/control-plane/connect-inject/envoy_sidecar_test.go @@ -394,6 +394,32 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) { } } +func TestHandlerEnvoySidecar_UserVolumeMounts(t *testing.T) { + + h := MeshWebhook{ + ImageConsul: "hashicorp/consul:latest", + ImageEnvoy: "hashicorp/consul-k8s:latest", + } + + pod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationEnvoyExtraArgs: "--log-level debug --admin-address-path \"/tmp/consul/foo bar\"", + annotationConsulSidecarUserVolumeMount: "[{\"name\": \"tls-cert\", \"mountPath\": \"/custom/path\"}]", + }, + }, + } + volMount1 := corev1.VolumeMount{ + Name: "tls-cert", + MountPath: "/custom/path", + } + + c, err := h.envoySidecar(testNS, pod, multiPortInfo{}) + require.NoError(t, err) + require.Len(t, c.VolumeMounts, 2) + require.Equal(t, c.VolumeMounts[1], volMount1) +} + func TestHandlerEnvoySidecar_Resources(t *testing.T) { mem1 := resource.MustParse("100Mi") mem2 := resource.MustParse("200Mi") diff --git a/control-plane/connect-inject/mesh_webhook.go b/control-plane/connect-inject/mesh_webhook.go index 73b56704fb..b5951f5cdd 100644 --- a/control-plane/connect-inject/mesh_webhook.go +++ b/control-plane/connect-inject/mesh_webhook.go @@ -328,6 +328,16 @@ func (w *MeshWebhook) Handle(ctx context.Context, req admission.Request) admissi } } + // Optionally add any volumes that are to be used by the envoy sidecar. + if _, ok := pod.Annotations[annotationConsulSidecarUserVolume]; ok { + var userVolumes []corev1.Volume + err := json.Unmarshal([]byte(pod.Annotations[annotationConsulSidecarUserVolume]), &userVolumes) + if err != nil { + return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error unmarshalling sidecar user volumes: %s", err)) + } + pod.Spec.Volumes = append(pod.Spec.Volumes, userVolumes...) + } + // Now that the consul-sidecar no longer needs to re-register services periodically // (that functionality lives in the endpoints-controller), // we only need the consul sidecar to run the metrics merging server. From 65666ad09ff51054903aa331c41eb206ca68e781 Mon Sep 17 00:00:00 2001 From: Kyle Schochenmaier Date: Wed, 29 Jun 2022 19:36:10 -0500 Subject: [PATCH 2/4] add tests for the webhook --- .../connect-inject/envoy_sidecar_test.go | 75 ++++++++++++---- control-plane/connect-inject/mesh_webhook.go | 20 ++--- .../connect-inject/mesh_webhook_test.go | 88 ++++++++++++++++++- 3 files changed, 153 insertions(+), 30 deletions(-) diff --git a/control-plane/connect-inject/envoy_sidecar_test.go b/control-plane/connect-inject/envoy_sidecar_test.go index 4bd76d3b05..79d11966c9 100644 --- a/control-plane/connect-inject/envoy_sidecar_test.go +++ b/control-plane/connect-inject/envoy_sidecar_test.go @@ -395,29 +395,66 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) { } func TestHandlerEnvoySidecar_UserVolumeMounts(t *testing.T) { - - h := MeshWebhook{ - ImageConsul: "hashicorp/consul:latest", - ImageEnvoy: "hashicorp/consul-k8s:latest", - } - - pod := corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationEnvoyExtraArgs: "--log-level debug --admin-address-path \"/tmp/consul/foo bar\"", - annotationConsulSidecarUserVolumeMount: "[{\"name\": \"tls-cert\", \"mountPath\": \"/custom/path\"}]", + cases := []struct { + name string + pod corev1.Pod + expectedContainerVolumeMounts []corev1.VolumeMount + expErr string + }{ + { + name: "able to set a sidecar container volume mount via annotation", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationEnvoyExtraArgs: "--log-level debug --admin-address-path \"/tmp/consul/foo bar\"", + annotationConsulSidecarUserVolumeMount: "[{\"name\": \"tls-cert\", \"mountPath\": \"/custom/path\"}, {\"name\": \"tls-ca\", \"mountPath\": \"/custom/path2\"}]", + }, + }, + }, + expectedContainerVolumeMounts: []corev1.VolumeMount{ + corev1.VolumeMount{ + Name: "consul-connect-inject-data", + MountPath: "/consul/connect-inject", + }, + corev1.VolumeMount{ + Name: "tls-cert", + MountPath: "/custom/path", + }, + corev1.VolumeMount{ + Name: "tls-ca", + MountPath: "/custom/path2", + }, }, }, + { + name: "invalid annotation results in error", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationEnvoyExtraArgs: "--log-level debug --admin-address-path \"/tmp/consul/foo bar\"", + annotationConsulSidecarUserVolumeMount: "[abcdefg]", + }, + }, + }, + expErr: "invalid character 'a' looking ", + }, } - volMount1 := corev1.VolumeMount{ - Name: "tls-cert", - MountPath: "/custom/path", + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := MeshWebhook{ + ImageConsul: "hashicorp/consul:latest", + ImageEnvoy: "hashicorp/consul-k8s:latest", + } + c, err := h.envoySidecar(testNS, tc.pod, multiPortInfo{}) + if tc.expErr == "" { + require.NoError(t, err) + require.Equal(t, tc.expectedContainerVolumeMounts, c.VolumeMounts) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expErr) + } + }) } - - c, err := h.envoySidecar(testNS, pod, multiPortInfo{}) - require.NoError(t, err) - require.Len(t, c.VolumeMounts, 2) - require.Equal(t, c.VolumeMounts[1], volMount1) } func TestHandlerEnvoySidecar_Resources(t *testing.T) { diff --git a/control-plane/connect-inject/mesh_webhook.go b/control-plane/connect-inject/mesh_webhook.go index b5951f5cdd..cd1e49cd25 100644 --- a/control-plane/connect-inject/mesh_webhook.go +++ b/control-plane/connect-inject/mesh_webhook.go @@ -220,6 +220,16 @@ func (w *MeshWebhook) Handle(ctx context.Context, req admission.Request) admissi // Optionally mount data volume to other containers w.injectVolumeMount(pod) + // Optionally add any volumes that are to be used by the envoy sidecar. + if _, ok := pod.Annotations[annotationConsulSidecarUserVolume]; ok { + var userVolumes []corev1.Volume + err := json.Unmarshal([]byte(pod.Annotations[annotationConsulSidecarUserVolume]), &userVolumes) + if err != nil { + return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error unmarshalling sidecar user volumes: %s", err)) + } + pod.Spec.Volumes = append(pod.Spec.Volumes, userVolumes...) + } + // Add the upstream services as environment variables for easy // service discovery. containerEnvVars := w.containerEnvVars(pod) @@ -328,16 +338,6 @@ func (w *MeshWebhook) Handle(ctx context.Context, req admission.Request) admissi } } - // Optionally add any volumes that are to be used by the envoy sidecar. - if _, ok := pod.Annotations[annotationConsulSidecarUserVolume]; ok { - var userVolumes []corev1.Volume - err := json.Unmarshal([]byte(pod.Annotations[annotationConsulSidecarUserVolume]), &userVolumes) - if err != nil { - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error unmarshalling sidecar user volumes: %s", err)) - } - pod.Spec.Volumes = append(pod.Spec.Volumes, userVolumes...) - } - // Now that the consul-sidecar no longer needs to re-register services periodically // (that functionality lives in the endpoints-controller), // we only need the consul sidecar to run the metrics merging server. diff --git a/control-plane/connect-inject/mesh_webhook_test.go b/control-plane/connect-inject/mesh_webhook_test.go index bbebd48c30..8b37462506 100644 --- a/control-plane/connect-inject/mesh_webhook_test.go +++ b/control-plane/connect-inject/mesh_webhook_test.go @@ -384,7 +384,93 @@ func TestHandlerHandle(t *testing.T) { }, }, }, - + { + "pod with sidecar volume mount annotation", + MeshWebhook{ + Log: logrtest.TestLogger{T: t}, + AllowK8sNamespacesSet: mapset.NewSetWith("*"), + DenyK8sNamespacesSet: mapset.NewSet(), + decoder: decoder, + Clientset: defaultTestClientWithNamespace(), + }, + admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Namespace: namespaces.DefaultNamespace, + Object: encodeRaw(t, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationConsulSidecarUserVolume: "[{\"name\":\"bbb\",\"csi\":{\"driver\":\"bob\"}}]", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "web", + }, + }, + }, + }), + }, + }, + "", + []jsonpatch.Operation{ + { + Operation: "add", + Path: "/spec/volumes", + }, + { + Operation: "add", + Path: "/spec/containers/1", + }, + { + Operation: "add", + Path: "/spec/initContainers", + }, + { + Operation: "add", + Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), + }, + { + Operation: "add", + Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), + }, + { + Operation: "add", + Path: "/metadata/labels", + }, + }, + }, + { + "pod with sidecar invalid volume mount annotation", + MeshWebhook{ + Log: logrtest.TestLogger{T: t}, + AllowK8sNamespacesSet: mapset.NewSetWith("*"), + DenyK8sNamespacesSet: mapset.NewSet(), + decoder: decoder, + Clientset: defaultTestClientWithNamespace(), + }, + admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Namespace: namespaces.DefaultNamespace, + Object: encodeRaw(t, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationConsulSidecarUserVolume: "[a]", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "web", + }, + }, + }, + }), + }, + }, + "error unmarshalling sidecar user volumes: invalid character 'a' looking for beginning of value", + nil, + }, { "pod with service annotation", MeshWebhook{ From 855bbeb9119eb70d827d6150a50b151342bedb2c Mon Sep 17 00:00:00 2001 From: Kyle Schochenmaier Date: Wed, 29 Jun 2022 19:45:17 -0500 Subject: [PATCH 3/4] gofmt --- control-plane/connect-inject/envoy_sidecar_test.go | 6 +++--- control-plane/connect-inject/mesh_webhook_ent_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/control-plane/connect-inject/envoy_sidecar_test.go b/control-plane/connect-inject/envoy_sidecar_test.go index 79d11966c9..8baa0d30b9 100644 --- a/control-plane/connect-inject/envoy_sidecar_test.go +++ b/control-plane/connect-inject/envoy_sidecar_test.go @@ -412,15 +412,15 @@ func TestHandlerEnvoySidecar_UserVolumeMounts(t *testing.T) { }, }, expectedContainerVolumeMounts: []corev1.VolumeMount{ - corev1.VolumeMount{ + { Name: "consul-connect-inject-data", MountPath: "/consul/connect-inject", }, - corev1.VolumeMount{ + { Name: "tls-cert", MountPath: "/custom/path", }, - corev1.VolumeMount{ + { Name: "tls-ca", MountPath: "/custom/path2", }, diff --git a/control-plane/connect-inject/mesh_webhook_ent_test.go b/control-plane/connect-inject/mesh_webhook_ent_test.go index 5faaf4e0c1..7a34ee3d73 100644 --- a/control-plane/connect-inject/mesh_webhook_ent_test.go +++ b/control-plane/connect-inject/mesh_webhook_ent_test.go @@ -30,7 +30,7 @@ func TestHandler_MutateWithNamespaces(t *testing.T) { basicSpec := corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "web", }, }, @@ -285,7 +285,7 @@ func TestHandler_MutateWithNamespaces(t *testing.T) { func TestHandler_MutateWithNamespaces_ACLs(t *testing.T) { basicSpec := corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "web", }, }, From 084da01a0db5c33ae8006908713a77288975a0d9 Mon Sep 17 00:00:00 2001 From: Kyle Schochenmaier Date: Wed, 29 Jun 2022 19:58:14 -0500 Subject: [PATCH 4/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index efd3bb6015..4ed3f02f5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ FEATURES: 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)] + * Added annotations `consul.hashicorp.com/consul-sidecar-user-volume` and `consul.hashicorp.com/consul-sidecar-user-volume-mount` for attaching Volumes and VolumeMounts to the Envoy sidecar. Both should be JSON objects. [[GH-1315](https://github.com/hashicorp/consul-k8s/pull/1315)] * Helm * Added `connectInject.annotations` and `syncCatalog.annotations` values for setting annotations on connect inject and sync catalog deployments. [[GH-775](https://github.com/hashicorp/consul-k8s/pull/775)]