Fix nil dereference in etcdraft config parsing (#724)

The etcdraft config parsing code currently checks that the consensus
metadata is not nil, but it fails to check that the options are not nil.
This commit simply adds the additional check and backfills some testing
around this validation function.

Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

orderer/consensus/etcdraft/util.go

@@ -385,6 +385,10 @@ func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata) error {
 		return errors.Errorf("nil Raft config metadata")
 	}
 
+	if metadata.Options == nil {
+		return errors.Errorf("nil Raft config metadata options")
+	}
+
 	if metadata.Options.HeartbeatTick == 0 ||
 		metadata.Options.ElectionTick == 0 ||
 		metadata.Options.MaxInflightBlocks == 0 {
@@ -396,7 +400,7 @@ func CheckConfigMetadata(metadata *etcdraft.ConfigMetadata) error {
 	// check Raft options
 	if metadata.Options.ElectionTick <= metadata.Options.HeartbeatTick {
 		return errors.Errorf("ElectionTick (%d) must be greater than HeartbeatTick (%d)",
-			metadata.Options.HeartbeatTick, metadata.Options.HeartbeatTick)
+			metadata.Options.ElectionTick, metadata.Options.HeartbeatTick)
 	}
 
 	if d, err := time.ParseDuration(metadata.Options.TickInterval); err != nil {

orderer/consensus/etcdraft/util_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/hyperledger/fabric/orderer/consensus"
 	"github.com/hyperledger/fabric/orderer/mocks/common/multichannel"
 	"github.com/hyperledger/fabric/protos/common"
+	"github.com/hyperledger/fabric/protos/orderer/etcdraft"
 	"github.com/hyperledger/fabric/protos/utils"
 	"github.com/onsi/gomega"
 	"github.com/pkg/errors"
@@ -34,6 +35,31 @@ import (
 	"go.uber.org/zap/zapcore"
 )
 
+func TestCheckConfigMetadata(t *testing.T) {
+	tests := []struct {
+		metadata *etcdraft.ConfigMetadata
+		err      string
+	}{
+		{nil, "nil Raft config metadata"},
+		{&etcdraft.ConfigMetadata{}, "nil Raft config metadata options"},
+		{&etcdraft.ConfigMetadata{Options: &etcdraft.Options{}}, "none of HeartbeatTick (0), ElectionTick (0) and MaxInflightBlocks (0) can be zero"},
+		{&etcdraft.ConfigMetadata{Options: &etcdraft.Options{HeartbeatTick: 2, ElectionTick: 1, MaxInflightBlocks: 1}}, "ElectionTick (1) must be greater than HeartbeatTick (2)"},
+		{&etcdraft.ConfigMetadata{Options: &etcdraft.Options{HeartbeatTick: 1, ElectionTick: 2, MaxInflightBlocks: 1, TickInterval: "q"}}, "failed to parse TickInterval (q) to time duration: time: invalid duration q"},
+		{&etcdraft.ConfigMetadata{Options: &etcdraft.Options{HeartbeatTick: 1, ElectionTick: 2, MaxInflightBlocks: 1, TickInterval: "0"}}, "TickInterval cannot be zero"},
+		{&etcdraft.ConfigMetadata{Options: &etcdraft.Options{HeartbeatTick: 1, ElectionTick: 2, MaxInflightBlocks: 1, TickInterval: "1s"}}, "empty consenter set"},
+	}
+
+	for _, tc := range tests {
+		err := CheckConfigMetadata(tc.metadata)
+		if tc.err == "" {
+			assert.NoError(t, err)
+		} else {
+			assert.EqualError(t, err, tc.err)
+		}
+	}
+
+}
+
 func TestIsConsenterOfChannel(t *testing.T) {
 	certInsideConfigBlock, err := base64.StdEncoding.DecodeString("LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUNmekNDQWlhZ0F3SUJBZ0l" +
 		"SQUo4bjFLYTVzS1ZaTXRMTHJ1dldERDB3Q2dZSUtvWkl6ajBFQXdJd2JERUwKTUFrR0ExVUVCaE1DVlZNeEV6QVJCZ05WQkFnVENrTmhiR" +