Skip to content

Commit

Permalink
Remove hostname validation as it seems to break users
Browse files Browse the repository at this point in the history
Validation is still done by swarmkit on the service side.

Signed-off-by: Vincent Demeester <[email protected]>
  • Loading branch information
vdemeester committed Nov 30, 2016
1 parent 768f4ce commit ef39256
Show file tree
Hide file tree
Showing 12 changed files with 27 additions and 86 deletions.
6 changes: 3 additions & 3 deletions api/server/router/container/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
10 changes: 3 additions & 7 deletions api/server/router/container/container_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,15 @@ 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.
Commit(string, *backend.ContainerCommitConfig) (string, error)
// 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
Expand Down
2 changes: 1 addition & 1 deletion builder/dockerfile/dispatchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions builder/dockerfile/internals.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions daemon/cluster/executor/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 2 additions & 8 deletions daemon/cluster/executor/container/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -215,16 +213,14 @@ 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(),
Config: c.container.config(),
HostConfig: c.container.hostConfig(),
// Use the first network in container create
NetworkingConfig: c.container.createNetworkingConfig(),
}, validateHostname); err != nil {
}); err != nil {
return err
}

Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 1 addition & 14 deletions daemon/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package daemon
import (
"fmt"
"path/filepath"
"regexp"
"time"

"github.com/docker/docker/api/errors"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions daemon/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions daemon/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions daemon/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
36 changes: 0 additions & 36 deletions integration-cli/docker_cli_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit ef39256

Please sign in to comment.