Skip to content

Commit

Permalink
[FAB-6918] Fix linter errors in configtx package
Browse files Browse the repository at this point in the history 
This is the closing CR in a series to clean up the crufty configtx
package.  It fixes the linter errors, including inconsitent (and poor
naming) of the receiver, exported methods which did not need to be
exported, and key names with the wrong plurality.

Change-Id: I02424a84145dad8a702048a267daddc911dc1e16
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>
  • Loading branch information
@mastersingh24
Jason Yellick authored and mastersingh24 committed Nov 18, 2017
1 parent 7996cc7 commit 0a7c03b
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 126 deletions.
42 changes: 21 additions & 21 deletions common/configtx/configmap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,20 @@ import (
)

const (
GroupPrefix = "[Groups] "
ValuePrefix = "[Values] "
PolicyPrefix = "[Policy] " // The plurarility doesn't match, but, it makes the logs much easier being the same length as "Groups" and "Values"
groupPrefix = "[Group] "
valuePrefix = "[Value] "
policyPrefix = "[Policy] "

PathSeparator = "/"
pathSeparator = "/"

// Hacky fix constants, used in recurseConfigMap
hackyFixOrdererCapabilities = "[Values] /Channel/Orderer/Capabilities"
hackyFixOrdererCapabilities = "[Value] /Channel/Orderer/Capabilities"
hackyFixNewModPolicy = "Admins"
)

