Merge pull request #3747 from hashicorp/session-checks

Works around mapstructure behavior to enable sessions with no checks.

agent/session_endpoint.go

@@ -31,7 +31,8 @@ func (s *HTTPServer) SessionCreate(resp http.ResponseWriter, req *http.Request)
 		return nil, MethodNotAllowedError{req.Method, []string{"PUT"}}
 	}
 
-	// Default the session to our node + serf check + release session invalidate behavior
+	// Default the session to our node + serf check + release session
+	// invalidate behavior.
 	args := structs.SessionRequest{
 		Op: structs.SessionCreate,
 		Session: structs.Session{
@@ -47,7 +48,16 @@ func (s *HTTPServer) SessionCreate(resp http.ResponseWriter, req *http.Request)
 
 	// Handle optional request body
 	if req.ContentLength > 0 {
-		if err := decodeBody(req, &args.Session, FixupLockDelay); err != nil {
+		fixup := func(raw interface{}) error {
+			if err := FixupLockDelay(raw); err != nil {
+				return err
+			}
+			if err := FixupChecks(raw, &args.Session); err != nil {
+				return err
+			}
+			return nil
+		}
+		if err := decodeBody(req, &args.Session, fixup); err != nil {
 			resp.WriteHeader(http.StatusBadRequest)
 			fmt.Fprintf(resp, "Request decode failed: %v", err)
 			return nil, nil
@@ -103,6 +113,27 @@ func FixupLockDelay(raw interface{}) error {
 	return nil
 }
 
+// FixupChecks is used to handle parsing the JSON body to default-add the Serf
+// health check if they didn't specify any checks, but to allow an empty list
+// to take out the Serf health check. This behavior broke when mapstructure was
+// updated after 0.9.3, likely because we have a type wrapper around the string.
+func FixupChecks(raw interface{}, s *structs.Session) error {
+	rawMap, ok := raw.(map[string]interface{})
+	if !ok {
+		return nil
+	}
+	for k := range rawMap {
+		if strings.ToLower(k) == "checks" {
+			// If they supplied a checks key in the JSON, then
+			// remove the default entries and respect whatever they
+			// specified.
+			s.Checks = nil
+			return nil
+		}
+	}
+	return nil
+}
+
 // SessionDestroy is used to destroy an existing session
 func (s *HTTPServer) SessionDestroy(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
 	if req.Method != "PUT" {

agent/session_endpoint_test.go

@@ -11,8 +11,32 @@ import (
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/types"
+	"github.com/pascaldekloe/goe/verify"
 )
 
+func verifySession(t *testing.T, a *TestAgent, want structs.Session) {
+	t.Helper()
+
+	args := &structs.SessionSpecificRequest{
+		Datacenter: "dc1",
+		Session:    want.ID,
+	}
+	var out structs.IndexedSessions
+	if err := a.RPC("Session.Get", args, &out); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if len(out.Sessions) != 1 {
+		t.Fatalf("bad: %#v", out.Sessions)
+	}
+
+	// Make a copy so we don't modify the state store copy for an in-mem
+	// RPC and zero out the Raft info for the compare.
+	got := *(out.Sessions[0])
+	got.CreateIndex = 0
+	got.ModifyIndex = 0
+	verify.Values(t, "", got, want)
+}
+
 func TestSessionCreate(t *testing.T) {
 	t.Parallel()
 	a := NewTestAgent(t.Name(), "")
@@ -54,12 +78,18 @@ func TestSessionCreate(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if _, ok := obj.(sessionCreateResponse); !ok {
-		t.Fatalf("should work")
+	want := structs.Session{
+		ID:        obj.(sessionCreateResponse).ID,
+		Name:      "my-cool-session",
+		Node:      a.Config.NodeName,
+		Checks:    []types.CheckID{structs.SerfCheckID, "consul"},
+		LockDelay: 20 * time.Second,
+		Behavior:  structs.SessionKeysRelease,
 	}
+	verifySession(t, a, want)
 }
 
-func TestSessionCreateDelete(t *testing.T) {
+func TestSessionCreate_Delete(t *testing.T) {
 	t.Parallel()
 	a := NewTestAgent(t.Name(), "")
 	defer a.Shutdown()
@@ -101,9 +131,82 @@ func TestSessionCreateDelete(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if _, ok := obj.(sessionCreateResponse); !ok {
-		t.Fatalf("should work")
+	want := structs.Session{
+		ID:        obj.(sessionCreateResponse).ID,
+		Name:      "my-cool-session",
+		Node:      a.Config.NodeName,
+		Checks:    []types.CheckID{structs.SerfCheckID, "consul"},
+		LockDelay: 20 * time.Second,
+		Behavior:  structs.SessionKeysDelete,
+	}
+	verifySession(t, a, want)
+}
+
+func TestSessionCreate_DefaultCheck(t *testing.T) {
+	t.Parallel()
+	a := NewTestAgent(t.Name(), "")
+	defer a.Shutdown()
+
+	// Associate session with node and 2 health checks
+	body := bytes.NewBuffer(nil)
+	enc := json.NewEncoder(body)
+	raw := map[string]interface{}{
+		"Name":      "my-cool-session",
+		"Node":      a.Config.NodeName,
+		"LockDelay": "20s",
+	}
+	enc.Encode(raw)
+
+	req, _ := http.NewRequest("PUT", "/v1/session/create", body)
+	resp := httptest.NewRecorder()
+	obj, err := a.srv.SessionCreate(resp, req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	want := structs.Session{
+		ID:        obj.(sessionCreateResponse).ID,
+		Name:      "my-cool-session",
+		Node:      a.Config.NodeName,
+		Checks:    []types.CheckID{structs.SerfCheckID},
+		LockDelay: 20 * time.Second,
+		Behavior:  structs.SessionKeysRelease,
+	}
+	verifySession(t, a, want)
+}
+
+func TestSessionCreate_NoCheck(t *testing.T) {
+	t.Parallel()
+	a := NewTestAgent(t.Name(), "")
+	defer a.Shutdown()
+
+	// Associate session with node and 2 health checks
+	body := bytes.NewBuffer(nil)
+	enc := json.NewEncoder(body)
+	raw := map[string]interface{}{
+		"Name":      "my-cool-session",
+		"Node":      a.Config.NodeName,
+		"Checks":    []types.CheckID{},
+		"LockDelay": "20s",
+	}
+	enc.Encode(raw)
+
+	req, _ := http.NewRequest("PUT", "/v1/session/create", body)
+	resp := httptest.NewRecorder()
+	obj, err := a.srv.SessionCreate(resp, req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	want := structs.Session{
+		ID:        obj.(sessionCreateResponse).ID,
+		Name:      "my-cool-session",
+		Node:      a.Config.NodeName,
+		Checks:    []types.CheckID{},
+		LockDelay: 20 * time.Second,
+		Behavior:  structs.SessionKeysRelease,
 	}
+	verifySession(t, a, want)
 }
 
 func TestFixupLockDelay(t *testing.T) {

api/session_test.go

@@ -5,6 +5,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/pascaldekloe/goe/verify"
 )
 
 func TestAPI_SessionCreateDestroy(t *testing.T) {
@@ -289,7 +291,6 @@ func TestAPI_SessionInfo(t *testing.T) {
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
-
 	if qm.LastIndex == 0 {
 		t.Fatalf("bad: %v", qm)
 	}
@@ -297,33 +298,59 @@ func TestAPI_SessionInfo(t *testing.T) {
 		t.Fatalf("bad: %v", qm)
 	}
 
-	if info == nil {
-		t.Fatalf("should get session")
-	}
 	if info.CreateIndex == 0 {
 		t.Fatalf("bad: %v", info)
 	}
-	if info.ID != id {
-		t.Fatalf("bad: %v", info)
+	info.CreateIndex = 0
+
+	want := &SessionEntry{
+		ID:        id,
+		Node:      s.Config.NodeName,
+		Checks:    []string{"serfHealth"},
+		LockDelay: 15 * time.Second,
+		Behavior:  SessionBehaviorRelease,
 	}
-	if info.Name != "" {
-		t.Fatalf("bad: %v", info)
+	verify.Values(t, "", info, want)
 }
-	if info.Node == "" {
-		t.Fatalf("bad: %v", info)
+
+func TestAPI_SessionInfo_NoChecks(t *testing.T) {
+	t.Parallel()
+	c, s := makeClient(t)
+	defer s.Stop()
+
+	session := c.Session()
+
+	id, _, err := session.CreateNoChecks(nil, nil)
+	if err != nil {
+		t.Fatalf("err: %v", err)
 	}
-	if len(info.Checks) == 0 {
-		t.Fatalf("bad: %v", info)
+	defer session.Destroy(id, nil)
+
+	info, qm, err := session.Info(id, nil)
+	if err != nil {
+		t.Fatalf("err: %v", err)
 	}
-	if info.LockDelay == 0 {
-		t.Fatalf("bad: %v", info)
+
+	if qm.LastIndex == 0 {
+		t.Fatalf("bad: %v", qm)
 	}
-	if info.Behavior != "release" {
-		t.Fatalf("bad: %v", info)
+	if !qm.KnownLeader {
+		t.Fatalf("bad: %v", qm)
 	}
-	if info.TTL != "" {
+
+	if info.CreateIndex == 0 {
 		t.Fatalf("bad: %v", info)
 	}
+	info.CreateIndex = 0
+
+	want := &SessionEntry{
+		ID:        id,
+		Node:      s.Config.NodeName,
+		Checks:    []string{},
+		LockDelay: 15 * time.Second,
+		Behavior:  SessionBehaviorRelease,
+	}
+	verify.Values(t, "", info, want)
 }
 
 func TestAPI_SessionNode(t *testing.T) {