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.

Sign up for GitHub

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

New securityContext options and annotations #131

Merged
merged 10 commits into from
May 21, 2020
Merged

New securityContext options and annotations #131

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6e1aef8
Implement RunAsSameUser option
eye0fra Apr 20, 2020
d7f4d62
Update agent-inject/agent/annotations.go
eye0fra Apr 30, 2020
f97f533
Update subcommand/injector/flags.go
eye0fra Apr 30, 2020
0e5e632
Update agent-inject/agent/annotations.go
eye0fra Apr 30, 2020
50eb32a
Update subcommand/injector/command.go
eye0fra Apr 30, 2020
69de19e
Update subcommand/injector/flags.go
eye0fra Apr 30, 2020
f8f00ea
Updated tests and docs for RunAsSameUser
tvoran May 5, 2020
d7fef2a
RunAsSameUser annotation and SetSecurityContext
tvoran May 14, 2020
2bb3a86
passing SetSecurityContext all the way to the pod
tvoran May 15, 2020
3c1cf60
review feedback (no root, neater tests)
tvoran May 20, 2020
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
14 changes: 14 additions & 0 deletions agent-inject/agent/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ type AgentConfig struct {
RevokeOnShutdown bool
UserID string
GroupID string
SameID bool
}

// Init configures the expected annotations required to create a new instance
Expand Down Expand Up @@ -244,9 +245,22 @@ func Init(pod *corev1.Pod, cfg AgentConfig) error {
}

if _, ok := pod.ObjectMeta.Annotations[AnnotationAgentRunAsUser]; !ok {

if cfg.UserID == "" {
cfg.UserID = strconv.Itoa(DefaultAgentRunAsUser)
}
if cfg.SameID {
if len(pod.Spec.Containers) == 0 {
return errors.New("No containers found in Pod Spec")
}
if pod.Spec.Containers[0].SecurityContext == nil {
return errors.New("No SecurityContext found for Container 0")
}
if pod.Spec.Containers[0].SecurityContext.RunAsUser == nil {
return errors.New("RunAsUser is nil for Container 0's SecurityContext")
}
cfg.UserID = strconv.FormatInt(*pod.Spec.Containers[0].SecurityContext.RunAsUser, 10)
Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder if this should be looking at pod.Spec.SecurityContext as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Spec.SecurityContext is common for all containers running inside the pod.
The Spec.Container[0].SecurityContext overrides the settings at the pod level.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, and after running this on OpenShift via CRC, I see that RunAsUser is being set at the container level.

I'm also now thinking that it might be simpler to just not set a SecurityContext at all on the init and sidecar containers, since OpenShift will do that for you based on the project/namespace.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm I have to double check it, but the reason that we explicitly put the security context is because we are injecting the side/init-container with the webhook and it is not OpenShift doing it.

}
pod.ObjectMeta.Annotations[AnnotationAgentRunAsUser] = cfg.UserID
}

