Skip to content

Commit

Permalink
fix: remove identifier limits
Browse files Browse the repository at this point in the history
Signed-off-by: Shubharanshu Mahapatra <shubhum@amazon.com>
  • Loading branch information
Shubhranshu153 committed Aug 13, 2024
1 parent 012ffe8 commit 9a5afdc
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 32 deletions.
11 changes: 5 additions & 6 deletions cmd/nerdctl/network_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/composer/composer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
}

Expand Down
8 changes: 5 additions & 3 deletions pkg/composer/serviceparser/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions pkg/composer/serviceparser/serviceparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions pkg/identifiers/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
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.
*/

// This package implements functions for docker compatible identifier validation
package identifiers

import (
"fmt"
"regexp"

"github.com/containerd/errdefs"
)

const AllowedIdentfierChars = `[a-zA-Z0-9][a-zA-Z0-9_.-]`

var AllowedIdentifierPattern = regexp.MustCompile(`^` + AllowedIdentfierChars + `+$`)

// ValidateDockerCompat implements docker compatible identifier validation.
// Containerd Implementation allows single character identifier. Docker compatible Implementation requires atleast 2 characters for identifiers.
// Containerd Implementation enforces a maximum length constraint of 76 characters. Docker compatible Implementation omits the length check entirely.
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
}
4 changes: 2 additions & 2 deletions pkg/mountutil/mountutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
15 changes: 8 additions & 7 deletions pkg/mountutil/volumestore/volumestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 7 additions & 8 deletions pkg/namestore/namestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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 name %q: %w", newName, err)
}
fn := func() error {
oldFileName := filepath.Join(x.dir, oldName)
Expand Down

0 comments on commit 9a5afdc

Please sign in to comment.