From 380852bf8f0552f8cd7a6fb39aed0ee98754bccf Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Fri, 16 Dec 2022 08:44:32 -0500 Subject: [PATCH] add support for enabling shareProcessNamespace (#408) Co-authored-by: David Wolffberg <1350533+wolffberg@users.noreply.github.com> --- CHANGELOG.md | 1 + Makefile | 3 ++- agent-inject/agent/agent.go | 14 +++++++++++- agent-inject/agent/annotations.go | 31 ++++++++++++++++++++++++++ agent-inject/agent/annotations_test.go | 12 ++++++++++ agent-inject/agent/patch.go | 6 +++++ agent-inject/handler.go | 2 ++ agent-inject/handler_test.go | 8 ++++++- 8 files changed, 74 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13f5b0ea..4f627aae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Improvements: * Building with Go 1.19.4 * Update golang.org/x/net to v0.0.0-20220722155237-a158d28d115b +* Add support for enabling `sharedProcessNamespace` on the Pod `spec` [GH-408](https://github.com/hashicorp/vault-k8s/pull/408) Bugs: * Preserve metadata when updating the cert secret [GH-401](https://github.com/hashicorp/vault-k8s/pull/401) diff --git a/Makefile b/Makefile index 0967a2f9..f7cbe576 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,7 @@ GOFMT_FILES?=$$(find . -name '*.go' | grep -v vendor) XC_PUBLISH?= PKG=github.com/hashicorp/vault-k8s/version LDFLAGS?="-X '$(PKG).Version=v$(VERSION)'" +TESTARGS ?= '-test.v' .PHONY: all test build image clean version all: build @@ -35,7 +36,7 @@ clean: test: unit-test unit-test: - go test -race ./... + go test -race $(TESTARGS) ./... .PHONY: mod mod: diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index 94ecafd1..00181407 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -19,6 +19,7 @@ const ( DefaultAgentRunAsUser = 100 DefaultAgentRunAsGroup = 1000 DefaultAgentRunAsSameUser = false + DefaultAgentShareProcessNamespace = false DefaultAgentAllowPrivilegeEscalation = false DefaultAgentDropCapabilities = "ALL" DefaultAgentSetSecurityContext = true @@ -137,6 +138,9 @@ type Agent struct { // same as the first application container RunAsSameID bool + // ShareProcessNamespace sets the shareProcessNamespace value on the pod spec. + ShareProcessNamespace bool + // SetSecurityContext controls whether the injected containers have a // SecurityContext set. SetSecurityContext bool @@ -433,6 +437,11 @@ func New(pod *corev1.Pod) (*Agent, error) { return agent, err } + agent.ShareProcessNamespace, err = agent.setShareProcessNamespace(pod) + if err != nil { + return agent, err + } + agent.SetSecurityContext, err = agent.setSecurityContext() if err != nil { return agent, err @@ -607,7 +616,7 @@ func (a *Agent) Patch() ([]byte, error) { // to run first. if a.InitFirst { - // Remove all init containers from the document so we can re-add them after the agent. + // Remove all init containers from the document, so we can re-add them after the agent. if len(a.Pod.Spec.InitContainers) != 0 { patches = append(patches, removeContainers("/spec/initContainers")...) } @@ -636,6 +645,9 @@ func (a *Agent) Patch() ([]byte, error) { a.ContainerVolumeMounts(), fmt.Sprintf("/spec/initContainers/%d/volumeMounts", i))...) } + + // Add shareProcessNamespace + patches = append(patches, updateShareProcessNamespace(a.ShareProcessNamespace)...) } // Sidecar Container diff --git a/agent-inject/agent/annotations.go b/agent-inject/agent/annotations.go index 7e1b6002..8c455082 100644 --- a/agent-inject/agent/annotations.go +++ b/agent-inject/agent/annotations.go @@ -156,6 +156,9 @@ const ( // Pod Spec. AnnotationAgentRunAsSameUser = "vault.hashicorp.com/agent-run-as-same-user" + // AnnotationAgentShareProcessNamespace sets the shareProcessNamespace value on the pod spec. + AnnotationAgentShareProcessNamespace = "vault.hashicorp.com/agent-share-process-namespace" + // AnnotationAgentSetSecurityContext controls whether a SecurityContext (uid // and gid) is set on the injected Vault Agent containers AnnotationAgentSetSecurityContext = "vault.hashicorp.com/agent-set-security-context" @@ -312,6 +315,7 @@ type AgentConfig struct { GroupID string SameID bool SetSecurityContext bool + ShareProcessNamespace bool ProxyAddress string DefaultTemplate string ResourceRequestCPU string @@ -336,6 +340,7 @@ func Init(pod *corev1.Pod, cfg AgentConfig) error { var runAsUserIsSet bool var runAsSameUserIsSet bool var runAsGroupIsSet bool + var shareProcessNamespaceIsSet bool if pod == nil { return errors.New("pod is empty") @@ -455,6 +460,10 @@ func Init(pod *corev1.Pod, cfg AgentConfig) error { pod.ObjectMeta.Annotations[AnnotationAgentRunAsSameUser] = strconv.FormatBool(cfg.SameID) } + if _, shareProcessNamespaceIsSet = pod.ObjectMeta.Annotations[AnnotationAgentShareProcessNamespace]; !shareProcessNamespaceIsSet { + pod.ObjectMeta.Annotations[AnnotationAgentShareProcessNamespace] = strconv.FormatBool(cfg.ShareProcessNamespace) + } + if _, runAsGroupIsSet = pod.ObjectMeta.Annotations[AnnotationAgentRunAsGroup]; !runAsGroupIsSet { if cfg.GroupID == "" { cfg.GroupID = strconv.Itoa(DefaultAgentRunAsGroup) @@ -736,6 +745,28 @@ func (a *Agent) runAsSameID(pod *corev1.Pod) (bool, error) { return runAsSameID, nil } +func (a *Agent) setShareProcessNamespace(pod *corev1.Pod) (bool, error) { + annotation := AnnotationAgentShareProcessNamespace + raw, ok := a.Annotations[annotation] + if !ok { + return DefaultAgentShareProcessNamespace, nil + } + shareProcessNamespace, err := strconv.ParseBool(raw) + if err != nil { + return DefaultAgentShareProcessNamespace, fmt.Errorf( + "invalid value %v for annotation %q, err=%w", raw, annotation, err) + } + if pod.Spec.ShareProcessNamespace != nil { + if !*pod.Spec.ShareProcessNamespace && shareProcessNamespace { + return DefaultAgentShareProcessNamespace, + errors.New("shareProcessNamespace explicitly disabled on the pod, " + + "refusing to enable it") + } + } + + return shareProcessNamespace, nil +} + func (a *Agent) setSecurityContext() (bool, error) { raw, ok := a.Annotations[AnnotationAgentSetSecurityContext] if !ok { diff --git a/agent-inject/agent/annotations_test.go b/agent-inject/agent/annotations_test.go index 2902358f..746fe20f 100644 --- a/agent-inject/agent/annotations_test.go +++ b/agent-inject/agent/annotations_test.go @@ -94,6 +94,7 @@ func TestInitDefaults(t *testing.T) { {annotationKey: AnnotationAgentImage, annotationValue: DefaultVaultImage}, {annotationKey: AnnotationAgentRunAsUser, annotationValue: strconv.Itoa(DefaultAgentRunAsUser)}, {annotationKey: AnnotationAgentRunAsGroup, annotationValue: strconv.Itoa(DefaultAgentRunAsGroup)}, + {annotationKey: AnnotationAgentShareProcessNamespace, annotationValue: strconv.FormatBool(DefaultAgentShareProcessNamespace)}, } for _, tt := range tests { @@ -704,6 +705,14 @@ func TestCouldErrorAnnotations(t *testing.T) { {AnnotationAgentRunAsGroup, "100", true}, {AnnotationAgentRunAsGroup, "root", false}, + {AnnotationAgentShareProcessNamespace, "true", true}, + {AnnotationAgentShareProcessNamespace, "false", true}, + {AnnotationAgentShareProcessNamespace, "TRUE", true}, + {AnnotationAgentShareProcessNamespace, "FALSE", true}, + {AnnotationAgentShareProcessNamespace, "tRuE", false}, + {AnnotationAgentShareProcessNamespace, "fAlSe", false}, + {AnnotationAgentShareProcessNamespace, "", false}, + {AnnotationAgentSetSecurityContext, "true", true}, {AnnotationAgentSetSecurityContext, "false", true}, {AnnotationAgentSetSecurityContext, "secure", false}, @@ -976,6 +985,7 @@ func TestInjectContainers(t *testing.T) { internal.AddOp("/spec/containers/1/volumeMounts/-", nil), internal.AddOp("/spec/containers/2/volumeMounts/-", nil), internal.AddOp("/spec/initContainers", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(AnnotationAgentStatus), nil), }, @@ -990,6 +1000,7 @@ func TestInjectContainers(t *testing.T) { internal.AddOp("/spec/volumes", nil), internal.AddOp("/spec/containers/1/volumeMounts/-", nil), internal.AddOp("/spec/initContainers", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(AnnotationAgentStatus), nil), }, @@ -1005,6 +1016,7 @@ func TestInjectContainers(t *testing.T) { internal.AddOp("/spec/containers/1/volumeMounts/-", nil), internal.AddOp("/spec/containers/2/volumeMounts/-", nil), internal.AddOp("/spec/initContainers", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(AnnotationAgentStatus), nil), }, diff --git a/agent-inject/agent/patch.go b/agent-inject/agent/patch.go index f003362b..c05d27b5 100644 --- a/agent-inject/agent/patch.go +++ b/agent-inject/agent/patch.go @@ -82,3 +82,9 @@ func updateAnnotations(target, annotations map[string]string) jsonpatch.Patch { return result } + +func updateShareProcessNamespace(shareProcessNamespace bool) jsonpatch.Patch { + return []jsonpatch.Operation{ + internal.AddOp("/spec/shareProcessNamespace", shareProcessNamespace), + } +} diff --git a/agent-inject/handler.go b/agent-inject/handler.go index cd802613..e3139ae5 100644 --- a/agent-inject/handler.go +++ b/agent-inject/handler.go @@ -57,6 +57,7 @@ type Handler struct { UserID string GroupID string SameID bool + ShareProcessNamespace bool SetSecurityContext bool DefaultTemplate string ResourceRequestCPU string @@ -195,6 +196,7 @@ func (h *Handler) Mutate(req *admissionv1.AdmissionRequest) *admissionv1.Admissi GroupID: h.GroupID, SameID: h.SameID, SetSecurityContext: h.SetSecurityContext, + ShareProcessNamespace: h.ShareProcessNamespace, DefaultTemplate: h.DefaultTemplate, ResourceRequestCPU: h.ResourceRequestCPU, ResourceRequestMem: h.ResourceRequestMem, diff --git a/agent-inject/handler_test.go b/agent-inject/handler_test.go index 628334d9..9a154b90 100644 --- a/agent-inject/handler_test.go +++ b/agent-inject/handler_test.go @@ -157,6 +157,7 @@ func TestHandlerHandle(t *testing.T) { internal.AddOp("/spec/containers/0/volumeMounts/-", nil), internal.AddOp("/spec/initContainers/-", nil), internal.AddOp("/spec/initContainers/0/volumeMounts/-", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(agent.AnnotationAgentStatus), nil), }, @@ -188,6 +189,7 @@ func TestHandlerHandle(t *testing.T) { internal.AddOp("/spec/initContainers", nil), internal.AddOp("/spec/initContainers/-", nil), internal.AddOp("/spec/initContainers/1/volumeMounts/-", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(agent.AnnotationAgentStatus), nil), }, @@ -217,11 +219,11 @@ func TestHandlerHandle(t *testing.T) { internal.AddOp("/spec/containers/0/volumeMounts/-", nil), internal.AddOp("/spec/initContainers/-", nil), internal.AddOp("/spec/initContainers/0/volumeMounts/-", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(agent.AnnotationAgentStatus), nil), }, }, - { "tls pod injection", basicHandler(), @@ -248,6 +250,7 @@ func TestHandlerHandle(t *testing.T) { internal.AddOp("/spec/containers/0/volumeMounts/-", nil), internal.AddOp("/spec/initContainers/-", nil), internal.AddOp("/spec/initContainers/0/volumeMounts/-", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(agent.AnnotationAgentStatus), nil), }, @@ -278,6 +281,7 @@ func TestHandlerHandle(t *testing.T) { internal.AddOp("/spec/containers/0/volumeMounts/-", nil), internal.AddOp("/spec/initContainers/-", nil), internal.AddOp("/spec/initContainers/0/volumeMounts/-", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(agent.AnnotationAgentStatus), nil), }, @@ -336,6 +340,7 @@ func TestHandlerHandle(t *testing.T) { internal.AddOp("/spec/containers/0/volumeMounts/-", nil), internal.AddOp("/spec/initContainers/-", nil), internal.AddOp("/spec/initContainers/0/volumeMounts/-", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(agent.AnnotationAgentStatus), nil), }, }, @@ -364,6 +369,7 @@ func TestHandlerHandle(t *testing.T) { internal.AddOp("/spec/containers/0/volumeMounts/-", nil), internal.AddOp("/spec/initContainers/-", nil), internal.AddOp("/spec/initContainers/0/volumeMounts/-", nil), + internal.AddOp("/spec/shareProcessNamespace", nil), internal.AddOp("/spec/containers/-", nil), internal.AddOp("/metadata/annotations/"+internal.EscapeJSONPointer(agent.AnnotationAgentStatus), nil), },