daemon/logger/fluentd: add coverage for ValidateLogOpt(), parseAddress()

thaJeztah · pull[bot] · commit 9f9e405b66dd · 2023-04-13T13:39:00.000Z
This exposed a bug where host is ignored on some valid cases (to be fixed).

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
diff --git a/daemon/logger/fluentd/fluentd_test.go b/daemon/logger/fluentd/fluentd_test.go
@@ -2,6 +2,7 @@ package fluentd // import "github.com/docker/docker/daemon/logger/fluentd"
 import (
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"gotest.tools/v3/assert"
 )
 
@@ -22,3 +23,128 @@ func TestValidateLogOptReconnectInterval(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateLogOptAddress(t *testing.T) {
+
+	// paths to try
+	paths := []string{"/", "/some-path"}
+
+	tests := []struct {
+		addr        string
+		paths       []string // paths to append to addr, should be an error for tcp/udp
+		expected    location
+		expectedErr string
+	}{
+		{
+			addr: "",
+			expected: location{
+				protocol: defaultProtocol,
+				host:     defaultHost,
+				port:     defaultPort,
+			},
+		},
+		{
+			addr:  "192.168.1.1",
+			paths: paths,
+			expected: location{
+				port:     defaultPort,
+				protocol: defaultProtocol,
+			},
+		},
+		{
+			addr:  "[::1]",
+			paths: paths,
+			expected: location{
+				port:     defaultPort,
+				protocol: defaultProtocol,
+			},
+		},
+		{
+			addr:  "example.com",
+			paths: paths,
+			expected: location{
+				port:     defaultPort,
+				protocol: defaultProtocol,
+			},
+		},
+		{
+			addr:  "tcp://",
+			paths: paths,
+			expected: location{
+				protocol: "tcp",
+				port:     defaultPort,
+			},
+		},
+		{
+			addr:  "tcp://example.com",
+			paths: paths,
+			expected: location{
+				protocol: "tcp",
+				port:     defaultPort,
+			},
+		},
+		{
+			addr:  "tcp://example.com:65535",
+			paths: paths,
+			expected: location{
+				protocol: "tcp",
+				host:     "example.com",
+				port:     65535,
+			},
+		},
+		{
+			addr:        "://",
+			expectedErr: "invalid syntax",
+		},
+		{
+			addr:        "something://",
+			expectedErr: "invalid syntax",
+		},
+		{
+			addr:        "corrupted:c",
+			expectedErr: "invalid syntax",
+		},
+		{
+			addr:        "tcp://example.com:port",
+			expectedErr: "invalid port",
+		},
+		{
+			addr:        "tcp://example.com:-1",
+			expectedErr: "invalid port",
+		},
+		{
+			addr: "unix:///some/socket.sock",
+			expected: location{
+				protocol: "unix",
+				path:     "/some/socket.sock",
+			},
+		},
+		{
+			addr: "unix:///some/socket.sock:80", // unusual, but technically valid
+			expected: location{
+				protocol: "unix",
+				path:     "/some/socket.sock:80",
+			},
+		},
+	}
+	for _, tc := range tests {
+		tc := tc
+		if len(tc.paths) == 0 {
+			tc.paths = []string{""}
+		}
+		for _, path := range tc.paths {
+			address := tc.addr + path
+			t.Run(address, func(t *testing.T) {
+				err := ValidateLogOpt(map[string]string{addressKey: address})
+				if tc.expectedErr != "" {
+					assert.ErrorContains(t, err, tc.expectedErr)
+					return
+				}
+
+				assert.NilError(t, err)
+				addr, _ := parseAddress(address)
+				assert.DeepEqual(t, tc.expected, *addr, cmp.AllowUnexported(location{}))
+			})
+		}
+	}
+}
diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go
@@ -1671,14 +1671,6 @@ func (s *DockerDaemonSuite) TestDaemonRestartLocalVolumes(c *testing.T) {
 	assert.NilError(c, err, out)
 }
 
-// FIXME(vdemeester) should be a unit test
-func (s *DockerDaemonSuite) TestDaemonCorruptedFluentdAddress(c *testing.T) {
-	d := daemon.New(c, dockerBinary, dockerdBinary, testdaemon.WithEnvironment(testEnv.Execution))
-	assert.Assert(c, d.StartWithError("--log-driver=fluentd", "--log-opt", "fluentd-address=corrupted:c") != nil)
-	expected := "invalid fluentd-address corrupted:c: "
-	icmd.RunCommand("grep", expected, d.LogFileName()).Assert(c, icmd.Success)
-}
-
 // FIXME(vdemeester) Use a new daemon instance instead of the Suite one
 func (s *DockerDaemonSuite) TestDaemonStartWithoutHost(c *testing.T) {
 	s.d.UseDefaultHost = true
