fix: remove identifier limits

Signed-off-by: Shubharanshu Mahapatra <[email protected]>
containerd · Aug 13, 2024 · 185de34 · 185de34
1 parent 012ffe8
commit 185de34
Show file tree

Hide file tree

Showing 9 changed files with 82 additions and 32 deletions.
diff --git a/cmd/nerdctl/network_create.go b/cmd/nerdctl/network_create.go
@@ -19,13 +19,12 @@ package main
 import (
 	"fmt"
 
-	"github.com/spf13/cobra"
-
-	"github.com/containerd/containerd/v2/pkg/identifiers"
-
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/cmd/network"
+	"github.com/containerd/nerdctl/v2/pkg/identifiers"
 	"github.com/containerd/nerdctl/v2/pkg/strutil"
+
+	"github.com/spf13/cobra"
 )
 
 func newNetworkCreateCommand() *cobra.Command {
@@ -58,8 +57,8 @@ func networkCreateAction(cmd *cobra.Command, args []string) error {
 		return err
 	}
 	name := args[0]
-	if err := identifiers.Validate(name); err != nil {
-		return fmt.Errorf("malformed name %s: %w", name, err)
+	if err := identifiers.ValidateDockerCompat(name); err != nil {
+		return fmt.Errorf("invalid network name: %w", err)
 	}
 	driver, err := cmd.Flags().GetString("driver")
 	if err != nil {

diff --git a/docs/ValidateDockerCompat.md b/docs/ValidateDockerCompat.md
@@ -0,0 +1,10 @@
+This document explains the changes made in the identifier validation logic between the containerd identifier implementation and the Docker compatible implementation `ValidateDockerCompat`.
+
+#### 1. Minimum Length Constraint
+   - Containerd Implementation: Allows single character identifier.
+   - Docker compatible Implementation: Requires atleast 2 characters for identifiers.
+
+#### 2. Maximum Length Constraint
+   - Containerd Implementation: Enforces a maximum length constraint of 76 characters.
+   - Docker compatible Implementation: Omits the length check entirely.
+g
diff --git a/pkg/composer/composer.go b/pkg/composer/composer.go
@@ -27,10 +27,10 @@ import (
 	compose "github.com/compose-spec/compose-go/v2/types"
 
 	containerd "github.com/containerd/containerd/v2/client"
-	"github.com/containerd/containerd/v2/pkg/identifiers"
 	"github.com/containerd/log"
 
 	"github.com/containerd/nerdctl/v2/pkg/composer/serviceparser"
+	"github.com/containerd/nerdctl/v2/pkg/identifiers"
 	"github.com/containerd/nerdctl/v2/pkg/reflectutil"
 )
 
@@ -63,8 +63,8 @@ func New(o Options, client *containerd.Client) (*Composer, error) {
 	}
 
 	if o.Project != "" {
-		if err := identifiers.Validate(o.Project); err != nil {
-			return nil, fmt.Errorf("got invalid project name %q: %w", o.Project, err)
+		if err := identifiers.ValidateDockerCompat(o.Project); err != nil {
+			return nil, fmt.Errorf("invalid project name: %w", err)
 		}
 	}
 

diff --git a/pkg/composer/serviceparser/build.go b/pkg/composer/serviceparser/build.go
@@ -25,9 +25,9 @@ import (
 	"github.com/compose-spec/compose-go/v2/types"
 	securejoin "github.com/cyphar/filepath-securejoin"
 
-	"github.com/containerd/containerd/v2/pkg/identifiers"
 	"github.com/containerd/errdefs"
 	"github.com/containerd/log"
+	"github.com/containerd/nerdctl/v2/pkg/identifiers"
 
 	"github.com/containerd/nerdctl/v2/pkg/reflectutil"
 )
@@ -84,9 +84,11 @@ func parseBuildConfig(c *types.BuildConfig, project *types.Project, imageName st
 
 	for _, s := range c.Secrets {
 		fileRef := types.FileReferenceConfig(s)
-		if err := identifiers.Validate(fileRef.Source); err != nil {
-			return nil, fmt.Errorf("secret source %q is invalid: %w", fileRef.Source, err)
+
+		if err := identifiers.ValidateDockerCompat(fileRef.Source); err != nil {
+			return nil, fmt.Errorf("invalid secret source name: %w", err)
 		}
+
 		projectSecret, ok := project.Secrets[fileRef.Source]
 		if !ok {
 			return nil, fmt.Errorf("build: secret %s is undefined", fileRef.Source)

diff --git a/pkg/composer/serviceparser/serviceparser.go b/pkg/composer/serviceparser/serviceparser.go
@@ -31,9 +31,9 @@ import (
 	"github.com/compose-spec/compose-go/v2/types"
 
 	"github.com/containerd/containerd/v2/contrib/nvidia"
-	"github.com/containerd/containerd/v2/pkg/identifiers"
 	"github.com/containerd/log"
 
+	"github.com/containerd/nerdctl/v2/pkg/identifiers"
 	"github.com/containerd/nerdctl/v2/pkg/reflectutil"
 )
 
@@ -851,8 +851,8 @@ func fileReferenceConfigToFlagV(c types.FileReferenceConfig, project *types.Proj
 		log.L.Warnf("Ignoring: %s: %+v", objType, unknown)
 	}
 
-	if err := identifiers.Validate(c.Source); err != nil {
-		return "", fmt.Errorf("%s source %q is invalid: %w", objType, c.Source, err)
+	if err := identifiers.ValidateDockerCompat(c.Source); err != nil {
+		return "", fmt.Errorf("invalid source name for %s: %w", objType, err)
 	}
 
 	var obj types.FileObjectConfig

diff --git a/pkg/identifiers/validate.go b/pkg/identifiers/validate.go
@@ -0,0 +1,39 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package identifiers
+
+import (
+	"fmt"
+	"regexp"
+
+	"github.com/containerd/errdefs"
+)
+
+const AllowedIdentfierChars = `[a-zA-Z0-9][a-zA-Z0-9_.-]`
+
+var AllowedIdentifierPattern = regexp.MustCompile(`^` + AllowedIdentfierChars + `+$`)
+
+func ValidateDockerCompat(s string) error {
+	if len(s) == 0 {
+		return fmt.Errorf("identifier must not be empty %w", errdefs.ErrInvalidArgument)
+	}
+
+	if !AllowedIdentifierPattern.MatchString(s) {
+		return fmt.Errorf("identifier %q must match pattern %q: %w", s, AllowedIdentfierChars, errdefs.ErrInvalidArgument)
+	}
+	return nil
+}
diff --git a/pkg/mountutil/mountutil.go b/pkg/mountutil/mountutil.go
@@ -26,10 +26,10 @@ import (
 	"github.com/moby/sys/userns"
 	"github.com/opencontainers/runtime-spec/specs-go"
 
-	"github.com/containerd/containerd/v2/pkg/identifiers"
 	"github.com/containerd/containerd/v2/pkg/oci"
 	"github.com/containerd/log"
 
+	"github.com/containerd/nerdctl/v2/pkg/identifiers"
 	"github.com/containerd/nerdctl/v2/pkg/idgen"
 	"github.com/containerd/nerdctl/v2/pkg/mountutil/volumestore"
 	"github.com/containerd/nerdctl/v2/pkg/strutil"
@@ -260,7 +260,7 @@ func createDirOnHost(src string, createDir bool) error {
 }
 
 func isNamedVolume(s string) bool {
-	err := identifiers.Validate(s)
+	err := identifiers.ValidateDockerCompat(s)
 
 	// If the volume name is invalid, we assume it is a path
 	return err == nil

diff --git a/pkg/mountutil/volumestore/volumestore.go b/pkg/mountutil/volumestore/volumestore.go
@@ -23,10 +23,10 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/containerd/containerd/v2/pkg/identifiers"
 	"github.com/containerd/errdefs"
 	"github.com/containerd/log"
 
+	"github.com/containerd/nerdctl/v2/pkg/identifiers"
 	"github.com/containerd/nerdctl/v2/pkg/inspecttypes/native"
 	"github.com/containerd/nerdctl/v2/pkg/lockutil"
 	"github.com/containerd/nerdctl/v2/pkg/strutil"
@@ -109,9 +109,10 @@ func (vs *volumeStore) Unlock() error {
 // Create will create a new volume, or return an existing one if there is one already by that name
 // Besides a possible locking error, it might return ErrInvalidArgument, hard filesystem errors, json errors
 func (vs *volumeStore) Create(name string, labels []string) (*native.Volume, error) {
-	if err := identifiers.Validate(name); err != nil {
-		return nil, fmt.Errorf("malformed volume name: %w (%w)", err, errdefs.ErrInvalidArgument)
+	if err := identifiers.ValidateDockerCompat(name); err != nil {
+		return nil, fmt.Errorf("invalid volume name: %w", err)
 	}
+
 	volPath := filepath.Join(vs.dir, name)
 	volDataPath := filepath.Join(volPath, dataDirName)
 	volFilePath := filepath.Join(volPath, volumeJSONFileName)
@@ -178,8 +179,8 @@ func (vs *volumeStore) Create(name string, labels []string) (*native.Volume, err
 // Get retrieves a native volume from the store
 // Besides a possible locking error, it might return ErrInvalidArgument, ErrNotFound, or a filesystem error
 func (vs *volumeStore) Get(name string, size bool) (*native.Volume, error) {
-	if err := identifiers.Validate(name); err != nil {
-		return nil, fmt.Errorf("malformed volume name %q: %w", name, err)
+	if err := identifiers.ValidateDockerCompat(name); err != nil {
+		return nil, fmt.Errorf("invalid volume name: %w", err)
 	}
 	volPath := filepath.Join(vs.dir, name)
 	volDataPath := filepath.Join(volPath, dataDirName)
@@ -274,8 +275,8 @@ func (vs *volumeStore) Remove(names []string) (removed []string, warns []error,
 	fn := func() error {
 		for _, name := range names {
 			// Invalid name, soft error
-			if err := identifiers.Validate(name); err != nil {
-				warns = append(warns, fmt.Errorf("malformed volume name: %w (%w)", err, errdefs.ErrInvalidArgument))
+			if err := identifiers.ValidateDockerCompat(name); err != nil {
+				warns = append(warns, fmt.Errorf("invalid volume name: %w", err))
 				continue
 			}
 			dir := filepath.Join(vs.dir, name)

diff --git a/pkg/namestore/namestore.go b/pkg/namestore/namestore.go
@@ -22,8 +22,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/containerd/containerd/v2/pkg/identifiers"
-
+	"github.com/containerd/nerdctl/v2/pkg/identifiers"
 	"github.com/containerd/nerdctl/v2/pkg/lockutil"
 )
 
@@ -49,8 +48,8 @@ type nameStore struct {
 }
 
 func (x *nameStore) Acquire(name, id string) error {
-	if err := identifiers.Validate(name); err != nil {
-		return fmt.Errorf("invalid name %q: %w", name, err)
+	if err := identifiers.ValidateDockerCompat(name); err != nil {
+		return fmt.Errorf("invalid name: %w", err)
 	}
 	if strings.TrimSpace(id) != id {
 		return fmt.Errorf("untrimmed ID %q", id)
@@ -69,8 +68,8 @@ func (x *nameStore) Release(name, id string) error {
 	if name == "" {
 		return nil
 	}
-	if err := identifiers.Validate(name); err != nil {
-		return fmt.Errorf("invalid name %q: %w", name, err)
+	if err := identifiers.ValidateDockerCompat(name); err != nil {
+		return fmt.Errorf("invalid name: %w", err)
 	}
 	if strings.TrimSpace(id) != id {
 		return fmt.Errorf("untrimmed ID %q", id)
@@ -96,8 +95,8 @@ func (x *nameStore) Rename(oldName, id, newName string) error {
 	if oldName == "" || newName == "" {
 		return nil
 	}
-	if err := identifiers.Validate(newName); err != nil {
-		return fmt.Errorf("invalid name %q: %w", oldName, err)
+	if err := identifiers.ValidateDockerCompat(newName); err != nil {
+		return fmt.Errorf("invalid new name: %w", err)
 	}
 	fn := func() error {
 		oldFileName := filepath.Join(x.dir, oldName)