From c46ac53d0a06c7fe2f206a67bcbacb0b74af19b7 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Wed, 30 Nov 2016 19:22:07 +0100 Subject: [PATCH] Remove hostname validation as it seems to break users Validation is still done by swarmkit on the service side. Signed-off-by: Vincent Demeester Upstream-commit: ef39256dfb711f8382a5c021b85d6c7d613282b0 Component: engine --- .../api/server/router/container/backend.go | 6 ++-- .../router/container/container_routes.go | 10 ++---- components/engine/builder/builder.go | 4 +-- .../engine/builder/dockerfile/dispatchers.go | 2 +- .../engine/builder/dockerfile/internals.go | 6 ++-- .../engine/daemon/cluster/executor/backend.go | 4 +-- .../cluster/executor/container/adapter.go | 10 ++---- components/engine/daemon/container.go | 15 +------- components/engine/daemon/create.go | 12 +++---- components/engine/daemon/start.go | 4 +-- components/engine/daemon/update.go | 4 +-- .../integration-cli/docker_cli_run_test.go | 36 ------------------- 12 files changed, 27 insertions(+), 86 deletions(-) diff --git a/components/engine/api/server/router/container/backend.go b/components/engine/api/server/router/container/backend.go index 0c9f081471..5b4134eba5 100644 --- a/components/engine/api/server/router/container/backend.go +++ b/components/engine/api/server/router/container/backend.go @@ -32,17 +32,17 @@ type copyBackend interface { // stateBackend includes functions to implement to provide container state lifecycle functionality. type stateBackend interface { - ContainerCreate(config types.ContainerCreateConfig, validateHostname bool) (container.ContainerCreateCreatedBody, error) + ContainerCreate(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error) ContainerKill(name string, sig uint64) error ContainerPause(name string) error ContainerRename(oldName, newName string) error ContainerResize(name string, height, width int) error ContainerRestart(name string, seconds *int) error ContainerRm(name string, config *types.ContainerRmConfig) error - ContainerStart(name string, hostConfig *container.HostConfig, validateHostname bool, checkpoint string, checkpointDir string) error + ContainerStart(name string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error ContainerStop(name string, seconds *int) error ContainerUnpause(name string) error - ContainerUpdate(name string, hostConfig *container.HostConfig, validateHostname bool) (container.ContainerUpdateOKBody, error) + ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error) ContainerWait(name string, timeout time.Duration) (int, error) } diff --git a/components/engine/api/server/router/container/container_routes.go b/components/engine/api/server/router/container/container_routes.go index b4eacbc816..18bc50bcac 100644 --- a/components/engine/api/server/router/container/container_routes.go +++ b/components/engine/api/server/router/container/container_routes.go @@ -156,8 +156,7 @@ func (s *containerRouter) postContainersStart(ctx context.Context, w http.Respon checkpoint := r.Form.Get("checkpoint") checkpointDir := r.Form.Get("checkpoint-dir") - validateHostname := versions.GreaterThanOrEqualTo(version, "1.24") - if err := s.backend.ContainerStart(vars["name"], hostConfig, validateHostname, checkpoint, checkpointDir); err != nil { + if err := s.backend.ContainerStart(vars["name"], hostConfig, checkpoint, checkpointDir); err != nil { return err } @@ -332,7 +331,6 @@ func (s *containerRouter) postContainerUpdate(ctx context.Context, w http.Respon return err } - version := httputils.VersionFromContext(ctx) var updateConfig container.UpdateConfig decoder := json.NewDecoder(r.Body) @@ -346,8 +344,7 @@ func (s *containerRouter) postContainerUpdate(ctx context.Context, w http.Respon } name := vars["name"] - validateHostname := versions.GreaterThanOrEqualTo(version, "1.24") - resp, err := s.backend.ContainerUpdate(name, hostConfig, validateHostname) + resp, err := s.backend.ContainerUpdate(name, hostConfig) if err != nil { return err } @@ -372,14 +369,13 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo version := httputils.VersionFromContext(ctx) adjustCPUShares := versions.LessThan(version, "1.19") - validateHostname := versions.GreaterThanOrEqualTo(version, "1.24") ccr, err := s.backend.ContainerCreate(types.ContainerCreateConfig{ Name: name, Config: config, HostConfig: hostConfig, NetworkingConfig: networkingConfig, AdjustCPUShares: adjustCPUShares, - }, validateHostname) + }) if err != nil { return err } diff --git a/components/engine/builder/builder.go b/components/engine/builder/builder.go index b25a5cd043..ced19e81e6 100644 --- a/components/engine/builder/builder.go +++ b/components/engine/builder/builder.go @@ -116,7 +116,7 @@ type Backend interface { // ContainerAttachRaw attaches to container. ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool) error // ContainerCreate creates a new Docker container and returns potential warnings - ContainerCreate(config types.ContainerCreateConfig, validateHostname bool) (container.ContainerCreateCreatedBody, error) + ContainerCreate(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error) // ContainerRm removes a container specified by `id`. ContainerRm(name string, config *types.ContainerRmConfig) error // Commit creates a new Docker image from an existing Docker container. @@ -124,7 +124,7 @@ type Backend interface { // ContainerKill stops the container execution abruptly. ContainerKill(containerID string, sig uint64) error // ContainerStart starts a new container - ContainerStart(containerID string, hostConfig *container.HostConfig, validateHostname bool, checkpoint string, checkpointDir string) error + ContainerStart(containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error // ContainerWait stops processing until the given container is stopped. ContainerWait(containerID string, timeout time.Duration) (int, error) // ContainerUpdateCmdOnBuild updates container.Path and container.Args diff --git a/components/engine/builder/dockerfile/dispatchers.go b/components/engine/builder/dockerfile/dispatchers.go index 64f0341d40..cfaaa59d00 100644 --- a/components/engine/builder/dockerfile/dispatchers.go +++ b/components/engine/builder/dockerfile/dispatchers.go @@ -301,7 +301,7 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str return nil } - container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{Config: b.runConfig}, true) + container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{Config: b.runConfig}) if err != nil { return err } diff --git a/components/engine/builder/dockerfile/internals.go b/components/engine/builder/dockerfile/internals.go index 0b6c994894..6f0a367842 100644 --- a/components/engine/builder/dockerfile/internals.go +++ b/components/engine/builder/dockerfile/internals.go @@ -180,7 +180,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD return nil } - container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{Config: b.runConfig}, true) + container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{Config: b.runConfig}) if err != nil { return err } @@ -496,7 +496,7 @@ func (b *Builder) create() (string, error) { c, err := b.docker.ContainerCreate(types.ContainerCreateConfig{ Config: b.runConfig, HostConfig: hostConfig, - }, true) + }) if err != nil { return "", err } @@ -537,7 +537,7 @@ func (b *Builder) run(cID string) (err error) { } }() - if err := b.docker.ContainerStart(cID, nil, true, "", ""); err != nil { + if err := b.docker.ContainerStart(cID, nil, "", ""); err != nil { close(finished) if cancelErr := <-cancelErrCh; cancelErr != nil { logrus.Debugf("Build cancelled (%v) and got an error from ContainerStart: %v", diff --git a/components/engine/daemon/cluster/executor/backend.go b/components/engine/daemon/cluster/executor/backend.go index db7991b4dd..5cbbf4da15 100644 --- a/components/engine/daemon/cluster/executor/backend.go +++ b/components/engine/daemon/cluster/executor/backend.go @@ -28,8 +28,8 @@ type Backend interface { FindNetwork(idName string) (libnetwork.Network, error) SetupIngress(req clustertypes.NetworkCreateRequest, nodeIP string) error PullImage(ctx context.Context, image, tag string, metaHeaders map[string][]string, authConfig *types.AuthConfig, outStream io.Writer) error - CreateManagedContainer(config types.ContainerCreateConfig, validateHostname bool) (container.ContainerCreateCreatedBody, error) - ContainerStart(name string, hostConfig *container.HostConfig, validateHostname bool, checkpoint string, checkpointDir string) error + CreateManagedContainer(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error) + ContainerStart(name string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error ContainerStop(name string, seconds *int) error ContainerLogs(context.Context, string, *backend.ContainerLogsConfig, chan struct{}) error ConnectContainerToNetwork(containerName, networkName string, endpointConfig *network.EndpointSettings) error diff --git a/components/engine/daemon/cluster/executor/container/adapter.go b/components/engine/daemon/cluster/executor/container/adapter.go index 1613b1d114..09325be25a 100644 --- a/components/engine/daemon/cluster/executor/container/adapter.go +++ b/components/engine/daemon/cluster/executor/container/adapter.go @@ -11,12 +11,10 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/distribution/digest" - "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/events" - "github.com/docker/docker/api/types/versions" "github.com/docker/docker/daemon/cluster/convert" executorpkg "github.com/docker/docker/daemon/cluster/executor" "github.com/docker/docker/reference" @@ -215,8 +213,6 @@ func (c *containerAdapter) waitForDetach(ctx context.Context) error { func (c *containerAdapter) create(ctx context.Context) error { var cr containertypes.ContainerCreateCreatedBody var err error - version := httputils.VersionFromContext(ctx) - validateHostname := versions.GreaterThanOrEqualTo(version, "1.24") if cr, err = c.backend.CreateManagedContainer(types.ContainerCreateConfig{ Name: c.container.name(), @@ -224,7 +220,7 @@ func (c *containerAdapter) create(ctx context.Context) error { HostConfig: c.container.hostConfig(), // Use the first network in container create NetworkingConfig: c.container.createNetworkingConfig(), - }, validateHostname); err != nil { + }); err != nil { return err } @@ -263,9 +259,7 @@ func (c *containerAdapter) create(ctx context.Context) error { } func (c *containerAdapter) start(ctx context.Context) error { - version := httputils.VersionFromContext(ctx) - validateHostname := versions.GreaterThanOrEqualTo(version, "1.24") - return c.backend.ContainerStart(c.container.name(), nil, validateHostname, "", "") + return c.backend.ContainerStart(c.container.name(), nil, "", "") } func (c *containerAdapter) inspect(ctx context.Context) (types.ContainerJSON, error) { diff --git a/components/engine/daemon/container.go b/components/engine/daemon/container.go index d27ed27dbf..23a9d24604 100644 --- a/components/engine/daemon/container.go +++ b/components/engine/daemon/container.go @@ -3,7 +3,6 @@ package daemon import ( "fmt" "path/filepath" - "regexp" "time" "github.com/docker/docker/api/errors" @@ -204,7 +203,7 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig * // verifyContainerSettings performs validation of the hostconfig and config // structures. -func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool, validateHostname bool) ([]string, error) { +func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) ([]string, error) { // First perform verification of settings common across all platforms. if config != nil { @@ -222,18 +221,6 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon } } - // Validate if the given hostname is RFC 1123 (https://tools.ietf.org/html/rfc1123) compliant. - if validateHostname && len(config.Hostname) > 0 { - // RFC1123 specifies that 63 bytes is the maximium length - // Windows has the limitation of 63 bytes in length - // Linux hostname is limited to HOST_NAME_MAX=64, not including the terminating null byte. - // We limit the length to 63 bytes here to match RFC1035 and RFC1123. - matched, _ := regexp.MatchString("^(([[:alnum:]]|[[:alnum:]][[:alnum:]\\-]*[[:alnum:]])\\.)*([[:alnum:]]|[[:alnum:]][[:alnum:]\\-]*[[:alnum:]])$", config.Hostname) - if len(config.Hostname) > 63 || !matched { - return nil, fmt.Errorf("invalid hostname format: %s", config.Hostname) - } - } - // Validate if Env contains empty variable or not (e.g., ``, `=foo`) for _, env := range config.Env { if _, err := opts.ValidateEnv(env); err != nil { diff --git a/components/engine/daemon/create.go b/components/engine/daemon/create.go index 33fbaac6b1..a4205aacc8 100644 --- a/components/engine/daemon/create.go +++ b/components/engine/daemon/create.go @@ -25,22 +25,22 @@ import ( ) // CreateManagedContainer creates a container that is managed by a Service -func (daemon *Daemon) CreateManagedContainer(params types.ContainerCreateConfig, validateHostname bool) (containertypes.ContainerCreateCreatedBody, error) { - return daemon.containerCreate(params, true, validateHostname) +func (daemon *Daemon) CreateManagedContainer(params types.ContainerCreateConfig) (containertypes.ContainerCreateCreatedBody, error) { + return daemon.containerCreate(params, true) } // ContainerCreate creates a regular container -func (daemon *Daemon) ContainerCreate(params types.ContainerCreateConfig, validateHostname bool) (containertypes.ContainerCreateCreatedBody, error) { - return daemon.containerCreate(params, false, validateHostname) +func (daemon *Daemon) ContainerCreate(params types.ContainerCreateConfig) (containertypes.ContainerCreateCreatedBody, error) { + return daemon.containerCreate(params, false) } -func (daemon *Daemon) containerCreate(params types.ContainerCreateConfig, managed bool, validateHostname bool) (containertypes.ContainerCreateCreatedBody, error) { +func (daemon *Daemon) containerCreate(params types.ContainerCreateConfig, managed bool) (containertypes.ContainerCreateCreatedBody, error) { start := time.Now() if params.Config == nil { return containertypes.ContainerCreateCreatedBody{}, fmt.Errorf("Config cannot be empty in order to create a container") } - warnings, err := daemon.verifyContainerSettings(params.HostConfig, params.Config, false, validateHostname) + warnings, err := daemon.verifyContainerSettings(params.HostConfig, params.Config, false) if err != nil { return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, err } diff --git a/components/engine/daemon/start.go b/components/engine/daemon/start.go index 807fe569db..1a5b0e7559 100644 --- a/components/engine/daemon/start.go +++ b/components/engine/daemon/start.go @@ -19,7 +19,7 @@ import ( ) // ContainerStart starts a container. -func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.HostConfig, validateHostname bool, checkpoint string, checkpointDir string) error { +func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.HostConfig, checkpoint string, checkpointDir string) error { if checkpoint != "" && !daemon.HasExperimental() { return apierrors.NewBadRequestError(fmt.Errorf("checkpoint is only supported in experimental mode")) } @@ -73,7 +73,7 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos // check if hostConfig is in line with the current system settings. // It may happen cgroups are umounted or the like. - if _, err = daemon.verifyContainerSettings(container.HostConfig, nil, false, validateHostname); err != nil { + if _, err = daemon.verifyContainerSettings(container.HostConfig, nil, false); err != nil { return err } // Adapt for old containers in case we have updates in this function and diff --git a/components/engine/daemon/update.go b/components/engine/daemon/update.go index 3e05047ae2..67c0e59bf1 100644 --- a/components/engine/daemon/update.go +++ b/components/engine/daemon/update.go @@ -7,10 +7,10 @@ import ( ) // ContainerUpdate updates configuration of the container -func (daemon *Daemon) ContainerUpdate(name string, hostConfig *container.HostConfig, validateHostname bool) (container.ContainerUpdateOKBody, error) { +func (daemon *Daemon) ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error) { var warnings []string - warnings, err := daemon.verifyContainerSettings(hostConfig, nil, true, validateHostname) + warnings, err := daemon.verifyContainerSettings(hostConfig, nil, true) if err != nil { return container.ContainerUpdateOKBody{Warnings: warnings}, err } diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index 7cfdb8ec29..f65f2586b5 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -4354,42 +4354,6 @@ func (s *DockerSuite) TestRunVolumeCopyFlag(c *check.C) { c.Assert(err, checker.NotNil, check.Commentf(out)) } -func (s *DockerSuite) TestRunTooLongHostname(c *check.C) { - // Test case in #21445 - hostname1 := "this-is-a-way-too-long-hostname-but-it-should-give-a-nice-error.local" - out, _, err := dockerCmdWithError("run", "--hostname", hostname1, "busybox", "echo", "test") - c.Assert(err, checker.NotNil, check.Commentf("Expected docker run to fail!")) - c.Assert(out, checker.Contains, "invalid hostname format:", check.Commentf("Expected to have 'invalid hostname format:' in the output, get: %s!", out)) - - // Additional test cases - validHostnames := map[string]string{ - "hostname": "hostname", - "host-name": "host-name", - "hostname123": "hostname123", - "123hostname": "123hostname", - "hostname-of-63-bytes-long-should-be-valid-and-without-any-error": "hostname-of-63-bytes-long-should-be-valid-and-without-any-error", - } - for hostname := range validHostnames { - dockerCmd(c, "run", "--hostname", hostname, "busybox", "echo", "test") - } - - invalidHostnames := map[string]string{ - "^hostname": "invalid hostname format: ^hostname", - "hostname%": "invalid hostname format: hostname%", - "host&name": "invalid hostname format: host&name", - "-hostname": "invalid hostname format: -hostname", - "host_name": "invalid hostname format: host_name", - "hostname-of-64-bytes-long-should-be-invalid-and-be-with-an-error": "invalid hostname format: hostname-of-64-bytes-long-should-be-invalid-and-be-with-an-error", - } - - for hostname, expectedError := range invalidHostnames { - out, _, err = dockerCmdWithError("run", "--hostname", hostname, "busybox", "echo", "test") - c.Assert(err, checker.NotNil, check.Commentf("Expected docker run to fail!")) - c.Assert(out, checker.Contains, expectedError, check.Commentf("Expected to have '%s' in the output, get: %s!", expectedError, out)) - - } -} - // Test case for #21976 func (s *DockerSuite) TestRunDNSInHostMode(c *check.C) { testRequires(c, DaemonIsLinux, NotUserNamespace)