Expand Down
26 changes: 13 additions & 13 deletions agent-inject/agent/annotations_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestInitCanSet(t *testing.T) {
annotations := make(map[string]string)
pod := testPod(annotations)

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"foobar-image", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -48,7 +48,7 @@ func TestInitDefaults(t *testing.T) {
annotations := make(map[string]string)
pod := testPod(annotations)

err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "", ""})
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "", "", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -78,7 +78,7 @@ func TestInitError(t *testing.T) {
annotations := make(map[string]string)
pod := testPod(annotations)

err := Init(pod, AgentConfig{"image", "", "authPath", "namespace", true, "1000", "100"})
err := Init(pod, AgentConfig{"image", "", "authPath", "namespace", true, "1000", "100", false})
if err == nil {
t.Error("expected error no address, got none")
}
Expand All @@ -88,7 +88,7 @@ func TestInitError(t *testing.T) {
t.Errorf("expected '%s' error, got %s", errMsg, err)
}

err = Init(pod, AgentConfig{"image", "address", "", "namespace", true, "1000", "100"})
err = Init(pod, AgentConfig{"image", "address", "", "namespace", true, "1000", "100", false})
if err == nil {
t.Error("expected error no authPath, got none")
}
Expand All @@ -98,7 +98,7 @@ func TestInitError(t *testing.T) {
t.Errorf("expected '%s' error, got %s", errMsg, err)
}

err = Init(pod, AgentConfig{"image", "address", "authPath", "", true, "1000", "100"})
err = Init(pod, AgentConfig{"image", "address", "authPath", "", true, "1000", "100", false})
if err == nil {
t.Error("expected error for no namespace, got none")
}
Expand Down Expand Up @@ -134,7 +134,7 @@ func TestSecretAnnotationsWithPreserveCaseSensitivityFlagOff(t *testing.T) {
pod := testPod(annotation)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -181,7 +181,7 @@ func TestSecretAnnotationsWithPreserveCaseSensitivityFlagOn(t *testing.T) {
pod := testPod(annotation)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -257,7 +257,7 @@ func TestSecretTemplateAnnotations(t *testing.T) {
pod := testPod(tt.annotations)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -313,7 +313,7 @@ func TestTemplateShortcuts(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
pod := testPod(tt.annotations)
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -369,7 +369,7 @@ func TestSecretCommandAnnotations(t *testing.T) {

for _, tt := range tests {
pod := testPod(tt.annotations)
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -480,7 +480,7 @@ func TestCouldErrorAnnotations(t *testing.T) {
pod := testPod(annotations)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand All @@ -497,7 +497,7 @@ func TestCouldErrorAnnotations(t *testing.T) {
func TestInitEmptyPod(t *testing.T) {
var pod *corev1.Pod

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"foobar-image", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err == nil {
t.Errorf("got no error, should have")
}
Expand All @@ -522,7 +522,7 @@ func TestVaultNamespaceAnnotation(t *testing.T) {
pod := testPod(annotation)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:8200", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"foobar-image", "http://foobar:8200", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down
98 changes: 93 additions & 5 deletions agent-inject/agent/container_sidecar_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strconv"
"testing"

"github.com/hashicorp/vault/sdk/helper/pointerutil"
"github.com/mattbaird/jsonpatch"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -32,7 +33,7 @@ func TestContainerSidecarVolume(t *testing.T) {
pod := testPod(annotations)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -83,7 +84,7 @@ func TestContainerSidecar(t *testing.T) {
pod := testPod(annotations)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", false, "1000", "100"})
err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", false, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -184,7 +185,7 @@ func TestContainerSidecarRevokeHook(t *testing.T) {
pod := testPod(annotations)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", tt.revokeFlag, "1000", "100"})
err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", tt.revokeFlag, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -233,7 +234,7 @@ func TestContainerSidecarConfigMap(t *testing.T) {
pod := testPod(annotations)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand Down Expand Up @@ -588,7 +589,7 @@ func TestContainerSidecarSecurityContext(t *testing.T) {
pod := testPod(annotations)
var patches []*jsonpatch.JsonPatchOperation

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", true, "1000", "100"})
err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", true, "1000", "100", false})
if err != nil {
t.Errorf("got error, shouldn't have: %s", err)
}
Expand All @@ -615,3 +616,90 @@ func TestContainerSidecarSecurityContext(t *testing.T) {
})
}
}

func TestContainerSidecar_RunAsSameUser(t *testing.T) {

tests := []struct {
name string
runAsSameUser bool
AppSCC *corev1.SecurityContext
expectedSidecarSCC *corev1.SecurityContext
expectedErr bool
}{
{
name: "false with no app SCC",
runAsSameUser: false,
AppSCC: nil,
expectedSidecarSCC: &corev1.SecurityContext{
RunAsUser: pointerutil.Int64Ptr(DefaultAgentRunAsUser),
RunAsGroup: pointerutil.Int64Ptr(DefaultAgentRunAsGroup),
RunAsNonRoot: pointerutil.BoolPtr(true),
},
expectedErr: false,
},
{
name: "true with app SCC",
runAsSameUser: true,
AppSCC: &corev1.SecurityContext{
RunAsUser: pointerutil.Int64Ptr(123456),
},
expectedSidecarSCC: &corev1.SecurityContext{
RunAsUser: pointerutil.Int64Ptr(123456),
RunAsGroup: pointerutil.Int64Ptr(DefaultAgentRunAsGroup),
RunAsNonRoot: pointerutil.BoolPtr(true),
},
expectedErr: false,
},
{
name: "false with app SCC",
runAsSameUser: false,
AppSCC: &corev1.SecurityContext{
RunAsUser: pointerutil.Int64Ptr(123456),
},
expectedSidecarSCC: &corev1.SecurityContext{
RunAsUser: pointerutil.Int64Ptr(DefaultAgentRunAsUser),
RunAsGroup: pointerutil.Int64Ptr(DefaultAgentRunAsGroup),
RunAsNonRoot: pointerutil.BoolPtr(true),
},
expectedErr: false,
},
{
name: "true with no app SCC",
runAsSameUser: true,
AppSCC: nil,
expectedSidecarSCC: &corev1.SecurityContext{
RunAsUser: pointerutil.Int64Ptr(DefaultAgentRunAsUser),
RunAsGroup: pointerutil.Int64Ptr(DefaultAgentRunAsGroup),
RunAsNonRoot: pointerutil.BoolPtr(true),
},
expectedErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
annotations := map[string]string{
AnnotationVaultRole: "foobar",
}
pod := testPod(annotations)
var patches []*jsonpatch.JsonPatchOperation

pod.Spec.Containers[0].SecurityContext = tt.AppSCC

err := Init(pod, AgentConfig{"foobar-image", "http://foobar:1234", "test", "test", true, "100", "1000", tt.runAsSameUser})
require.Equal(t, err != nil, tt.expectedErr)
if err != nil {
return
}

agent, err := New(pod, patches)
require.NoError(t, err)
require.NoError(t, agent.Validate())

container, err := agent.ContainerSidecar()
require.NoError(t, err)

require.Equal(t, tt.expectedSidecarSCC, container.SecurityContext)
})
}
}
2 changes: 2 additions & 0 deletions agent-inject/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type Handler struct {
RevokeOnShutdown bool
UserID string
GroupID string
SameID bool
}

// Handle is the http.HandlerFunc implementation that actually handles the
Expand Down Expand Up @@ -146,6 +147,7 @@ func (h *Handler) Mutate(req *v1beta1.AdmissionRequest) *v1beta1.AdmissionRespon
RevokeOnShutdown: h.RevokeOnShutdown,
UserID: h.UserID,
GroupID: h.GroupID,
SameID: h.SameID,
}
err = agent.Init(&pod, cfg)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions subcommand/injector/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type Command struct {
flagRevokeOnShutdown bool // Revoke Vault Token on pod shutdown
flagRunAsUser string // User (uid) to run Vault agent as
flagRunAsGroup string // Group (gid) to run Vault agent as
flagRunAsSameUser bool // Run Vault agent as the User (uid) of the first application container

flagSet *flag.FlagSet

Expand Down Expand Up @@ -122,6 +123,7 @@ func (c *Command) Run(args []string) int {
RevokeOnShutdown: c.flagRevokeOnShutdown,
UserID: c.flagRunAsUser,
GroupID: c.flagRunAsGroup,
SameID: c.flagRunAsSameUser,
}

mux := http.NewServeMux()
Expand Down
14 changes: 14 additions & 0 deletions subcommand/injector/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ type Specification struct {

// RunAsGroup is the AGENT_INJECT_RUN_AS_GROUP environment variable. (gid)
RunAsGroup string `envconfig:"AGENT_INJECT_RUN_AS_GROUP"`

// RunAsSameUser is the AGENT_INJECT_RUN_AS_SAME_USER environment variable.
RunAsSameUser string `envconfig:"AGENT_INJECT_RUN_AS_SAME_USER"`
}

func (c *Command) init() {
Expand Down Expand Up @@ -88,6 +91,10 @@ func (c *Command) init() {
fmt.Sprintf("User (uid) to run Vault agent as. Defaults to %d.", agent.DefaultAgentRunAsUser))
c.flagSet.StringVar(&c.flagRunAsGroup, "run-as-group", strconv.Itoa(agent.DefaultAgentRunAsGroup),
fmt.Sprintf("Group (gid) to run Vault agent as. Defaults to %d.", agent.DefaultAgentRunAsGroup))
c.flagSet.BoolVar(&c.flagRunAsSameUser, "run-as-same-user", false,
"Run the injected Vault agent containers as the User (uid) of the first application container in the pod. "+
"Requires Spec.Containers[0].SecurityContext.RunAsUser to be set in the pod spec. "+
"Defaults to false.")

c.help = flags.Usage(help, c.flagSet)
}
Expand Down Expand Up @@ -176,5 +183,12 @@ func (c *Command) parseEnvs() error {
c.flagRunAsGroup = envs.RunAsGroup
}

if envs.RunAsSameUser != "" {
c.flagRunAsSameUser, err = strconv.ParseBool(envs.RunAsSameUser)
if err != nil {
return err
}
}

return nil
}
2 changes: 2 additions & 0 deletions subcommand/injector/flags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ func TestCommandEnvBools(t *testing.T) {
}{
{env: "AGENT_INJECT_REVOKE_ON_SHUTDOWN", value: true, cmdPtr: &cmd.flagRevokeOnShutdown},
{env: "AGENT_INJECT_REVOKE_ON_SHUTDOWN", value: false, cmdPtr: &cmd.flagRevokeOnShutdown},
{env: "AGENT_INJECT_RUN_AS_SAME_USER", value: true, cmdPtr: &cmd.flagRunAsSameUser},
{env: "AGENT_INJECT_RUN_AS_SAME_USER", value: false, cmdPtr: &cmd.flagRunAsSameUser},
}

for _, tt := range tests {
Expand Down