From 0de8e8459e529d854645459ba1e3594636fccaaa Mon Sep 17 00:00:00 2001
From: Christopher Swenson <swenson@swenson.io>
Date: Thu, 7 Apr 2022 11:03:38 -0700
Subject: [PATCH] Agent error log level is mismatched (#14424)

* [VAULT-1618] Agent error log level is mismatched

`logLevelToStringPtr` translates `go-hclog`'s `ERROR` to `"ERROR"` for
Consul Template's runner, but that expects `ERR` and is quite strict
about it.

This will address https://github.com/hashicorp/vault-k8s/issues/223
after it is set as the default image in `vault-k8s`.

I didn't find a simple way to test this other than starting up a full
server and agent and letting them run, which is unfortunately fairly
slow.

I confirmed that this addresses the original issue by modifying the helm
chart with the values in this commit and patching the log level to `err`.

* VAULT-1618 Add changelog/14424.txt

* VAULT-1618 Update changelog/14424.txt based on @kalafut suggestion

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>

* VAULT-1618 Move cancel and server stop into defer in tests

* VAULT-1618 Triggering CircleCI tests

* VAULT-1618 Replace ioutil with os functions for agent template tests

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
---
 changelog/14424.txt                     |  3 +
 command/agent/template/template.go      |  2 +-
 command/agent/template/template_test.go | 78 +++++++++++++++++++++++--
 3 files changed, 77 insertions(+), 6 deletions(-)
 create mode 100644 changelog/14424.txt

diff --git a/changelog/14424.txt b/changelog/14424.txt
new file mode 100644
index 000000000..cc54058f9
--- /dev/null
+++ b/changelog/14424.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+agent: Fix log level mismatch between ERR and ERROR
+```
diff --git a/command/agent/template/template.go b/command/agent/template/template.go
index 3ad546787..cf21944ab 100644
--- a/command/agent/template/template.go
+++ b/command/agent/template/template.go
@@ -341,7 +341,7 @@ func logLevelToStringPtr(level hclog.Level) *string {
 	case hclog.Warn:
 		levelStr = "WARN"
 	case hclog.Error:
-		levelStr = "ERROR"
+		levelStr = "ERR"
 	default:
 		levelStr = "INFO"
 	}
diff --git a/command/agent/template/template_test.go b/command/agent/template/template_test.go
index 9f0464730..77f166aff 100644
--- a/command/agent/template/template_test.go
+++ b/command/agent/template/template_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -188,7 +187,7 @@ func TestCacheConfigNoListener(t *testing.T) {
 	assert.NotNil(t, ctConfig.Vault.Transport.CustomDialer)
 }
 
-func TestServerRun(t *testing.T) {
+func createHttpTestServer() *httptest.Server {
 	// create http test server
 	mux := http.NewServeMux()
 	mux.HandleFunc("/v1/kv/myapp/config", func(w http.ResponseWriter, r *http.Request) {
@@ -203,9 +202,14 @@ func TestServerRun(t *testing.T) {
 		fmt.Fprintln(w, `{"errors":["1 error occurred:\n\t* permission denied\n\n"]}`)
 	})
 
-	ts := httptest.NewServer(mux)
+	return httptest.NewServer(mux)
+}
+
+func TestServerRun(t *testing.T) {
+	ts := createHttpTestServer()
 	defer ts.Close()
-	tmpDir, err := ioutil.TempDir("", "agent-tests")
+
+	tmpDir, err := os.MkdirTemp("", "agent-tests")
 	defer os.RemoveAll(tmpDir)
 	if err != nil {
 		t.Fatal(err)
@@ -379,7 +383,7 @@ func TestServerRun(t *testing.T) {
 				if template.Destination == nil {
 					t.Fatal("nil template destination")
 				}
-				content, err := ioutil.ReadFile(*template.Destination)
+				content, err := os.ReadFile(*template.Destination)
 				if err != nil {
 					t.Fatal(err)
 				}
@@ -400,6 +404,70 @@ func TestServerRun(t *testing.T) {
 	}
 }
 
+// TestNewServerLogLevels tests that the server can be started with any log
+// level.
+func TestNewServerLogLevels(t *testing.T) {
+	ts := createHttpTestServer()
+	defer ts.Close()
+
+	tmpDir, err := os.MkdirTemp("", "agent-tests")
+	defer os.RemoveAll(tmpDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	levels := []hclog.Level{hclog.NoLevel, hclog.Trace, hclog.Debug, hclog.Info, hclog.Warn, hclog.Error}
+	for _, level := range levels {
+		name := fmt.Sprintf("log_%s", level)
+		t.Run(name, func(t *testing.T) {
+			server := NewServer(&ServerConfig{
+				Logger:        logging.NewVaultLogger(level),
+				LogWriter:     hclog.DefaultOutput,
+				LogLevel:      level,
+				ExitAfterAuth: true,
+				AgentConfig: &config.Config{
+					Vault: &config.Vault{
+						Address: ts.URL,
+					},
+				},
+			})
+			if server == nil {
+				t.Fatal("nil server returned")
+			}
+			defer server.Stop()
+
+			templateTokenCh := make(chan string, 1)
+
+			templateTest := &ctconfig.TemplateConfig{
+				Contents: pointerutil.StringPtr(templateContents),
+			}
+			dstFile := fmt.Sprintf("%s/%s", tmpDir, name)
+			templateTest.Destination = pointerutil.StringPtr(dstFile)
+			templatesToRender := []*ctconfig.TemplateConfig{templateTest}
+
+			ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
+			defer cancel()
+
+			errCh := make(chan error)
+			go func() {
+				errCh <- server.Run(ctx, templateTokenCh, templatesToRender)
+			}()
+
+			// send a dummy value to trigger auth so the server will exit
+			templateTokenCh <- "test"
+
+			select {
+			case <-ctx.Done():
+				t.Fatal("timeout reached before templates were rendered")
+			case err := <-errCh:
+				if err != nil {
+					t.Fatalf("did not expect error, got: %v", err)
+				}
+			}
+		})
+	}
+}
+
 var jsonResponse = `
 {
   "request_id": "8af096e9-518c-7351-eff5-5ba20554b21f",