From 22d2d5c9c76d6899f79d332edebc11e3ae5019c3 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Thu, 16 Feb 2017 23:31:49 -0500 Subject: [PATCH] [FAB-2322] Allow mod_policy to be relative https://jira.hyperledger.org/browse/FAB-2322 With the notion of hierarchical policies, the configuration mod_policy needs to be able to refer to policies in the current group (relatively) or policies relative to the channel root configuration (absolute). This CR modifies the policy lookup to ask the relative policy manager unless the policy begins with a "/" in which case the policy is retrieved from the root policy manager. Change-Id: Id14e327a5c37c4c1848521abd691aa857a12039d Signed-off-by: Jason Yellick --- common/configtx/update.go | 23 +++++++-- common/configtx/update_test.go | 84 +++++++++++++++++++++++++++++++ common/mocks/policies/policies.go | 21 ++++++-- 3 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 common/configtx/update_test.go diff --git a/common/configtx/update.go b/common/configtx/update.go index f74b59470cf..9a840b70ca5 100644 --- a/common/configtx/update.go +++ b/common/configtx/update.go @@ -18,6 +18,7 @@ package configtx import ( "fmt" + "strings" "github.com/hyperledger/fabric/common/policies" cb "github.com/hyperledger/fabric/protos/common" @@ -94,12 +95,15 @@ func (cm *configManager) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop // Get the modification policy for this config item if one was previously specified // or accept it if it is new, as the group policy will be evaluated for its inclusion - var policy policies.Policy if ok { - policy, _ = cm.PolicyManager().GetPolicy(oldValue.modPolicy()) + policy, ok := cm.policyForItem(oldValue) + if !ok { + return nil, fmt.Errorf("Unexpected missing policy %s for item %s", oldValue.modPolicy(), key) + } + // Ensure the policy is satisfied if err = policy.Evaluate(signedData); err != nil { - return nil, err + return nil, fmt.Errorf("Policy for %s not satisfied: %s", key, err) } } @@ -118,6 +122,19 @@ func (cm *configManager) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop return cm.computeUpdateResult(configMap), nil } +func (cm *configManager) policyForItem(item comparable) (policies.Policy, bool) { + if strings.HasPrefix(item.modPolicy(), PathSeparator) { + return cm.PolicyManager().GetPolicy(item.modPolicy()[1:]) + } + + // path is always at least of length 1 + manager, ok := cm.PolicyManager().Manager(item.path[1:]) + if !ok { + return nil, ok + } + return manager.GetPolicy(item.modPolicy()) +} + // computeUpdateResult takes a configMap generated by an update and produces a new configMap overlaying it onto the old config func (cm *configManager) computeUpdateResult(updatedConfig map[string]comparable) map[string]comparable { newConfigMap := make(map[string]comparable) diff --git a/common/configtx/update_test.go b/common/configtx/update_test.go new file mode 100644 index 00000000000..dc7f0ac5997 --- /dev/null +++ b/common/configtx/update_test.go @@ -0,0 +1,84 @@ +/* +Copyright IBM Corp. 2017 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. +*/ + +package configtx + +import ( + "fmt" + "testing" + + mockconfigtx "github.com/hyperledger/fabric/common/mocks/configtx" + mockpolicies "github.com/hyperledger/fabric/common/mocks/policies" + cb "github.com/hyperledger/fabric/protos/common" + + "github.com/stretchr/testify/assert" +) + +func TestPolicyForItem(t *testing.T) { + // Policies are set to different error values to differentiate them in equal assertion + rootPolicy := &mockpolicies.Policy{Err: fmt.Errorf("rootPolicy")} + fooPolicy := &mockpolicies.Policy{Err: fmt.Errorf("fooPolicy")} + + cm := &configManager{ + Resources: &mockconfigtx.Resources{ + PolicyManagerVal: &mockpolicies.Manager{ + BasePathVal: "root", + Policy: rootPolicy, + SubManagersMap: map[string]*mockpolicies.Manager{ + "foo": &mockpolicies.Manager{ + Policy: fooPolicy, + BasePathVal: "foo", + }, + }, + }, + }, + } + + policy, ok := cm.policyForItem(comparable{ + path: []string{"root"}, + ConfigValue: &cb.ConfigValue{ + ModPolicy: "rootPolicy", + }, + }) + assert.True(t, ok) + assert.Equal(t, policy, rootPolicy, "Should have found relative policy off the root manager") + + policy, ok = cm.policyForItem(comparable{ + path: []string{"root", "wrong"}, + ConfigValue: &cb.ConfigValue{ + ModPolicy: "rootPolicy", + }, + }) + assert.False(t, ok, "Should not have found rootPolicy off a non-existant manager") + + policy, ok = cm.policyForItem(comparable{ + path: []string{"root", "foo"}, + ConfigValue: &cb.ConfigValue{ + ModPolicy: "foo", + }, + }) + assert.True(t, ok) + assert.Equal(t, policy, fooPolicy, "Should not have found relative foo policy the foo manager") + + policy, ok = cm.policyForItem(comparable{ + path: []string{"root", "foo"}, + ConfigValue: &cb.ConfigValue{ + ModPolicy: "/rootPolicy", + }, + }) + assert.True(t, ok) + assert.Equal(t, policy, rootPolicy, "Should not have found absolute root policy from the foo path position") +} diff --git a/common/mocks/policies/policies.go b/common/mocks/policies/policies.go index da0d7dcf910..c6d65e44886 100644 --- a/common/mocks/policies/policies.go +++ b/common/mocks/policies/policies.go @@ -43,11 +43,20 @@ type Manager struct { // PolicyMap is returned is used to look up Policies in PolicyMap map[string]*Policy + + // SubManagers is used for the return value of Manager and SubManagers + SubManagersMap map[string]*Manager } -// Managers panics +// Managers returns the values of SubManagers func (m *Manager) SubManagers() []policies.Manager { - panic("Unimplimented") + result := make([]policies.Manager, len(m.SubManagersMap)) + i := 0 + for _, manager := range m.SubManagersMap { + result[i] = manager + i++ + } + return result } // PolicyNames panics @@ -60,9 +69,13 @@ func (m *Manager) BasePath() string { return m.BasePathVal } -// Manager always returns itself +// Manager returns the Manager from SubManagers for the last component of the path func (m *Manager) Manager(path []string) (policies.Manager, bool) { - return m, true + if len(path) == 0 { + return m, true + } + manager, ok := m.SubManagersMap[path[len(path)-1]] + return manager, ok } // GetPolicy returns the value of Manager.Policy and whether it was nil or not