Skip to content

Commit d769f03

Browse files
samuelkarpsamuelkarp
authored
Merge pull request #7882 from kinvolk/rata/userns-stateless-pods
2 parents 426175e + 72ef986 commit d769f03

File tree

3 files changed

+135
-7
lines changed

3 files changed

+135
-7
lines changed

‎pkg/cri/server/container_create_linux.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,12 @@ func (c *criService) containerSpec(
316316
return nil, fmt.Errorf("user namespace configuration: %w", err)
317317
}
318318

319+
// Check sandbox userns config is consistent with container config.
320+
sandboxUsernsOpts := sandboxConfig.GetLinux().GetSecurityContext().GetNamespaceOptions().GetUsernsOptions()
321+
if !sameUsernsConfig(sandboxUsernsOpts, nsOpts.GetUsernsOptions()) {
322+
return nil, fmt.Errorf("user namespace config for sandbox is different from container. Sandbox userns config: %v - Container userns config: %v", sandboxUsernsOpts, nsOpts.GetUsernsOptions())
323+
}
324+
319325
specOpts = append(specOpts,
320326
customopts.WithOOMScoreAdj(config, c.config.RestrictOOMScoreAdj),
321327
customopts.WithPodNamespaces(securityContext, sandboxPid, targetPid, uids, gids),

‎pkg/cri/server/container_create_linux_test.go‎

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,11 @@ func TestUserNamespace(t *testing.T) {
814814
ContainerId: 1000,
815815
Length: 10,
816816
}
817+
otherIDMap := runtime.IDMapping{
818+
HostId: 2000,
819+
ContainerId: 2000,
820+
Length: 10,
821+
}
817822
expIDMap := runtimespec.LinuxIDMapping{
818823
HostID: 1000,
819824
ContainerID: 1000,
@@ -824,6 +829,7 @@ func TestUserNamespace(t *testing.T) {
824829
c := newTestCRIService()
825830
for desc, test := range map[string]struct {
826831
userNS *runtime.UserNamespace
832+
sandboxUserNS *runtime.UserNamespace
827833
expNS *runtimespec.LinuxNamespace
828834
expNotNS *runtimespec.LinuxNamespace // Does NOT contain this namespace
829835
expUIDMapping []runtimespec.LinuxIDMapping
@@ -871,6 +877,58 @@ func TestUserNamespace(t *testing.T) {
871877
expUIDMapping: []runtimespec.LinuxIDMapping{expIDMap},
872878
expGIDMapping: []runtimespec.LinuxIDMapping{expIDMap},
873879
},
880+
"pod namespace mode with inconsistent sandbox config (different GIDs)": {
881+
userNS: &runtime.UserNamespace{
882+
Mode: runtime.NamespaceMode_POD,
883+
Uids: []*runtime.IDMapping{&idMap},
884+
Gids: []*runtime.IDMapping{&idMap},
885+
},
886+
sandboxUserNS: &runtime.UserNamespace{
887+
Mode: runtime.NamespaceMode_POD,
888+
Uids: []*runtime.IDMapping{&idMap},
889+
Gids: []*runtime.IDMapping{&otherIDMap},
890+
},
891+
err: true,
892+
},
893+
"pod namespace mode with inconsistent sandbox config (different UIDs)": {
894+
userNS: &runtime.UserNamespace{
895+
Mode: runtime.NamespaceMode_POD,
896+
Uids: []*runtime.IDMapping{&idMap},
897+
Gids: []*runtime.IDMapping{&idMap},
898+
},
899+
sandboxUserNS: &runtime.UserNamespace{
900+
Mode: runtime.NamespaceMode_POD,
901+
Uids: []*runtime.IDMapping{&otherIDMap},
902+
Gids: []*runtime.IDMapping{&idMap},
903+
},
904+
err: true,
905+
},
906+
"pod namespace mode with inconsistent sandbox config (different len)": {
907+
userNS: &runtime.UserNamespace{
908+
Mode: runtime.NamespaceMode_POD,
909+
Uids: []*runtime.IDMapping{&idMap},
910+
Gids: []*runtime.IDMapping{&idMap},
911+
},
912+
sandboxUserNS: &runtime.UserNamespace{
913+
Mode: runtime.NamespaceMode_POD,
914+
Uids: []*runtime.IDMapping{&idMap, &idMap},
915+
Gids: []*runtime.IDMapping{&idMap, &idMap},
916+
},
917+
err: true,
918+
},
919+
"pod namespace mode with inconsistent sandbox config (different mode)": {
920+
userNS: &runtime.UserNamespace{
921+
Mode: runtime.NamespaceMode_POD,
922+
Uids: []*runtime.IDMapping{&idMap},
923+
Gids: []*runtime.IDMapping{&idMap},
924+
},
925+
sandboxUserNS: &runtime.UserNamespace{
926+
Mode: runtime.NamespaceMode_NODE,
927+
Uids: []*runtime.IDMapping{&idMap},
928+
Gids: []*runtime.IDMapping{&idMap},
929+
},
930+
err: true,
931+
},
874932
"pod namespace mode with several mappings": {
875933
userNS: &runtime.UserNamespace{
876934
Mode: runtime.NamespaceMode_POD,
@@ -892,14 +950,23 @@ func TestUserNamespace(t *testing.T) {
892950
test := test
893951
t.Run(desc, func(t *testing.T) {
894952
containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{UsernsOptions: test.userNS}
953+
// By default, set sandbox and container config to the same (this is
954+
// required by containerSpec). However, if the test wants to test for what
955+
// happens when they don't match, the test.sandboxUserNS should be set and
956+
// we just use that.
957+
sandboxUserns := test.userNS
958+
if test.sandboxUserNS != nil {
959+
sandboxUserns = test.sandboxUserNS
960+
}
961+
sandboxConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{UsernsOptions: sandboxUserns}
895962
spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
896963

897964
if test.err {
898-
assert.Error(t, err)
965+
require.Error(t, err)
899966
assert.Nil(t, spec)
900967
return
901968
}
902-
assert.NoError(t, err)
969+
require.NoError(t, err)
903970
assert.Equal(t, spec.Linux.UIDMappings, test.expUIDMapping)
904971
assert.Equal(t, spec.Linux.GIDMappings, test.expGIDMapping)
905972

‎pkg/cri/server/helpers_linux.go‎

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -319,15 +319,12 @@ func parseUsernsIDs(userns *runtime.UserNamespace) (uids, gids []specs.LinuxIDMa
319319
return nil, nil, nil
320320
}
321321

322-
uidRuntimeMap := userns.GetUids()
323-
gidRuntimeMap := userns.GetGids()
324-
325-
uids, err := parseUsernsIDMap(uidRuntimeMap)
322+
uids, err := parseUsernsIDMap(userns.GetUids())
326323
if err != nil {
327324
return nil, nil, fmt.Errorf("UID mapping: %w", err)
328325
}
329326

330-
gids, err = parseUsernsIDMap(gidRuntimeMap)
327+
gids, err = parseUsernsIDMap(userns.GetGids())
331328
if err != nil {
332329
return nil, nil, fmt.Errorf("GID mapping: %w", err)
333330
}
@@ -349,6 +346,64 @@ func parseUsernsIDs(userns *runtime.UserNamespace) (uids, gids []specs.LinuxIDMa
349346
return uids, gids, nil
350347
}
351348

349+
// sameUsernsConfig checks if the userns configs are the same. If the mappings
350+
// on each config are the same but in different order, it returns false.
351+
// XXX: If the runtime.UserNamespace struct changes, we should update this
352+
// function accordingly.
353+
func sameUsernsConfig(a, b *runtime.UserNamespace) bool {
354+
// If both are nil, they are the same.
355+
if a == nil && b == nil {
356+
return true
357+
}
358+
// If only one is nil, they are different.
359+
if a == nil || b == nil {
360+
return false
361+
}
362+
// At this point, a is not nil nor b.
363+
364+
if a.GetMode() != b.GetMode() {
365+
return false
366+
}
367+
368+
aUids, aGids, err := parseUsernsIDs(a)
369+
if err != nil {
370+
return false
371+
}
372+
bUids, bGids, err := parseUsernsIDs(b)
373+
if err != nil {
374+
return false
375+
}
376+
377+
if !sameMapping(aUids, bUids) {
378+
return false
379+
}
380+
if !sameMapping(aGids, bGids) {
381+
return false
382+
}
383+
return true
384+
}
385+
386+
// sameMapping checks if the mappings are the same. If the mappings are the same
387+
// but in different order, it returns false.
388+
func sameMapping(a, b []specs.LinuxIDMapping) bool {
389+
if len(a) != len(b) {
390+
return false
391+
}
392+
393+
for x := range a {
394+
if a[x].ContainerID != b[x].ContainerID {
395+
return false
396+
}
397+
if a[x].HostID != b[x].HostID {
398+
return false
399+
}
400+
if a[x].Size != b[x].Size {
401+
return false
402+
}
403+
}
404+
return true
405+
}
406+
352407
func snapshotterRemapOpts(nsOpts *runtime.NamespaceOption) ([]snapshots.Opt, error) {
353408
snapshotOpt := []snapshots.Opt{}
354409
usernsOpts := nsOpts.GetUsernsOptions()

0 commit comments

Comments
 (0)