Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add annotations to support specifying userVolumes and userVolumeMounts for the envoy sidecar #1315

Merged
merged 4 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)]

Expand Down
4 changes: 4 additions & 0 deletions control-plane/connect-inject/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clarifying q for my understanding: these will reference a volume that exists on the pod? and then mount that to the sidecar container as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct!
The user will need to either:

  • specify both annotations, which would add a volume to the pod + mount to the sidecar.
  • specify only the volumeMount annotation and already have the volume defined in the pod's spec.

annotationConsulSidecarUserVolumeMount = "consul.hashicorp.com/consul-sidecar-user-volume-mount"

// annotations for sidecar concurrency.
annotationEnvoyProxyConcurrency = "consul.hashicorp.com/consul-envoy-proxy-concurrency"

Expand Down
11 changes: 11 additions & 0 deletions control-plane/connect-inject/envoy_sidecar.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package connectinject

import (
"encoding/json"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -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
Expand Down
63 changes: 63 additions & 0 deletions control-plane/connect-inject/envoy_sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,69 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
}
}

func TestHandlerEnvoySidecar_UserVolumeMounts(t *testing.T) {
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{
{
Name: "consul-connect-inject-data",
MountPath: "/consul/connect-inject",
},
{
Name: "tls-cert",
MountPath: "/custom/path",
},
{
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 ",
},
}
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)
}
})
}
}

func TestHandlerEnvoySidecar_Resources(t *testing.T) {
mem1 := resource.MustParse("100Mi")
mem2 := resource.MustParse("200Mi")
Expand Down
10 changes: 10 additions & 0 deletions control-plane/connect-inject/mesh_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions control-plane/connect-inject/mesh_webhook_ent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestHandler_MutateWithNamespaces(t *testing.T) {

basicSpec := corev1.PodSpec{
Containers: []corev1.Container{
corev1.Container{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linter was forcing this.

{
Name: "web",
},
},
Expand Down Expand Up @@ -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",
},
},
Expand Down
88 changes: 87 additions & 1 deletion control-plane/connect-inject/mesh_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down