Fix 'rename' error msg and error checking
`docker rename foo ''` would result in:
```
usage: docker rename OLD_NAME NEW_NAME
```
which is the old engine's way of return errors - yes that's in the
daemon code. So I fixed that error msg to just be normal.
While doing that I noticed that using an empty string for the
source container name failed but didn't print any error message at all.
This is because we would generate a URL like: ../containers//rename/..
which would cause a 301 redirect to ../containers/rename/..
however the CLI code doesn't actually deal with 301's - it just ignores
them and returns back to the CLI code/caller.
Rather than changing the CLI to deal with 3xx error codes, which would
probably be a good thing to do in a follow-on PR, for this immediate
issue I just added a cli-side check for empty strings for both old and
new names. This way we catch it even before we hit the daemon.
API callers will get a 404, assuming they follow the 301, for the
case of the src being empty, and the new error msg when the destination
is empty - so we should be good now.
Add tests for both cases too.
Signed-off-by: Doug Davis <dug@us.ibm.com>
Doug Davis authored on 2015/09/19 03:05:04Showing 3 changed files
| ... | ... |
@@ -2,6 +2,7 @@ package client |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 | 4 |
"fmt" |
| 5 |
+ "strings" |
|
| 5 | 6 |
|
| 6 | 7 |
Cli "github.com/docker/docker/cli" |
| 7 | 8 |
flag "github.com/docker/docker/pkg/mflag" |
| ... | ... |
@@ -16,8 +17,12 @@ func (cli *DockerCli) CmdRename(args ...string) error {
|
| 16 | 16 |
|
| 17 | 17 |
cmd.ParseFlags(args, true) |
| 18 | 18 |
|
| 19 |
- oldName := cmd.Arg(0) |
|
| 20 |
- newName := cmd.Arg(1) |
|
| 19 |
+ oldName := strings.TrimSpace(cmd.Arg(0)) |
|
| 20 |
+ newName := strings.TrimSpace(cmd.Arg(1)) |
|
| 21 |
+ |
|
| 22 |
+ if oldName == "" || newName == "" {
|
|
| 23 |
+ return fmt.Errorf("Error: Neither old nor new names may be empty")
|
|
| 24 |
+ } |
|
| 21 | 25 |
|
| 22 | 26 |
if _, _, err := readBody(cli.call("POST", fmt.Sprintf("/containers/%s/rename?name=%s", oldName, newName), nil, nil)); err != nil {
|
| 23 | 27 |
fmt.Fprintf(cli.err, "%s\n", err) |
| ... | ... |
@@ -9,7 +9,7 @@ import ( |
| 9 | 9 |
// reserved. |
| 10 | 10 |
func (daemon *Daemon) ContainerRename(oldName, newName string) error {
|
| 11 | 11 |
if oldName == "" || newName == "" {
|
| 12 |
- return fmt.Errorf("usage: docker rename OLD_NAME NEW_NAME")
|
|
| 12 |
+ return fmt.Errorf("Neither old nor new names may be empty")
|
|
| 13 | 13 |
} |
| 14 | 14 |
|
| 15 | 15 |
container, err := daemon.Get(oldName) |
| ... | ... |
@@ -74,6 +74,14 @@ func (s *DockerSuite) TestRenameInvalidName(c *check.C) {
|
| 74 | 74 |
c.Fatalf("Renaming container to invalid name should have failed: %s\n%v", out, err)
|
| 75 | 75 |
} |
| 76 | 76 |
|
| 77 |
+ if out, _, err := dockerCmdWithError("rename", "myname", ""); err == nil || !strings.Contains(out, "may be empty") {
|
|
| 78 |
+ c.Fatalf("Renaming container to empty name should have failed: %s\n%v", out, err)
|
|
| 79 |
+ } |
|
| 80 |
+ |
|
| 81 |
+ if out, _, err := dockerCmdWithError("rename", "", "newname"); err == nil || !strings.Contains(out, "may be empty") {
|
|
| 82 |
+ c.Fatalf("Renaming container to empty name should have failed: %s\n%v", out, err)
|
|
| 83 |
+ } |
|
| 84 |
+ |
|
| 77 | 85 |
if out, _, err := dockerCmdWithError("ps", "-a"); err != nil || !strings.Contains(out, "myname") {
|
| 78 | 86 |
c.Fatalf("Output of docker ps should have included 'myname': %s\n%v", out, err)
|
| 79 | 87 |
} |