Skip to content

Commit

Permalink
add support for enabling shareProcessNamespace (#408)
Browse files Browse the repository at this point in the history
Co-authored-by: David Wolffberg <1350533+wolffberg@users.noreply.github.com>
  • Loading branch information
benashz and wolffberg authored Dec 16, 2022
1 parent bd97989 commit 380852b
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -35,7 +36,7 @@ clean:
test: unit-test

unit-test:
go test -race ./...
go test -race $(TESTARGS) ./...

.PHONY: mod
mod:
Expand Down
14 changes: 13 additions & 1 deletion agent-inject/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const (
DefaultAgentRunAsUser = 100
DefaultAgentRunAsGroup = 1000
DefaultAgentRunAsSameUser = false
DefaultAgentShareProcessNamespace = false
DefaultAgentAllowPrivilegeEscalation = false
DefaultAgentDropCapabilities = "ALL"
DefaultAgentSetSecurityContext = true
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")...)
}
Expand Down Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions agent-inject/agent/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -312,6 +315,7 @@ type AgentConfig struct {
GroupID string
SameID bool
SetSecurityContext bool
ShareProcessNamespace bool
ProxyAddress string
DefaultTemplate string
ResourceRequestCPU string
Expand All @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 12 additions & 0 deletions agent-inject/agent/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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),
},
Expand All @@ -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),
},
Expand All @@ -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),
},
Expand Down
6 changes: 6 additions & 0 deletions agent-inject/agent/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
2 changes: 2 additions & 0 deletions agent-inject/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ type Handler struct {
UserID string
GroupID string
SameID bool
ShareProcessNamespace bool
SetSecurityContext bool
DefaultTemplate string
ResourceRequestCPU string
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 7 additions & 1 deletion agent-inject/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
Expand Down Expand Up @@ -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),
},
Expand Down Expand Up @@ -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(),
Expand All @@ -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),
},
Expand Down Expand Up @@ -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),
},
Expand Down Expand Up @@ -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),
},
},
Expand Down Expand Up @@ -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),
},
Expand Down

0 comments on commit 380852b

Please sign in to comment.