Merge pull request #9379 from gyuho/fix-election

*: fix server panic on invalid Election Proclaim/Resign HTTP requests

Author: gyuho

e2e/util.go

@@ -42,9 +42,14 @@ func spawnWithExpect(args []string, expected string) error {
 }
 
 func spawnWithExpects(args []string, xs ...string) error {
+	_, err := spawnWithExpectLines(args, xs...)
+	return err
+}
+
+func spawnWithExpectLines(args []string, xs ...string) ([]string, error) {
 	proc, err := spawnCmd(args)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	// process until either stdout or stderr contains
 	// the expected string
@@ -57,7 +62,7 @@ func spawnWithExpects(args []string, xs ...string) error {
 			l, lerr := proc.ExpectFunc(lineFunc)
 			if lerr != nil {
 				proc.Close()
-				return fmt.Errorf("%v (expected %q, got %q)", lerr, txt, lines)
+				return nil, fmt.Errorf("%v (expected %q, got %q)", lerr, txt, lines)
 			}
 			lines = append(lines, l)
 			if strings.Contains(l, txt) {
@@ -67,9 +72,9 @@ func spawnWithExpects(args []string, xs ...string) error {
 	}
 	perr := proc.Close()
 	if len(xs) == 0 && proc.LineCount() != noOutputLineCount { // expect no output
-		return fmt.Errorf("unexpected output (got lines %q, line count %d)", lines, proc.LineCount())
+		return nil, fmt.Errorf("unexpected output (got lines %q, line count %d)", lines, proc.LineCount())
 	}
-	return perr
+	return lines, perr
 }
 
 func closeWithTimeout(p *expect.ExpectProcess, d time.Duration) error {

e2e/v3_curl_test.go

@@ -15,10 +15,13 @@
 package e2e
 
 import (
+	"encoding/base64"
 	"encoding/json"
 	"path"
+	"strconv"
 	"testing"
 
+	epb "github.com/coreos/etcd/etcdserver/api/v3election/v3electionpb"
 	pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
 	"github.com/coreos/etcd/pkg/testutil"
 
@@ -245,3 +248,115 @@ func testV3CurlAuth(cx ctlCtx) {
 		cx.t.Fatalf("failed testV3CurlAuth auth put with curl using prefix (%s) (%v)", p, err)
 	}
 }
+
+func TestV3CurlCampaignNoTLS(t *testing.T) {
+	for _, p := range apiPrefix {
+		testCtl(t, testV3CurlCampaign, withApiPrefix(p), withCfg(configNoTLS))
+	}
+}
+
+func testV3CurlCampaign(cx ctlCtx) {
+	cdata, err := json.Marshal(&epb.CampaignRequest{
+		Name:  []byte("/election-prefix"),
+		Value: []byte("v1"),
+	})
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+	cargs := cURLPrefixArgs(cx.epc, "POST", cURLReq{
+		endpoint: path.Join(cx.apiPrefix, "/election/campaign"),
+		value:    string(cdata),
+	})
+	lines, err := spawnWithExpectLines(cargs, `"leader":{"name":"`)
+	if err != nil {
+		cx.t.Fatalf("failed post campaign request (%s) (%v)", cx.apiPrefix, err)
+	}
+	if len(lines) != 1 {
+		cx.t.Fatalf("len(lines) expected 1, got %+v", lines)
+	}
+
+	var cresp campaignResponse
+	if err = json.Unmarshal([]byte(lines[0]), &cresp); err != nil {
+		cx.t.Fatalf("failed to unmarshal campaign response %v", err)
+	}
+	ndata, err := base64.StdEncoding.DecodeString(cresp.Leader.Name)
+	if err != nil {
+		cx.t.Fatalf("failed to decode leader key %v", err)
+	}
+	kdata, err := base64.StdEncoding.DecodeString(cresp.Leader.Key)
+	if err != nil {
+		cx.t.Fatalf("failed to decode leader key %v", err)
+	}
+
+	rev, _ := strconv.ParseInt(cresp.Leader.Rev, 10, 64)
+	lease, _ := strconv.ParseInt(cresp.Leader.Lease, 10, 64)
+	pdata, err := json.Marshal(&epb.ProclaimRequest{
+		Leader: &epb.LeaderKey{
+			Name:  ndata,
+			Key:   kdata,
+			Rev:   rev,
+			Lease: lease,
+		},
+		Value: []byte("v2"),
+	})
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+	if err = cURLPost(cx.epc, cURLReq{
+		endpoint: path.Join(cx.apiPrefix, "/election/proclaim"),
+		value:    string(pdata),
+		expected: `"revision":`,
+	}); err != nil {
+		cx.t.Fatalf("failed post proclaim request (%s) (%v)", cx.apiPrefix, err)
+	}
+}
+
+func TestV3CurlProclaimMissiongLeaderKeyNoTLS(t *testing.T) {
+	for _, p := range apiPrefix {
+		testCtl(t, testV3CurlProclaimMissiongLeaderKey, withApiPrefix(p), withCfg(configNoTLS))
+	}
+}
+
+func testV3CurlProclaimMissiongLeaderKey(cx ctlCtx) {
+	pdata, err := json.Marshal(&epb.ProclaimRequest{Value: []byte("v2")})
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+	if err = cURLPost(cx.epc, cURLReq{
+		endpoint: path.Join(cx.apiPrefix, "/election/proclaim"),
+		value:    string(pdata),
+		expected: `{"error":"\"leader\" field must be provided","code":2}`,
+	}); err != nil {
+		cx.t.Fatalf("failed post proclaim request (%s) (%v)", cx.apiPrefix, err)
+	}
+}
+
+func TestV3CurlResignMissiongLeaderKeyNoTLS(t *testing.T) {
+	for _, p := range apiPrefix {
+		testCtl(t, testV3CurlResignMissiongLeaderKey, withApiPrefix(p), withCfg(configNoTLS))
+	}
+}
+
+func testV3CurlResignMissiongLeaderKey(cx ctlCtx) {
+	if err := cURLPost(cx.epc, cURLReq{
+		endpoint: path.Join(cx.apiPrefix, "/election/resign"),
+		value:    `{}`,
+		expected: `{"error":"\"leader\" field must be provided","code":2}`,
+	}); err != nil {
+		cx.t.Fatalf("failed post resign request (%s) (%v)", cx.apiPrefix, err)
+	}
+}
+
+// to manually decode; JSON marshals integer fields with
+// string types, so can't unmarshal with epb.CampaignResponse
+type campaignResponse struct {
+	Leader struct {
+		Name  string `json:"name,omitempty"`
+		Key   string `json:"key,omitempty"`
+		Rev   string `json:"rev,omitempty"`
+		Lease string `json:"lease,omitempty"`
+	} `json:"leader,omitempty"`
+}

etcdserver/api/v3election/election.go

@@ -16,12 +16,17 @@ package v3election
 
 import (
 	"context"
+	"errors"
 
 	"github.com/coreos/etcd/clientv3"
 	"github.com/coreos/etcd/clientv3/concurrency"
 	epb "github.com/coreos/etcd/etcdserver/api/v3election/v3electionpb"
 )
 
+// ErrMissingLeaderKey is returned when election API request
+// is missing the "leader" field.
+var ErrMissingLeaderKey = errors.New(`"leader" field must be provided`)
+
 type electionServer struct {
 	c *clientv3.Client
 }
@@ -51,6 +56,9 @@ func (es *electionServer) Campaign(ctx context.Context, req *epb.CampaignRequest
 }
 
 func (es *electionServer) Proclaim(ctx context.Context, req *epb.ProclaimRequest) (*epb.ProclaimResponse, error) {
+	if req.Leader == nil {
+		return nil, ErrMissingLeaderKey
+	}
 	s, err := es.session(ctx, req.Leader.Lease)
 	if err != nil {
 		return nil, err
@@ -98,6 +106,9 @@ func (es *electionServer) Leader(ctx context.Context, req *epb.LeaderRequest) (*
 }
 
 func (es *electionServer) Resign(ctx context.Context, req *epb.ResignRequest) (*epb.ResignResponse, error) {
+	if req.Leader == nil {
+		return nil, ErrMissingLeaderKey
+	}
 	s, err := es.session(ctx, req.Leader.Lease)
 	if err != nil {
 		return nil, err