From f26264aaca6d513828caa0fac5742ed62d3fef14 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Tue, 31 Oct 2017 15:53:10 -0400 Subject: [PATCH] [FAB-6841] configtx to errs pkg, improve test The configtx package was written before the pkg/errors package gained acceptance in the fabric codebase. Consequently, it contains many fmt.Errorf references. This CR updates the errors with the newer framework, including fixing capitalization, adds some additional unit tests, and generally cleans up configtx cruft. It improves test coverage from 76.2% to 87.3%. Change-Id: I834769ac44ba6d238f95a4a3e5f5d175c2c1f153 Signed-off-by: Jason Yellick --- common/configtx/compare.go | 14 +-- common/configtx/compare_test.go | 14 +-- common/configtx/configmap.go | 14 +-- common/configtx/configmap_test.go | 14 +-- common/configtx/update.go | 65 +++++----- common/configtx/update_test.go | 26 ++-- common/configtx/util.go | 14 +-- common/configtx/util_test.go | 111 ++++++++++++++++-- common/configtx/{manager.go => validator.go} | 78 ++++++------ .../{manager_test.go => validator_test.go} | 47 ++++++++ 10 files changed, 227 insertions(+), 170 deletions(-) rename common/configtx/{manager.go => validator.go} (68%) rename common/configtx/{manager_test.go => validator_test.go} (87%) diff --git a/common/configtx/compare.go b/common/configtx/compare.go index 736a6b4c183..c7586225cad 100644 --- a/common/configtx/compare.go +++ b/common/configtx/compare.go @@ -1,17 +1,7 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +SPDX-License-Identifier: Apache-2.0 */ package configtx diff --git a/common/configtx/compare_test.go b/common/configtx/compare_test.go index 58f23a1dec1..8a81868c2e2 100644 --- a/common/configtx/compare_test.go +++ b/common/configtx/compare_test.go @@ -1,17 +1,7 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +SPDX-License-Identifier: Apache-2.0 */ package configtx diff --git a/common/configtx/configmap.go b/common/configtx/configmap.go index d138cda0430..510bd041149 100644 --- a/common/configtx/configmap.go +++ b/common/configtx/configmap.go @@ -1,17 +1,7 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +SPDX-License-Identifier: Apache-2.0 */ package configtx diff --git a/common/configtx/configmap_test.go b/common/configtx/configmap_test.go index a48164ff1aa..793aad0feff 100644 --- a/common/configtx/configmap_test.go +++ b/common/configtx/configmap_test.go @@ -1,17 +1,7 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +SPDX-License-Identifier: Apache-2.0 */ package configtx diff --git a/common/configtx/update.go b/common/configtx/update.go index 1bada0a360a..e6405990d68 100644 --- a/common/configtx/update.go +++ b/common/configtx/update.go @@ -1,39 +1,30 @@ /* -Copyright IBM Corp. 2016-2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +SPDX-License-Identifier: Apache-2.0 */ package configtx import ( - "fmt" "strings" "github.com/hyperledger/fabric/common/policies" cb "github.com/hyperledger/fabric/protos/common" "github.com/hyperledger/fabric/protos/utils" + + "github.com/pkg/errors" ) -func (c *configSet) verifyReadSet(readSet map[string]comparable) error { +func (c *ValidatorImpl) verifyReadSet(readSet map[string]comparable) error { for key, value := range readSet { existing, ok := c.configMap[key] if !ok { - return fmt.Errorf("Existing config does not contain element for %s but was in the read set", key) + return errors.Errorf("existing config does not contain element for %s but was in the read set", key) } if existing.version() != value.version() { - return fmt.Errorf("Readset expected key %s at version %d, but got version %d", key, value.version(), existing.version()) + return errors.Errorf("readset expected key %s at version %d, but got version %d", key, value.version(), existing.version()) } } return nil @@ -49,7 +40,7 @@ func ComputeDeltaSet(readSet, writeSet map[string]comparable) map[string]compara } // If the key in the readset is a different version, we include it - // Error checking on the sanity of the update is done against the current config + // Error checking on the sanity of the update is done against the config result[key] = value } return result @@ -57,7 +48,7 @@ func ComputeDeltaSet(readSet, writeSet map[string]comparable) map[string]compara func validateModPolicy(modPolicy string) error { if modPolicy == "" { - return fmt.Errorf("mod_policy not set") + return errors.Errorf("mod_policy not set") } trimmed := modPolicy @@ -68,7 +59,7 @@ func validateModPolicy(modPolicy string) error { for i, pathElement := range strings.Split(trimmed, PathSeparator) { err := validateConfigID(pathElement) if err != nil { - return fmt.Errorf("path element at %d is invalid: %s", i, err) + return errors.Wrapf(err, "path element at %d is invalid", i) } } return nil @@ -77,36 +68,36 @@ func validateModPolicy(modPolicy string) error { func (cm *ValidatorImpl) verifyDeltaSet(deltaSet map[string]comparable, signedData []*cb.SignedData) error { if len(deltaSet) == 0 { - return fmt.Errorf("Delta set was empty. Update would have no effect.") + return errors.Errorf("delta set was empty -- update would have no effect") } for key, value := range deltaSet { logger.Debugf("Processing change to key: %s", key) if err := validateModPolicy(value.modPolicy()); err != nil { - return fmt.Errorf("invalid mod_policy for element %s: %s", key, err) + return errors.Wrapf(err, "invalid mod_policy for element %s", key) } - existing, ok := cm.current.configMap[key] + existing, ok := cm.configMap[key] if !ok { if value.version() != 0 { - return fmt.Errorf("Attempted to set key %s to version %d, but key does not exist", key, value.version()) + return errors.Errorf("attempted to set key %s to version %d, but key does not exist", key, value.version()) } else { continue } } if value.version() != existing.version()+1 { - return fmt.Errorf("Attempt to set key %s to version %d, but key is at version %d", key, value.version(), existing.version()) + return errors.Errorf("attempt to set key %s to version %d, but key is at version %d", key, value.version(), existing.version()) } policy, ok := cm.policyForItem(existing) if !ok { - return fmt.Errorf("Unexpected missing policy %s for item %s", existing.modPolicy(), key) + return errors.Errorf("unexpected missing policy %s for item %s", existing.modPolicy(), key) } // Ensure the policy is satisfied if err := policy.Evaluate(signedData); err != nil { - return fmt.Errorf("Policy for %s not satisfied: %s", key, err) + return errors.Wrapf(err, "policy for %s not satisfied", key) } } return nil @@ -115,7 +106,7 @@ func (cm *ValidatorImpl) verifyDeltaSet(deltaSet map[string]comparable, signedDa func verifyFullProposedConfig(writeSet, fullProposedConfig map[string]comparable) error { for key := range writeSet { if _, ok := fullProposedConfig[key]; !ok { - return fmt.Errorf("Writeset contained key %s which did not appear in proposed config", key) + return errors.Errorf("writeset contained key %s which did not appear in proposed config", key) } } return nil @@ -125,7 +116,7 @@ func verifyFullProposedConfig(writeSet, fullProposedConfig map[string]comparable // it returns a map of the modified config func (cm *ValidatorImpl) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelope) (map[string]comparable, error) { if configUpdateEnv == nil { - return nil, fmt.Errorf("Cannot process nil ConfigUpdateEnvelope") + return nil, errors.Errorf("cannot process nil ConfigUpdateEnvelope") } configUpdate, err := UnmarshalConfigUpdate(configUpdateEnv.ConfigUpdate) @@ -133,22 +124,22 @@ func (cm *ValidatorImpl) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop return nil, err } - if configUpdate.ChannelId != cm.current.channelID { - return nil, fmt.Errorf("Update not for correct channel: %s for %s", configUpdate.ChannelId, cm.current.channelID) + if configUpdate.ChannelId != cm.channelID { + return nil, errors.Errorf("Update not for correct channel: %s for %s", configUpdate.ChannelId, cm.channelID) } readSet, err := MapConfig(configUpdate.ReadSet, cm.namespace) if err != nil { - return nil, fmt.Errorf("Error mapping ReadSet: %s", err) + return nil, errors.Wrapf(err, "error mapping ReadSet") } - err = cm.current.verifyReadSet(readSet) + err = cm.verifyReadSet(readSet) if err != nil { - return nil, fmt.Errorf("Error validating ReadSet: %s", err) + return nil, errors.Wrapf(err, "error validating ReadSet") } writeSet, err := MapConfig(configUpdate.WriteSet, cm.namespace) if err != nil { - return nil, fmt.Errorf("Error mapping WriteSet: %s", err) + return nil, errors.Wrapf(err, "error mapping WriteSet") } deltaSet := ComputeDeltaSet(readSet, writeSet) @@ -158,12 +149,12 @@ func (cm *ValidatorImpl) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop } if err = cm.verifyDeltaSet(deltaSet, signedData); err != nil { - return nil, fmt.Errorf("Error validating DeltaSet: %s", err) + return nil, errors.Wrapf(err, "error validating DeltaSet") } fullProposedConfig := cm.computeUpdateResult(deltaSet) if err := verifyFullProposedConfig(writeSet, fullProposedConfig); err != nil { - return nil, fmt.Errorf("Full config did not verify: %s", err) + return nil, errors.Wrapf(err, "full config did not verify") } return fullProposedConfig, nil @@ -203,7 +194,7 @@ func (cm *ValidatorImpl) policyForItem(item comparable) (policies.Policy, bool) // computeUpdateResult takes a configMap generated by an update and produces a new configMap overlaying it onto the old config func (cm *ValidatorImpl) computeUpdateResult(updatedConfig map[string]comparable) map[string]comparable { newConfigMap := make(map[string]comparable) - for key, value := range cm.current.configMap { + for key, value := range cm.configMap { newConfigMap[key] = value } diff --git a/common/configtx/update_test.go b/common/configtx/update_test.go index 0f3dfa4b57d..8b756ae43a6 100644 --- a/common/configtx/update_test.go +++ b/common/configtx/update_test.go @@ -1,17 +1,7 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +SPDX-License-Identifier: Apache-2.0 */ package configtx @@ -27,7 +17,7 @@ import ( ) func TestReadSetNotPresent(t *testing.T) { - cm := &configSet{ + cm := &ValidatorImpl{ configMap: make(map[string]comparable), } @@ -42,7 +32,7 @@ func TestReadSetNotPresent(t *testing.T) { } func TestReadSetBackVersioned(t *testing.T) { - cm := &configSet{ + cm := &ValidatorImpl{ configMap: make(map[string]comparable), } @@ -76,12 +66,10 @@ func TestVerifyDeltaSet(t *testing.T) { pm: &mockpolicies.Manager{ Policy: &mockpolicies.Policy{}, }, - current: &configSet{ - configMap: make(map[string]comparable), - }, + configMap: make(map[string]comparable), } - cm.current.configMap["foo"] = comparable{path: []string{"foo"}} + cm.configMap["foo"] = comparable{path: []string{"foo"}} t.Run("Green path", func(t *testing.T) { deltaSet := make(map[string]comparable) @@ -127,7 +115,7 @@ func TestVerifyDeltaSet(t *testing.T) { t.Run("Empty delta set", func(t *testing.T) { err := (&ValidatorImpl{}).verifyDeltaSet(map[string]comparable{}, nil) assert.Error(t, err, "Empty delta set should be rejected") - assert.Contains(t, err.Error(), "Delta set was empty. Update would have no effect.") + assert.Contains(t, err.Error(), "delta set was empty -- update would have no effect") }) } diff --git a/common/configtx/util.go b/common/configtx/util.go index eca9f4266b9..e51a2e767d5 100644 --- a/common/configtx/util.go +++ b/common/configtx/util.go @@ -1,17 +1,7 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +SPDX-License-Identifier: Apache-2.0 */ package configtx diff --git a/common/configtx/util_test.go b/common/configtx/util_test.go index 7a7c7324edc..dea7ee5f9e3 100644 --- a/common/configtx/util_test.go +++ b/common/configtx/util_test.go @@ -1,17 +1,7 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +SPDX-License-Identifier: Apache-2.0 */ package configtx @@ -19,6 +9,11 @@ package configtx import ( "math/rand" "testing" + + cb "github.com/hyperledger/fabric/protos/common" + "github.com/hyperledger/fabric/protos/utils" + + "github.com/stretchr/testify/assert" ) // TestValidConfigID checks that the constraints on chain IDs are enforced properly @@ -120,3 +115,95 @@ func randomAlphaString(size int) string { } return string(output) } + +func TestUnmarshalConfig(t *testing.T) { + goodConfigBytes := utils.MarshalOrPanic(&cb.Config{}) + badConfigBytes := []byte("garbage") + + t.Run("GoodUnmarshalNormal", func(t *testing.T) { + _, err := UnmarshalConfig(goodConfigBytes) + assert.NoError(t, err) + }) + + t.Run("GoodUnmarshalOrpanic", func(t *testing.T) { + assert.NotPanics(t, func() { UnmarshalConfigOrPanic(goodConfigBytes) }) + }) + + t.Run("BadUnmarshalNormal", func(t *testing.T) { + _, err := UnmarshalConfig(badConfigBytes) + assert.Error(t, err) + }) + + t.Run("BadUnmarshalOrpanic", func(t *testing.T) { + assert.Panics(t, func() { UnmarshalConfigOrPanic(badConfigBytes) }) + }) +} + +func TestUnmarshalConfigEnvelope(t *testing.T) { + goodConfigEnvelopeBytes := utils.MarshalOrPanic(&cb.ConfigEnvelope{}) + badConfigEnvelopeBytes := []byte("garbage") + + t.Run("GoodUnmarshalNormal", func(t *testing.T) { + _, err := UnmarshalConfigEnvelope(goodConfigEnvelopeBytes) + assert.NoError(t, err) + }) + + t.Run("GoodUnmarshalOrpanic", func(t *testing.T) { + assert.NotPanics(t, func() { UnmarshalConfigEnvelopeOrPanic(goodConfigEnvelopeBytes) }) + }) + + t.Run("BadUnmarshalNormal", func(t *testing.T) { + _, err := UnmarshalConfigEnvelope(badConfigEnvelopeBytes) + assert.Error(t, err) + }) + + t.Run("BadUnmarshalOrpanic", func(t *testing.T) { + assert.Panics(t, func() { UnmarshalConfigEnvelopeOrPanic(badConfigEnvelopeBytes) }) + }) +} + +func TestUnmarshalConfigUpdate(t *testing.T) { + goodConfigUpdateBytes := utils.MarshalOrPanic(&cb.ConfigUpdate{}) + badConfigUpdateBytes := []byte("garbage") + + t.Run("GoodUnmarshalNormal", func(t *testing.T) { + _, err := UnmarshalConfigUpdate(goodConfigUpdateBytes) + assert.NoError(t, err) + }) + + t.Run("GoodUnmarshalOrpanic", func(t *testing.T) { + assert.NotPanics(t, func() { UnmarshalConfigUpdateOrPanic(goodConfigUpdateBytes) }) + }) + + t.Run("BadUnmarshalNormal", func(t *testing.T) { + _, err := UnmarshalConfigUpdate(badConfigUpdateBytes) + assert.Error(t, err) + }) + + t.Run("BadUnmarshalOrpanic", func(t *testing.T) { + assert.Panics(t, func() { UnmarshalConfigUpdateOrPanic(badConfigUpdateBytes) }) + }) +} + +func TestUnmarshalConfigUpdateEnvelope(t *testing.T) { + goodConfigUpdateEnvelopeBytes := utils.MarshalOrPanic(&cb.ConfigUpdateEnvelope{}) + badConfigUpdateEnvelopeBytes := []byte("garbage") + + t.Run("GoodUnmarshalNormal", func(t *testing.T) { + _, err := UnmarshalConfigUpdateEnvelope(goodConfigUpdateEnvelopeBytes) + assert.NoError(t, err) + }) + + t.Run("GoodUnmarshalOrpanic", func(t *testing.T) { + assert.NotPanics(t, func() { UnmarshalConfigUpdateEnvelopeOrPanic(goodConfigUpdateEnvelopeBytes) }) + }) + + t.Run("BadUnmarshalNormal", func(t *testing.T) { + _, err := UnmarshalConfigUpdateEnvelope(badConfigUpdateEnvelopeBytes) + assert.Error(t, err) + }) + + t.Run("BadUnmarshalOrpanic", func(t *testing.T) { + assert.Panics(t, func() { UnmarshalConfigUpdateEnvelopeOrPanic(badConfigUpdateEnvelopeBytes) }) + }) +} diff --git a/common/configtx/manager.go b/common/configtx/validator.go similarity index 68% rename from common/configtx/manager.go rename to common/configtx/validator.go index 03f84da23ea..874d2354d5e 100644 --- a/common/configtx/manager.go +++ b/common/configtx/validator.go @@ -7,7 +7,6 @@ SPDX-License-Identifier: Apache-2.0 package configtx import ( - "fmt" "regexp" "github.com/hyperledger/fabric/common/flogging" @@ -15,6 +14,7 @@ import ( cb "github.com/hyperledger/fabric/protos/common" "github.com/golang/protobuf/proto" + "github.com/pkg/errors" ) var logger = flogging.MustGetLogger("common/configtx") @@ -30,17 +30,13 @@ var ( } ) -type configSet struct { +type ValidatorImpl struct { channelID string sequence uint64 configMap map[string]comparable configProto *cb.Config -} - -type ValidatorImpl struct { - namespace string - pm policies.Manager - current *configSet + namespace string + pm policies.Manager } // validateConfigID makes sure that the config element names (ie map key of @@ -52,19 +48,19 @@ func validateConfigID(configID string) error { re, _ := regexp.Compile(configAllowedChars) // Length if len(configID) <= 0 { - return fmt.Errorf("config ID illegal, cannot be empty") + return errors.New("config ID illegal, cannot be empty") } if len(configID) > maxLength { - return fmt.Errorf("config ID illegal, cannot be longer than %d", maxLength) + return errors.Errorf("config ID illegal, cannot be longer than %d", maxLength) } // Illegal name if _, ok := illegalNames[configID]; ok { - return fmt.Errorf("name '%s' for config ID is not allowed", configID) + return errors.Errorf("name '%s' for config ID is not allowed", configID) } // Illegal characters matched := re.FindString(configID) if len(matched) != len(configID) { - return fmt.Errorf("config ID '%s' contains illegal characters", configID) + return errors.Errorf("config ID '%s' contains illegal characters", configID) } return nil @@ -84,16 +80,16 @@ func validateChannelID(channelID string) error { re, _ := regexp.Compile(channelAllowedChars) // Length if len(channelID) <= 0 { - return fmt.Errorf("channel ID illegal, cannot be empty") + return errors.Errorf("channel ID illegal, cannot be empty") } if len(channelID) > maxLength { - return fmt.Errorf("channel ID illegal, cannot be longer than %d", maxLength) + return errors.Errorf("channel ID illegal, cannot be longer than %d", maxLength) } // Illegal characters matched := re.FindString(channelID) if len(matched) != len(channelID) { - return fmt.Errorf("channel ID '%s' contains illegal characters", channelID) + return errors.Errorf("channel ID '%s' contains illegal characters", channelID) } return nil @@ -101,31 +97,29 @@ func validateChannelID(channelID string) error { func NewValidatorImpl(channelID string, config *cb.Config, namespace string, pm policies.Manager) (*ValidatorImpl, error) { if config == nil { - return nil, fmt.Errorf("Nil config envelope Config") + return nil, errors.Errorf("nil config parameter") } if config.ChannelGroup == nil { - return nil, fmt.Errorf("nil channel group") + return nil, errors.Errorf("nil channel group") } if err := validateChannelID(channelID); err != nil { - return nil, fmt.Errorf("Bad channel id: %s", err) + return nil, errors.Errorf("bad channel ID: %s", err) } configMap, err := MapConfig(config.ChannelGroup, namespace) if err != nil { - return nil, fmt.Errorf("Error converting config to map: %s", err) + return nil, errors.Errorf("error converting config to map: %s", err) } return &ValidatorImpl{ - namespace: namespace, - pm: pm, - current: &configSet{ - sequence: config.Sequence, - configMap: configMap, - channelID: channelID, - configProto: config, - }, + namespace: namespace, + pm: pm, + sequence: config.Sequence, + configMap: configMap, + channelID: channelID, + configProto: config, }, nil } @@ -138,22 +132,22 @@ func (cm *ValidatorImpl) ProposeConfigUpdate(configtx *cb.Envelope) (*cb.ConfigE func (cm *ValidatorImpl) proposeConfigUpdate(configtx *cb.Envelope) (*cb.ConfigEnvelope, error) { configUpdateEnv, err := envelopeToConfigUpdate(configtx) if err != nil { - return nil, fmt.Errorf("Error converting envelope to config update: %s", err) + return nil, errors.Errorf("error converting envelope to config update: %s", err) } configMap, err := cm.authorizeUpdate(configUpdateEnv) if err != nil { - return nil, fmt.Errorf("Error authorizing update: %s", err) + return nil, errors.Errorf("error authorizing update: %s", err) } channelGroup, err := configMapToConfig(configMap, cm.namespace) if err != nil { - return nil, fmt.Errorf("Could not turn configMap back to channelGroup: %s", err) + return nil, errors.Errorf("could not turn configMap back to channelGroup: %s", err) } return &cb.ConfigEnvelope{ Config: &cb.Config{ - Sequence: cm.current.sequence + 1, + Sequence: cm.sequence + 1, ChannelGroup: channelGroup, }, LastUpdate: configtx, @@ -163,15 +157,15 @@ func (cm *ValidatorImpl) proposeConfigUpdate(configtx *cb.Envelope) (*cb.ConfigE // Validate simulates applying a ConfigEnvelope to become the new config func (cm *ValidatorImpl) Validate(configEnv *cb.ConfigEnvelope) error { if configEnv == nil { - return fmt.Errorf("config envelope is nil") + return errors.Errorf("config envelope is nil") } if configEnv.Config == nil { - return fmt.Errorf("config envelope has nil config") + return errors.Errorf("config envelope has nil config") } - if configEnv.Config.Sequence != cm.current.sequence+1 { - return fmt.Errorf("config currently at sequence %d, cannot validate config at sequence %d", cm.current.sequence, configEnv.Config.Sequence) + if configEnv.Config.Sequence != cm.sequence+1 { + return errors.Errorf("config currently at sequence %d, cannot validate config at sequence %d", cm.sequence, configEnv.Config.Sequence) } configUpdateEnv, err := envelopeToConfigUpdate(configEnv.LastUpdate) @@ -186,12 +180,12 @@ func (cm *ValidatorImpl) Validate(configEnv *cb.ConfigEnvelope) error { channelGroup, err := configMapToConfig(configMap, cm.namespace) if err != nil { - return fmt.Errorf("Could not turn configMap back to channelGroup: %s", err) + return errors.Errorf("could not turn configMap back to channelGroup: %s", err) } // reflect.Equal will not work here, because it considers nil and empty maps as different if !proto.Equal(channelGroup, configEnv.Config.ChannelGroup) { - return fmt.Errorf("ConfigEnvelope LastUpdate did not produce the supplied config result") + return errors.Errorf("ConfigEnvelope LastUpdate did not produce the supplied config result") } return nil @@ -199,15 +193,15 @@ func (cm *ValidatorImpl) Validate(configEnv *cb.ConfigEnvelope) error { // ChainID retrieves the chain ID associated with this manager func (cm *ValidatorImpl) ChainID() string { - return cm.current.channelID + return cm.channelID } -// Sequence returns the current sequence number of the config +// Sequence returns the sequence number of the config func (cm *ValidatorImpl) Sequence() uint64 { - return cm.current.sequence + return cm.sequence } -// ConfigEnvelope returns the current config envelope +// ConfigEnvelope returns the config proto which initialized this Validator func (cm *ValidatorImpl) ConfigProto() *cb.Config { - return cm.current.configProto + return cm.configProto } diff --git a/common/configtx/manager_test.go b/common/configtx/validator_test.go similarity index 87% rename from common/configtx/manager_test.go rename to common/configtx/validator_test.go index 7c25e482618..75b3710c758 100644 --- a/common/configtx/manager_test.go +++ b/common/configtx/validator_test.go @@ -363,3 +363,50 @@ func TestInvalidProposal(t *testing.T) { t.Error("Should have errored proposing config because the handler rejected it") } } + +func TestValidateErrors(t *testing.T) { + t.Run("TestNilConfigEnv", func(t *testing.T) { + err := (&ValidatorImpl{}).Validate(nil) + assert.Error(t, err) + assert.Regexp(t, "config envelope is nil", err.Error()) + }) + + t.Run("TestNilConfig", func(t *testing.T) { + err := (&ValidatorImpl{}).Validate(&cb.ConfigEnvelope{}) + assert.Error(t, err) + assert.Regexp(t, "config envelope has nil config", err.Error()) + }) + + t.Run("TestSequenceSkip", func(t *testing.T) { + err := (&ValidatorImpl{}).Validate(&cb.ConfigEnvelope{ + Config: &cb.Config{ + Sequence: 2, + }, + }) + assert.Error(t, err) + assert.Regexp(t, "config currently at sequence 0", err.Error()) + }) +} + +func TestConstructionErrors(t *testing.T) { + t.Run("NilConfig", func(t *testing.T) { + v, err := NewValidatorImpl("test", nil, "foonamespace", &mockpolicies.Manager{}) + assert.Nil(t, v) + assert.Error(t, err) + assert.Regexp(t, "nil config parameter", err.Error()) + }) + + t.Run("NilChannelGroup", func(t *testing.T) { + v, err := NewValidatorImpl("test", &cb.Config{}, "foonamespace", &mockpolicies.Manager{}) + assert.Nil(t, v) + assert.Error(t, err) + assert.Regexp(t, "nil channel group", err.Error()) + }) + + t.Run("BadChannelID", func(t *testing.T) { + v, err := NewValidatorImpl("*&$#@*&@$#*&", &cb.Config{ChannelGroup: &cb.ConfigGroup{}}, "foonamespace", &mockpolicies.Manager{}) + assert.Nil(t, v) + assert.Error(t, err) + assert.Regexp(t, "bad channel ID", err.Error()) + }) +}