Skip to content

Commit

Permalink
Allow container visitor to operate on selected container types
Browse files Browse the repository at this point in the history 
Signed-off-by: Ted Yu <[email protected]>
  • Loading branch information
@tedyu
tedyu committed Mar 5, 2020
1 parent d682c83 commit af0e131
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 26 deletions.
71 changes: 49 additions & 22 deletions pkg/api/pod/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,55 @@ import (
"k8s.io/kubernetes/pkg/security/apparmor"
)

// ContainerType signifies container type
type ContainerType int

// DefaultContainers defines default behavior: Iterate containers based on feature gates
const DefaultContainers ContainerType = 0

const (
// Containers is for normal containers
Containers ContainerType = 1 << iota
// InitContainers is for init containers
InitContainers
// EphemeralContainers is for ephemeral containers
EphemeralContainers
)

// AllContainers specifies that all containers be visited
const AllContainers ContainerType = (InitContainers | Containers | EphemeralContainers)

// ContainerVisitor is called with each container spec, and returns true
// if visiting should continue.
type ContainerVisitor func(container *api.Container) (shouldContinue bool)
type ContainerVisitor func(container *api.Container, containerType ContainerType) (shouldContinue bool)

// VisitContainers invokes the visitor function with a pointer to the container
// spec of every container in the given pod spec. If visitor returns false,
// visiting is short-circuited. VisitContainers returns true if visiting completes,
// false if visiting was short-circuited.
func VisitContainers(podSpec *api.PodSpec, visitor ContainerVisitor) bool {
for i := range podSpec.InitContainers {
if !visitor(&podSpec.InitContainers[i]) {
return false
//
// With the default mask (zero value or DefaultContainers) VisitContainers will visit all containers
// enabled by current feature gates. If mask is non-zero, VisitContainers will unconditionally visit
// container types specified by mask, and no feature gate checks will be performed.
func VisitContainers(podSpec *api.PodSpec, mask ContainerType, visitor ContainerVisitor) bool {
if mask == DefaultContainers || (mask&InitContainers) > 0 {
for i := range podSpec.InitContainers {
if !visitor(&podSpec.InitContainers[i], InitContainers) {
return false
}
}
}
for i := range podSpec.Containers {
if !visitor(&podSpec.Containers[i]) {
return false
if mask == DefaultContainers || (mask&Containers) > 0 {
for i := range podSpec.Containers {
if !visitor(&podSpec.Containers[i], Containers) {
return false
}
}
}
if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) {
if (mask == DefaultContainers && utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers)) ||
(mask&EphemeralContainers) > 0 {
for i := range podSpec.EphemeralContainers {
if !visitor((*api.Container)(&podSpec.EphemeralContainers[i].EphemeralContainerCommon)) {
if !visitor((*api.Container)(&podSpec.EphemeralContainers[i].EphemeralContainerCommon), EphemeralContainers) {
return false
}
}
Expand All @@ -68,7 +95,7 @@ func VisitPodSecretNames(pod *api.Pod, visitor Visitor) bool {
return false
}
}
VisitContainers(&pod.Spec, func(c *api.Container) bool {
VisitContainers(&pod.Spec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
return visitContainerSecretNames(c, visitor)
})
var source *api.VolumeSource
Expand Down Expand Up @@ -151,7 +178,7 @@ func visitContainerSecretNames(container *api.Container, visitor Visitor) bool {
// Transitive references (e.g. pod -> pvc -> pv -> secret) are not visited.
// Returns true if visiting completed, false if visiting was short-circuited.
func VisitPodConfigmapNames(pod *api.Pod, visitor Visitor) bool {
VisitContainers(&pod.Spec, func(c *api.Container) bool {
VisitContainers(&pod.Spec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
return visitContainerConfigmapNames(c, visitor)
})
var source *api.VolumeSource
Expand Down Expand Up @@ -356,7 +383,7 @@ func dropDisabledFields(

if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) && !subpathInUse(oldPodSpec) {
// drop subpath from the pod if the feature is disabled and the old spec did not specify subpaths
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
for i := range c.VolumeMounts {
c.VolumeMounts[i].SubPath = ""
}
Expand All @@ -369,7 +396,7 @@ func dropDisabledFields(

if (!utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) || !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpathEnvExpansion)) && !subpathExprInUse(oldPodSpec) {
// drop subpath env expansion from the pod if either of the subpath features is disabled and the old spec did not specify subpath env expansion
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
for i := range c.VolumeMounts {
c.VolumeMounts[i].SubPathExpr = ""
}
Expand All @@ -379,7 +406,7 @@ func dropDisabledFields(

if !utilfeature.DefaultFeatureGate.Enabled(features.StartupProbe) && !startupProbeInUse(oldPodSpec) {
// drop startupProbe from all containers if the feature is disabled
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
c.StartupProbe = nil
return true
})
Expand Down Expand Up @@ -421,7 +448,7 @@ func dropDisabledRunAsGroupField(podSpec, oldPodSpec *api.PodSpec) {
if podSpec.SecurityContext != nil {
podSpec.SecurityContext.RunAsGroup = nil
}
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
if c.SecurityContext != nil {
c.SecurityContext.RunAsGroup = nil
}
Expand All @@ -435,7 +462,7 @@ func dropDisabledRunAsGroupField(podSpec, oldPodSpec *api.PodSpec) {
func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.ProcMountType) && !procMountInUse(oldPodSpec) {
defaultProcMount := api.DefaultProcMount
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
if c.SecurityContext != nil && c.SecurityContext.ProcMount != nil {
// The ProcMount field was improperly forced to non-nil in 1.12.
// If the feature is disabled, and the existing object is not using any non-default values, and the ProcMount field is present in the incoming object, force to the default value.
Expand Down Expand Up @@ -471,7 +498,7 @@ func subpathInUse(podSpec *api.PodSpec) bool {
}

var inUse bool
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
for i := range c.VolumeMounts {
if len(c.VolumeMounts[i].SubPath) > 0 {
inUse = true
Expand Down Expand Up @@ -521,7 +548,7 @@ func procMountInUse(podSpec *api.PodSpec) bool {
}

var inUse bool
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
if c.SecurityContext == nil || c.SecurityContext.ProcMount == nil {
return true
}
Expand Down Expand Up @@ -609,7 +636,7 @@ func runAsGroupInUse(podSpec *api.PodSpec) bool {
}

var inUse bool
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
if c.SecurityContext != nil && c.SecurityContext.RunAsGroup != nil {
inUse = true
return false
Expand All @@ -627,7 +654,7 @@ func subpathExprInUse(podSpec *api.PodSpec) bool {
}

var inUse bool
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
for i := range c.VolumeMounts {
if len(c.VolumeMounts[i].SubPathExpr) > 0 {
inUse = true
Expand All @@ -647,7 +674,7 @@ func startupProbeInUse(podSpec *api.PodSpec) bool {
}

var inUse bool
VisitContainers(podSpec, func(c *api.Container) bool {
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
if c.StartupProbe != nil {
inUse = true
return false
Expand Down
29 changes: 28 additions & 1 deletion pkg/api/pod/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,13 @@ func TestVisitContainers(t *testing.T) {
description string
haveSpec *api.PodSpec
wantNames []string
mask ContainerType
}{
{
"empty podspec",
&api.PodSpec{},
[]string{},
DefaultContainers,
},
{
"regular containers",
Expand All @@ -56,6 +58,7 @@ func TestVisitContainers(t *testing.T) {
},
},
[]string{"c1", "c2"},
DefaultContainers,
},
{
"init containers",
Expand All @@ -66,6 +69,7 @@ func TestVisitContainers(t *testing.T) {
},
},
[]string{"i1", "i2"},
DefaultContainers,
},
{
"regular and init containers",
Expand All @@ -80,6 +84,7 @@ func TestVisitContainers(t *testing.T) {
},
},
[]string{"i1", "i2", "c1", "c2"},
DefaultContainers,
},
{
"ephemeral containers",
Expand All @@ -93,6 +98,7 @@ func TestVisitContainers(t *testing.T) {
},
},
[]string{"c1", "c2", "e1"},
DefaultContainers,
},
{
"all container types",
Expand All @@ -111,6 +117,26 @@ func TestVisitContainers(t *testing.T) {
},
},
[]string{"i1", "i2", "c1", "c2", "e1", "e2"},
DefaultContainers,
},
{
"all container types with init and regular container types chosen",
&api.PodSpec{
Containers: []api.Container{
{Name: "c1"},
{Name: "c2"},
},
InitContainers: []api.Container{
{Name: "i1"},
{Name: "i2"},
},
EphemeralContainers: []api.EphemeralContainer{
{EphemeralContainerCommon: api.EphemeralContainerCommon{Name: "e1"}},
{EphemeralContainerCommon: api.EphemeralContainerCommon{Name: "e2"}},
},
},
[]string{"i1", "i2", "c1", "c2"},
Containers | InitContainers,
},
{
"dropping fields",
Expand All @@ -129,12 +155,13 @@ func TestVisitContainers(t *testing.T) {
},
},
[]string{"i1", "i2", "c1", "c2", "e1", "e2"},
DefaultContainers,
},
}

for _, tc := range testCases {
gotNames := []string{}
VisitContainers(tc.haveSpec, func(c *api.Container) bool {
VisitContainers(tc.haveSpec, tc.mask, func(c *api.Container, containerType ContainerType) bool {
gotNames = append(gotNames, c.Name)
if c.SecurityContext != nil {
c.SecurityContext = nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/registry/core/pod/strategy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func LogLocation(

func podHasContainerWithName(pod *api.Pod, containerName string) bool {
var hasContainer bool
podutil.VisitContainers(&pod.Spec, func(c *api.Container) bool {
podutil.VisitContainers(&pod.Spec, podutil.DefaultContainers, func(c *api.Container, containerType podutil.ContainerType) bool {
if c.Name == containerName {
hasContainer = true
return false
Expand Down Expand Up @@ -554,7 +554,7 @@ func validateContainer(container string, pod *api.Pod) (string, error) {
return "", errors.NewBadRequest(fmt.Sprintf("a container name must be specified for pod %s", pod.Name))
default:
var containerNames []string
podutil.VisitContainers(&pod.Spec, func(c *api.Container) bool {
podutil.VisitContainers(&pod.Spec, podutil.DefaultContainers, func(c *api.Container, containerType podutil.ContainerType) bool {
containerNames = append(containerNames, c.Name)
return true
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/security/podsecuritypolicy/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (s *simpleProvider) MutatePod(pod *api.Pod) error {
}

var retErr error
podutil.VisitContainers(&pod.Spec, func(c *api.Container) bool {
podutil.VisitContainers(&pod.Spec, podutil.AllContainers, func(c *api.Container, containerType podutil.ContainerType) bool {
retErr = s.mutateContainer(pod, c)
if retErr != nil {
return false
Expand Down

0 comments on commit af0e131

Please sign in to comment.