nsd: add support for specifying check.method in nomad service checks

Unblock 'check.method' in service validation. Add tests around making
sure this value gets plumbed through.

client/serviceregistration/checks/client_test.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/nomad/ci"
+	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/freeport"
 	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/hashicorp/nomad/nomad/mock"
@@ -20,6 +21,13 @@ import (
 	"oss.indeed.com/go/libtime/libtimetest"
 )
 
+func splitURL(u string) (string, string) {
+	// get the address and port for http server
+	tokens := strings.Split(u, ":")
+	addr, port := strings.TrimPrefix(tokens[1], "//"), tokens[2]
+	return addr, port
+}
+
 func TestChecker_Do_HTTP(t *testing.T) {
 	ci.Parallel(t)
 
@@ -49,8 +57,7 @@ func TestChecker_Do_HTTP(t *testing.T) {
 	defer ts.Close()
 
 	// get the address and port for http server
-	tokens := strings.Split(ts.URL, ":")
-	addr, port := strings.TrimPrefix(tokens[1], "//"), tokens[2]
+	addr, port := splitURL(ts.URL)
 
 	// create a mock clock so we can assert time is set
 	now := time.Date(2022, 1, 2, 3, 4, 5, 6, time.UTC)
@@ -200,6 +207,101 @@ func bigResponse() (string, string) {
 	return s, s[:outputSizeLimit]
 }
 
+func TestChecker_Do_HTTP_extras(t *testing.T) {
+	ci.Parallel(t)
+
+	// record the method, body, and headers of the request
+	var (
+		method  string
+		body    []byte
+		headers map[string][]string
+	)
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		method = r.Method
+		body, _ = io.ReadAll(r.Body)
+		headers = helper.CopyMap(r.Header)
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer ts.Close()
+
+	// get the address and port for http server
+	addr, port := splitURL(ts.URL)
+
+	// make headers from key-value pairs
+	makeHeaders := func(more ...[2]string) http.Header {
+		h := make(http.Header)
+		for _, extra := range more {
+			h.Set(extra[0], extra[1])
+		}
+		return h
+	}
+
+	encoding := [2]string{"Accept-Encoding", "gzip"}
+	agent := [2]string{"User-Agent", "Go-http-client/1.1"}
+
+	cases := []struct {
+		name    string
+		method  string
+		body    string
+		headers map[string][]string
+	}{
+		{
+			name:    "method GET",
+			method:  "GET",
+			headers: makeHeaders(encoding, agent),
+		},
+		{
+			name:    "method Get",
+			method:  "Get",
+			headers: makeHeaders(encoding, agent),
+		},
+		{
+			name:    "method HEAD",
+			method:  "HEAD",
+			headers: makeHeaders(agent),
+		},
+	}
+
+	for _, tc := range cases {
+		qc := &QueryContext{
+			ID:               "abc123",
+			CustomAddress:    addr,
+			ServicePortLabel: port,
+			Networks:         nil,
+			NetworkStatus:    mock.NewNetworkStatus(addr),
+			Ports:            nil,
+			Group:            "group",
+			Task:             "task",
+			Service:          "service",
+			Check:            "check",
+		}
+
+		q := &Query{
+			Mode:        structs.Healthiness,
+			Type:        "http",
+			Timeout:     1 * time.Second,
+			AddressMode: "auto",
+			PortLabel:   port,
+			Protocol:    "http",
+			Path:        "/",
+			Method:      tc.method,
+		}
+
+		logger := testlog.HCLogger(t)
+		c := New(logger)
+		ctx := context.Background()
+		result := c.Do(ctx, qc, q)
+		must.Eq(t, http.StatusOK, result.StatusCode,
+			must.Sprintf("test.URL: %s", ts.URL),
+			must.Sprintf("headers: %v", tc.headers),
+		)
+		must.Eq(t, tc.method, method)
+		must.Eq(t, tc.body, string(body))
+		must.Eq(t, tc.headers, headers)
+	}
+}
+
 func TestChecker_Do_TCP(t *testing.T) {
 	ci.Parallel(t)
 

helper/funcs.go

@@ -3,6 +3,7 @@ package helper
 import (
 	"crypto/sha512"
 	"fmt"
+	"net/http"
 	"path/filepath"
 	"reflect"
 	"regexp"
@@ -716,3 +717,21 @@ func NewSafeTimer(duration time.Duration) (*time.Timer, StopFunc) {
 
 	return t, cancel
 }
+
+// IsMethodHTTP returns whether s is a known HTTP method, ignoring case.
+func IsMethodHTTP(s string) bool {
+	switch strings.ToUpper(s) {
+	case http.MethodGet:
+	case http.MethodHead:
+	case http.MethodPost:
+	case http.MethodPut:
+	case http.MethodPatch:
+	case http.MethodDelete:
+	case http.MethodConnect:
+	case http.MethodOptions:
+	case http.MethodTrace:
+	default:
+		return false
+	}
+	return true
+}

helper/funcs_test.go

@@ -546,3 +546,31 @@ func Test_NewSafeTimer(t *testing.T) {
 		<-timer.C
 	})
 }
+
+func Test_IsMethodHTTP(t *testing.T) {
+	t.Run("is method", func(t *testing.T) {
+		cases := []string{
+			"GET", "Get", "get",
+			"HEAD", "Head", "head",
+			"POST", "Post", "post",
+			"PUT", "Put", "put",
+			"PATCH", "Patch", "patch",
+			"DELETE", "Delete", "delete",
+			"CONNECT", "Connect", "connect",
+			"OPTIONS", "Options", "options",
+			"TRACE", "Trace", "trace",
+		}
+		for _, tc := range cases {
+			result := IsMethodHTTP(tc)
+			must.True(t, result)
+		}
+	})
+
+	t.Run("is not method", func(t *testing.T) {
+		not := []string{"GETTER", "!GET", ""}
+		for _, tc := range not {
+			result := IsMethodHTTP(tc)
+			must.False(t, result)
+		}
+	})
+}

nomad/structs/services.go

@@ -200,6 +200,7 @@ func (sc *ServiceCheck) Canonicalize(serviceName string) {
 		sc.Args = nil
 	}
 
+	// Ensure empty slices are nil
 	if len(sc.Header) == 0 {
 		sc.Header = nil
 	} else {
@@ -210,10 +211,12 @@ func (sc *ServiceCheck) Canonicalize(serviceName string) {
 		}
 	}
 
+	// Ensure a default name for the check
 	if sc.Name == "" {
 		sc.Name = fmt.Sprintf("service: %q check", serviceName)
 	}
 
+	// Ensure OnUpdate defaults to require_healthy (i.e. healthiness check)
 	if sc.OnUpdate == "" {
 		sc.OnUpdate = OnUpdateRequireHealthy
 	}
@@ -344,15 +347,16 @@ func (sc *ServiceCheck) validateNomad() error {
 	}
 
 	if sc.Type == "http" {
-		if sc.Method != "" && sc.Method != "GET" {
-			// unset turns into GET
-			return fmt.Errorf("http checks may only use GET method in Nomad services")
+		if sc.Method != "" && !helper.IsMethodHTTP(sc.Method) {
+			return fmt.Errorf("method type %q not supported in Nomad http check", sc.Method)
 		}
 
+		// todo(shoenig) support headers
 		if len(sc.Header) > 0 {
 			return fmt.Errorf("http checks may not set headers in Nomad services")
 		}
 
+		// todo(shoenig) support body
 		if len(sc.Body) > 0 {
 			return fmt.Errorf("http checks may not set Body in Nomad services")
 		}

nomad/structs/services_test.go

@@ -57,6 +57,41 @@ func TestServiceCheck_Hash(t *testing.T) {
 	})
 }
 
+func TestServiceCheck_Canonicalize(t *testing.T) {
+	ci.Parallel(t)
+
+	t.Run("defaults", func(t *testing.T) {
+		sc := &ServiceCheck{
+			Args:     []string{},
+			Header:   make(map[string][]string),
+			Method:   "",
+			OnUpdate: "",
+		}
+		sc.Canonicalize("MyService")
+		must.Nil(t, sc.Args)
+		must.Nil(t, sc.Header)
+		must.Eq(t, `service: "MyService" check`, sc.Name)
+		must.Eq(t, "", sc.Method)
+		must.Eq(t, OnUpdateRequireHealthy, sc.OnUpdate)
+	})
+
+	t.Run("check name set", func(t *testing.T) {
+		sc := &ServiceCheck{
+			Name: "Some Check",
+		}
+		sc.Canonicalize("MyService")
+		must.Eq(t, "Some Check", sc.Name)
+	})
+
+	t.Run("on_update is set", func(t *testing.T) {
+		sc := &ServiceCheck{
+			OnUpdate: OnUpdateIgnore,
+		}
+		sc.Canonicalize("MyService")
+		must.Eq(t, OnUpdateIgnore, sc.OnUpdate)
+	})
+}
+
 func TestServiceCheck_validate_PassingTypes(t *testing.T) {
 	ci.Parallel(t)
 
@@ -268,7 +303,17 @@ func TestServiceCheck_validateNomad(t *testing.T) {
 				Path:     "/health",
 				Method:   "HEAD",
 			},
-			exp: `http checks may only use GET method in Nomad services`,
+		},
+		{
+			name: "http unknown method type",
+			sc: &ServiceCheck{
+				Type:     ServiceCheckHTTP,
+				Interval: 3 * time.Second,
+				Timeout:  1 * time.Second,
+				Path:     "/health",
+				Method:   "Invalid",
+			},
+			exp: `method type "Invalid" not supported in Nomad http check`,
 		},
 		{
 			name: "http with headers",