From a137c1dfe0f4f5136d8f0e207b192db7692eb5a1 Mon Sep 17 00:00:00 2001
From: Michael Schurter <mschurter@hashicorp.com>
Date: Thu, 24 Feb 2022 17:06:07 -0800
Subject: [PATCH] cli: add tests and minor fixes for op api

Trimmed spaces around header values.

Fixed method getting forced to GET.
---
 command/operator_api.go      |   8 +--
 command/operator_api_test.go | 103 +++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 5 deletions(-)
 create mode 100644 command/operator_api_test.go

diff --git a/command/operator_api.go b/command/operator_api.go
index dc9bb8200..8b28ec4c1 100644
--- a/command/operator_api.go
+++ b/command/operator_api.go
@@ -106,7 +106,7 @@ func (c *OperatorAPICommand) Run(args []string) int {
 	flags.BoolVar(&dryrun, "dryrun", false, "")
 	flags.StringVar(&filter, "filter", "", "")
 	flags.BoolVar(&c.verboseFlag, "verbose", false, "")
-	flags.StringVar(&c.method, "X", "", "")
+	flags.StringVar(&c.method, "X", "GET", "")
 	flags.Var(headerFlags, "H", "")
 
 	if err := flags.Parse(args); err != nil {
@@ -145,8 +145,6 @@ func (c *OperatorAPICommand) Run(args []string) int {
 		if c.method == "" {
 			c.method = "POST"
 		}
-	} else {
-		c.method = "GET"
 	}
 
 	config := c.clientConfig()
@@ -339,7 +337,7 @@ func (c *OperatorAPICommand) apiToCurl(config *api.Config, headers http.Header,
 	// environment variable.
 	if headers.Get("X-Nomad-Token") == "" {
 		if c.Meta.token != "" {
-			parts = append(parts, fmt.Sprintf(`-H "X-Nomad-Token: %s"`, c.Meta.token))
+			parts = append(parts, fmt.Sprintf(`-H 'X-Nomad-Token: %s'`, c.Meta.token))
 		} else if v := os.Getenv("NOMAD_TOKEN"); v != "" {
 			parts = append(parts, `-H "X-Nomad-Token: ${NOMAD_TOKEN}"`)
 		}
@@ -443,6 +441,6 @@ func (h *headerFlags) Set(v string) error {
 		return fmt.Errorf("Headers must be in the form 'Key: Value' but found: %q", v)
 	}
 
-	h.headers.Add(parts[0], parts[1])
+	h.headers.Add(parts[0], strings.TrimSpace(parts[1]))
 	return nil
 }
diff --git a/command/operator_api_test.go b/command/operator_api_test.go
new file mode 100644
index 000000000..937db63aa
--- /dev/null
+++ b/command/operator_api_test.go
@@ -0,0 +1,103 @@
+package command
+
+import (
+	"bytes"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+	"time"
+
+	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/require"
+)
+
+// TestOperatorAPICommand_Paths asserts that the op api command normalizes
+// various path formats to the proper full address.
+func TestOperatorAPICommand_Paths(t *testing.T) {
+	hits := make(chan *url.URL, 1)
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		hits <- r.URL
+	}))
+	defer ts.Close()
+
+	// Always expect the same URL to be hit
+	expected := "/v1/jobs"
+
+	buf := bytes.NewBuffer(nil)
+	ui := &cli.BasicUi{
+		ErrorWriter: buf,
+		Writer:      buf,
+	}
+	cmd := &OperatorAPICommand{Meta: Meta{Ui: ui}}
+
+	// Assert that absolute paths are appended to the configured address
+	exitCode := cmd.Run([]string{"-address=" + ts.URL, "/v1/jobs"})
+	require.Zero(t, exitCode, buf.String())
+
+	select {
+	case hit := <-hits:
+		require.Equal(t, expected, hit.String())
+	case <-time.After(10 * time.Second):
+		t.Fatalf("timed out waiting for hit")
+	}
+
+	buf.Reset()
+
+	// Assert that full URLs are used as-is even if an invalid address is
+	// set.
+	exitCode = cmd.Run([]string{"-address=ftp://127.0.0.2:1", ts.URL + "/v1/jobs"})
+	require.Zero(t, exitCode, buf.String())
+
+	select {
+	case hit := <-hits:
+		require.Equal(t, expected, hit.String())
+	case <-time.After(10 * time.Second):
+		t.Fatalf("timed out waiting for hit")
+	}
+
+	buf.Reset()
+
+	// Assert that URLs lacking a scheme are used even if an invalid
+	// address is set.
+	exitCode = cmd.Run([]string{"-address=ftp://127.0.0.2:1", ts.Listener.Addr().String() + "/v1/jobs"})
+	require.Zero(t, exitCode, buf.String())
+
+	select {
+	case hit := <-hits:
+		require.Equal(t, expected, hit.String())
+	case <-time.After(10 * time.Second):
+		t.Fatalf("timed out waiting for hit")
+	}
+}
+
+// TestOperatorAPICommand_Curl asserts that -dryrun outputs a valid curl
+// command.
+func TestOperatorAPICommand_Curl(t *testing.T) {
+	buf := bytes.NewBuffer(nil)
+	ui := &cli.BasicUi{
+		ErrorWriter: buf,
+		Writer:      buf,
+	}
+	cmd := &OperatorAPICommand{Meta: Meta{Ui: ui}}
+
+	exitCode := cmd.Run([]string{
+		"-dryrun",
+		"-address=http://127.0.0.1:1",
+		"-region=not even a valid region",
+		`-filter=this == "that" or this != "foo"`,
+		"-X", "POST",
+		"-token=acl-token",
+		"-H", "Some-Other-Header: ok",
+		"/url",
+	})
+	require.Zero(t, exitCode, buf.String())
+
+	expected := `curl \
+  -X POST \
+  -H 'Some-Other-Header: ok' \
+  -H 'X-Nomad-Token: acl-token' \
+  http://127.0.0.1:1/url?filter=this+%3D%3D+%22that%22+or+this+%21%3D+%22foo%22&region=not+even+a+valid+region
+`
+	require.Equal(t, expected, buf.String())
+}