Skip to content

Commit

Permalink
Fix session backwards incompatibility with 1.6.x and earlier.
Browse files Browse the repository at this point in the history
  • Loading branch information
mkeeler committed Mar 5, 2020
1 parent ee24f4d commit 7584dfe
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 1 deletion.
15 changes: 15 additions & 0 deletions agent/consul/session_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ type Session struct {
logger hclog.Logger
}

// in v1.7.0 we renamed Session -> SessionID. While its more descriptive of what
// we actually expect, it did break the RPC API for the SessionSpecificRequest. Now
// we have to put back the original name and support both with the new name being
// the canonical name and the other being considered only when the main one is empty.
func fixupSessionSpecificRequest(args *structs.SessionSpecificRequest) {
if args.SessionID == "" {
args.SessionID = args.Session
}
}

// Apply is used to apply a modifying request to the data store. This should
// only be used for operations that modify the data
func (s *Session) Apply(args *structs.SessionRequest, reply *string) error {
Expand Down Expand Up @@ -156,6 +166,8 @@ func (s *Session) Get(args *structs.SessionSpecificRequest,
return err
}

fixupSessionSpecificRequest(args)

var authzContext acl.AuthorizerContext
authz, err := s.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
if err != nil {
Expand Down Expand Up @@ -262,6 +274,9 @@ func (s *Session) Renew(args *structs.SessionSpecificRequest,
if done, err := s.srv.forward("Session.Renew", args, args, reply); done {
return err
}

fixupSessionSpecificRequest(args)

defer metrics.MeasureSince([]string{"session", "renew"}, time.Now())

// Fetch the ACL token, if any, and apply the policy.
Expand Down
105 changes: 104 additions & 1 deletion agent/consul/session_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/testrpc"
"github.com/hashicorp/net-rpc-msgpackrpc"
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
)

func TestSession_Apply(t *testing.T) {
Expand Down Expand Up @@ -280,6 +280,52 @@ func TestSession_Get(t *testing.T) {
}
}

func TestSession_Get_Compat(t *testing.T) {
t.Parallel()
dir1, s1 := testServer(t)
defer os.RemoveAll(dir1)
defer s1.Shutdown()

codec := rpcClient(t, s1)
defer codec.Close()

testrpc.WaitForLeader(t, s1.RPC, "dc1")

s1.fsm.State().EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"})
arg := structs.SessionRequest{
Datacenter: "dc1",
Op: structs.SessionCreate,
Session: structs.Session{
Node: "foo",
},
}
var out string
if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &out); err != nil {
t.Fatalf("err: %v", err)
}

getR := structs.SessionSpecificRequest{
Datacenter: "dc1",
// this should get converted to the SessionID field internally
Session: out,
}
var sessions structs.IndexedSessions
if err := msgpackrpc.CallWithCodec(codec, "Session.Get", &getR, &sessions); err != nil {
t.Fatalf("err: %v", err)
}

if sessions.Index == 0 {
t.Fatalf("Bad: %v", sessions)
}
if len(sessions.Sessions) != 1 {
t.Fatalf("Bad: %v", sessions)
}
s := sessions.Sessions[0]
if s.ID != out {
t.Fatalf("bad: %v", s)
}
}

func TestSession_List(t *testing.T) {
t.Parallel()
dir1, s1 := testServer(t)
Expand Down Expand Up @@ -793,6 +839,63 @@ session "foo" {
}
}

func TestSession_Renew_Compat(t *testing.T) {
// This method is timing sensitive, disable Parallel
//t.Parallel()
ttl := 5 * time.Second
TTL := ttl.String()

dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.SessionTTLMin = ttl
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
testrpc.WaitForTestAgent(t, s1.RPC, "dc1")

codec := rpcClient(t, s1)
defer codec.Close()

s1.fsm.State().EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"})
var id string
arg := structs.SessionRequest{
Datacenter: "dc1",
Op: structs.SessionCreate,
Session: structs.Session{
Node: "foo",
TTL: TTL,
},
}
if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &id); err != nil {
t.Fatalf("err: %v", err)
}

// renew the session
renewR := structs.SessionSpecificRequest{
Datacenter: "dc1",
// this will get ranslated internally to the SessionID field
Session: id,
}
var session structs.IndexedSessions
if err := msgpackrpc.CallWithCodec(codec, "Session.Renew", &renewR, &session); err != nil {
t.Fatalf("err: %v", err)
}

if session.Index == 0 {
t.Fatalf("Bad: %v", session)
}
if len(session.Sessions) != 1 {
t.Fatalf("Bad: %v", session.Sessions)
}

s := session.Sessions[0]
if id != s.ID {
t.Fatalf("bad: %v", s)
}
if s.Node != "foo" {
t.Fatalf("bad: %v", s)
}
}

func TestSession_NodeSessions(t *testing.T) {
t.Parallel()
dir1, s1 := testServer(t)
Expand Down
2 changes: 2 additions & 0 deletions agent/session_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ func (s *HTTPServer) SessionRenew(resp http.ResponseWriter, req *http.Request) (

// Pull out the session id
args.SessionID = strings.TrimPrefix(req.URL.Path, "/v1/session/renew/")
args.Session = args.SessionID
if args.SessionID == "" {
resp.WriteHeader(http.StatusBadRequest)
fmt.Fprint(resp, "Missing session")
Expand Down Expand Up @@ -128,6 +129,7 @@ func (s *HTTPServer) SessionGet(resp http.ResponseWriter, req *http.Request) (in

// Pull out the session id
args.SessionID = strings.TrimPrefix(req.URL.Path, "/v1/session/info/")
args.Session = args.SessionID
if args.SessionID == "" {
resp.WriteHeader(http.StatusBadRequest)
fmt.Fprint(resp, "Missing session")
Expand Down
2 changes: 2 additions & 0 deletions agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -2036,6 +2036,8 @@ func (r *SessionRequest) RequestDatacenter() string {
type SessionSpecificRequest struct {
Datacenter string
SessionID string
// DEPRECATED in 1.7.0
Session string
EnterpriseMeta
QueryOptions
}
Expand Down

0 comments on commit 7584dfe

Please sign in to comment.