[CONSUL-491] Support admin_access_log_path value for Windows (#71)

command/connect/envoy/envoy.go

@@ -7,6 +7,7 @@ import (
 	"net"
 	"os"
 	"os/exec"
+	"runtime"
 	"strings"
 
 	"github.com/mitchellh/cli"
@@ -22,13 +23,14 @@ import (
 	"github.com/hashicorp/consul/tlsutil"
 )
 
-func New(ui cli.Ui) *cmd {
+func New(ui cli.Ui, osPlatform string) *cmd {
 	c := &cmd{UI: ui}
-	c.init()
+	c.init(osPlatform)
 	return c
 }
 
-const DefaultAdminAccessLogPath = "/dev/null"
+const DefaultUnixAdminAccessLogPath = "/dev/null"
+const DefaultWindowsAdminAccessLogPath = "nul"
 
 type cmd struct {
 	UI     cli.Ui
@@ -82,7 +84,7 @@ var supportedGateways = map[string]api.ServiceKind{
 	"ingress":     api.ServiceKindIngressGateway,
 }
 
-func (c *cmd) init() {
+func (c *cmd) init(osPlatform string) {
 	c.flags = flag.NewFlagSet("", flag.ContinueOnError)
 
 	c.flags.StringVar(&c.proxyID, "proxy-id", os.Getenv("CONNECT_PROXY_ID"),
@@ -108,10 +110,17 @@ func (c *cmd) init() {
 		"The full path to the envoy binary to run. By default will just search "+
 			"$PATH. Ignored if -bootstrap is used.")
 
-	c.flags.StringVar(&c.adminAccessLogPath, "admin-access-log-path", DefaultAdminAccessLogPath,
-		fmt.Sprintf("The path to write the access log for the administration server. If no access "+
-			"log is desired specify %q. By default it will use %q.",
-			DefaultAdminAccessLogPath, DefaultAdminAccessLogPath))
+	if osPlatform == "windows" {
+		c.flags.StringVar(&c.adminAccessLogPath, "admin-access-log-path", DefaultWindowsAdminAccessLogPath,
+			fmt.Sprintf("The path to write the access log for the administration server. If no access "+
+				"log is desired specify %q. By default it will use %q.",
+				DefaultWindowsAdminAccessLogPath, DefaultWindowsAdminAccessLogPath))
+	} else {
+		c.flags.StringVar(&c.adminAccessLogPath, "admin-access-log-path", DefaultUnixAdminAccessLogPath,
+			fmt.Sprintf("The path to write the access log for the administration server. If no access "+
+				"log is desired specify %q. By default it will use %q.",
+				DefaultUnixAdminAccessLogPath, DefaultUnixAdminAccessLogPath))
+	}
 
 	c.flags.StringVar(&c.adminBind, "admin-bind", "localhost:19000",
 		"The address:port to start envoy's admin server on. Envoy requires this "+
@@ -259,10 +268,11 @@ func (c *cmd) Run(args []string) int {
 		return 1
 	}
 	// TODO: refactor
-	return c.run(c.flags.Args())
+	osPlatform := runtime.GOOS
+	return c.run(c.flags.Args(), osPlatform)
 }
 
-func (c *cmd) run(args []string) int {
+func (c *cmd) run(args []string, osPlatform string) int {
 
 	if c.nodeName != "" && c.proxyID == "" {
 		c.UI.Error("'-node-name' requires '-proxy-id'")
@@ -429,7 +439,7 @@ func (c *cmd) run(args []string) int {
 	}
 
 	// Generate config
-	bootstrapJson, err := c.generateConfig()
+	bootstrapJson, err := c.generateConfig(osPlatform)
 	if err != nil {
 		c.UI.Error(err.Error())
 		return 1
@@ -472,7 +482,7 @@ func (c *cmd) findBinary() (string, error) {
 	return exec.LookPath("envoy")
 }
 
-func (c *cmd) templateArgs() (*BootstrapTplArgs, error) {
+func (c *cmd) templateArgs(osPlatform string) (*BootstrapTplArgs, error) {
 	httpCfg := api.DefaultConfig()
 	c.http.MergeOntoConfig(httpCfg)
 
@@ -515,7 +525,11 @@ func (c *cmd) templateArgs() (*BootstrapTplArgs, error) {
 
 	adminAccessLogPath := c.adminAccessLogPath
 	if adminAccessLogPath == "" {
-		adminAccessLogPath = DefaultAdminAccessLogPath
+		if osPlatform == "windows" {
+			adminAccessLogPath = DefaultWindowsAdminAccessLogPath
+		} else {
+			adminAccessLogPath = DefaultUnixAdminAccessLogPath
+		}
 	}
 
 	// Fallback to the old certificate configuration, if none was defined.
@@ -556,8 +570,8 @@ func (c *cmd) templateArgs() (*BootstrapTplArgs, error) {
 	}, nil
 }
 
-func (c *cmd) generateConfig() ([]byte, error) {
-	args, err := c.templateArgs()
+func (c *cmd) generateConfig(osPlatform string) ([]byte, error) {
+	args, err := c.templateArgs(osPlatform)
 	if err != nil {
 		return nil, err
 	}

command/connect/envoy/envoy_test.go

@@ -24,9 +24,11 @@ import (
 
 var update = flag.Bool("update", false, "update golden files")
 
+const defaultOSPlatform = "linux"
+
 func TestEnvoyCommand_noTabs(t *testing.T) {
 	t.Parallel()
-	if strings.ContainsRune(New(nil).Help(), '\t') {
+	if strings.ContainsRune(New(nil, defaultOSPlatform).Help(), '\t') {
 		t.Fatal("help has tabs")
 	}
 }
@@ -64,8 +66,8 @@ func TestEnvoyGateway_Validation(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			ui := cli.NewMockUi()
-			c := New(ui)
-			c.init()
+			c := New(ui, defaultOSPlatform)
+			c.init(defaultOSPlatform)
 
 			code := c.Run(tc.args)
 			if code == 0 {
@@ -120,6 +122,7 @@ type generateConfigTestCase struct {
 	AgentSelf110      bool            // fake the agent API from versions v1.10 and earlier
 	WantArgs          BootstrapTplArgs
 	WantErr           string
+	OSPlatform        string
 }
 
 // This tests the args we use to generate the template directly because they
@@ -160,6 +163,28 @@ func TestGenerateConfig(t *testing.T) {
 				PrometheusScrapePath:  "/metrics",
 			},
 		},
+		{
+			Name:  "defaults-windows",
+			Flags: []string{"-proxy-id", "test-proxy"},
+			WantArgs: BootstrapTplArgs{
+				ProxyCluster: "test-proxy",
+				ProxyID:      "test-proxy",
+				// We don't know this til after the lookup so it will be empty in the
+				// initial args call we are testing here.
+				ProxySourceService: "",
+				GRPC: GRPC{
+					AgentAddress: "127.0.0.1",
+					AgentPort:    "8502", // Note this is the gRPC port
+				},
+				AdminAccessLogPath:    "nul",
+				AdminBindAddress:      "127.0.0.1",
+				AdminBindPort:         "19000",
+				LocalAgentClusterName: xds.LocalAgentClusterName,
+				PrometheusBackendPort: "",
+				PrometheusScrapePath:  "/metrics",
+			},
+			OSPlatform: "windows",
+		},
 		{
 			Name:  "defaults-nodemeta",
 			Flags: []string{"-proxy-id", "test-proxy", "-node-name", "test-node"},
@@ -1101,8 +1126,14 @@ func TestGenerateConfig(t *testing.T) {
 			client, err := api.NewClient(&api.Config{Address: srv.URL, TLSConfig: api.TLSConfig{InsecureSkipVerify: true}})
 			require.NoError(t, err)
 
+			// Default OS Platform "linux". Custom value should be set in the test case
+			osPlatform := "linux"
+			if tc.OSPlatform == "windows" {
+				osPlatform = tc.OSPlatform
+			}
+
 			ui := cli.NewMockUi()
-			c := New(ui)
+			c := New(ui, osPlatform)
 			// explicitly set the client to one which can connect to the httptest.Server
 			c.client = client
 
@@ -1111,7 +1142,7 @@ func TestGenerateConfig(t *testing.T) {
 			args := append([]string{"-bootstrap"}, myFlags...)
 
 			require.NoError(t, c.flags.Parse(args))
-			code := c.run(c.flags.Args())
+			code := c.run(c.flags.Args(), osPlatform)
 			if tc.WantErr == "" {
 				require.Equal(t, 0, code, ui.ErrorWriter.String())
 			} else {
@@ -1122,7 +1153,8 @@ func TestGenerateConfig(t *testing.T) {
 
 			// Verify we handled the env and flags right first to get correct template
 			// args.
-			got, err := c.templateArgs()
+			got, err := c.templateArgs(osPlatform)
+
 			require.NoError(t, err) // Error cases should have returned above
 			require.Equal(t, &tc.WantArgs, got)
 
@@ -1215,7 +1247,7 @@ func TestEnvoy_GatewayRegistration(t *testing.T) {
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
 			ui := cli.NewMockUi()
-			c := New(ui)
+			c := New(ui, defaultOSPlatform)
 
 			code := c.Run(tc.args)
 			if code != 0 {