diff --git a/api/docs/v1.24.md b/api/docs/v1.24.md index 2d374a4a84..bec30bb0b3 100644 --- a/api/docs/v1.24.md +++ b/api/docs/v1.24.md @@ -1095,6 +1095,7 @@ Start the container `id` - **204** – no error - **304** – container already started +- **400** – bad parameter, including an invalid checkpoint ID - **404** – no such container - **500** – server error diff --git a/api/docs/v1.25.yaml b/api/docs/v1.25.yaml index 35fb1bbf46..fdac6a3bfd 100644 --- a/api/docs/v1.25.yaml +++ b/api/docs/v1.25.yaml @@ -3596,6 +3596,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.26.yaml b/api/docs/v1.26.yaml index 048bd56cf3..99b3620b5e 100644 --- a/api/docs/v1.26.yaml +++ b/api/docs/v1.26.yaml @@ -3601,6 +3601,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.27.yaml b/api/docs/v1.27.yaml index 3223479601..d7e5e56771 100644 --- a/api/docs/v1.27.yaml +++ b/api/docs/v1.27.yaml @@ -3675,6 +3675,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.28.yaml b/api/docs/v1.28.yaml index b36f040110..e0e0314027 100644 --- a/api/docs/v1.28.yaml +++ b/api/docs/v1.28.yaml @@ -3766,6 +3766,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.29.yaml b/api/docs/v1.29.yaml index 21d761daf5..b5e08f2358 100644 --- a/api/docs/v1.29.yaml +++ b/api/docs/v1.29.yaml @@ -3799,6 +3799,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.30.yaml b/api/docs/v1.30.yaml index a153bec75c..d536ac0b6f 100644 --- a/api/docs/v1.30.yaml +++ b/api/docs/v1.30.yaml @@ -4024,6 +4024,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.31.yaml b/api/docs/v1.31.yaml index a7e2c4f5f3..76fea3b178 100644 --- a/api/docs/v1.31.yaml +++ b/api/docs/v1.31.yaml @@ -4097,6 +4097,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.32.yaml b/api/docs/v1.32.yaml index 47a1c3ab37..7bf882e927 100644 --- a/api/docs/v1.32.yaml +++ b/api/docs/v1.32.yaml @@ -5304,6 +5304,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.33.yaml b/api/docs/v1.33.yaml index 1ee9c1c171..1bb601aa35 100644 --- a/api/docs/v1.33.yaml +++ b/api/docs/v1.33.yaml @@ -5308,6 +5308,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.34.yaml b/api/docs/v1.34.yaml index 5978bc7a36..92e297061b 100644 --- a/api/docs/v1.34.yaml +++ b/api/docs/v1.34.yaml @@ -5336,6 +5336,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.35.yaml b/api/docs/v1.35.yaml index a4e1177102..4c4df12f15 100644 --- a/api/docs/v1.35.yaml +++ b/api/docs/v1.35.yaml @@ -5345,6 +5345,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.36.yaml b/api/docs/v1.36.yaml index ab2d6e0c82..cd6464d5ae 100644 --- a/api/docs/v1.36.yaml +++ b/api/docs/v1.36.yaml @@ -5375,6 +5375,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.37.yaml b/api/docs/v1.37.yaml index 4739ec7491..453e8631c3 100644 --- a/api/docs/v1.37.yaml +++ b/api/docs/v1.37.yaml @@ -5395,6 +5395,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.38.yaml b/api/docs/v1.38.yaml index 063f8bfeff..5af1c3844f 100644 --- a/api/docs/v1.38.yaml +++ b/api/docs/v1.38.yaml @@ -5446,6 +5446,10 @@ paths: description: "container already started" schema: $ref: "#/definitions/ErrorResponse" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.39.yaml b/api/docs/v1.39.yaml index b38152eb1d..17a96112fe 100644 --- a/api/docs/v1.39.yaml +++ b/api/docs/v1.39.yaml @@ -6737,6 +6737,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.40.yaml b/api/docs/v1.40.yaml index ce2dfb3dfc..bb35f9fa7c 100644 --- a/api/docs/v1.40.yaml +++ b/api/docs/v1.40.yaml @@ -7052,6 +7052,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.41.yaml b/api/docs/v1.41.yaml index 86b55b43a6..8c3c1ea335 100644 --- a/api/docs/v1.41.yaml +++ b/api/docs/v1.41.yaml @@ -7341,6 +7341,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.42.yaml b/api/docs/v1.42.yaml index f084a533f7..72f3148cfc 100644 --- a/api/docs/v1.42.yaml +++ b/api/docs/v1.42.yaml @@ -7556,6 +7556,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.43.yaml b/api/docs/v1.43.yaml index c9df834dce..1897321ed4 100644 --- a/api/docs/v1.43.yaml +++ b/api/docs/v1.43.yaml @@ -7574,6 +7574,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.44.yaml b/api/docs/v1.44.yaml index 64e455b903..d1aa8976e5 100644 --- a/api/docs/v1.44.yaml +++ b/api/docs/v1.44.yaml @@ -7714,6 +7714,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.45.yaml b/api/docs/v1.45.yaml index 5bad034618..16ba143cf1 100644 --- a/api/docs/v1.45.yaml +++ b/api/docs/v1.45.yaml @@ -7700,6 +7700,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.46.yaml b/api/docs/v1.46.yaml index 4fccaaebcc..7e33997503 100644 --- a/api/docs/v1.46.yaml +++ b/api/docs/v1.46.yaml @@ -7821,6 +7821,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.47.yaml b/api/docs/v1.47.yaml index 1552907a76..75328e3eb5 100644 --- a/api/docs/v1.47.yaml +++ b/api/docs/v1.47.yaml @@ -7957,6 +7957,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.48.yaml b/api/docs/v1.48.yaml index 7b11c5d000..13fd4ba354 100644 --- a/api/docs/v1.48.yaml +++ b/api/docs/v1.48.yaml @@ -8513,6 +8513,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.49.yaml b/api/docs/v1.49.yaml index bde55ec241..d391ba0856 100644 --- a/api/docs/v1.49.yaml +++ b/api/docs/v1.49.yaml @@ -8513,6 +8513,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.50.yaml b/api/docs/v1.50.yaml index c92e53de8c..e30818562c 100644 --- a/api/docs/v1.50.yaml +++ b/api/docs/v1.50.yaml @@ -8343,6 +8343,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.51.yaml b/api/docs/v1.51.yaml index 079c98623b..dac4bb1075 100644 --- a/api/docs/v1.51.yaml +++ b/api/docs/v1.51.yaml @@ -8364,6 +8364,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.52.yaml b/api/docs/v1.52.yaml index d0d48411ed..83f90a9c4d 100644 --- a/api/docs/v1.52.yaml +++ b/api/docs/v1.52.yaml @@ -8653,6 +8653,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.53.yaml b/api/docs/v1.53.yaml index 5dd8853148..9a6db2d41d 100644 --- a/api/docs/v1.53.yaml +++ b/api/docs/v1.53.yaml @@ -8901,6 +8901,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.54.yaml b/api/docs/v1.54.yaml index c1a4d8db7c..10fa714bfc 100644 --- a/api/docs/v1.54.yaml +++ b/api/docs/v1.54.yaml @@ -8908,6 +8908,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/docs/v1.55.yaml b/api/docs/v1.55.yaml index 0d0eb1f4ce..a400164b9f 100644 --- a/api/docs/v1.55.yaml +++ b/api/docs/v1.55.yaml @@ -8927,6 +8927,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/api/swagger.yaml b/api/swagger.yaml index 4c5215b393..676e2fe334 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -8938,6 +8938,10 @@ paths: description: "no error" 304: description: "container already started" + 400: + description: "bad parameter, including an invalid checkpoint ID" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such container" schema: diff --git a/daemon/checkpoint.go b/daemon/checkpoint.go index b6ac9b17ca..948c8e90ba 100644 --- a/daemon/checkpoint.go +++ b/daemon/checkpoint.go @@ -2,30 +2,37 @@ package daemon import ( "context" + "errors" "fmt" "os" "path/filepath" + "strings" "github.com/moby/moby/api/types/checkpoint" "github.com/moby/moby/api/types/events" "github.com/moby/moby/v2/daemon/names" "github.com/moby/moby/v2/daemon/server/backend" + "github.com/moby/moby/v2/errdefs" ) -var ( - validCheckpointNameChars = names.RestrictedNameChars - validCheckpointNamePattern = names.RestrictedNamePattern -) - -// getCheckpointDir verifies checkpoint directory for create,remove, list options and checks if checkpoint already exists -func getCheckpointDir(checkDir, checkpointID, ctrName, ctrID, ctrCheckpointDir string, create bool) (string, error) { - var checkpointDir string - var err2 error - if checkDir != "" { - checkpointDir = checkDir - } else { - checkpointDir = ctrCheckpointDir +func validateCheckpointID(checkpointID string) error { + if !names.RestrictedNamePattern.MatchString(checkpointID) || strings.HasSuffix(checkpointID, ".") { + return errdefs.InvalidParameter(fmt.Errorf("invalid checkpoint ID %q, only %s are allowed", checkpointID, names.RestrictedNameChars)) } + return nil +} + +func checkpointRoot(checkDir, ctrCheckpointDir string) string { + if checkDir != "" { + return checkDir + } + return ctrCheckpointDir +} + +// getCheckpointDir resolves and verifies a named checkpoint directory. +func getCheckpointDir(checkDir, checkpointID, ctrName, ctrID, ctrCheckpointDir string, create bool) (string, error) { + checkpointDir := checkpointRoot(checkDir, ctrCheckpointDir) + var err2 error checkpointAbsDir := filepath.Join(checkpointDir, checkpointID) stat, err := os.Stat(checkpointAbsDir) if create { @@ -52,8 +59,32 @@ func getCheckpointDir(checkDir, checkpointID, ctrName, ctrID, ctrCheckpointDir s return checkpointAbsDir, err2 } +// getCheckpointRoot resolves the checkpoint root used by CheckpointList. +func getCheckpointRoot(checkDir, ctrName, ctrCheckpointDir string) (string, error) { + checkpointDir := checkpointRoot(checkDir, ctrCheckpointDir) + // The API intentionally supports caller-selected checkpoint roots. + stat, err := os.Stat(checkpointDir) + if err != nil { + if checkDir != "" && errors.Is(err, os.ErrNotExist) { + return checkpointDir, errdefs.NotFound(fmt.Errorf("checkpoint directory %q does not exist for container %s: %w", checkpointDir, ctrName, err)) + } + return checkpointDir, fmt.Errorf("failed to stat checkpoint directory %q for container %s: %w", checkpointDir, ctrName, err) + } + if !stat.IsDir() { + if checkDir != "" { + return checkpointDir, errdefs.InvalidParameter(fmt.Errorf("checkpoint directory %q exists but is not a directory", checkpointDir)) + } + return checkpointDir, fmt.Errorf("%s exists and is not a directory", checkpointDir) + } + return checkpointDir, nil +} + // CheckpointCreate checkpoints the process running in a container with CRIU func (daemon *Daemon) CheckpointCreate(name string, config checkpoint.CreateRequest) error { + if err := validateCheckpointID(config.CheckpointID); err != nil { + return err + } + container, err := daemon.GetContainer(name) if err != nil { return err @@ -66,13 +97,9 @@ func (daemon *Daemon) CheckpointCreate(name string, config checkpoint.CreateRequ return err } - if !validCheckpointNamePattern.MatchString(config.CheckpointID) { - return fmt.Errorf("Invalid checkpoint ID (%s), only %s are allowed", config.CheckpointID, validCheckpointNameChars) - } - checkpointDir, err := getCheckpointDir(config.CheckpointDir, config.CheckpointID, name, container.ID, container.CheckpointDir(), true) if err != nil { - return fmt.Errorf("cannot checkpoint container %s: %s", name, err) + return fmt.Errorf("cannot checkpoint container %s: %w", name, err) } err = tsk.CreateCheckpoint(context.Background(), checkpointDir, config.Exit) @@ -88,6 +115,10 @@ func (daemon *Daemon) CheckpointCreate(name string, config checkpoint.CreateRequ // CheckpointDelete deletes the specified checkpoint func (daemon *Daemon) CheckpointDelete(name string, config backend.CheckpointDeleteOptions) error { + if err := validateCheckpointID(config.CheckpointID); err != nil { + return err + } + container, err := daemon.GetContainer(name) if err != nil { return err @@ -108,17 +139,22 @@ func (daemon *Daemon) CheckpointList(name string, config backend.CheckpointListO return nil, err } - checkpointDir, err := getCheckpointDir(config.CheckpointDir, "", name, container.ID, container.CheckpointDir(), false) + checkpointDir, err := getCheckpointRoot(config.CheckpointDir, name, container.CheckpointDir()) if err != nil { - return nil, err - } - - if err := os.MkdirAll(checkpointDir, 0o755); err != nil { + if config.CheckpointDir == "" && errors.Is(err, os.ErrNotExist) { + return out, nil + } return nil, err } dirs, err := os.ReadDir(checkpointDir) if err != nil { + if errors.Is(err, os.ErrNotExist) { + if config.CheckpointDir == "" { + return out, nil + } + return nil, errdefs.NotFound(fmt.Errorf("checkpoint directory %q does not exist for container %s: %w", checkpointDir, name, err)) + } return nil, err } diff --git a/daemon/checkpoint_test.go b/daemon/checkpoint_test.go new file mode 100644 index 0000000000..c98a1e7d4c --- /dev/null +++ b/daemon/checkpoint_test.go @@ -0,0 +1,106 @@ +package daemon + +import ( + "context" + "path/filepath" + "runtime" + "testing" + + cerrdefs "github.com/containerd/errdefs" + "github.com/moby/moby/api/types/checkpoint" + "github.com/moby/moby/v2/daemon/container" + "github.com/moby/moby/v2/daemon/server/backend" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestValidateCheckpointID(t *testing.T) { + type testCase struct { + name string + checkpointID string + valid bool + } + tests := []testCase{ + {name: "restricted name", checkpointID: "a1", valid: true}, + {name: "space", checkpointID: "space name"}, + {name: "unicode", checkpointID: "unicode-世界"}, + {name: "dots", checkpointID: "name.with-dots", valid: true}, + {name: "empty", checkpointID: ""}, + {name: "dot", checkpointID: "."}, + {name: "dot-dot", checkpointID: ".."}, + {name: "traversal", checkpointID: "../outside"}, + {name: "nested traversal", checkpointID: "foo/../outside"}, + {name: "separator", checkpointID: "foo/bar"}, + {name: "windows separator", checkpointID: `foo\bar`}, + {name: "NUL", checkpointID: "contains\x00nul"}, + {name: "absolute path", checkpointID: filepath.Join(string(filepath.Separator), "absolute")}, + } + if runtime.GOOS == "windows" { + tests = append(tests, + testCase{name: "dot trailing space", checkpointID: ". "}, + testCase{name: "dot-dot trailing space", checkpointID: ".. "}, + testCase{name: "trailing dot", checkpointID: "name."}, + testCase{name: "trailing space", checkpointID: "name "}, + ) + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateCheckpointID(tc.checkpointID) + if tc.valid { + assert.NilError(t, err) + return + } + assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument)) + }) + } +} + +func TestCheckpointOperationsValidateBeforeContainerLookup(t *testing.T) { + daemon := &Daemon{} + tests := []struct { + name string + operation func() error + }{ + { + name: "create", + operation: func() error { + return daemon.CheckpointCreate("missing", checkpoint.CreateRequest{CheckpointID: "../outside"}) + }, + }, + { + name: "delete", + operation: func() error { + return daemon.CheckpointDelete("missing", backend.CheckpointDeleteOptions{CheckpointID: "../outside"}) + }, + }, + { + name: "restore", + operation: func() error { + return daemon.ContainerStart(context.Background(), "missing", "../outside", "") + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.operation() + assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument)) + assert.Check(t, is.ErrorContains(err, `invalid checkpoint ID "../outside"`)) + }) + } +} + +func TestCheckpointListMissingDefaultDirectory(t *testing.T) { + t.Parallel() + + root := t.TempDir() + ctr := container.NewBaseContainer("container-id", root) + store := container.NewMemoryStore() + store.Add(ctr.ID, ctr) + daemon := &Daemon{containers: store} + + checkpoints, err := daemon.CheckpointList(ctr.ID, backend.CheckpointListOptions{}) + assert.NilError(t, err) + assert.Assert(t, is.Len(checkpoints, 0)) +} diff --git a/daemon/start.go b/daemon/start.go index c979e0b342..6b85f4ccff 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -47,6 +47,12 @@ func validateState(ctr *container.Container) error { // ContainerStart starts a container. func (daemon *Daemon) ContainerStart(ctx context.Context, name string, checkpoint string, checkpointDir string) error { + if checkpoint != "" { + if err := validateCheckpointID(checkpoint); err != nil { + return err + } + } + daemonCfg := daemon.config() if checkpoint != "" && !daemonCfg.Experimental { return errdefs.InvalidParameter(errors.New("checkpoint is only supported in experimental mode")) @@ -84,6 +90,11 @@ func (daemon *Daemon) containerStart(ctx context.Context, daemonCfg *configStore span.End() }() + if checkpointDir != "" { + // TODO(mlaventure): how would we support that? + return errdefs.Forbidden(errors.New("custom checkpointdir is not supported")) + } + start := time.Now() container.Lock() defer container.Unlock() @@ -96,11 +107,6 @@ func (daemon *Daemon) containerStart(ctx context.Context, daemonCfg *configStore return errdefs.Conflict(errors.New("container is marked for removal and cannot be started")) } - if checkpointDir != "" { - // TODO(mlaventure): how would we support that? - return errdefs.Forbidden(errors.New("custom checkpointdir is not supported")) - } - // if we encounter an error during start we need to ensure that any other // setup has been cleaned up properly defer func() { diff --git a/integration/container/checkpoint_test.go b/integration/container/checkpoint_test.go index 5a9c02a723..a9fe8899d0 100644 --- a/integration/container/checkpoint_test.go +++ b/integration/container/checkpoint_test.go @@ -1,11 +1,16 @@ package container import ( + "encoding/json" + "net/http" + "net/url" "os/exec" "regexp" "sort" "testing" + "github.com/moby/moby/api/types/common" + containertypes "github.com/moby/moby/api/types/container" mounttypes "github.com/moby/moby/api/types/mount" "github.com/moby/moby/client" @@ -17,6 +22,19 @@ import ( "gotest.tools/v3/skip" ) +func TestCheckpointInvalidID(t *testing.T) { + ctx := setupTest(t) + query := url.Values{"checkpoint": {"../outside"}} + res, body, err := request.Post(ctx, "/containers/missing/start?"+query.Encode()) + assert.NilError(t, err) + defer body.Close() + assert.Equal(t, res.StatusCode, http.StatusBadRequest) + + var errorResponse common.ErrorResponse + assert.NilError(t, json.NewDecoder(body).Decode(&errorResponse)) + assert.Check(t, is.ErrorContains(errorResponse, `invalid checkpoint ID "../outside"`)) +} + func TestCheckpoint(t *testing.T) { t.Skip("TestCheckpoint is broken; see https://github.com/moby/moby/issues/38963") skip.If(t, testEnv.DaemonInfo.OSType == "windows")