feat: Add support to auto-mount service account tokens for plugins. (a…

…rgoproj#8176) Signed-off-by: Alex Collins <alex_collins@intuit.com>
whybeyoung · Apr 27, 2022 · 0e5a01f · 0e5a01f
1 parent 9c08aed
commit 0e5a01f
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 21 deletions.
diff --git a/pkg/plugins/spec/plugin_types.go b/pkg/plugins/spec/plugin_types.go
@@ -25,7 +25,9 @@ type PluginSpec struct {
 }
 
 type Sidecar struct {
-	Container apiv1.Container `json:"container"`
+	// AutomountServiceAccount mounts the service account's token. The service account must have the same name as the plugin.
+	AutomountServiceAccountToken bool            `json:"automountServiceAccountToken,omitempty"`
+	Container                    apiv1.Container `json:"container"`
 }
 
 func (s Sidecar) Validate() error {

diff --git a/test/e2e/executor_plugins_test.go b/test/e2e/executor_plugins_test.go
@@ -41,16 +41,38 @@ func (s *ExecutorPluginsSuite) TestTemplateExecutor() {
 					RunAsUser:    pointer.Int64(8737),
 					RunAsNonRoot: pointer.BoolPtr(true),
 				}, spec.SecurityContext)
+				if assert.Len(t, spec.Volumes, 4) {
+					assert.Contains(t, spec.Volumes[0].Name, "kube-api-access-")
+					assert.Equal(t, spec.Volumes[1].Name, "var-run-argo")
+					assert.Contains(t, spec.Volumes[2].Name, "kube-api-access-")
+					assert.Equal(t, spec.Volumes[3].Name, "argo-workflows-agent-ca-certificates")
+				}
 				if assert.Len(t, spec.Containers, 2) {
-					agent := spec.Containers[1]
-					if assert.Equal(t, "main", agent.Name) {
-						assert.Equal(t, &apiv1.SecurityContext{
-							RunAsUser:                pointer.Int64(8737),
-							RunAsNonRoot:             pointer.BoolPtr(true),
-							AllowPrivilegeEscalation: pointer.BoolPtr(false),
-							ReadOnlyRootFilesystem:   pointer.BoolPtr(true),
-							Capabilities:             &apiv1.Capabilities{Drop: []apiv1.Capability{"ALL"}},
-						}, agent.SecurityContext)
+					{
+						plug := spec.Containers[0]
+						if assert.Equal(t, "hello-executor-plugin", plug.Name) {
+							if assert.Len(t, plug.VolumeMounts, 2) {
+								assert.Equal(t, "var-run-argo", plug.VolumeMounts[0].Name)
+								assert.Contains(t, plug.VolumeMounts[1].Name, "kube-api-access-")
+							}
+						}
+					}
+					{
+						agent := spec.Containers[1]
+						if assert.Equal(t, "main", agent.Name) {
+							if assert.Len(t, agent.VolumeMounts, 3) {
+								assert.Equal(t, "var-run-argo", agent.VolumeMounts[0].Name)
+								assert.Contains(t, agent.VolumeMounts[1].Name, "kube-api-access-")
+								assert.Equal(t, agent.VolumeMounts[2].Name, "argo-workflows-agent-ca-certificates")
+							}
+							assert.Equal(t, &apiv1.SecurityContext{
+								RunAsUser:                pointer.Int64(8737),
+								RunAsNonRoot:             pointer.BoolPtr(true),
+								AllowPrivilegeEscalation: pointer.BoolPtr(false),
+								ReadOnlyRootFilesystem:   pointer.BoolPtr(true),
+								Capabilities:             &apiv1.Capabilities{Drop: []apiv1.Capability{"ALL"}},
+							}, agent.SecurityContext)
+						}
 					}
 				}
 			}

diff --git a/test/e2e/manifests/plugins/hello-executor-plugin-configmap.yaml b/test/e2e/manifests/plugins/hello-executor-plugin-configmap.yaml
@@ -1,6 +1,6 @@
-# This is an auto-generated file. DO NOT EDIT
 apiVersion: v1
 data:
+  sidecar.automountServiceAccountToken: "true"
   sidecar.container: |
     args:
     - |

diff --git a/test/e2e/manifests/plugins/hello-executor-plugin-serviceaccount.yaml b/test/e2e/manifests/plugins/hello-executor-plugin-serviceaccount.yaml
@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: hello-executor-plugin
diff --git a/test/e2e/manifests/plugins/kustomization.yaml b/test/e2e/manifests/plugins/kustomization.yaml
@@ -3,6 +3,7 @@ kind: Kustomization
 
 resources:
   - ../minimal
+  - hello-executor-plugin-serviceaccount.yaml
   - hello-executor-plugin-configmap.yaml
 
 commonLabels:

diff --git a/workflow/controller/agent.go b/workflow/controller/agent.go
@@ -128,7 +128,11 @@ func (woc *wfOperationCtx) createAgentPod(ctx context.Context) (*apiv1.Pod, erro
 		return nil, err
 	}
 
-	pluginSidecars := woc.getExecutorPlugins()
+	pluginSidecars, pluginVolumes, err := woc.getExecutorPlugins(ctx)
+	if err != nil {
+		return nil, err
+	}
+
 	envVars := []apiv1.EnvVar{
 		{Name: common.EnvVarWorkflowName, Value: woc.wf.Name},
 		{Name: common.EnvAgentPatchRate, Value: env.LookupEnvStringOr(common.EnvAgentPatchRate, GetRequeueTime().String())},
@@ -150,10 +154,11 @@ func (woc *wfOperationCtx) createAgentPod(ctx context.Context) (*apiv1.Pod, erro
 		return nil, fmt.Errorf("failed to get token volumes: %w", err)
 	}
 
-	podVolumes := []apiv1.Volume{
+	podVolumes := append(
+		pluginVolumes,
 		volumeVarArgo,
 		*tokenVolume,
-	}
+	)
 	podVolumeMounts := []apiv1.VolumeMount{
 		volumeMountVarArgo,
 		*tokenVolumeMount,
@@ -251,25 +256,35 @@ func (woc *wfOperationCtx) createAgentPod(ctx context.Context) (*apiv1.Pod, erro
 	return created, nil
 }
 
-func (woc *wfOperationCtx) getExecutorPlugins() []apiv1.Container {
+func (woc *wfOperationCtx) getExecutorPlugins(ctx context.Context) ([]apiv1.Container, []apiv1.Volume, error) {
 	var sidecars []apiv1.Container
+	var volumes []apiv1.Volume
 	namespaces := map[string]bool{} // de-dupes executorPlugins when their namespaces are the same
 	namespaces[woc.controller.namespace] = true
 	namespaces[woc.wf.Namespace] = true
 	for namespace := range namespaces {
 		for _, plug := range woc.controller.executorPlugins[namespace] {
-			c := plug.Spec.Sidecar.Container.DeepCopy()
+			s := plug.Spec.Sidecar
+			c := s.Container.DeepCopy()
 			c.VolumeMounts = append(c.VolumeMounts, apiv1.VolumeMount{
 				Name:      volumeMountVarArgo.Name,
 				MountPath: volumeMountVarArgo.MountPath,
 				ReadOnly:  true,
 				// only mount the token for this plugin, not others
 				SubPath: c.Name,
 			})
+			if s.AutomountServiceAccountToken {
+				volume, volumeMount, err := woc.getServiceAccountTokenVolume(ctx, plug.Name+"-executor-plugin")
+				if err != nil {
+					return nil, nil, err
+				}
+				volumes = append(volumes, *volume)
+				c.VolumeMounts = append(c.VolumeMounts, *volumeMount)
+			}
 			sidecars = append(sidecars, *c)
 		}
 	}
-	return sidecars
+	return sidecars, volumes, nil
 }
 
 func addresses(containers []apiv1.Container) []string {

diff --git a/workflow/util/plugins/configmap.go b/workflow/util/plugins/configmap.go
@@ -34,7 +34,8 @@ func ToConfigMap(p *spec.Plugin) (*apiv1.ConfigMap, error) {
 			Namespace: p.Namespace,
 		},
 		Data: map[string]string{
-			"sidecar.container": string(data),
+			"sidecar.automountServiceAccountToken": fmt.Sprint(p.Spec.Sidecar.AutomountServiceAccountToken),
+			"sidecar.container":                    string(data),
 		},
 	}
 	for k, v := range p.Annotations {
@@ -64,6 +65,7 @@ func FromConfigMap(cm *apiv1.ConfigMap) (*spec.Plugin, error) {
 		p.Labels[k] = v
 	}
 	delete(p.Labels, common.LabelKeyConfigMapType)
+	p.Spec.Sidecar.AutomountServiceAccountToken = cm.Data["sidecar.automountServiceAccountToken"] == "true"
 	if err := yaml.UnmarshalStrict([]byte(cm.Data["sidecar.container"]), &p.Spec.Sidecar.Container); err != nil {
 		return nil, err
 	}

diff --git a/workflow/util/plugins/configmap_test.go b/workflow/util/plugins/configmap_test.go
@@ -33,6 +33,7 @@ func TestToConfigMap(t *testing.T) {
 			},
 			Spec: spec.PluginSpec{
 				Sidecar: spec.Sidecar{
+					AutomountServiceAccountToken: true,
 					Container: apiv1.Container{
 						Ports: []apiv1.ContainerPort{{ContainerPort: 1234}},
 						Resources: apiv1.ResourceRequirements{
@@ -52,7 +53,8 @@ func TestToConfigMap(t *testing.T) {
 				"workflows.argoproj.io/configmap-type": "ExecutorPlugin",
 			}, cm.Labels)
 			assert.Equal(t, map[string]string{
-				"sidecar.container": "name: \"\"\nports:\n- containerPort: 1234\nresources: {}\nsecurityContext: {}\n",
+				"sidecar.automountServiceAccountToken": "true",
+				"sidecar.container":                    "name: \"\"\nports:\n- containerPort: 1234\nresources: {}\nsecurityContext: {}\n",
 			}, cm.Data)
 		}
 	})
@@ -76,15 +78,16 @@ func TestFromConfigMap(t *testing.T) {
 				},
 			},
 			Data: map[string]string{
-				"sidecar.address":   "http://my-addr",
-				"sidecar.container": "{'name': 'my-name', 'ports': [{}], 'resources': {'requests': {}, 'limits': {}}, 'securityContext': {}}",
+				"sidecar.automountServiceAccountToken": "true",
+				"sidecar.container":                    "{'name': 'my-name', 'ports': [{}], 'resources': {'requests': {}, 'limits': {}}, 'securityContext': {}}",
 			},
 		})
 		if assert.NoError(t, err) {
 			assert.Equal(t, "ExecutorPlugin", p.Kind)
 			assert.Equal(t, "my-plug", p.Name)
 			assert.Len(t, p.Annotations, 1)
 			assert.Len(t, p.Labels, 1)
+			assert.True(t, p.Spec.Sidecar.AutomountServiceAccountToken)
 			assert.Equal(t, apiv1.Container{
 				Name:  "my-name",
 				Ports: []apiv1.ContainerPort{{}},