Merge pull request #3723 from apostasie/namespace-validate

Cleanup namespace validation

Author: AkihiroSuda

cmd/nerdctl/main.go

@@ -49,6 +49,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/errutil"
 	"github.com/containerd/nerdctl/v2/pkg/logging"
 	"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
+	"github.com/containerd/nerdctl/v2/pkg/store"
 	"github.com/containerd/nerdctl/v2/pkg/version"
 )
 
@@ -239,6 +240,16 @@ Config file ($NERDCTL_TOML): %s
 				return fmt.Errorf("invalid cgroup-manager %q (supported values: \"systemd\", \"cgroupfs\", \"none\")", cgroupManager)
 			}
 		}
+
+		// Since we store containers' stateful information on the filesystem per namespace, we need namespaces to be
+		// valid, safe path segments. This is enforced by store.ValidatePathComponent.
+		// Note that the container runtime will further enforce additional restrictions on namespace names
+		// (containerd treats namespaces as valid identifiers - eg: alphanumericals + dash, starting with a letter)
+		// See https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#path-segment-names for
+		// considerations about path segments identifiers.
+		if err = store.ValidatePathComponent(globalOptions.Namespace); err != nil {
+			return err
+		}
 		if appNeedsRootlessParentMain(cmd, args) {
 			// reexec /proc/self/exe with `nsenter` into RootlessKit namespaces
 			return rootlessutil.ParentMain(globalOptions.HostGatewayIP)

pkg/containerutil/containerutil.go

@@ -46,7 +46,6 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/ipcutil"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
 	"github.com/containerd/nerdctl/v2/pkg/labels/k8slabels"
-	"github.com/containerd/nerdctl/v2/pkg/nsutil"
 	"github.com/containerd/nerdctl/v2/pkg/portutil"
 	"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
 	"github.com/containerd/nerdctl/v2/pkg/signalutil"
@@ -529,9 +528,6 @@ func Unpause(ctx context.Context, client *containerd.Client, id string) error {
 
 // ContainerStateDirPath returns the path to the Nerdctl-managed state directory for the container with the given ID.
 func ContainerStateDirPath(ns, dataStore, id string) (string, error) {
-	if err := nsutil.ValidateNamespaceName(ns); err != nil {
-		return "", fmt.Errorf("invalid namespace name %q for determining state dir of container %q: %s", ns, id, err)
-	}
 	return filepath.Join(dataStore, "containers", ns, id), nil
 }
 

pkg/nsutil/nsutil.go

@@ -204,7 +204,7 @@ func (vs *fileStore) List(key ...string) ([]string, error) {
 
 	// Unlike Get, Set and Delete, List can have zero length key
 	for _, k := range key {
-		if err := validatePathComponent(k); err != nil {
+		if err := ValidatePathComponent(k); err != nil {
 			return nil, err
 		}
 	}
@@ -333,8 +333,8 @@ func (vs *fileStore) GroupSize(key ...string) (int64, error) {
 	return size, nil
 }
 
-// validatePathComponent will enforce os specific filename restrictions on a single path component
-func validatePathComponent(pathComponent string) error {
+// ValidatePathComponent will enforce os specific filename restrictions on a single path component
+func ValidatePathComponent(pathComponent string) error {
 	// https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
 	if len(pathComponent) > 255 {
 		return errors.Join(ErrInvalidArgument, errors.New("identifiers must be stricly shorter than 256 characters"))
@@ -358,7 +358,7 @@ func validateAllPathComponents(pathComponent ...string) error {
 	}
 
 	for _, key := range pathComponent {
-		if err := validatePathComponent(key); err != nil {
+		if err := ValidatePathComponent(key); err != nil {
 			return err
 		}
 	}

pkg/nsutil/nsutil_test.go

@@ -267,12 +267,12 @@ func TestFileStoreFilesystemRestrictions(t *testing.T) {
 	}
 
 	for _, v := range invalid {
-		err := validatePathComponent(v)
+		err := ValidatePathComponent(v)
 		assert.ErrorIs(t, err, ErrInvalidArgument, v)
 	}
 
 	for _, v := range valid {
-		err := validatePathComponent(v)
+		err := ValidatePathComponent(v)
 		assert.NilError(t, err, v)
 	}