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

Kube like pods should share ipc,net,uts by default #10268

Merged
merged 1 commit into from
May 10, 2021
Merged

Kube like pods should share ipc,net,uts by default #10268

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
4 changes: 4 additions & 0 deletions pkg/specgen/generate/kube/kube.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ func ToPodGen(ctx context.Context, podName string, podYAML *v1.PodTemplateSpec)
p := specgen.NewPodSpecGenerator()
p.Name = podName
p.Labels = podYAML.ObjectMeta.Labels
// Kube pods must share {ipc, net, uts} by default
p.SharedNamespaces = append(p.SharedNamespaces, "ipc")
p.SharedNamespaces = append(p.SharedNamespaces, "net")
p.SharedNamespaces = append(p.SharedNamespaces, "uts")
// TODO we only configure Process namespace. We also need to account for Host{IPC,Network,PID}
// which is not currently possible with pod create
if podYAML.Spec.ShareProcessNamespace != nil && *podYAML.Spec.ShareProcessNamespace {
Expand Down
56 changes: 56 additions & 0 deletions test/e2e/play_kube_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,44 @@ metadata:
spec:
hostname: unknown
`
var sharedNamespacePodYaml = `
apiVersion: v1
kind: Pod
metadata:
creationTimestamp: "2021-05-07T17:25:01Z"
labels:
app: testpod1
name: testpod1
spec:
containers:
- command:
- top
- -d
- "1.5"
env:
- name: PATH
value: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
- name: TERM
value: xterm
- name: container
value: podman
- name: HOSTNAME
value: label-pod
image: quay.io/libpod/alpine:latest
name: alpine
resources: {}
securityContext:
allowPrivilegeEscalation: true
capabilities: {}
privileged: false
readOnlyRootFilesystem: false
seLinuxOptions: {}
workingDir: /
dnsConfig: {}
restartPolicy: Never
shareProcessNamespace: true
status: {}
`

var selinuxLabelPodYaml = `
apiVersion: v1
Expand Down Expand Up @@ -1004,6 +1042,24 @@ var _ = Describe("Podman play kube", func() {
Expect(label).To(ContainSubstring("unconfined_u:system_r:spc_t:s0"))
})

It("podman play kube should share ipc,net,uts when shareProcessNamespace is set", func() {
SkipIfRootless("Requires root priviledges for sharing few namespaces")
err := writeYaml(sharedNamespacePodYaml, kubeYaml)
Expect(err).To(BeNil())

kube := podmanTest.Podman([]string{"play", "kube", kubeYaml})
kube.WaitWithDefaultTimeout()
Expect(kube.ExitCode()).To(Equal(0))

inspect := podmanTest.Podman([]string{"inspect", "testpod1", "--format", "'{{ .SharedNamespaces }}'"})
inspect.WaitWithDefaultTimeout()
sharednamespaces := inspect.OutputToString()
Expect(sharednamespaces).To(ContainSubstring("ipc"))
Copy link
Member

Choose a reason for hiding this comment

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

can you add tests for uts and net too, or am I being oblivious and I'm missing them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@TomSweeneyRedHat You are right I missed checking string for uts and net. Thanks for pointing this out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah seems like CI flake is still there will open/close PR to re-kick CI.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@TomSweeneyRedHat this is resolved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

also need pid namespace

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@zhangguanzhang Bug was with only ipc, net, uts but sure case should also pass with pid

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@zhangguanzhang @TomSweeneyRedHat Added pid as well let me know if anything else needs to be done.

I think rdoproject.org/github-check is a flake , repush should fix it. But let me know if this check is mandatory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@zhangguanzhang ah it was just a flake force pushed made checks happy again.

Expect(sharednamespaces).To(ContainSubstring("net"))
Expect(sharednamespaces).To(ContainSubstring("uts"))
Expect(sharednamespaces).To(ContainSubstring("pid"))
})

It("podman play kube fail with nonexistent authfile", func() {
err := generateKubeYaml("pod", getPod(), kubeYaml)
Expect(err).To(BeNil())
Expand Down