diff --git a/common/configtx/update.go b/common/configtx/update.go index 2c41c30dc05..3f1377b3308 100644 --- a/common/configtx/update.go +++ b/common/configtx/update.go @@ -18,7 +18,6 @@ package configtx import ( "fmt" - "strings" "github.com/hyperledger/fabric/common/policies" cb "github.com/hyperledger/fabric/protos/common" @@ -123,10 +122,6 @@ func (cm *configManager) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop } 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 { diff --git a/common/configtx/update_test.go b/common/configtx/update_test.go index dc7f0ac5997..0031baad2b8 100644 --- a/common/configtx/update_test.go +++ b/common/configtx/update_test.go @@ -72,13 +72,4 @@ func TestPolicyForItem(t *testing.T) { }) 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/policies/policy.go b/common/policies/policy.go index 2bee39ecc22..1aba1ffd05a 100644 --- a/common/policies/policy.go +++ b/common/policies/policy.go @@ -18,6 +18,7 @@ package policies import ( "fmt" + "strings" cb "github.com/hyperledger/fabric/protos/common" @@ -25,8 +26,17 @@ import ( ) const ( + // Path separator is used to separate policy names in paths + PathSeparator = "/" + + // ChannelPrefix is used in the path of standard channel policy managers + ChannelPrefix = "Channel" + + // ApplicationPrefix is used in the path of standard application policy paths + ApplicationPrefix = "Application" + // ChannelApplicationReaders is the label for the channel's application readers policy - ChannelApplicationReaders = "/channel/Application/Readers" + ChannelApplicationReaders = "/" + ChannelPrefix + "/" + ApplicationPrefix + "/Readers" ) var logger = logging.MustGetLogger("common/policies") @@ -45,7 +55,7 @@ type Manager interface { // Manager returns the sub-policy manager for a given path and whether it exists Manager(path []string) (Manager, bool) - // Basepath returns the basePath the manager was instnatiated with + // Basepath returns the basePath the manager was instantiated with BasePath() string // Policies returns all policy names defined in the manager @@ -85,7 +95,9 @@ type policyConfig struct { // ManagerImpl is an implementation of Manager and configtx.ConfigHandler // In general, it should only be referenced as an Impl for the configtx.ConfigManager type ManagerImpl struct { + parent *ManagerImpl basePath string + fqPrefix string providers map[int32]Provider config *policyConfig pendingConfig *policyConfig @@ -100,6 +112,7 @@ func NewManagerImpl(basePath string, providers map[int32]Provider) *ManagerImpl return &ManagerImpl{ basePath: basePath, + fqPrefix: PathSeparator + basePath + PathSeparator, providers: providers, config: &policyConfig{ policies: make(map[string]Policy), @@ -145,15 +158,36 @@ func (pm *ManagerImpl) Manager(path []string) (Manager, bool) { // GetPolicy returns a policy and true if it was the policy requested, or false if it is the default reject policy func (pm *ManagerImpl) GetPolicy(id string) (Policy, bool) { - policy, ok := pm.config.policies[id] + if id == "" { + logger.Errorf("Returning dummy reject all policy because no policy ID supplied") + return rejectPolicy(id), false + } + var relpath string + + if strings.HasPrefix(id, PathSeparator) { + if pm.parent != nil { + return pm.parent.GetPolicy(id) + } + if !strings.HasPrefix(id, pm.fqPrefix) { + if logger.IsEnabledFor(logging.DEBUG) { + logger.Debugf("Requested policy from root manager with wrong basePath: %s, returning rejectAll", id) + } + return rejectPolicy(id), false + } + relpath = id[len(pm.fqPrefix):] + } else { + relpath = id + } + + policy, ok := pm.config.policies[relpath] if !ok { if logger.IsEnabledFor(logging.DEBUG) { - logger.Debugf("Returning dummy reject all policy because %s could not be found in %s", id, pm.basePath) + logger.Debugf("Returning dummy reject all policy because %s could not be found in /%s/%s", id, pm.basePath, relpath) } - return rejectPolicy(id), false + return rejectPolicy(relpath), false } if logger.IsEnabledFor(logging.DEBUG) { - logger.Debugf("Returning policy %s for evaluation", id) + logger.Debugf("Returning policy %s for evaluation", relpath) } return policy, true } @@ -172,6 +206,7 @@ func (pm *ManagerImpl) BeginPolicyProposals(groups []string) ([]Proposer, error) managers := make([]Proposer, len(groups)) for i, group := range groups { newManager := NewManagerImpl(group, pm.providers) + newManager.parent = pm pm.pendingConfig.managers[group] = newManager managers[i] = newManager } @@ -196,7 +231,7 @@ func (pm *ManagerImpl) CommitProposals() { for managerPath, m := range pm.pendingConfig.managers { for _, policyName := range m.PolicyNames() { - fqKey := managerPath + "/" + policyName + fqKey := managerPath + PathSeparator + policyName pm.pendingConfig.policies[fqKey], _ = m.GetPolicy(policyName) logger.Debugf("In commit adding relative sub-policy %s to %s", fqKey, pm.basePath) } @@ -209,6 +244,20 @@ func (pm *ManagerImpl) CommitProposals() { pm.config = pm.pendingConfig pm.pendingConfig = nil + + if pm.parent == nil && pm.basePath == ChannelPrefix { + if _, ok := pm.config.managers[ApplicationPrefix]; ok { + // Check for default application policies if the application component is defined + for _, policyName := range []string{ChannelApplicationReaders} { + _, ok := pm.GetPolicy(policyName) + if !ok { + logger.Warningf("Current configuration has no policy '%s', this will likely cause problems in production systems", policyName) + } else { + logger.Debugf("As expected, current configuration has policy '%s'", policyName) + } + } + } + } } // ProposePolicy takes key, path, and ConfigPolicy and registers it in the proposed PolicyManager, or errors diff --git a/common/policies/policy_test.go b/common/policies/policy_test.go index e43f09940e4..7de47971f7c 100644 --- a/common/policies/policy_test.go +++ b/common/policies/policy_test.go @@ -74,6 +74,7 @@ func TestUnnestedManager(t *testing.T) { func TestNestedManager(t *testing.T) { m := NewManagerImpl("test", defaultProviders()) + absPrefix := "/test/" nesting1, err := m.BeginPolicyProposals([]string{"nest1"}) assert.NoError(t, err) assert.Len(t, nesting1, 1, "Should not have returned exactly one additional manager") @@ -137,6 +138,10 @@ func TestNestedManager(t *testing.T) { for _, policyName := range policyNames { _, ok := m.GetPolicy(policyName) assert.True(t, ok, "Should have found policy %s", policyName) + + absName := absPrefix + policyName + _, ok = m.GetPolicy(absName) + assert.True(t, ok, "Should have found absolute policy %s", absName) } for _, policyName := range n1PolicyNames { @@ -145,6 +150,12 @@ func TestNestedManager(t *testing.T) { _, ok = m.GetPolicy(n1.BasePath() + "/" + policyName) assert.True(t, ok, "Should have found policy %s", policyName) + + for i, abs := range []Manager{n1, m} { + absName := absPrefix + n1.BasePath() + "/" + policyName + _, ok = abs.GetPolicy(absName) + assert.True(t, ok, "Should have found absolutely policy for manager %d", i) + } } for _, policyName := range n2aPolicyNames { @@ -156,6 +167,12 @@ func TestNestedManager(t *testing.T) { _, ok = m.GetPolicy(n1.BasePath() + "/" + n2a.BasePath() + "/" + policyName) assert.True(t, ok, "Should have found policy %s", policyName) + + for i, abs := range []Manager{n2a, n1, m} { + absName := absPrefix + n1.BasePath() + "/" + n2a.BasePath() + "/" + policyName + _, ok = abs.GetPolicy(absName) + assert.True(t, ok, "Should have found absolutely policy for manager %d", i) + } } for _, policyName := range n2bPolicyNames { @@ -167,5 +184,11 @@ func TestNestedManager(t *testing.T) { _, ok = m.GetPolicy(n1.BasePath() + "/" + n2b.BasePath() + "/" + policyName) assert.True(t, ok, "Should have found policy %s", policyName) + + for i, abs := range []Manager{n2b, n1, m} { + absName := absPrefix + n1.BasePath() + "/" + n2b.BasePath() + "/" + policyName + _, ok = abs.GetPolicy(absName) + assert.True(t, ok, "Should have found absolutely policy for manager %d", i) + } } }