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

set proper file permission for projected service account volume #89193

Merged
merged 1 commit into from
May 5, 2020
Merged

set proper file permission for projected service account volume #89193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions pkg/securitycontext/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ go_test(
"//pkg/apis/core:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//vendor/k8s.io/utils/pointer:go_default_library",
],
)

Expand Down
21 changes: 20 additions & 1 deletion pkg/securitycontext/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package securitycontext

import (
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
)

// HasPrivilegedRequest returns the value of SecurityContext.Privileged, taking into account
Expand Down Expand Up @@ -124,6 +124,25 @@ func DetermineEffectiveSecurityContext(pod *v1.Pod, container *v1.Container) *v1
return effectiveSc
}

// DetermineEffectiveRunAsUser returns a pointer of UID from the provided pod's
// and container's security context and a bool value to indicate if it is absent.
// Container's runAsUser take precedence in cases where both are set.
func DetermineEffectiveRunAsUser(pod *v1.Pod, container *v1.Container) (*int64, bool) {
var runAsUser *int64
if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.RunAsUser != nil {
runAsUser = new(int64)
*runAsUser = *pod.Spec.SecurityContext.RunAsUser
}
if container.SecurityContext != nil && container.SecurityContext.RunAsUser != nil {
zshihang marked this conversation as resolved.
Show resolved Hide resolved
runAsUser = new(int64)
*runAsUser = *container.SecurityContext.RunAsUser
}
if runAsUser == nil {
return nil, false
}
return runAsUser, true
}

func securityContextFromPodSecurityContext(pod *v1.Pod) *v1.SecurityContext {
if pod.Spec.SecurityContext == nil {
return nil
Expand Down
92 changes: 91 additions & 1 deletion pkg/securitycontext/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import (
"reflect"
"testing"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
utilptr "k8s.io/utils/pointer"
)

func TestAddNoNewPrivileges(t *testing.T) {
Expand Down Expand Up @@ -120,3 +121,92 @@ func TestConvertToRuntimeReadonlyPaths(t *testing.T) {
}
}
}

func TestDetermineEffectiveRunAsUser(t *testing.T) {
tests := []struct {
desc string
pod *v1.Pod
container *v1.Container
wantRunAsUser *int64
}{
{
desc: "no securityContext in pod, no securityContext in container",
pod: &v1.Pod{
Spec: v1.PodSpec{},
},
container: &v1.Container{},
wantRunAsUser: nil,
},
{
desc: "no runAsUser in pod, no runAsUser in container",
pod: &v1.Pod{
Spec: v1.PodSpec{
SecurityContext: &v1.PodSecurityContext{},
},
},
container: &v1.Container{
SecurityContext: &v1.SecurityContext{},
},
wantRunAsUser: nil,
},
{
desc: "runAsUser in pod, no runAsUser in container",
pod: &v1.Pod{
Spec: v1.PodSpec{
SecurityContext: &v1.PodSecurityContext{
RunAsUser: new(int64),
},
},
},
container: &v1.Container{
SecurityContext: &v1.SecurityContext{},
},
wantRunAsUser: new(int64),
},
{
desc: "no runAsUser in pod, runAsUser in container",
pod: &v1.Pod{
Spec: v1.PodSpec{
SecurityContext: &v1.PodSecurityContext{},
},
},
container: &v1.Container{
SecurityContext: &v1.SecurityContext{
RunAsUser: new(int64),
},
},
wantRunAsUser: new(int64),
},
{
desc: "no runAsUser in pod, runAsUser in container",
pod: &v1.Pod{
Spec: v1.PodSpec{
SecurityContext: &v1.PodSecurityContext{
RunAsUser: new(int64),
},
},
},
container: &v1.Container{
SecurityContext: &v1.SecurityContext{
RunAsUser: utilptr.Int64Ptr(1),
},
},
wantRunAsUser: utilptr.Int64Ptr(1),
},
}

for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
runAsUser, ok := DetermineEffectiveRunAsUser(test.pod, test.container)
if !ok && test.wantRunAsUser != nil {
t.Errorf("DetermineEffectiveRunAsUser(%v, %v) = %v, want %d", test.pod, test.container, runAsUser, *test.wantRunAsUser)
}
if ok && test.wantRunAsUser == nil {
t.Errorf("DetermineEffectiveRunAsUser(%v, %v) = %d, want %v", test.pod, test.container, *runAsUser, test.wantRunAsUser)
}
if ok && test.wantRunAsUser != nil && *runAsUser != *test.wantRunAsUser {
t.Errorf("DetermineEffectiveRunAsUser(%v, %v) = %d, want %d", test.pod, test.container, *runAsUser, *test.wantRunAsUser)
}
})
}
}
3 changes: 2 additions & 1 deletion pkg/volume/projected/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
"//staging/src/k8s.io/client-go/testing:go_default_library",
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
"//vendor/k8s.io/utils/pointer:go_default_library",
],
)

Expand Down
16 changes: 12 additions & 4 deletions pkg/volume/projected/projected.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (s *projectedVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterA
return err
}

data, err := s.collectData()
data, err := s.collectData(mounterArgs)
if err != nil {
klog.Errorf("Error preparing data for projected volume %v for pod %v/%v: %s", s.volName, s.pod.Namespace, s.pod.Name, err.Error())
return err
Expand Down Expand Up @@ -248,7 +248,7 @@ func (s *projectedVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterA
return nil
}

func (s *projectedVolumeMounter) collectData() (map[string]volumeutil.FileProjection, error) {
func (s *projectedVolumeMounter) collectData(mounterArgs volume.MounterArgs) (map[string]volumeutil.FileProjection, error) {
if s.source.DefaultMode == nil {
return nil, fmt.Errorf("No defaultMode used, not even the default value for it")
}
Expand Down Expand Up @@ -328,6 +328,13 @@ func (s *projectedVolumeMounter) collectData() (map[string]volumeutil.FileProjec
}
tp := source.ServiceAccountToken

// When FsGroup is set, we depend on SetVolumeOwnership to
// change from 0600 to 0640.
mode := *s.source.DefaultMode
mikedanese marked this conversation as resolved.
Show resolved Hide resolved
mikedanese marked this conversation as resolved.
Show resolved Hide resolved
if mounterArgs.FsUser != nil || mounterArgs.FsGroup != nil {
mode = 0600
}

Copy link
Member

Choose a reason for hiding this comment

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

So one flip side of this implementation is - it won't easily work for other ephemeral volume types, which was discussed in sig-storage call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, it is intended to limit the scope to only projected service account token but this can be easily extended to other projected sources, non-projected secrets, non-projected configMap..etc.

var auds []string
if len(tp.Audience) != 0 {
auds = []string{tp.Audience}
Expand All @@ -349,8 +356,9 @@ func (s *projectedVolumeMounter) collectData() (map[string]volumeutil.FileProjec
continue
}
payload[tp.Path] = volumeutil.FileProjection{
Data: []byte(tr.Status.Token),
Mode: 0600,
Data: []byte(tr.Status.Token),
Mode: mode,
FsUser: mounterArgs.FsUser,
}
}
}
Expand Down
Loading