From af22452427d077f65f63e0a50b684eeed11ccbd4 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Tue, 18 Feb 2020 14:45:17 +0000 Subject: [PATCH 01/14] Add projected ServiceAccount token support --- agent-inject/agent/agent.go | 9 +++++++++ agent-inject/agent/annotations.go | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index 6117dfe3..295aa2ee 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -578,6 +578,15 @@ func (a *Agent) Validate() error { func serviceaccount(pod *corev1.Pod) (string, string) { var serviceAccountName, serviceAccountPath string + if volumeName := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountVolumeName]; volumeName != "" { + for _, container := range pod.Spec.Containers { + for _, volumes := range container.VolumeMounts { + if volumes.Name == volumeName { + return volumes.Name, volumes.MountPath + } + } + } + } for _, container := range pod.Spec.Containers { for _, volumes := range container.VolumeMounts { if strings.Contains(volumes.MountPath, "serviceaccount") { diff --git a/agent-inject/agent/annotations.go b/agent-inject/agent/annotations.go index 5dd0eb46..cf2c5833 100644 --- a/agent-inject/agent/annotations.go +++ b/agent-inject/agent/annotations.go @@ -138,6 +138,10 @@ const ( // and gid) is set on the injected Vault Agent containers AnnotationAgentSetSecurityContext = "vault.hashicorp.com/agent-set-security-context" + // AnnotationAgentServiceAccountTokenVolumeName is the optional name of a volume containing a + // projected service account token + AnnotationAgentServiceAccountTokenVolumeName = "vault.hashicorp.com/agent-service-account-token-volume-name" + // AnnotationVaultService is the name of the Vault server. This can be overridden by the // user but will be set by a flag on the deployment. AnnotationVaultService = "vault.hashicorp.com/service" From 4bab1212dc7845a007cad2207f9a2041e517b047 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Sat, 21 Mar 2020 07:36:16 +0000 Subject: [PATCH 02/14] Add volume path annotation --- agent-inject/agent/agent.go | 2 +- agent-inject/agent/annotations.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index 295aa2ee..c2245339 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -578,7 +578,7 @@ func (a *Agent) Validate() error { func serviceaccount(pod *corev1.Pod) (string, string) { var serviceAccountName, serviceAccountPath string - if volumeName := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountVolumeName]; volumeName != "" { + if volumeName := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName]; volumeName != "" { for _, container := range pod.Spec.Containers { for _, volumes := range container.VolumeMounts { if volumes.Name == volumeName { diff --git a/agent-inject/agent/annotations.go b/agent-inject/agent/annotations.go index cf2c5833..8dfc1ac6 100644 --- a/agent-inject/agent/annotations.go +++ b/agent-inject/agent/annotations.go @@ -142,6 +142,11 @@ const ( // projected service account token AnnotationAgentServiceAccountTokenVolumeName = "vault.hashicorp.com/agent-service-account-token-volume-name" + // AnnotationAgentServiceAccountTokenVolumePath is the optional path to a projected service + // account token within a volume named by + // "vault.hashicorp.com/agent-service-account-token-volume-name" - the default path is "token" + AnnotationAgentServiceAccountTokenVolumePath = "vault.hashicorp.com/agent-service-account-token-volume-path" + // AnnotationVaultService is the name of the Vault server. This can be overridden by the // user but will be set by a flag on the deployment. AnnotationVaultService = "vault.hashicorp.com/service" @@ -346,6 +351,14 @@ func Init(pod *corev1.Pod, cfg AgentConfig) error { pod.ObjectMeta.Annotations[AnnotationVaultLogFormat] = DefaultAgentLogFormat } + if _, ok := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName]; !ok { + pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName] = "" + } + + if _, ok := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumePath]; !ok { + pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumePath] = "token" + } + if _, securityContextIsSet = pod.ObjectMeta.Annotations[AnnotationAgentSetSecurityContext]; !securityContextIsSet { pod.ObjectMeta.Annotations[AnnotationAgentSetSecurityContext] = strconv.FormatBool(cfg.SetSecurityContext) } From 643c9d9b5e3d1436b1f7728c196c0b14baf7c780 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Sat, 21 Mar 2020 08:24:03 +0000 Subject: [PATCH 03/14] Implement projected service account tokens fully --- agent-inject/agent/agent.go | 87 ++++++++++----- agent-inject/agent/agent_test.go | 109 ++++++++++++------- agent-inject/agent/annotations.go | 8 +- agent-inject/agent/container_init_sidecar.go | 7 +- agent-inject/agent/container_sidecar.go | 7 +- agent-inject/agent/container_sidecar_test.go | 6 +- 6 files changed, 143 insertions(+), 81 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index c2245339..4efb6985 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -29,6 +29,7 @@ const ( DefaultAgentCacheListenerPort = "8200" DefaultAgentCacheExitOnErr = false DefaultAgentUseLeaderElector = false + DefaultServiceAccountPath = "/var/run/secrets/vault.hashicorp.com/serviceaccount" ) // Agent is the top level structure holding all the @@ -93,14 +94,10 @@ type Agent struct { //found, and the unique name of the secret which will be used for the filename. Secrets []*Secret - // ServiceAccountName is the Kubernetes service account name for the pod. - // This is used when we mount the service account to the Vault Agent container(s). - ServiceAccountName string - - // ServiceAccountPath is the path on disk where the service account JWT - // can be located. This is used when we mount the service account to the - // Vault Agent container(s). - ServiceAccountPath string + // ServiceAccountTokenVolume holds details of a volume mount for a Kubernetes service account + // token for the pod. This is used when we mount the service account to the Vault Agent + // container(s). + ServiceAccountTokenVolume *ServiceAccountTokenVolume // Status is the current injection status. The only status considered is "injected", // which prevents further mutations. A user can patch this annotation to force a new @@ -140,6 +137,17 @@ type Agent struct { CopyVolumeMounts string } +type ServiceAccountTokenVolume struct { + // The name of the volume + Name string + + // The mount path of the volume within vault agent containers + MountPath string + + // The path to the JWT token within the volume + TokenPath string +} + type Secret struct { // Name of the secret used to identify other annotation directives, and used // as the filename for the rendered secret file (unless FilePathAndName is @@ -252,22 +260,25 @@ func New(pod *corev1.Pod, patches []*jsonpatch.JsonPatchOperation) (*Agent, erro saName, saPath := serviceaccount(pod) agent := &Agent{ - Annotations: pod.Annotations, - ConfigMapName: pod.Annotations[AnnotationAgentConfigMap], - ImageName: pod.Annotations[AnnotationAgentImage], - DefaultTemplate: pod.Annotations[AnnotationAgentInjectDefaultTemplate], - LimitsCPU: pod.Annotations[AnnotationAgentLimitsCPU], - LimitsMem: pod.Annotations[AnnotationAgentLimitsMem], - Namespace: pod.Annotations[AnnotationAgentRequestNamespace], - Patches: patches, - Pod: pod, - RequestsCPU: pod.Annotations[AnnotationAgentRequestsCPU], - RequestsMem: pod.Annotations[AnnotationAgentRequestsMem], - ServiceAccountName: saName, - ServiceAccountPath: saPath, - Status: pod.Annotations[AnnotationAgentStatus], - ExtraSecret: pod.Annotations[AnnotationAgentExtraSecret], - CopyVolumeMounts: pod.Annotations[AnnotationAgentCopyVolumeMounts], + Annotations: pod.Annotations, + ConfigMapName: pod.Annotations[AnnotationAgentConfigMap], + ImageName: pod.Annotations[AnnotationAgentImage], + DefaultTemplate: pod.Annotations[AnnotationAgentInjectDefaultTemplate], + LimitsCPU: pod.Annotations[AnnotationAgentLimitsCPU], + LimitsMem: pod.Annotations[AnnotationAgentLimitsMem], + Namespace: pod.Annotations[AnnotationAgentRequestNamespace], + Patches: patches, + Pod: pod, + RequestsCPU: pod.Annotations[AnnotationAgentRequestsCPU], + RequestsMem: pod.Annotations[AnnotationAgentRequestsMem], + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: saName, + MountPath: saPath, + TokenPath: pod.Annotations[AnnotationAgentServiceAccountTokenVolumePath], + }, + Status: pod.Annotations[AnnotationAgentStatus], + ExtraSecret: pod.Annotations[AnnotationAgentExtraSecret], + CopyVolumeMounts: pod.Annotations[AnnotationAgentCopyVolumeMounts], Vault: Vault{ Address: pod.Annotations[AnnotationVaultService], ProxyAddress: pod.Annotations[AnnotationProxyAddress], @@ -547,7 +558,10 @@ func (a *Agent) Validate() error { return errors.New("namespace missing from request") } - if a.ServiceAccountName == "" || a.ServiceAccountPath == "" { + if a.ServiceAccountTokenVolume == nil || + (a.ServiceAccountTokenVolume.Name == "" || + a.ServiceAccountTokenVolume.MountPath == "" || + a.ServiceAccountTokenVolume.TokenPath == "") { return errors.New("no service account name or path found") } @@ -577,16 +591,28 @@ func (a *Agent) Validate() error { } func serviceaccount(pod *corev1.Pod) (string, string) { - var serviceAccountName, serviceAccountPath string + + // Attempt to find existing mount point of named volume and copy mount path if volumeName := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName]; volumeName != "" { for _, container := range pod.Spec.Containers { - for _, volumes := range container.VolumeMounts { - if volumes.Name == volumeName { - return volumes.Name, volumes.MountPath + for _, volumeMount := range container.VolumeMounts { + if volumeMount.Name == volumeName { + return volumeMount.Name, volumeMount.MountPath } } } } + + // Otherwise, check the volume exists and fallback to `DefaultServiceAccountPath` + if volumeName := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName]; volumeName != "" { + for _, volume := range pod.Spec.Volumes { + if volume.Name == volumeName { + return volume.Name, DefaultServiceAccountPath + } + } + } + + // Fallback to searching for normal service account token for _, container := range pod.Spec.Containers { for _, volumes := range container.VolumeMounts { if strings.Contains(volumes.MountPath, "serviceaccount") { @@ -594,7 +620,8 @@ func serviceaccount(pod *corev1.Pod) (string, string) { } } } - return serviceAccountName, serviceAccountPath + + return "", "" } func (a *Agent) vaultCliFlags() []string { diff --git a/agent-inject/agent/agent_test.go b/agent-inject/agent/agent_test.go index 5972df49..7a750c1e 100644 --- a/agent-inject/agent/agent_test.go +++ b/agent-inject/agent/agent_test.go @@ -66,19 +66,25 @@ func TestValidate(t *testing.T) { }{ { Agent{ - Namespace: "test", - ServiceAccountPath: "foobar", - ServiceAccountName: "foobar", - ImageName: "test", - ConfigMapName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "test", + ConfigMapName: "test", }, true, }, { Agent{ - Namespace: "test", - ServiceAccountPath: "foobar", - ServiceAccountName: "foobar", - ImageName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "test", Vault: Vault{ Role: "test", Address: "https://foobar.com:8200", @@ -89,46 +95,61 @@ func TestValidate(t *testing.T) { }, { Agent{ - Namespace: "", - ServiceAccountPath: "foobar", - ServiceAccountName: "foobar", - ImageName: "test", - ConfigMapName: "test", + Namespace: "", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "test", + ConfigMapName: "test", }, false, }, { Agent{ - Namespace: "test", - ServiceAccountPath: "", - ServiceAccountName: "foobar", - ImageName: "test", - ConfigMapName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "", + TokenPath: "foobar", + }, + ImageName: "test", + ConfigMapName: "test", }, false, }, { Agent{ - Namespace: "test", - ServiceAccountPath: "foobar", - ServiceAccountName: "", - ImageName: "test", - ConfigMapName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "test", + ConfigMapName: "test", }, false, }, { Agent{ - Namespace: "test", - ServiceAccountPath: "foobar", - ServiceAccountName: "foobar", - ImageName: "", - ConfigMapName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "", + ConfigMapName: "test", }, false, }, { Agent{ - Namespace: "test", - ServiceAccountPath: "foobar", - ServiceAccountName: "foobar", - ImageName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "test", Vault: Vault{ Role: "", Address: "https://foobar.com:8200", @@ -138,10 +159,13 @@ func TestValidate(t *testing.T) { }, { Agent{ - Namespace: "test", - ServiceAccountPath: "foobar", - ServiceAccountName: "foobar", - ImageName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "test", Vault: Vault{ Role: "test", Address: "", @@ -151,10 +175,13 @@ func TestValidate(t *testing.T) { }, { Agent{ - Namespace: "test", - ServiceAccountPath: "foobar", - ServiceAccountName: "foobar", - ImageName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "test", Vault: Vault{ Role: "test", Address: "https://foobar.com:8200", diff --git a/agent-inject/agent/annotations.go b/agent-inject/agent/annotations.go index 8dfc1ac6..214d486f 100644 --- a/agent-inject/agent/annotations.go +++ b/agent-inject/agent/annotations.go @@ -139,12 +139,12 @@ const ( AnnotationAgentSetSecurityContext = "vault.hashicorp.com/agent-set-security-context" // AnnotationAgentServiceAccountTokenVolumeName is the optional name of a volume containing a - // projected service account token + // service account token AnnotationAgentServiceAccountTokenVolumeName = "vault.hashicorp.com/agent-service-account-token-volume-name" - // AnnotationAgentServiceAccountTokenVolumePath is the optional path to a projected service - // account token within a volume named by - // "vault.hashicorp.com/agent-service-account-token-volume-name" - the default path is "token" + // AnnotationAgentServiceAccountTokenVolumePath is the optional path to a service account token + // within a volume named by "vault.hashicorp.com/agent-service-account-token-volume-name" or + // discovered by searching for service account volume mounts - the default is "token" AnnotationAgentServiceAccountTokenVolumePath = "vault.hashicorp.com/agent-service-account-token-volume-path" // AnnotationVaultService is the name of the Vault server. This can be overridden by the diff --git a/agent-inject/agent/container_init_sidecar.go b/agent-inject/agent/container_init_sidecar.go index af5c7dee..57ad171f 100644 --- a/agent-inject/agent/container_init_sidecar.go +++ b/agent-inject/agent/container_init_sidecar.go @@ -2,6 +2,7 @@ package agent import ( "fmt" + "path" corev1 "k8s.io/api/core/v1" ) @@ -19,8 +20,10 @@ func (a *Agent) ContainerInitSidecar() (corev1.Container, error) { ReadOnly: false, }, { - Name: a.ServiceAccountName, - MountPath: a.ServiceAccountPath, + Name: a.ServiceAccountTokenVolume.Name, + // Only mount the token + MountPath: path.Join(a.ServiceAccountTokenVolume.MountPath, a.ServiceAccountTokenVolume.TokenPath), + SubPath: a.ServiceAccountTokenVolume.TokenPath, ReadOnly: true, }, } diff --git a/agent-inject/agent/container_sidecar.go b/agent-inject/agent/container_sidecar.go index 09e9fd3c..af4dd0e0 100644 --- a/agent-inject/agent/container_sidecar.go +++ b/agent-inject/agent/container_sidecar.go @@ -2,6 +2,7 @@ package agent import ( "fmt" + "path" "strings" "github.com/hashicorp/vault/sdk/helper/pointerutil" @@ -26,8 +27,10 @@ const ( func (a *Agent) ContainerSidecar() (corev1.Container, error) { volumeMounts := []corev1.VolumeMount{ { - Name: a.ServiceAccountName, - MountPath: a.ServiceAccountPath, + Name: a.ServiceAccountTokenVolume.Name, + // Only mount the token + MountPath: path.Join(a.ServiceAccountTokenVolume.MountPath, a.ServiceAccountTokenVolume.TokenPath), + SubPath: a.ServiceAccountTokenVolume.TokenPath, ReadOnly: true, }, { diff --git a/agent-inject/agent/container_sidecar_test.go b/agent-inject/agent/container_sidecar_test.go index 63b4efd4..30c55b53 100644 --- a/agent-inject/agent/container_sidecar_test.go +++ b/agent-inject/agent/container_sidecar_test.go @@ -2,6 +2,7 @@ package agent import ( "fmt" + "path" "strconv" "testing" @@ -65,8 +66,9 @@ func TestContainerSidecarVolume(t *testing.T) { t, []corev1.VolumeMount{ corev1.VolumeMount{ - Name: agent.ServiceAccountName, - MountPath: agent.ServiceAccountPath, + Name: agent.ServiceAccountTokenVolume.Name, + MountPath: path.Join(agent.ServiceAccountTokenVolume.MountPath, agent.ServiceAccountTokenVolume.TokenPath), + SubPath: agent.ServiceAccountTokenVolume.TokenPath, ReadOnly: true, }, corev1.VolumeMount{ From ea88357bafa6f6355e1f7dd0d50612115927e7c5 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Sat, 21 Mar 2020 08:36:58 +0000 Subject: [PATCH 04/14] Fix tests --- agent-inject/agent/agent.go | 9 ++++----- agent-inject/agent/agent_test.go | 34 +++++++++++++++++++++----------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index 4efb6985..a180c94e 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -558,11 +558,10 @@ func (a *Agent) Validate() error { return errors.New("namespace missing from request") } - if a.ServiceAccountTokenVolume == nil || - (a.ServiceAccountTokenVolume.Name == "" || - a.ServiceAccountTokenVolume.MountPath == "" || - a.ServiceAccountTokenVolume.TokenPath == "") { - return errors.New("no service account name or path found") + if a.ServiceAccountTokenVolume.Name == "" || + a.ServiceAccountTokenVolume.MountPath == "" || + a.ServiceAccountTokenVolume.TokenPath == "" { + return errors.New("no service account token volume name, mount path or token path found") } if a.ImageName == "" { diff --git a/agent-inject/agent/agent_test.go b/agent-inject/agent/agent_test.go index 7a750c1e..0d5e2f53 100644 --- a/agent-inject/agent/agent_test.go +++ b/agent-inject/agent/agent_test.go @@ -67,7 +67,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -79,7 +79,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -96,7 +96,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -108,7 +108,19 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + Name: "", + MountPath: "foobar", + TokenPath: "foobar", + }, + ImageName: "test", + ConfigMapName: "test", + }, false, + }, + { + Agent{ + Namespace: "test", + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ Name: "foobar", MountPath: "", TokenPath: "foobar", @@ -120,10 +132,10 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ - Name: "", + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + Name: "foobar", MountPath: "foobar", - TokenPath: "foobar", + TokenPath: "", }, ImageName: "test", ConfigMapName: "test", @@ -132,7 +144,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -144,7 +156,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -160,7 +172,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -176,7 +188,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", From c7a0adbaecbb9b41c60428f0ed0dda71c190d921 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Fri, 24 Jul 2020 11:52:43 +0100 Subject: [PATCH 05/14] Fix token mounting --- agent-inject/agent/container_init_sidecar.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/agent-inject/agent/container_init_sidecar.go b/agent-inject/agent/container_init_sidecar.go index 57ad171f..42359d4b 100644 --- a/agent-inject/agent/container_init_sidecar.go +++ b/agent-inject/agent/container_init_sidecar.go @@ -2,7 +2,6 @@ package agent import ( "fmt" - "path" corev1 "k8s.io/api/core/v1" ) @@ -20,10 +19,8 @@ func (a *Agent) ContainerInitSidecar() (corev1.Container, error) { ReadOnly: false, }, { - Name: a.ServiceAccountTokenVolume.Name, - // Only mount the token - MountPath: path.Join(a.ServiceAccountTokenVolume.MountPath, a.ServiceAccountTokenVolume.TokenPath), - SubPath: a.ServiceAccountTokenVolume.TokenPath, + Name: a.ServiceAccountTokenVolume.Name, + MountPath: a.ServiceAccountTokenVolume.MountPath, ReadOnly: true, }, } From 59dd8544ef1899f51b07b65c070c6918d89e3fb4 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Sat, 25 Jul 2020 21:21:03 +0100 Subject: [PATCH 06/14] Fix token mounting --- agent-inject/agent/container_sidecar.go | 7 ++----- go.sum | 2 ++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/agent-inject/agent/container_sidecar.go b/agent-inject/agent/container_sidecar.go index af4dd0e0..3b6b4458 100644 --- a/agent-inject/agent/container_sidecar.go +++ b/agent-inject/agent/container_sidecar.go @@ -2,7 +2,6 @@ package agent import ( "fmt" - "path" "strings" "github.com/hashicorp/vault/sdk/helper/pointerutil" @@ -27,10 +26,8 @@ const ( func (a *Agent) ContainerSidecar() (corev1.Container, error) { volumeMounts := []corev1.VolumeMount{ { - Name: a.ServiceAccountTokenVolume.Name, - // Only mount the token - MountPath: path.Join(a.ServiceAccountTokenVolume.MountPath, a.ServiceAccountTokenVolume.TokenPath), - SubPath: a.ServiceAccountTokenVolume.TokenPath, + Name: a.ServiceAccountTokenVolume.Name, + MountPath: a.ServiceAccountTokenVolume.MountPath, ReadOnly: true, }, { diff --git a/go.sum b/go.sum index 516dd45f..d4d982f1 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,10 @@ +cloud.google.com/go v0.26.0 h1:e0WKqKTd5BnrG8aKH3J3h+QvEIQtSUcf2n5UZ5ZgLtQ= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/Azure/azure-sdk-for-go v16.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8= github.com/Azure/go-autorest v10.7.0+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24= github.com/Azure/go-autorest v10.15.3+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24= +github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/DataDog/datadog-go v0.0.0-20160329135253-cc2f4770f4d6/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ= github.com/DataDog/datadog-go v3.2.0+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ= From cb4f9697934c28a66d0694e886da7aea283f3c60 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Sat, 25 Jul 2020 21:21:54 +0100 Subject: [PATCH 07/14] Fix token mounting --- agent-inject/agent/container_sidecar_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/agent-inject/agent/container_sidecar_test.go b/agent-inject/agent/container_sidecar_test.go index 30c55b53..11b57477 100644 --- a/agent-inject/agent/container_sidecar_test.go +++ b/agent-inject/agent/container_sidecar_test.go @@ -2,7 +2,6 @@ package agent import ( "fmt" - "path" "strconv" "testing" @@ -67,8 +66,7 @@ func TestContainerSidecarVolume(t *testing.T) { []corev1.VolumeMount{ corev1.VolumeMount{ Name: agent.ServiceAccountTokenVolume.Name, - MountPath: path.Join(agent.ServiceAccountTokenVolume.MountPath, agent.ServiceAccountTokenVolume.TokenPath), - SubPath: agent.ServiceAccountTokenVolume.TokenPath, + MountPath: agent.ServiceAccountTokenVolume.MountPath, ReadOnly: true, }, corev1.VolumeMount{ From 585f9cf516be6548c3e121d734a9e11081dcd7a3 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Mon, 27 Jul 2020 21:40:03 +0100 Subject: [PATCH 08/14] Deduplicate volume search logic --- agent-inject/agent/agent.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index a180c94e..ef502461 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -591,8 +591,8 @@ func (a *Agent) Validate() error { func serviceaccount(pod *corev1.Pod) (string, string) { - // Attempt to find existing mount point of named volume and copy mount path if volumeName := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName]; volumeName != "" { + // Attempt to find existing mount point of named volume and copy mount path for _, container := range pod.Spec.Containers { for _, volumeMount := range container.VolumeMounts { if volumeMount.Name == volumeName { @@ -600,10 +600,7 @@ func serviceaccount(pod *corev1.Pod) (string, string) { } } } - } - - // Otherwise, check the volume exists and fallback to `DefaultServiceAccountPath` - if volumeName := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName]; volumeName != "" { + // Otherwise, check the volume exists and fallback to `DefaultServiceAccountPath` for _, volume := range pod.Spec.Volumes { if volume.Name == volumeName { return volume.Name, DefaultServiceAccountPath From e984048245c0ddbe87ebd43e7df97dd7dc30cb12 Mon Sep 17 00:00:00 2001 From: Luke Addison Date: Sun, 11 Apr 2021 16:09:45 +0100 Subject: [PATCH 09/14] Rebase --- agent-inject/agent/annotations.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/agent-inject/agent/annotations.go b/agent-inject/agent/annotations.go index 214d486f..28b88757 100644 --- a/agent-inject/agent/annotations.go +++ b/agent-inject/agent/annotations.go @@ -2,6 +2,7 @@ package agent import ( "fmt" + "path" "strconv" "strings" @@ -634,5 +635,9 @@ func (a *Agent) authConfig() map[string]interface{} { authConfig["role"] = a.Vault.Role } + if a.ServiceAccountTokenVolume.MountPath != "" && a.ServiceAccountTokenVolume.TokenPath != "" { + authConfig["token_path"] = path.Join(a.ServiceAccountTokenVolume.MountPath, a.ServiceAccountTokenVolume.TokenPath) + } + return authConfig } From e421828501d1d106fa7c8f4db34cb99150de149c Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Tue, 24 Aug 2021 11:19:01 -0700 Subject: [PATCH 10/14] Implement PR feedback Fixed tests and added more tests --- agent-inject/agent/agent.go | 90 ++++++---- agent-inject/agent/agent_test.go | 220 +++++++++++++++++++++++-- agent-inject/agent/annotations_test.go | 7 +- 3 files changed, 272 insertions(+), 45 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index ef502461..208029c7 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -29,7 +29,7 @@ const ( DefaultAgentCacheListenerPort = "8200" DefaultAgentCacheExitOnErr = false DefaultAgentUseLeaderElector = false - DefaultServiceAccountPath = "/var/run/secrets/vault.hashicorp.com/serviceaccount" + DefaultServiceAccountMount = "/var/run/secrets/vault.hashicorp.com/serviceaccount" ) // Agent is the top level structure holding all the @@ -257,28 +257,27 @@ type VaultAgentCache struct { // New creates a new instance of Agent by parsing all the Kubernetes annotations. func New(pod *corev1.Pod, patches []*jsonpatch.JsonPatchOperation) (*Agent, error) { - saName, saPath := serviceaccount(pod) + sa, err := serviceaccount(pod) + if err != nil { + return nil, err + } agent := &Agent{ - Annotations: pod.Annotations, - ConfigMapName: pod.Annotations[AnnotationAgentConfigMap], - ImageName: pod.Annotations[AnnotationAgentImage], - DefaultTemplate: pod.Annotations[AnnotationAgentInjectDefaultTemplate], - LimitsCPU: pod.Annotations[AnnotationAgentLimitsCPU], - LimitsMem: pod.Annotations[AnnotationAgentLimitsMem], - Namespace: pod.Annotations[AnnotationAgentRequestNamespace], - Patches: patches, - Pod: pod, - RequestsCPU: pod.Annotations[AnnotationAgentRequestsCPU], - RequestsMem: pod.Annotations[AnnotationAgentRequestsMem], - ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ - Name: saName, - MountPath: saPath, - TokenPath: pod.Annotations[AnnotationAgentServiceAccountTokenVolumePath], - }, - Status: pod.Annotations[AnnotationAgentStatus], - ExtraSecret: pod.Annotations[AnnotationAgentExtraSecret], - CopyVolumeMounts: pod.Annotations[AnnotationAgentCopyVolumeMounts], + Annotations: pod.Annotations, + ConfigMapName: pod.Annotations[AnnotationAgentConfigMap], + ImageName: pod.Annotations[AnnotationAgentImage], + DefaultTemplate: pod.Annotations[AnnotationAgentInjectDefaultTemplate], + LimitsCPU: pod.Annotations[AnnotationAgentLimitsCPU], + LimitsMem: pod.Annotations[AnnotationAgentLimitsMem], + Namespace: pod.Annotations[AnnotationAgentRequestNamespace], + Patches: patches, + Pod: pod, + RequestsCPU: pod.Annotations[AnnotationAgentRequestsCPU], + RequestsMem: pod.Annotations[AnnotationAgentRequestsMem], + ServiceAccountTokenVolume: sa, + Status: pod.Annotations[AnnotationAgentStatus], + ExtraSecret: pod.Annotations[AnnotationAgentExtraSecret], + CopyVolumeMounts: pod.Annotations[AnnotationAgentCopyVolumeMounts], Vault: Vault{ Address: pod.Annotations[AnnotationVaultService], ProxyAddress: pod.Annotations[AnnotationProxyAddress], @@ -299,7 +298,6 @@ func New(pod *corev1.Pod, patches []*jsonpatch.JsonPatchOperation) (*Agent, erro }, } - var err error agent.Secrets = agent.secrets() agent.Vault.AuthConfig = agent.authConfig() agent.Inject, err = agent.inject() @@ -589,35 +587,69 @@ func (a *Agent) Validate() error { return nil } -func serviceaccount(pod *corev1.Pod) (string, string) { - +func serviceaccount(pod *corev1.Pod) (*ServiceAccountTokenVolume, error) { if volumeName := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName]; volumeName != "" { // Attempt to find existing mount point of named volume and copy mount path for _, container := range pod.Spec.Containers { for _, volumeMount := range container.VolumeMounts { if volumeMount.Name == volumeName { - return volumeMount.Name, volumeMount.MountPath + return &ServiceAccountTokenVolume{ + Name: volumeMount.Name, + MountPath: volumeMount.MountPath, + TokenPath: getTokenPath(pod, volumeName), + }, nil } } } - // Otherwise, check the volume exists and fallback to `DefaultServiceAccountPath` + + // Otherwise, check the volume exists and fallback to `DefaultServiceAccountMount` for _, volume := range pod.Spec.Volumes { if volume.Name == volumeName { - return volume.Name, DefaultServiceAccountPath + return &ServiceAccountTokenVolume{ + Name: volume.Name, + MountPath: DefaultServiceAccountMount, + TokenPath: getTokenPathFromVolume(volume), + }, nil } } + + return nil, fmt.Errorf("failed to find service account volume %q", volumeName) } // Fallback to searching for normal service account token for _, container := range pod.Spec.Containers { for _, volumes := range container.VolumeMounts { if strings.Contains(volumes.MountPath, "serviceaccount") { - return volumes.Name, volumes.MountPath + return &ServiceAccountTokenVolume{ + Name: volumes.Name, + MountPath: volumes.MountPath, + TokenPath: "token", + }, nil } } } - return "", "" + return nil, fmt.Errorf("failed to find service account volume mount") +} + +func getTokenPath(pod *corev1.Pod, volumeName string) string { + for _, volume := range pod.Spec.Volumes { + if volume.Name == volumeName { + return getTokenPathFromVolume(volume) + } + } + return "" +} + +func getTokenPathFromVolume(volume corev1.Volume) string { + if volume.Projected != nil { + for _, source := range volume.Projected.Sources { + if source.ServiceAccountToken != nil { + return source.ServiceAccountToken.Path + } + } + } + return "token" } func (a *Agent) vaultCliFlags() []string { diff --git a/agent-inject/agent/agent_test.go b/agent-inject/agent/agent_test.go index 0d5e2f53..75931403 100644 --- a/agent-inject/agent/agent_test.go +++ b/agent-inject/agent/agent_test.go @@ -3,8 +3,10 @@ package agent import ( "testing" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func testPod(annotations map[string]string) *corev1.Pod { @@ -67,7 +69,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -79,7 +81,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -96,7 +98,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -108,7 +110,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "", MountPath: "foobar", TokenPath: "foobar", @@ -120,7 +122,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "", TokenPath: "foobar", @@ -132,7 +134,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "", @@ -144,7 +146,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -156,7 +158,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -172,7 +174,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -188,7 +190,7 @@ func TestValidate(t *testing.T) { { Agent{ Namespace: "test", - ServiceAccountTokenVolume: ServiceAccountTokenVolume{ + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ Name: "foobar", MountPath: "foobar", TokenPath: "foobar", @@ -204,10 +206,13 @@ func TestValidate(t *testing.T) { }, { Agent{ - Namespace: "test", - ServiceAccountPath: "foobar", - ServiceAccountName: "foobar", - ImageName: "test", + Namespace: "test", + ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ + Name: "foobar", + MountPath: "foobar", + TokenPath: "", + }, + ImageName: "test", Vault: Vault{ Role: "test", Address: "https://foobar.com:8200", @@ -230,3 +235,190 @@ func TestValidate(t *testing.T) { } } } + +func Test_serviceaccount(t *testing.T) { + tests := map[string]struct { + pod *corev1.Pod + expected *ServiceAccountTokenVolume + expectedError string + }{ + "no service accounts": { + expected: nil, + expectedError: "failed to find service account volume mount", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{}, + }, + }, + "token volume doesn't exist": { + expected: nil, + expectedError: `failed to find service account volume "projected-token"`, + pod: &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "vault.hashicorp.com/agent-service-account-token-volume-name": "projected-token", + }, + }, + }, + }, + "regular service account": { + expected: &ServiceAccountTokenVolume{ + Name: "internal-app-token-n4pjn", + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + TokenPath: "token", + }, + expectedError: "", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + VolumeMounts: []corev1.VolumeMount{ + { + Name: "internal-app-token-n4pjn", + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "internal-app-token-n4pjn", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "internal-app-token-n4pjn", + }, + }, + }, + }, + }, + }, + }, + "projected default service account": { + expected: &ServiceAccountTokenVolume{ + Name: "kube-api-access-4bfzq", + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + TokenPath: "token", + }, + expectedError: "", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + VolumeMounts: []corev1.VolumeMount{ + { + Name: "kube-api-access-4bfzq", + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "kube-api-access-4bfzq", + VolumeSource: corev1.VolumeSource{ + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Path: "token", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + "projected service account with annotation": { + expected: &ServiceAccountTokenVolume{ + Name: "projected-token", + MountPath: "/var/run/secrets/special/serviceaccount", + TokenPath: "vault-token", + }, + expectedError: "", + pod: &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "vault.hashicorp.com/agent-service-account-token-volume-name": "projected-token", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + VolumeMounts: []corev1.VolumeMount{ + { + Name: "projected-token", + MountPath: "/var/run/secrets/special/serviceaccount", + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "projected-token", + VolumeSource: corev1.VolumeSource{ + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Path: "vault-token", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + "projected service account with annotation but not mounted": { + expected: &ServiceAccountTokenVolume{ + Name: "projected-token", + MountPath: "/var/run/secrets/vault.hashicorp.com/serviceaccount", + TokenPath: "vault-token", + }, + expectedError: "", + pod: &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "vault.hashicorp.com/agent-service-account-token-volume-name": "projected-token", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + VolumeMounts: []corev1.VolumeMount{}, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "projected-token", + VolumeSource: corev1.VolumeSource{ + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Path: "vault-token", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result, err := serviceaccount(tc.pod) + if len(tc.expectedError) > 0 { + assert.EqualError(t, err, tc.expectedError) + } + assert.Equal(t, tc.expected, result) + }) + } +} diff --git a/agent-inject/agent/annotations_test.go b/agent-inject/agent/annotations_test.go index 0d332efd..d9b91992 100644 --- a/agent-inject/agent/annotations_test.go +++ b/agent-inject/agent/annotations_test.go @@ -920,7 +920,8 @@ func TestAuthConfigAnnotations(t *testing.T) { "vault.hashicorp.com/role": "backwardscompat", }, map[string]interface{}{ - "role": "backwardscompat", + "role": "backwardscompat", + "token_path": "serviceaccount/somewhere/token", }, }, { @@ -929,7 +930,8 @@ func TestAuthConfigAnnotations(t *testing.T) { "vault.hashicorp.com/auth-config-role": "lowerprio", }, map[string]interface{}{ - "role": "backwardscompat", + "role": "backwardscompat", + "token_path": "serviceaccount/somewhere/token", }, }, { @@ -946,6 +948,7 @@ func TestAuthConfigAnnotations(t *testing.T) { "client_cert": "baz", "credential_poll_interval": "1", // string->int conversion left up to consuming app HCL parser "remove_secret_id_file_after_reading": "false", // string->bool, same as above + "token_path": "serviceaccount/somewhere/token", }, }, } From 2430452288a3866cf1860e5dc43a806994de36b4 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Thu, 26 Aug 2021 15:15:14 -0700 Subject: [PATCH 11/14] remove agent-service-account-token-volume-path --- agent-inject/agent/agent.go | 6 +++--- agent-inject/agent/annotations.go | 9 --------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index 49fa7761..1ea3178d 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -96,9 +96,9 @@ type Agent struct { //found, and the unique name of the secret which will be used for the filename. Secrets []*Secret - // ServiceAccountTokenVolume holds details of a volume mount for a Kubernetes service account - // token for the pod. This is used when we mount the service account to the Vault Agent - // container(s). + // ServiceAccountTokenVolume holds details of a volume mount for a + // Kubernetes service account token for the pod. This is used when we mount + // the service account to the Vault Agent container(s). ServiceAccountTokenVolume *ServiceAccountTokenVolume // Status is the current injection status. The only status considered is "injected", diff --git a/agent-inject/agent/annotations.go b/agent-inject/agent/annotations.go index dc631ed9..67b65765 100644 --- a/agent-inject/agent/annotations.go +++ b/agent-inject/agent/annotations.go @@ -151,11 +151,6 @@ const ( // service account token AnnotationAgentServiceAccountTokenVolumeName = "vault.hashicorp.com/agent-service-account-token-volume-name" - // AnnotationAgentServiceAccountTokenVolumePath is the optional path to a service account token - // within a volume named by "vault.hashicorp.com/agent-service-account-token-volume-name" or - // discovered by searching for service account volume mounts - the default is "token" - AnnotationAgentServiceAccountTokenVolumePath = "vault.hashicorp.com/agent-service-account-token-volume-path" - // AnnotationVaultService is the name of the Vault server. This can be overridden by the // user but will be set by a flag on the deployment. AnnotationVaultService = "vault.hashicorp.com/service" @@ -376,10 +371,6 @@ func Init(pod *corev1.Pod, cfg AgentConfig) error { pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumeName] = "" } - if _, ok := pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumePath]; !ok { - pod.ObjectMeta.Annotations[AnnotationAgentServiceAccountTokenVolumePath] = "token" - } - if _, securityContextIsSet = pod.ObjectMeta.Annotations[AnnotationAgentSetSecurityContext]; !securityContextIsSet { pod.ObjectMeta.Annotations[AnnotationAgentSetSecurityContext] = strconv.FormatBool(cfg.SetSecurityContext) } From 7882683bc502bcc331be2b1736daa4854633807a Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Thu, 26 Aug 2021 22:42:00 -0700 Subject: [PATCH 12/14] Make the tokenPath lookups more strict --- agent-inject/agent/agent.go | 20 ++++++++---- agent-inject/agent/agent_test.go | 53 ++++++++++++++++++++++++++++---- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index 1ea3178d..cf625ed1 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -646,10 +646,14 @@ func serviceaccount(pod *corev1.Pod) (*ServiceAccountTokenVolume, error) { for _, container := range pod.Spec.Containers { for _, volumeMount := range container.VolumeMounts { if volumeMount.Name == volumeName { + tokenPath := getProjectedTokenPath(pod, volumeName) + if len(tokenPath) == 0 { + return nil, fmt.Errorf("failed to find tokenPath for service account volume mount %q", volumeName) + } return &ServiceAccountTokenVolume{ Name: volumeMount.Name, MountPath: volumeMount.MountPath, - TokenPath: getTokenPath(pod, volumeName), + TokenPath: tokenPath, }, nil } } @@ -658,10 +662,14 @@ func serviceaccount(pod *corev1.Pod) (*ServiceAccountTokenVolume, error) { // Otherwise, check the volume exists and fallback to `DefaultServiceAccountMount` for _, volume := range pod.Spec.Volumes { if volume.Name == volumeName { + tokenPath := getTokenPathFromProjectedVolume(volume) + if len(tokenPath) == 0 { + return nil, fmt.Errorf("failed to find tokenPath for service account volume %q", volumeName) + } return &ServiceAccountTokenVolume{ Name: volume.Name, MountPath: DefaultServiceAccountMount, - TokenPath: getTokenPathFromVolume(volume), + TokenPath: tokenPath, }, nil } } @@ -685,16 +693,16 @@ func serviceaccount(pod *corev1.Pod) (*ServiceAccountTokenVolume, error) { return nil, fmt.Errorf("failed to find service account volume mount") } -func getTokenPath(pod *corev1.Pod, volumeName string) string { +func getProjectedTokenPath(pod *corev1.Pod, volumeName string) string { for _, volume := range pod.Spec.Volumes { if volume.Name == volumeName { - return getTokenPathFromVolume(volume) + return getTokenPathFromProjectedVolume(volume) } } return "" } -func getTokenPathFromVolume(volume corev1.Volume) string { +func getTokenPathFromProjectedVolume(volume corev1.Volume) string { if volume.Projected != nil { for _, source := range volume.Projected.Sources { if source.ServiceAccountToken != nil { @@ -702,7 +710,7 @@ func getTokenPathFromVolume(volume corev1.Volume) string { } } } - return "token" + return "" } // IRSA support - get aws_iam_token volume mount details to inject to vault containers diff --git a/agent-inject/agent/agent_test.go b/agent-inject/agent/agent_test.go index 6b88bc4d..269866f2 100644 --- a/agent-inject/agent/agent_test.go +++ b/agent-inject/agent/agent_test.go @@ -280,7 +280,7 @@ func Test_serviceaccount(t *testing.T) { Spec: corev1.PodSpec{}, }, }, - "token volume doesn't exist": { + "missing token volume": { expected: nil, expectedError: `failed to find service account volume "projected-token"`, pod: &corev1.Pod{ @@ -291,6 +291,52 @@ func Test_serviceaccount(t *testing.T) { }, }, }, + "missing tokenPath for volume that's mounted": { + expected: nil, + expectedError: `failed to find tokenPath for service account volume mount "projected-token"`, + pod: &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "vault.hashicorp.com/agent-service-account-token-volume-name": "projected-token", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + VolumeMounts: []corev1.VolumeMount{ + { + Name: "projected-token", + MountPath: "/var/run/secrets/special/serviceaccount", + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "projected-token", + }, + }, + }, + }, + }, + "missing tokenPath in volume": { + expected: nil, + expectedError: `failed to find tokenPath for service account volume "projected-token"`, + pod: &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "vault.hashicorp.com/agent-service-account-token-volume-name": "projected-token", + }, + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "projected-token", + }, + }, + }, + }, + }, "regular service account": { expected: &ServiceAccountTokenVolume{ Name: "internal-app-token-n4pjn", @@ -418,11 +464,6 @@ func Test_serviceaccount(t *testing.T) { }, }, Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - VolumeMounts: []corev1.VolumeMount{}, - }, - }, Volumes: []corev1.Volume{ { Name: "projected-token", From edd2373d1da13c2d281a24079a03e7943fbedde5 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Fri, 27 Aug 2021 11:25:33 -0700 Subject: [PATCH 13/14] comment formatting for ServiceAccountTokenVolume Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com> --- agent-inject/agent/agent.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index cf625ed1..b3808bd1 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -157,13 +157,13 @@ type Agent struct { } type ServiceAccountTokenVolume struct { - // The name of the volume + // Name of the volume Name string - // The mount path of the volume within vault agent containers + // MountPath of the volume within vault agent containers MountPath string - // The path to the JWT token within the volume + // TokenPath to the JWT token within the volume TokenPath string } From 94b8263b598ec00c5441d370786eb10850288a7a Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Wed, 1 Sep 2021 14:39:52 -0700 Subject: [PATCH 14/14] review feedback Check for nil ServiceAccountTokenVolume and move error reporting into the get token functions. --- agent-inject/agent/agent.go | 29 ++++++++------- agent-inject/agent/agent_test.go | 60 ++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index cf625ed1..fd577081 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -609,7 +609,8 @@ func (a *Agent) Validate() error { return errors.New("namespace missing from request") } - if a.ServiceAccountTokenVolume.Name == "" || + if a.ServiceAccountTokenVolume == nil || + a.ServiceAccountTokenVolume.Name == "" || a.ServiceAccountTokenVolume.MountPath == "" || a.ServiceAccountTokenVolume.TokenPath == "" { return errors.New("no service account token volume name, mount path or token path found") @@ -646,9 +647,9 @@ func serviceaccount(pod *corev1.Pod) (*ServiceAccountTokenVolume, error) { for _, container := range pod.Spec.Containers { for _, volumeMount := range container.VolumeMounts { if volumeMount.Name == volumeName { - tokenPath := getProjectedTokenPath(pod, volumeName) - if len(tokenPath) == 0 { - return nil, fmt.Errorf("failed to find tokenPath for service account volume mount %q", volumeName) + tokenPath, err := getProjectedTokenPath(pod, volumeName) + if err != nil { + return nil, err } return &ServiceAccountTokenVolume{ Name: volumeMount.Name, @@ -662,9 +663,9 @@ func serviceaccount(pod *corev1.Pod) (*ServiceAccountTokenVolume, error) { // Otherwise, check the volume exists and fallback to `DefaultServiceAccountMount` for _, volume := range pod.Spec.Volumes { if volume.Name == volumeName { - tokenPath := getTokenPathFromProjectedVolume(volume) - if len(tokenPath) == 0 { - return nil, fmt.Errorf("failed to find tokenPath for service account volume %q", volumeName) + tokenPath, err := getTokenPathFromProjectedVolume(volume) + if err != nil { + return nil, err } return &ServiceAccountTokenVolume{ Name: volume.Name, @@ -693,24 +694,26 @@ func serviceaccount(pod *corev1.Pod) (*ServiceAccountTokenVolume, error) { return nil, fmt.Errorf("failed to find service account volume mount") } -func getProjectedTokenPath(pod *corev1.Pod, volumeName string) string { +// getProjectedTokenPath searches through a Pod's Volumes for volumeName, and +// attempts to retrieve the projected token path from that volume +func getProjectedTokenPath(pod *corev1.Pod, volumeName string) (string, error) { for _, volume := range pod.Spec.Volumes { if volume.Name == volumeName { return getTokenPathFromProjectedVolume(volume) } } - return "" + return "", fmt.Errorf("failed to find volume %q in Pod %q volumes", volumeName, pod.Name) } -func getTokenPathFromProjectedVolume(volume corev1.Volume) string { +func getTokenPathFromProjectedVolume(volume corev1.Volume) (string, error) { if volume.Projected != nil { for _, source := range volume.Projected.Sources { - if source.ServiceAccountToken != nil { - return source.ServiceAccountToken.Path + if source.ServiceAccountToken != nil && source.ServiceAccountToken.Path != "" { + return source.ServiceAccountToken.Path, nil } } } - return "" + return "", fmt.Errorf("failed to find tokenPath for projected volume %q", volume.Name) } // IRSA support - get aws_iam_token volume mount details to inject to vault containers diff --git a/agent-inject/agent/agent_test.go b/agent-inject/agent/agent_test.go index 269866f2..dd13642d 100644 --- a/agent-inject/agent/agent_test.go +++ b/agent-inject/agent/agent_test.go @@ -236,6 +236,7 @@ func TestValidate(t *testing.T) { }, false, }, { + // Missing ServiceAccountTokenVolume.TokenPath Agent{ Namespace: "test", ServiceAccountTokenVolume: &ServiceAccountTokenVolume{ @@ -252,6 +253,19 @@ func TestValidate(t *testing.T) { }, }, false, }, + { + // Missing ServiceAccountTokenVolume + Agent{ + Namespace: "test", + ImageName: "test", + Vault: Vault{ + Role: "test", + Address: "https://foobar.com:8200", + AuthPath: "test", + AuthType: "", + }, + }, false, + }, } for _, tt := range tests { @@ -291,9 +305,49 @@ func Test_serviceaccount(t *testing.T) { }, }, }, + "token volume name mount missing from volumes": { + expected: nil, + expectedError: `failed to find volume "missing" in Pod "test-pod" volumes`, + pod: &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: "test-pod", + Annotations: map[string]string{ + "vault.hashicorp.com/agent-service-account-token-volume-name": "missing", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + VolumeMounts: []corev1.VolumeMount{ + { + Name: "missing", + MountPath: "/not/a/projected/token/volume", + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "projected-token", + VolumeSource: corev1.VolumeSource{ + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Path: "token", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, "missing tokenPath for volume that's mounted": { expected: nil, - expectedError: `failed to find tokenPath for service account volume mount "projected-token"`, + expectedError: `failed to find tokenPath for projected volume "projected-token"`, pod: &corev1.Pod{ ObjectMeta: v1.ObjectMeta{ Annotations: map[string]string{ @@ -321,7 +375,7 @@ func Test_serviceaccount(t *testing.T) { }, "missing tokenPath in volume": { expected: nil, - expectedError: `failed to find tokenPath for service account volume "projected-token"`, + expectedError: `failed to find tokenPath for projected volume "projected-token"`, pod: &corev1.Pod{ ObjectMeta: v1.ObjectMeta{ Annotations: map[string]string{ @@ -337,7 +391,7 @@ func Test_serviceaccount(t *testing.T) { }, }, }, - "regular service account": { + "regular service account (not projected)": { expected: &ServiceAccountTokenVolume{ Name: "internal-app-token-n4pjn", MountPath: "/var/run/secrets/kubernetes.io/serviceaccount",