From f327ea041cc6f4bc8c3aa5f19d1e4b7d28af22f2 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Thu, 10 Aug 2017 10:56:41 -0400 Subject: [PATCH] [FAB-5710] Fix missing policy warnings The policy manager currently has a hardcoded check to look for policies of a particular name. This was fine, when the only policy manager was the channel policy manager, but with the resource policy manager, this does not make sense. Further, the log supression for the orderer channel templating was disabled during refactorying, this enables it once more. Change-Id: I6994425c3075c06503203132d857b5d54b9dba8c Signed-off-by: Jason Yellick --- common/config/channel/initializer.go | 69 ++++++++++++++++---- common/config/channel/realconfig_test.go | 9 +++ common/policies/policy.go | 40 ------------ orderer/common/msgprocessor/systemchannel.go | 2 +- 4 files changed, 67 insertions(+), 53 deletions(-) diff --git a/common/config/channel/initializer.go b/common/config/channel/initializer.go index 1daf0939970..04fc27527f5 100644 --- a/common/config/channel/initializer.go +++ b/common/config/channel/initializer.go @@ -113,31 +113,70 @@ func newResources() *resources { } type policyProposerRoot struct { - policyManager policies.Proposer + policyManager *policies.ManagerImpl + initializationLogSuppression bool } // BeginPolicyProposals is used to start a new config proposal -func (p *policyProposerRoot) BeginPolicyProposals(tx interface{}, groups []string) ([]policies.Proposer, error) { +func (ppr *policyProposerRoot) BeginPolicyProposals(tx interface{}, groups []string) ([]policies.Proposer, error) { if len(groups) != 1 { - logger.Panicf("Initializer only supports having one root group") + logger.Panicf("policyProposerRoot only supports having one root group") } - return []policies.Proposer{p.policyManager}, nil + return []policies.Proposer{ppr.policyManager}, nil } -func (i *policyProposerRoot) ProposePolicy(tx interface{}, key string, policy *cb.ConfigPolicy) (proto.Message, error) { +func (ppr *policyProposerRoot) ProposePolicy(tx interface{}, key string, policy *cb.ConfigPolicy) (proto.Message, error) { return nil, fmt.Errorf("Programming error, this should never be invoked") } // PreCommit is a no-op and returns nil -func (i *policyProposerRoot) PreCommit(tx interface{}) error { +func (ppr *policyProposerRoot) PreCommit(tx interface{}) error { return nil } // RollbackConfig is used to abandon a new config proposal -func (i *policyProposerRoot) RollbackProposals(tx interface{}) {} +func (ppr *policyProposerRoot) RollbackProposals(tx interface{}) {} // CommitConfig is used to commit a new config proposal -func (i *policyProposerRoot) CommitProposals(tx interface{}) {} +func (ppr *policyProposerRoot) CommitProposals(tx interface{}) { + if ppr.initializationLogSuppression { + ppr.initializationLogSuppression = false + } + + pm := ppr.policyManager + for _, policyName := range []string{policies.ChannelReaders, policies.ChannelWriters} { + _, 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) + } + } + if _, ok := pm.Manager([]string{policies.ApplicationPrefix}); ok { + // Check for default application policies if the application component is defined + for _, policyName := range []string{ + policies.ChannelApplicationReaders, + policies.ChannelApplicationWriters, + policies.ChannelApplicationAdmins} { + _, 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) + } + } + } + if _, ok := pm.Manager([]string{policies.OrdererPrefix}); ok { + for _, policyName := range []string{policies.BlockValidation} { + _, 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) + } + } + } +} type Initializer struct { *resources @@ -145,14 +184,22 @@ type Initializer struct { ppr *policyProposerRoot } +func NewWithOneTimeSuppressedPolicyWarnings(envConfig *cb.Envelope, callOnUpdate []func(*Initializer)) (*Initializer, error) { + return commonNew(envConfig, callOnUpdate, true) +} + // New creates a new channel config, complete with backing configtx manager -// TODO move configtx.Manager to resources, make func on Resources type func New(envConfig *cb.Envelope, callOnUpdate []func(*Initializer)) (*Initializer, error) { + return commonNew(envConfig, callOnUpdate, false) +} + +func commonNew(envConfig *cb.Envelope, callOnUpdate []func(*Initializer), suppressFirstPolicyWarnings bool) (*Initializer, error) { resources := newResources() i := &Initializer{ resources: resources, ppr: &policyProposerRoot{ - policyManager: resources.policyManager, + policyManager: resources.policyManager, + initializationLogSuppression: suppressFirstPolicyWarnings, }, } var err error @@ -164,7 +211,6 @@ func New(envConfig *cb.Envelope, callOnUpdate []func(*Initializer)) (*Initialize if !initialized { return } - logger.Criticalf("Making callback normally") for _, callback := range callOnUpdate { callback(i) } @@ -175,7 +221,6 @@ func New(envConfig *cb.Envelope, callOnUpdate []func(*Initializer)) (*Initialize } initialized = true i.resources.configtxManager = i.Manager - logger.Criticalf("Making callback manually") for _, callback := range callOnUpdate { callback(i) } diff --git a/common/config/channel/realconfig_test.go b/common/config/channel/realconfig_test.go index 1b61f5ca8da..9b3d32780dc 100644 --- a/common/config/channel/realconfig_test.go +++ b/common/config/channel/realconfig_test.go @@ -27,6 +27,7 @@ import ( "github.com/hyperledger/fabric/common/tools/configtxgen/localconfig" "github.com/hyperledger/fabric/common/tools/configtxgen/provisional" cb "github.com/hyperledger/fabric/protos/common" + "github.com/hyperledger/fabric/protos/utils" "github.com/stretchr/testify/assert" ) @@ -84,3 +85,11 @@ func TestSimpleConfig(t *testing.T) { func TestOneMSPConfig(t *testing.T) { commonTest(t, localconfig.SampleSingleMSPSoloProfile) } + +func TestWithRealConfigtx(t *testing.T) { + conf := localconfig.Load(localconfig.SampleSingleMSPSoloProfile) + gb := provisional.New(conf).GenesisBlockForChannel("foo") + env := utils.ExtractEnvelopeOrPanic(gb, 0) + _, err := New(env, nil) + assert.NoError(t, err) +} diff --git a/common/policies/policy.go b/common/policies/policy.go index 32ab65597fc..01e527c8e28 100644 --- a/common/policies/policy.go +++ b/common/policies/policy.go @@ -132,11 +132,6 @@ type ManagerImpl struct { config *policyConfig pendingConfig map[interface{}]*policyConfig pendingLock sync.RWMutex - - // SuppressSanityLogMessages when set to true will prevent the sanity checking log - // messages. Useful for novel cases like channel templates - // TODO, pull the sanity checking into chanel config - SuppressSanityLogMessages bool } // NewManagerImpl creates a new ManagerImpl with the given CryptoHelper @@ -294,41 +289,6 @@ func (pm *ManagerImpl) CommitProposals(tx interface{}) { pm.config = pendingConfig delete(pm.pendingConfig, tx) - - if pm.parent == nil && pm.basePath == ChannelPrefix && !pm.SuppressSanityLogMessages { - for _, policyName := range []string{ChannelReaders, ChannelWriters} { - _, 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) - } - } - if _, ok := pm.config.managers[ApplicationPrefix]; ok { - // Check for default application policies if the application component is defined - for _, policyName := range []string{ - ChannelApplicationReaders, - ChannelApplicationWriters, - ChannelApplicationAdmins} { - _, 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) - } - } - } - if _, ok := pm.config.managers[OrdererPrefix]; ok { - for _, policyName := range []string{BlockValidation} { - _, 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/orderer/common/msgprocessor/systemchannel.go b/orderer/common/msgprocessor/systemchannel.go index 03aa5a81a39..a361f62a554 100644 --- a/orderer/common/msgprocessor/systemchannel.go +++ b/orderer/common/msgprocessor/systemchannel.go @@ -269,5 +269,5 @@ func (dt *DefaultTemplator) NewChannelConfig(envConfigUpdate *cb.Envelope) (conf }, }, msgVersion, epoch) - return channelconfig.New(templateConfig, nil) + return channelconfig.NewWithOneTimeSuppressedPolicyWarnings(templateConfig, nil) }