Skip to content

Commit

Permalink
[FAB-5710] Fix missing policy warnings
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Jason Yellick committed Aug 12, 2017
1 parent fccd54d commit f327ea0
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 53 deletions.
69 changes: 57 additions & 12 deletions common/config/channel/initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,46 +113,93 @@ 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
configtxapi.Manager
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
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down
9 changes: 9 additions & 0 deletions common/config/channel/realconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
40 changes: 0 additions & 40 deletions common/policies/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion orderer/common/msgprocessor/systemchannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,5 +269,5 @@ func (dt *DefaultTemplator) NewChannelConfig(envConfigUpdate *cb.Envelope) (conf
},
}, msgVersion, epoch)

return channelconfig.New(templateConfig, nil)
return channelconfig.NewWithOneTimeSuppressedPolicyWarnings(templateConfig, nil)
}

0 comments on commit f327ea0

Please sign in to comment.