// MapConfig is intended to be called outside this file
// mapConfig is intended to be called outside this file
// it takes a ConfigGroup and generates a map of fqPath to comparables (or error on invalid keys)
func MapConfig(channelGroup *cb.ConfigGroup, rootGroupKey string) (map[string]comparable, error) {
func mapConfig(channelGroup *cb.ConfigGroup, rootGroupKey string) (map[string]comparable, error) {
result := make(map[string]comparable)
if channelGroup != nil {
err := recurseConfig(result, []string{rootGroupKey}, channelGroup)
Expand All @@ -40,27 +40,27 @@ func MapConfig(channelGroup *cb.ConfigGroup, rootGroupKey string) (map[string]co
return result, nil
}

// addToMap is used only internally by MapConfig
// addToMap is used only internally by mapConfig
func addToMap(cg comparable, result map[string]comparable) error {
var fqPath string

switch {
case cg.ConfigGroup != nil:
fqPath = GroupPrefix
fqPath = groupPrefix
case cg.ConfigValue != nil:
fqPath = ValuePrefix
fqPath = valuePrefix
case cg.ConfigPolicy != nil:
fqPath = PolicyPrefix
fqPath = policyPrefix
}

if err := validateConfigID(cg.key); err != nil {
return fmt.Errorf("Illegal characters in key: %s", fqPath)
}

if len(cg.path) == 0 {
fqPath += PathSeparator + cg.key
fqPath += pathSeparator + cg.key
} else {
fqPath += PathSeparator + strings.Join(cg.path, PathSeparator) + PathSeparator + cg.key
fqPath += pathSeparator + strings.Join(cg.path, pathSeparator) + pathSeparator + cg.key
}

logger.Debugf("Adding to config map: %s", fqPath)
Expand All @@ -70,7 +70,7 @@ func addToMap(cg comparable, result map[string]comparable) error {
return nil
}

// recurseConfig is used only internally by MapConfig
// recurseConfig is used only internally by mapConfig
func recurseConfig(result map[string]comparable, path []string, group *cb.ConfigGroup) error {
if err := addToMap(comparable{key: path[len(path)-1], path: path[:len(path)-1], ConfigGroup: group}, result); err != nil {
return err
Expand Down Expand Up @@ -103,14 +103,14 @@ func recurseConfig(result map[string]comparable, path []string, group *cb.Config
// configMapToConfig is intended to be called from outside this file
// It takes a configMap and converts it back into a *cb.ConfigGroup structure
func configMapToConfig(configMap map[string]comparable, rootGroupKey string) (*cb.ConfigGroup, error) {
rootPath := PathSeparator + rootGroupKey
rootPath := pathSeparator + rootGroupKey
return recurseConfigMap(rootPath, configMap)
}

// recurseConfigMap is used only internally by configMapToConfig
// Note, this function no longer mutates the cb.Config* entries within configMap
func recurseConfigMap(path string, configMap map[string]comparable) (*cb.ConfigGroup, error) {
groupPath := GroupPrefix + path
groupPath := groupPrefix + path
group, ok := configMap[groupPath]
if !ok {
return nil, fmt.Errorf("Missing group at path: %s", groupPath)
Expand All @@ -124,15 +124,15 @@ func recurseConfigMap(path string, configMap map[string]comparable) (*cb.ConfigG
proto.Merge(newConfigGroup, group.ConfigGroup)

for key := range group.Groups {
updatedGroup, err := recurseConfigMap(path+PathSeparator+key, configMap)
updatedGroup, err := recurseConfigMap(path+pathSeparator+key, configMap)
if err != nil {
return nil, err
}
newConfigGroup.Groups[key] = updatedGroup
}

for key := range group.Values {
valuePath := ValuePrefix + path + PathSeparator + key
valuePath := valuePrefix + path + pathSeparator + key
value, ok := configMap[valuePath]
if !ok {
return nil, fmt.Errorf("Missing value at path: %s", valuePath)
Expand All @@ -144,7 +144,7 @@ func recurseConfigMap(path string, configMap map[string]comparable) (*cb.ConfigG
}

for key := range group.Policies {
policyPath := PolicyPrefix + path + PathSeparator + key
policyPath := policyPrefix + path + pathSeparator + key
policy, ok := configMap[policyPath]
if !ok {
return nil, fmt.Errorf("Missing policy at path: %s", policyPath)
Expand Down Expand Up @@ -172,14 +172,14 @@ func recurseConfigMap(path string, configMap map[string]comparable) (*cb.ConfigG

for key, value := range newConfigGroup.Values {
if value.ModPolicy == "" {
logger.Debugf("Performing upgrade of value %s empty mod_policy", ValuePrefix+path+PathSeparator+key)
logger.Debugf("Performing upgrade of value %s empty mod_policy", valuePrefix+path+pathSeparator+key)
value.ModPolicy = hackyFixNewModPolicy
}
}

for key, policy := range newConfigGroup.Policies {
if policy.ModPolicy == "" {
logger.Debugf("Performing upgrade of policy %s empty mod_policy", PolicyPrefix+path+PathSeparator+key)
logger.Debugf("Performing upgrade of policy %s empty mod_policy", policyPrefix+path+pathSeparator+key)

policy.ModPolicy = hackyFixNewModPolicy
}
Expand Down
26 changes: 13 additions & 13 deletions common/configtx/configmap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ func TestConfigMapMultiGroup(t *testing.T) {
config.Groups["0"].Groups["1"].Groups["2.2"] = cb.NewConfigGroup()
config.Groups["0"].Groups["1"].Groups["2.2"].Values["Value"] = &cb.ConfigValue{}

confMap, err := MapConfig(config, "Channel")
confMap, err := mapConfig(config, "Channel")
assert.NoError(t, err)
assert.Equal(t, []string{"Channel", "0", "1", "2.1"}, confMap["[Values] /Channel/0/1/2.1/Value"].path)
assert.Equal(t, []string{"Channel", "0", "1", "2.2"}, confMap["[Values] /Channel/0/1/2.2/Value"].path)
assert.Equal(t, []string{"Channel", "0", "1", "2.1"}, confMap["[Value] /Channel/0/1/2.1/Value"].path)
assert.Equal(t, []string{"Channel", "0", "1", "2.2"}, confMap["[Value] /Channel/0/1/2.2/Value"].path)
}

func TestConfigMap(t *testing.T) {
Expand All @@ -48,25 +48,25 @@ func TestConfigMap(t *testing.T) {
config.Groups["0DeepGroup"].Groups["1DeepGroup"] = cb.NewConfigGroup()
config.Groups["0DeepGroup"].Groups["1DeepGroup"].Values["2DeepValue"] = &cb.ConfigValue{}

confMap, err := MapConfig(config, "Channel")
confMap, err := mapConfig(config, "Channel")
assert.NoError(t, err, "Should not have errored building map")

assert.Len(t, confMap, 7, "There should be 7 entries in the config map")

assert.Equal(t, comparable{key: "Channel", path: []string{}, ConfigGroup: config},
confMap["[Groups] /Channel"])
confMap["[Group] /Channel"])
assert.Equal(t, comparable{key: "0DeepGroup", path: []string{"Channel"}, ConfigGroup: config.Groups["0DeepGroup"]},
confMap["[Groups] /Channel/0DeepGroup"])
confMap["[Group] /Channel/0DeepGroup"])
assert.Equal(t, comparable{key: "0DeepValue1", path: []string{"Channel"}, ConfigValue: config.Values["0DeepValue1"]},
confMap["[Values] /Channel/0DeepValue1"])
confMap["[Value] /Channel/0DeepValue1"])
assert.Equal(t, comparable{key: "0DeepValue2", path: []string{"Channel"}, ConfigValue: config.Values["0DeepValue2"]},
confMap["[Values] /Channel/0DeepValue2"])
confMap["[Value] /Channel/0DeepValue2"])
assert.Equal(t, comparable{key: "1DeepPolicy", path: []string{"Channel", "0DeepGroup"}, ConfigPolicy: config.Groups["0DeepGroup"].Policies["1DeepPolicy"]},
confMap["[Policy] /Channel/0DeepGroup/1DeepPolicy"])
assert.Equal(t, comparable{key: "1DeepGroup", path: []string{"Channel", "0DeepGroup"}, ConfigGroup: config.Groups["0DeepGroup"].Groups["1DeepGroup"]},
confMap["[Groups] /Channel/0DeepGroup/1DeepGroup"])
confMap["[Group] /Channel/0DeepGroup/1DeepGroup"])
assert.Equal(t, comparable{key: "2DeepValue", path: []string{"Channel", "0DeepGroup", "1DeepGroup"}, ConfigValue: config.Groups["0DeepGroup"].Groups["1DeepGroup"].Values["2DeepValue"]},
confMap["[Values] /Channel/0DeepGroup/1DeepGroup/2DeepValue"])
confMap["[Value] /Channel/0DeepGroup/1DeepGroup/2DeepValue"])
}

func TestMapConfigBack(t *testing.T) {
Expand All @@ -78,7 +78,7 @@ func TestMapConfigBack(t *testing.T) {
config.Groups["0DeepGroup"].Groups["1DeepGroup"] = cb.NewConfigGroup()
config.Groups["0DeepGroup"].Groups["1DeepGroup"].Values["2DeepValue"] = &cb.ConfigValue{}

confMap, err := MapConfig(config, "Channel")
confMap, err := mapConfig(config, "Channel")
assert.NoError(t, err, "Should not have errored building map")

newConfig, err := configMapToConfig(confMap, "Channel")
Expand All @@ -90,7 +90,7 @@ func TestMapConfigBack(t *testing.T) {
assert.NotEqual(t, config, newConfig, "Mutating the new config should not mutate the existing config")
}

func TestHackInMapConfigBack(t *testing.T) {
func TestHackInmapConfigBack(t *testing.T) {
config := cb.NewConfigGroup()
config.Values["ChannelValue1"] = &cb.ConfigValue{}
config.Values["ChannelValue2"] = &cb.ConfigValue{}
Expand All @@ -103,7 +103,7 @@ func TestHackInMapConfigBack(t *testing.T) {
config.Groups["Application"].Policies["ApplicationPolicy"] = &cb.ConfigPolicy{}
config.Groups["Application"].Policies["ApplicationValue"] = &cb.ConfigPolicy{}

confMap, err := MapConfig(config, "Channel")
confMap, err := mapConfig(config, "Channel")
assert.NoError(t, err, "Should not have errored building map")

newConfig, err := configMapToConfig(confMap, "Channel")
Expand Down
43 changes: 21 additions & 22 deletions common/configtx/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import (
"github.com/pkg/errors"
)

func (c *ValidatorImpl) verifyReadSet(readSet map[string]comparable) error {
func (vi *ValidatorImpl) verifyReadSet(readSet map[string]comparable) error {
for key, value := range readSet {
existing, ok := c.configMap[key]
existing, ok := vi.configMap[key]
if !ok {
return errors.Errorf("existing config does not contain element for %s but was in the read set", key)
}
Expand All @@ -30,7 +30,7 @@ func (c *ValidatorImpl) verifyReadSet(readSet map[string]comparable) error {
return nil
}

func ComputeDeltaSet(readSet, writeSet map[string]comparable) map[string]comparable {
func computeDeltaSet(readSet, writeSet map[string]comparable) map[string]comparable {
result := make(map[string]comparable)
for key, value := range writeSet {
readVal, ok := readSet[key]
Expand All @@ -56,7 +56,7 @@ func validateModPolicy(modPolicy string) error {
trimmed = modPolicy[1:]
}

for i, pathElement := range strings.Split(trimmed, PathSeparator) {
for i, pathElement := range strings.Split(trimmed, pathSeparator) {
err := validateConfigID(pathElement)
if err != nil {
return errors.Wrapf(err, "path element at %d is invalid", i)
Expand All @@ -66,7 +66,7 @@ func validateModPolicy(modPolicy string) error {

}

func (cm *ValidatorImpl) verifyDeltaSet(deltaSet map[string]comparable, signedData []*cb.SignedData) error {
func (vi *ValidatorImpl) verifyDeltaSet(deltaSet map[string]comparable, signedData []*cb.SignedData) error {
if len(deltaSet) == 0 {
return errors.Errorf("delta set was empty -- update would have no effect")
}
Expand All @@ -77,20 +77,19 @@ func (cm *ValidatorImpl) verifyDeltaSet(deltaSet map[string]comparable, signedDa
return errors.Wrapf(err, "invalid mod_policy for element %s", key)
}

existing, ok := cm.configMap[key]
existing, ok := vi.configMap[key]
if !ok {
if value.version() != 0 {
return errors.Errorf("attempted to set key %s to version %d, but key does not exist", key, value.version())
} else {
continue
}

continue
}
if value.version() != existing.version()+1 {
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)
policy, ok := vi.policyForItem(existing)
if !ok {
return errors.Errorf("unexpected missing policy %s for item %s", existing.modPolicy(), key)
}
Expand All @@ -114,7 +113,7 @@ func verifyFullProposedConfig(writeSet, fullProposedConfig map[string]comparable

// authorizeUpdate validates that all modified config has the corresponding modification policies satisfied by the signature set
// it returns a map of the modified config
func (cm *ValidatorImpl) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelope) (map[string]comparable, error) {
func (vi *ValidatorImpl) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelope) (map[string]comparable, error) {
if configUpdateEnv == nil {
return nil, errors.Errorf("cannot process nil ConfigUpdateEnvelope")
}
Expand All @@ -124,44 +123,44 @@ func (cm *ValidatorImpl) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop
return nil, err
}

if configUpdate.ChannelId != cm.channelID {
return nil, errors.Errorf("Update not for correct channel: %s for %s", configUpdate.ChannelId, cm.channelID)
if configUpdate.ChannelId != vi.channelID {
return nil, errors.Errorf("Update not for correct channel: %s for %s", configUpdate.ChannelId, vi.channelID)
}

readSet, err := MapConfig(configUpdate.ReadSet, cm.namespace)
readSet, err := mapConfig(configUpdate.ReadSet, vi.namespace)
if err != nil {
return nil, errors.Wrapf(err, "error mapping ReadSet")
}
err = cm.verifyReadSet(readSet)
err = vi.verifyReadSet(readSet)
if err != nil {
return nil, errors.Wrapf(err, "error validating ReadSet")
}

writeSet, err := MapConfig(configUpdate.WriteSet, cm.namespace)
writeSet, err := mapConfig(configUpdate.WriteSet, vi.namespace)
if err != nil {
return nil, errors.Wrapf(err, "error mapping WriteSet")
}

deltaSet := ComputeDeltaSet(readSet, writeSet)
deltaSet := computeDeltaSet(readSet, writeSet)
signedData, err := configUpdateEnv.AsSignedData()
if err != nil {
return nil, err
}

if err = cm.verifyDeltaSet(deltaSet, signedData); err != nil {
if err = vi.verifyDeltaSet(deltaSet, signedData); err != nil {
return nil, errors.Wrapf(err, "error validating DeltaSet")
}

fullProposedConfig := cm.computeUpdateResult(deltaSet)
fullProposedConfig := vi.computeUpdateResult(deltaSet)
if err := verifyFullProposedConfig(writeSet, fullProposedConfig); err != nil {
return nil, errors.Wrapf(err, "full config did not verify")
}

return fullProposedConfig, nil
}

func (cm *ValidatorImpl) policyForItem(item comparable) (policies.Policy, bool) {
manager := cm.pm
func (vi *ValidatorImpl) policyForItem(item comparable) (policies.Policy, bool) {
manager := vi.pm

modPolicy := item.modPolicy()
logger.Debugf("Getting policy for item %s with mod_policy %s", item.key, modPolicy)
Expand Down Expand Up @@ -192,9 +191,9 @@ 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 {
func (vi *ValidatorImpl) computeUpdateResult(updatedConfig map[string]comparable) map[string]comparable {
newConfigMap := make(map[string]comparable)
for key, value := range cm.configMap {
for key, value := range vi.configMap {
newConfigMap[key] = value
}

Expand Down
Loading

0 comments on commit 0a7c03b

Please sign in to comment.