Update service health check to use newer patterns. (#27)

Author: thbishop-intuit

CHANGELOG.md

@@ -2,8 +2,22 @@
 
 ## Unreleased
 
+## v0.3.0 (2021-06-16)
+### Added
+* Health check tests.
+* More linting.
+
+### Fixed
+* Issues reported by linter.
+
+### Changed
+* Health check uses newer env var pattern.
+* Health check uses newer logging pattern.
+* Health check now responds with the same for success/failure (was json for
+  failures).
+
 ## v0.2.1 (2021-06-16)
-### Add
+### Added
 * CLI tests.
 * More linting.
 

service/handlers.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"os"
 	"regexp"
 	"strings"
 
@@ -103,15 +102,18 @@ func (h handler) validateWorkflowParameters(parameters map[string]string) error
 }
 
 // Service HealthCheck
-func (h handler) healthCheck(w http.ResponseWriter, r *http.Request) {
-	level.Debug(h.logger).Log("message", "executing health check")
-	vaultEndpoint := fmt.Sprintf("%s/v1/sys/health", os.Getenv("VAULT_ADDR"))
+func (h *handler) healthCheck(w http.ResponseWriter, r *http.Request) {
+	vaultEndpoint := fmt.Sprintf("%s/v1/sys/health", h.env.VaultAddress)
+
+	l := h.requestLogger(r, "op", "health-check", "vault-endpoint", vaultEndpoint)
+	level.Debug(l).Log("message", "executing")
 
 	// #nosec
 	response, err := http.Get(vaultEndpoint)
 	if err != nil {
-		level.Error(h.logger).Log("message", fmt.Sprintf("received error connecting to vault endpoint %s", vaultEndpoint))
-		h.errorResponse(w, "Health check failed", http.StatusServiceUnavailable)
+		level.Error(h.logger).Log("message", "received error connecting to vault", "error", err)
+		w.WriteHeader(http.StatusServiceUnavailable)
+		fmt.Fprintln(w, "Health check failed")
 		return
 	}
 
@@ -121,16 +123,18 @@ func (h handler) healthCheck(w http.ResponseWriter, r *http.Request) {
 	defer response.Body.Close()
 	_, err = ioutil.ReadAll(response.Body)
 	if err != nil {
-		level.Error(h.logger).Log("message", "unable to read body; continuing", "error", err)
+		level.Warn(h.logger).Log("message", "unable to read vault body; continuing", "error", err)
 		// Continue on and handle the actual response code from Vault accordingly.
 	}
 
 	if response.StatusCode != 200 && response.StatusCode != 429 {
-		level.Error(h.logger).Log("message", fmt.Sprintf("received code %d which is not 200 (initialized, unsealed, and active) or 429 (unsealed and standby) when connecting to vault endpoint %s", response.StatusCode, vaultEndpoint))
-		h.errorResponse(w, "Health check failed", http.StatusServiceUnavailable)
-	} else {
-		fmt.Fprintln(w, "Health check succeeded")
+		level.Error(h.logger).Log("message", fmt.Sprintf("received code %d which is not 200 (initialized, unsealed, and active) or 429 (unsealed and standby) when connecting to vault", response.StatusCode))
+		w.WriteHeader(http.StatusServiceUnavailable)
+		fmt.Fprintln(w, "Health check failed")
+		return
 	}
+
+	fmt.Fprintln(w, "Health check succeeded")
 }
 
 // Lists workflows

service/handlers_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -18,6 +19,7 @@ import (
 
 	"github.com/go-kit/kit/log"
 	vault "github.com/hashicorp/vault/api"
+	"github.com/stretchr/testify/assert"
 )
 
 const (
@@ -545,6 +547,105 @@ func TestListWorkflows(t *testing.T) {
 	runTests(t, tests)
 }
 
+func TestHealthCheck(t *testing.T) {
+	tests := []struct {
+		name                  string
+		endpoint              string // Used to cause a connection error.
+		vaultStatusCode       int
+		writeBadContentLength bool // Used to create response body error.
+		wantResponseBody      string
+		wantStatusCode        int
+	}{
+		{
+			name:             "good_vault_200",
+			vaultStatusCode:  http.StatusOK,
+			wantResponseBody: "Health check succeeded\n",
+			wantStatusCode:   http.StatusOK,
+		},
+		{
+			name:             "good_vault_429",
+			vaultStatusCode:  http.StatusTooManyRequests,
+			wantResponseBody: "Health check succeeded\n",
+			wantStatusCode:   http.StatusOK,
+		},
+		{
+			// We want successful health check in this vault error scenario.
+			name:                  "error_vault_read_response",
+			vaultStatusCode:       http.StatusOK,
+			writeBadContentLength: true,
+			wantResponseBody:      "Health check succeeded\n",
+			wantStatusCode:        http.StatusOK,
+		},
+		{
+			name:             "error_vault_connection",
+			endpoint:         string('\f'),
+			wantResponseBody: "Health check failed\n",
+			wantStatusCode:   http.StatusServiceUnavailable,
+		},
+		{
+			name:             "error_vault_unhealthy_status_code",
+			vaultStatusCode:  http.StatusInternalServerError,
+			wantResponseBody: "Health check failed\n",
+			wantStatusCode:   http.StatusServiceUnavailable,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			wantURL := "/v1/sys/health"
+
+			vaultSvc := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Path != wantURL {
+					http.NotFound(w, r)
+				}
+
+				if r.Method != http.MethodGet {
+					w.WriteHeader(http.StatusMethodNotAllowed)
+					return
+				}
+
+				if tt.writeBadContentLength {
+					w.Header().Set("Content-Length", "1")
+				}
+
+				w.WriteHeader(tt.vaultStatusCode)
+			}))
+			defer vaultSvc.Close()
+
+			vaultEndpoint := vaultSvc.URL
+			if tt.endpoint != "" {
+				vaultEndpoint = tt.endpoint
+			}
+
+			h := handler{
+				logger: log.NewNopLogger(),
+				env: env.Vars{
+					VaultAddress: vaultEndpoint,
+				},
+			}
+
+			// Dummy request.
+			req, err := http.NewRequest("", "", nil)
+			if err != nil {
+				assert.Nil(t, err)
+			}
+
+			resp := httptest.NewRecorder()
+
+			h.healthCheck(resp, req)
+
+			respResult := resp.Result()
+			defer respResult.Body.Close()
+
+			body, err := io.ReadAll(respResult.Body)
+			assert.Nil(t, err)
+
+			assert.Equal(t, tt.wantStatusCode, respResult.StatusCode)
+			assert.Equal(t, tt.wantResponseBody, string(body))
+		})
+	}
+}
+
 // Serialize a type to JSON-encoded byte buffer.
 func serialize(toMarshal interface{}) *bytes.Buffer {
 	jsonStr, _ := json.Marshal(toMarshal)