Skip to content

Commit

Permalink
Set HostUsers=false for unprivileged build pods + UserNamespaces
Browse files Browse the repository at this point in the history 
If the UserNamespacesStatelessPodsSupport feature is enabled, set
HostUsers=false for unprivileged build pods, in addition to the
non-standardized annotation that older versions of CRI-O could be
configured to accept instead.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
  • Loading branch information
@nalind
nalind committed Aug 8, 2023
1 parent 80c4923 commit 95db506
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 22 deletions.
3 changes: 2 additions & 1 deletion pkg/build/controller/strategy/custom.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func init() {

// CustomBuildStrategy creates a build using a custom builder image.
type CustomBuildStrategy struct {
HostUsersForUserNS bool // use "hostUsers: false" to ask to run in a user namespace because we're "stateless" (i.e., have no volumes which will persist beyond the life of this pod)
}

// CreateBuildPod creates the pod to be used for the Custom build
Expand Down Expand Up @@ -138,7 +139,7 @@ func (bs *CustomBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
setupBuildCAs(build, pod, additionalCAs, internalRegistryHost)
setupContainersStorage(pod, &pod.Spec.Containers[0])
if securityContext == nil || securityContext.Privileged == nil || !*securityContext.Privileged {
setupBuilderAutonsUser(build, strategy.Env, pod)
setupBuilderAutonsUser(build, strategy.Env, bs.HostUsersForUserNS, pod)
setupBuilderDeviceFUSE(pod)
}
return pod, nil
Expand Down
3 changes: 3 additions & 0 deletions pkg/build/controller/strategy/custom_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,5 +290,8 @@ func TestCustomCreateBuildPodAutonsUser(t *testing.T) {
func(build *buildv1.Build, env corev1.EnvVar) {
build.Spec.Strategy.CustomStrategy.Env = append(build.Spec.Strategy.CustomStrategy.Env, env)
},
func(hostUsersForUserNS bool) {
strategy.HostUsersForUserNS = hostUsersForUserNS
},
)
}
3 changes: 2 additions & 1 deletion pkg/build/controller/strategy/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func init() {
type DockerBuildStrategy struct {
Image string
BuildCSIVolumesEnabled bool
HostUsersForUserNS bool // use "hostUsers: false" to ask to run in a user namespace
}

// CreateBuildPod creates the pod to be used for the Docker build
Expand Down Expand Up @@ -210,7 +211,7 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
setupBuildCAs(build, pod, additionalCAs, internalRegistryHost)
setupContainersStorage(pod, &pod.Spec.Containers[0])
if securityContext == nil || securityContext.Privileged == nil || !*securityContext.Privileged {
setupBuilderAutonsUser(build, strategy.Env, pod)
setupBuilderAutonsUser(build, strategy.Env, bs.HostUsersForUserNS, pod)
setupBuilderDeviceFUSE(pod)
}
setupBlobCache(pod)
Expand Down
3 changes: 3 additions & 0 deletions pkg/build/controller/strategy/docker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,5 +251,8 @@ func TestDockerCreateBuildPodAutonsUser(t *testing.T) {
func(build *buildv1.Build, env corev1.EnvVar) {
build.Spec.Strategy.DockerStrategy.Env = append(build.Spec.Strategy.DockerStrategy.Env, env)
},
func(hostUsersForUserNS bool) {
strategy.HostUsersForUserNS = hostUsersForUserNS
},
)
}
3 changes: 2 additions & 1 deletion pkg/build/controller/strategy/sti.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type SourceBuildStrategy struct {
Image string
SecurityClient securityclient.SecurityV1Interface
BuildCSIVolumeseEnabled bool
HostUsersForUserNS bool // use "hostUsers: false" to ask to run in a user namespace
}

// DefaultDropCaps is the list of capabilities to drop if the current user cannot run as root
Expand Down Expand Up @@ -217,7 +218,7 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
setupBuildCAs(build, pod, additionalCAs, internalRegistryHost)
setupContainersStorage(pod, &pod.Spec.Containers[0])
if securityContext == nil || securityContext.Privileged == nil || !*securityContext.Privileged {
setupBuilderAutonsUser(build, strategy.Env, pod)
setupBuilderAutonsUser(build, strategy.Env, bs.HostUsersForUserNS, pod)
setupBuilderDeviceFUSE(pod)
}
setupBlobCache(pod)
Expand Down
3 changes: 3 additions & 0 deletions pkg/build/controller/strategy/sti_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,5 +317,8 @@ func TestS2ICreateBuildPodAutonsUser(t *testing.T) {
func(build *buildv1.Build, env corev1.EnvVar) {
build.Spec.Strategy.SourceStrategy.Env = append(build.Spec.Strategy.SourceStrategy.Env, env)
},
func(hostUsersForUserNS bool) {
strategy.HostUsersForUserNS = hostUsersForUserNS
},
)
}
6 changes: 5 additions & 1 deletion pkg/build/controller/strategy/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -570,9 +570,13 @@ func setupBuilderDeviceFUSE(pod *corev1.Pod) {

// Request that the builder be run in a user namespace, mapped from host ID
// ranges chosen by the node.
func setupBuilderAutonsUser(build *buildv1.Build, vars []corev1.EnvVar, pod *corev1.Pod) {
func setupBuilderAutonsUser(build *buildv1.Build, vars []corev1.EnvVar, hostUsersForUserNS bool, pod *corev1.Pod) {
metav1.SetMetaDataAnnotation(&pod.ObjectMeta, "io.openshift.builder", "")
metav1.SetMetaDataAnnotation(&pod.ObjectMeta, "io.kubernetes.cri-o.userns-mode", "auto:size=65536")
if hostUsersForUserNS {
f := false
pod.Spec.HostUsers = &f
}
}

// setupBuildCAs mounts certificate authorities for the build from a predetermined ConfigMap.
Expand Down
109 changes: 92 additions & 17 deletions pkg/build/controller/strategy/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -820,67 +820,135 @@ type buildPodCreator interface {
CreateBuildPod(build *buildv1.Build, additionalCAs map[string]string, internalRegistryHost string) (*corev1.Pod, error)
}

func testCreateBuildPodAutonsUser(t *testing.T, build *buildv1.Build, strategy buildPodCreator, addEnv func(build *buildv1.Build, env corev1.EnvVar)) {
func testCreateBuildPodAutonsUser(t *testing.T, build *buildv1.Build, strategy buildPodCreator, addEnv func(build *buildv1.Build, env corev1.EnvVar), setHostUsersForUserNS func(hostUsersForUserNS bool)) {
for _, testCase := range []struct {
env string
expectError bool
privileged bool
annotations map[string]string
noAnnotations []string
env string
expectError bool
privileged bool
useHostUsers bool
annotations map[string]string
noAnnotations []string
expectHostUsers bool
}{
{
env: "",
privileged: true,
env: "",
privileged: true,
useHostUsers: false,
noAnnotations: []string{
"io.openshift.builder",
"io.kubernetes.cri-o.Devices",
"io.kubernetes.cri-o.userns-mode",
},
expectHostUsers: true,
},
{
env: "BUILD_PRIVILEGED=0",
privileged: false,
env: "",
privileged: true,
useHostUsers: true,
noAnnotations: []string{
"io.openshift.builder",
"io.kubernetes.cri-o.Devices",
"io.kubernetes.cri-o.userns-mode",
},
expectHostUsers: true,
},
{
env: "BUILD_PRIVILEGED=0",
privileged: false,
useHostUsers: false,
annotations: map[string]string{
"io.openshift.builder": "",
"io.kubernetes.cri-o.Devices": "/dev/fuse:rwm",
"io.kubernetes.cri-o.userns-mode": "auto:size=65536",
},
expectHostUsers: true, // won't set HostUsers: false without the UserNamespacesStatelessPodsSupport feature
},
{
env: "BUILD_PRIVILEGED=42",
privileged: true,
env: "BUILD_PRIVILEGED=0",
privileged: false,
useHostUsers: true,
annotations: map[string]string{
"io.openshift.builder": "",
"io.kubernetes.cri-o.Devices": "/dev/fuse:rwm",
"io.kubernetes.cri-o.userns-mode": "auto:size=65536",
},
expectHostUsers: false,
},
{
env: "BUILD_PRIVILEGED=42",
privileged: true,
useHostUsers: false,
noAnnotations: []string{
"io.openshift.builder",
"io.kubernetes.cri-o.Devices",
"io.kubernetes.cri-o.userns-mode",
},
expectHostUsers: true,
},
{
env: "BUILD_PRIVILEGED=false",
privileged: false,
env: "BUILD_PRIVILEGED=42",
privileged: true,
useHostUsers: true,
noAnnotations: []string{
"io.openshift.builder",
"io.kubernetes.cri-o.Devices",
"io.kubernetes.cri-o.userns-mode",
},
expectHostUsers: true,
},
{
env: "BUILD_PRIVILEGED=false",
privileged: false,
useHostUsers: false,
annotations: map[string]string{
"io.openshift.builder": "",
"io.kubernetes.cri-o.Devices": "/dev/fuse:rwm",
"io.kubernetes.cri-o.userns-mode": "auto:size=65536",
},
expectHostUsers: true, // won't set HostUsers: false without the UserNamespacesStatelessPodsSupport feature
},
{
env: "BUILD_PRIVILEGED=false",
privileged: false,
useHostUsers: true,
annotations: map[string]string{
"io.openshift.builder": "",
"io.kubernetes.cri-o.Devices": "/dev/fuse:rwm",
"io.kubernetes.cri-o.userns-mode": "auto:size=65536",
},
expectHostUsers: false,
},
{
env: "BUILD_PRIVILEGED=true",
privileged: true,
env: "BUILD_PRIVILEGED=true",
privileged: true,
useHostUsers: false,
noAnnotations: []string{
"io.openshift.builder",
"io.kubernetes.cri-o.Devices",
"io.kubernetes.cri-o.userns-mode",
},
expectHostUsers: true,
},
{
env: "BUILD_PRIVILEGED=true",
privileged: true,
useHostUsers: true,
noAnnotations: []string{
"io.openshift.builder",
"io.kubernetes.cri-o.Devices",
"io.kubernetes.cri-o.userns-mode",
},
expectHostUsers: true,
},
} {
t.Run(testCase.env, func(t *testing.T) {
t.Run(testCase.env+fmt.Sprintf(",UseHostUsers=%t", testCase.useHostUsers), func(t *testing.T) {
build := build.DeepCopy()
for _, envVar := range strings.Split(testCase.env, ":") {
if env := strings.SplitN(envVar, "=", 2); len(env) > 1 {
addEnv(build, corev1.EnvVar{Name: env[0], Value: env[1]})
}
}
setHostUsersForUserNS(testCase.useHostUsers)
actual, err := strategy.CreateBuildPod(build, nil, testInternalRegistryHost)
if err != nil {
t.Errorf("Unexpected error: %v", err)
Expand Down Expand Up @@ -915,6 +983,13 @@ func testCreateBuildPodAutonsUser(t *testing.T, build *buildv1.Build, strategy b
t.Errorf("Expected no %q annotation, but got one", annotation)
}
}
hostUsers := true
if actual.Spec.HostUsers != nil {
hostUsers = *actual.Spec.HostUsers
}
if hostUsers != testCase.expectHostUsers {
t.Errorf("Expected to see pod HostUsers=%t, got %t (%#v)", testCase.expectHostUsers, hostUsers, actual.Spec.HostUsers)
}
})
}
}
12 changes: 11 additions & 1 deletion pkg/cmd/controller/build.go
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package controller

import (
"fmt"
"strings"

clientset "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/features"

buildclient "github.com/openshift/client-go/build/clientset/versioned"
buildcontroller "github.com/openshift/openshift-controller-manager/pkg/build/controller/build"
Expand Down Expand Up @@ -55,12 +57,16 @@ func RunBuildController(ctx *ControllerContext) (bool, error) {

fg := ctx.OpenshiftControllerConfig.FeatureGates
csiVolumesEnabled := false
hostUsersForUserNS := false
if fg != nil {
for _, v := range fg {
v = strings.TrimSpace(v)
if v == "BuildCSIVolumes=true" {
csiVolumesEnabled = true
}
if v == fmt.Sprintf("%s=true", string(features.UserNamespacesStatelessPodsSupport)) {
hostUsersForUserNS = true
}
}
}

Expand All @@ -85,13 +91,17 @@ func RunBuildController(ctx *ControllerContext) (bool, error) {
DockerBuildStrategy: &buildstrategy.DockerBuildStrategy{
Image: imageTemplate.ExpandOrDie("docker-builder"),
BuildCSIVolumesEnabled: csiVolumesEnabled,
HostUsersForUserNS: hostUsersForUserNS,
},
SourceBuildStrategy: &buildstrategy.SourceBuildStrategy{
Image: imageTemplate.ExpandOrDie("docker-builder"),
SecurityClient: securityClient.SecurityV1(),
BuildCSIVolumeseEnabled: csiVolumesEnabled,
HostUsersForUserNS: hostUsersForUserNS,
},
CustomBuildStrategy: &buildstrategy.CustomBuildStrategy{
HostUsersForUserNS: hostUsersForUserNS,
},
CustomBuildStrategy: &buildstrategy.CustomBuildStrategy{},
BuildDefaults: builddefaults.BuildDefaults{Config: ctx.OpenshiftControllerConfig.Build.BuildDefaults},
BuildOverrides: buildoverrides.BuildOverrides{Config: ctx.OpenshiftControllerConfig.Build.BuildOverrides},
InternalRegistryHostname: ctx.OpenshiftControllerConfig.DockerPullSecret.InternalRegistryHostname,
Expand Down

0 comments on commit 95db506

Please sign in to comment.