From e3c472426ff1dccf083e41dbbfdf5935921bf330 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Sun, 20 Sep 2015 13:03:09 +0200 Subject: [PATCH] daemon: logger: error out on daemon start if invalid logger address If an invalid logger address is provided on daemon start it will silently fail. As syslog driver is doing, this check should be done on daemon start and prevent it from starting even in other drivers. This patch also adds integration tests for this behavior. Signed-off-by: Antonio Murdaca --- daemon/logger/fluentd/fluentd.go | 73 ++++++++++++----------- daemon/logger/gelf/gelf.go | 43 +++++++------ integration-cli/docker_cli_daemon_test.go | 32 +++++++--- 3 files changed, 86 insertions(+), 62 deletions(-) diff --git a/daemon/logger/fluentd/fluentd.go b/daemon/logger/fluentd/fluentd.go index 3ce0747473..974c2e21d9 100644 --- a/daemon/logger/fluentd/fluentd.go +++ b/daemon/logger/fluentd/fluentd.go @@ -38,44 +38,20 @@ func init() { } } -func parseConfig(ctx logger.Context) (string, int, string, error) { - host := defaultHostName - port := defaultPort - - config := ctx.Config - - tag, err := loggerutils.ParseLogTag(ctx, "docker.{{.ID}}") - if err != nil { - return "", 0, "", err - } - - if address := config["fluentd-address"]; address != "" { - if h, p, err := net.SplitHostPort(address); err != nil { - if !strings.Contains(err.Error(), "missing port in address") { - return "", 0, "", err - } - host = h - } else { - portnum, err := strconv.Atoi(p) - if err != nil { - return "", 0, "", err - } - host = h - port = portnum - } - } - - return host, port, tag, nil -} - // New creates a fluentd logger using the configuration passed in on // the context. Supported context configuration variables are // fluentd-address & fluentd-tag. func New(ctx logger.Context) (logger.Logger, error) { - host, port, tag, err := parseConfig(ctx) + host, port, err := parseAddress(ctx.Config["fluentd-address"]) if err != nil { return nil, err } + + tag, err := loggerutils.ParseLogTag(ctx, "docker.{{.ID}}") + if err != nil { + return nil, err + } + logrus.Debugf("logging driver fluentd configured for container:%s, host:%s, port:%d, tag:%s.", ctx.ContainerID, host, port, tag) // logger tries to recoonect 2**32 - 1 times @@ -104,6 +80,14 @@ func (f *fluentd) Log(msg *logger.Message) error { return f.writer.PostWithTime(f.tag, msg.Timestamp, data) } +func (f *fluentd) Close() error { + return f.writer.Close() +} + +func (f *fluentd) Name() string { + return name +} + // ValidateLogOpt looks for fluentd specific log options fluentd-address & fluentd-tag. func ValidateLogOpt(cfg map[string]string) error { for key := range cfg { @@ -115,13 +99,30 @@ func ValidateLogOpt(cfg map[string]string) error { return fmt.Errorf("unknown log opt '%s' for fluentd log driver", key) } } + + if _, _, err := parseAddress(cfg["fluentd-address"]); err != nil { + return err + } + return nil } -func (f *fluentd) Close() error { - return f.writer.Close() -} +func parseAddress(address string) (string, int, error) { + if address == "" { + return defaultHostName, defaultPort, nil + } -func (f *fluentd) Name() string { - return name + host, port, err := net.SplitHostPort(address) + if err != nil { + if !strings.Contains(err.Error(), "missing port in address") { + return "", 0, fmt.Errorf("invalid fluentd-address %s: %s", address, err) + } + return host, defaultPort, nil + } + + portnum, err := strconv.Atoi(port) + if err != nil { + return "", 0, fmt.Errorf("invalid fluentd-address %s: %s", address, err) + } + return host, portnum, nil } diff --git a/daemon/logger/gelf/gelf.go b/daemon/logger/gelf/gelf.go index e980a6ce94..6a62672d72 100644 --- a/daemon/logger/gelf/gelf.go +++ b/daemon/logger/gelf/gelf.go @@ -147,28 +147,35 @@ func ValidateLogOpt(cfg map[string]string) error { return fmt.Errorf("unknown log opt '%s' for gelf log driver", key) } } + + if _, err := parseAddress(cfg["gelf-address"]); err != nil { + return err + } + return nil } func parseAddress(address string) (string, error) { - if urlutil.IsTransportURL(address) { - url, err := url.Parse(address) - if err != nil { - return "", err - } - - // we support only udp - if url.Scheme != "udp" { - return "", fmt.Errorf("gelf: endpoint needs to be UDP") - } - - // get host and port - if _, _, err = net.SplitHostPort(url.Host); err != nil { - return "", fmt.Errorf("gelf: please provide gelf-address as udp://host:port") - } - - return url.Host, nil + if address == "" { + return "", nil + } + if !urlutil.IsTransportURL(address) { + return "", fmt.Errorf("gelf-address should be in form proto://address, got %v", address) + } + url, err := url.Parse(address) + if err != nil { + return "", err } - return "", nil + // we support only udp + if url.Scheme != "udp" { + return "", fmt.Errorf("gelf: endpoint needs to be UDP") + } + + // get host and port + if _, _, err = net.SplitHostPort(url.Host); err != nil { + return "", fmt.Errorf("gelf: please provide gelf-address as udp://host:port") + } + + return url.Host, nil } diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index b3083b4461..f53400e66b 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -1609,14 +1609,6 @@ func (s *DockerDaemonSuite) TestDaemonRestartWithContainerWithRestartPolicyAlway c.Assert(strings.TrimSpace(out), check.Equals, id[:12]) } -func (s *DockerDaemonSuite) TestDaemonCorruptedSyslogAddress(c *check.C) { - c.Assert(s.d.Start("--log-driver=syslog", "--log-opt", "syslog-address=corrupted:1234"), check.NotNil) - runCmd := exec.Command("grep", "Failed to set log opts: syslog-address should be in form proto://address", s.d.LogfileName()) - if out, _, err := runCommandWithOutput(runCmd); err != nil { - c.Fatalf("Expected 'Error starting daemon' message; but doesn't exist in log: %q, err: %v", out, err) - } -} - func (s *DockerDaemonSuite) TestDaemonWideLogConfig(c *check.C) { c.Assert(s.d.Start("--log-driver=json-file", "--log-opt=max-size=1k"), check.IsNil) out, err := s.d.Cmd("run", "-d", "--name=logtest", "busybox", "top") @@ -1689,3 +1681,27 @@ func (s *DockerDaemonSuite) TestDaemonRestartLocalVolumes(c *check.C) { _, err = s.d.Cmd("volume", "inspect", "test") c.Assert(err, check.IsNil) } + +func (s *DockerDaemonSuite) TestDaemonCorruptedLogDriverAddress(c *check.C) { + for _, driver := range []string{ + "syslog", + "gelf", + } { + args := []string{"--log-driver=" + driver, "--log-opt", driver + "-address=corrupted:42"} + c.Assert(s.d.Start(args...), check.NotNil, check.Commentf(fmt.Sprintf("Expected daemon not to start with invalid %s-address provided", driver))) + expected := fmt.Sprintf("Failed to set log opts: %s-address should be in form proto://address", driver) + runCmd := exec.Command("grep", expected, s.d.LogfileName()) + if out, _, err := runCommandWithOutput(runCmd); err != nil { + c.Fatalf("Expected %q message; but doesn't exist in log: %q, err: %v", expected, out, err) + } + } +} + +func (s *DockerDaemonSuite) TestDaemonCorruptedFluentdAddress(c *check.C) { + c.Assert(s.d.Start("--log-driver=fluentd", "--log-opt", "fluentd-address=corrupted:c"), check.NotNil) + expected := "Failed to set log opts: invalid fluentd-address corrupted:c: " + runCmd := exec.Command("grep", expected, s.d.LogfileName()) + if out, _, err := runCommandWithOutput(runCmd); err != nil { + c.Fatalf("Expected %q message; but doesn't exist in log: %q, err: %v", expected, out, err) + } +}