Merge pull request #7882 from kinvolk/rata/userns-stateless-pods

pkg/cri/server/container_create_linux.go

@@ -316,6 +316,12 @@ func (c *criService) containerSpec(
 		return nil, fmt.Errorf("user namespace configuration: %w", err)
 	}
 
+	// Check sandbox userns config is consistent with container config.
+	sandboxUsernsOpts := sandboxConfig.GetLinux().GetSecurityContext().GetNamespaceOptions().GetUsernsOptions()
+	if !sameUsernsConfig(sandboxUsernsOpts, nsOpts.GetUsernsOptions()) {
+		return nil, fmt.Errorf("user namespace config for sandbox is different from container. Sandbox userns config: %v - Container userns config: %v", sandboxUsernsOpts, nsOpts.GetUsernsOptions())
+	}
+
 	specOpts = append(specOpts,
 		customopts.WithOOMScoreAdj(config, c.config.RestrictOOMScoreAdj),
 		customopts.WithPodNamespaces(securityContext, sandboxPid, targetPid, uids, gids),

pkg/cri/server/container_create_linux_test.go

@@ -814,6 +814,11 @@ func TestUserNamespace(t *testing.T) {
 		ContainerId: 1000,
 		Length:      10,
 	}
+	otherIDMap := runtime.IDMapping{
+		HostId:      2000,
+		ContainerId: 2000,
+		Length:      10,
+	}
 	expIDMap := runtimespec.LinuxIDMapping{
 		HostID:      1000,
 		ContainerID: 1000,
@@ -824,6 +829,7 @@ func TestUserNamespace(t *testing.T) {
 	c := newTestCRIService()
 	for desc, test := range map[string]struct {
 		userNS        *runtime.UserNamespace
+		sandboxUserNS *runtime.UserNamespace
 		expNS         *runtimespec.LinuxNamespace
 		expNotNS      *runtimespec.LinuxNamespace // Does NOT contain this namespace
 		expUIDMapping []runtimespec.LinuxIDMapping
@@ -871,6 +877,58 @@ func TestUserNamespace(t *testing.T) {
 			expUIDMapping: []runtimespec.LinuxIDMapping{expIDMap},
 			expGIDMapping: []runtimespec.LinuxIDMapping{expIDMap},
 		},
+		"pod namespace mode with inconsistent sandbox config (different GIDs)": {
+			userNS: &runtime.UserNamespace{
+				Mode: runtime.NamespaceMode_POD,
+				Uids: []*runtime.IDMapping{&idMap},
+				Gids: []*runtime.IDMapping{&idMap},
+			},
+			sandboxUserNS: &runtime.UserNamespace{
+				Mode: runtime.NamespaceMode_POD,
+				Uids: []*runtime.IDMapping{&idMap},
+				Gids: []*runtime.IDMapping{&otherIDMap},
+			},
+			err: true,
+		},
+		"pod namespace mode with inconsistent sandbox config (different UIDs)": {
+			userNS: &runtime.UserNamespace{
+				Mode: runtime.NamespaceMode_POD,
+				Uids: []*runtime.IDMapping{&idMap},
+				Gids: []*runtime.IDMapping{&idMap},
+			},
+			sandboxUserNS: &runtime.UserNamespace{
+				Mode: runtime.NamespaceMode_POD,
+				Uids: []*runtime.IDMapping{&otherIDMap},
+				Gids: []*runtime.IDMapping{&idMap},
+			},
+			err: true,
+		},
+		"pod namespace mode with inconsistent sandbox config (different len)": {
+			userNS: &runtime.UserNamespace{
+				Mode: runtime.NamespaceMode_POD,
+				Uids: []*runtime.IDMapping{&idMap},
+				Gids: []*runtime.IDMapping{&idMap},
+			},
+			sandboxUserNS: &runtime.UserNamespace{
+				Mode: runtime.NamespaceMode_POD,
+				Uids: []*runtime.IDMapping{&idMap, &idMap},
+				Gids: []*runtime.IDMapping{&idMap, &idMap},
+			},
+			err: true,
+		},
+		"pod namespace mode with inconsistent sandbox config (different mode)": {
+			userNS: &runtime.UserNamespace{
+				Mode: runtime.NamespaceMode_POD,
+				Uids: []*runtime.IDMapping{&idMap},
+				Gids: []*runtime.IDMapping{&idMap},
+			},
+			sandboxUserNS: &runtime.UserNamespace{
+				Mode: runtime.NamespaceMode_NODE,
+				Uids: []*runtime.IDMapping{&idMap},
+				Gids: []*runtime.IDMapping{&idMap},
+			},
+			err: true,
+		},
 		"pod namespace mode with several mappings": {
 			userNS: &runtime.UserNamespace{
 				Mode: runtime.NamespaceMode_POD,
@@ -892,14 +950,23 @@ func TestUserNamespace(t *testing.T) {
 		test := test
 		t.Run(desc, func(t *testing.T) {
 			containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{UsernsOptions: test.userNS}
+			// By default, set sandbox and container config to the same (this is
+			// required by containerSpec). However, if the test wants to test for what
+			// happens when they don't match, the test.sandboxUserNS should be set and
+			// we just use that.
+			sandboxUserns := test.userNS
+			if test.sandboxUserNS != nil {
+				sandboxUserns = test.sandboxUserNS
+			}
+			sandboxConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{UsernsOptions: sandboxUserns}
 			spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 
 			if test.err {
-				assert.Error(t, err)
+				require.Error(t, err)
 				assert.Nil(t, spec)
 				return
 			}
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, spec.Linux.UIDMappings, test.expUIDMapping)
 			assert.Equal(t, spec.Linux.GIDMappings, test.expGIDMapping)
 

pkg/cri/server/helpers_linux.go

@@ -319,15 +319,12 @@ func parseUsernsIDs(userns *runtime.UserNamespace) (uids, gids []specs.LinuxIDMa
 		return nil, nil, nil
 	}
 
-	uidRuntimeMap := userns.GetUids()
+	uids, err := parseUsernsIDMap(userns.GetUids())
-	gidRuntimeMap := userns.GetGids()
-
-	uids, err := parseUsernsIDMap(uidRuntimeMap)
 	if err != nil {
 		return nil, nil, fmt.Errorf("UID mapping: %w", err)
 	}
 
-	gids, err = parseUsernsIDMap(gidRuntimeMap)
+	gids, err = parseUsernsIDMap(userns.GetGids())
 	if err != nil {
 		return nil, nil, fmt.Errorf("GID mapping: %w", err)
 	}
@@ -349,6 +346,64 @@ func parseUsernsIDs(userns *runtime.UserNamespace) (uids, gids []specs.LinuxIDMa
 	return uids, gids, nil
 }
 
+// sameUsernsConfig checks if the userns configs are the same. If the mappings
+// on each config are the same but in different order, it returns false.
+// XXX: If the runtime.UserNamespace struct changes, we should update this
+// function accordingly.
+func sameUsernsConfig(a, b *runtime.UserNamespace) bool {
+	// If both are nil, they are the same.
+	if a == nil && b == nil {
+		return true
+	}
+	// If only one is nil, they are different.
+	if a == nil || b == nil {
+		return false
+	}
+	// At this point, a is not nil nor b.
+
+	if a.GetMode() != b.GetMode() {
+		return false
+	}
+
+	aUids, aGids, err := parseUsernsIDs(a)
+	if err != nil {
+		return false
+	}
+	bUids, bGids, err := parseUsernsIDs(b)
+	if err != nil {
+		return false
+	}
+
+	if !sameMapping(aUids, bUids) {
+		return false
+	}
+	if !sameMapping(aGids, bGids) {
+		return false
+	}
+	return true
+}
+
+// sameMapping checks if the mappings are the same. If the mappings are the same
+// but in different order, it returns false.
+func sameMapping(a, b []specs.LinuxIDMapping) bool {
+	if len(a) != len(b) {
+		return false
+	}
+
+	for x := range a {
+		if a[x].ContainerID != b[x].ContainerID {
+			return false
+		}
+		if a[x].HostID != b[x].HostID {
+			return false
+		}
+		if a[x].Size != b[x].Size {
+			return false
+		}
+	}
+	return true
+}
+
 func snapshotterRemapOpts(nsOpts *runtime.NamespaceOption) ([]snapshots.Opt, error) {
 	snapshotOpt := []snapshots.Opt{}
 	usernsOpts := nsOpts.GetUsernsOptions()