Added support for mod policies

[richardfelkl]

Type of change

• New feature

Description

Added support for mod policies in config groups: Application, ApplicationOrg, Channel, ConsortiumOrg, Orderer, OrdererOrg. Previous implementation wasn't able to set mod policies which is beneficial when creating production grade custom channel configurations.

Additional details

The previous code was setting mod policies to AdminModPolicyKey by default. This behavior was retained so when the mod policy is not set manually it is automatically set to AdminModPolicyKey.

There are two incompatible changes in methods SetPolicy and SetPolicies which omit first parameter called modPolicy because it's not needed anymore. There is a new field ModPolicy in Policy struct instead of it.

Those changes were tested in a production grade Fabric environment.

[sykesm]

I have not looked through the code yet but quickly ran gorelease against the changes. Each of the incompatible changes likely warrants a discussion.

➜  fabric-config git:(pr/45) gorelease
github.com/hyperledger/fabric-config/configtx
---------------------------------------------
Incompatible changes:
- (*ApplicationGroup).SetPolicies: changed from func(string, map[string]Policy) error to func(map[string]Policy) error
- (*ApplicationGroup).SetPolicy: changed from func(string, string, Policy) error to func(string, Policy) error
- (*ApplicationOrg).SetPolicies: changed from func(string, map[string]Policy) error to func(map[string]Policy) error
- (*ApplicationOrg).SetPolicy: changed from func(string, string, Policy) error to func(string, Policy) error
- (*ChannelGroup).SetPolicies: changed from func(string, map[string]Policy) error to func(map[string]Policy) error
- (*ChannelGroup).SetPolicy: changed from func(string, string, Policy) error to func(string, Policy) error
- (*OrdererGroup).SetPolicies: changed from func(string, map[string]Policy) error to func(map[string]Policy) error
- (*OrdererGroup).SetPolicy: changed from func(string, string, Policy) error to func(string, Policy) error
- (*OrdererOrg).SetPolicies: changed from func(string, map[string]Policy) error to func(map[string]Policy) error
- (*OrdererOrg).SetPolicy: changed from func(string, string, Policy) error to func(string, Policy) error
Compatible changes:
- (*ApplicationGroup).SetModPolicy: added
- (*ApplicationOrg).SetModPolicy: added
- (*ChannelGroup).SetModPolicy: added
- (*ConsortiumOrg).SetModPolicy: added
- (*OrdererGroup).SetModPolicy: added
- (*OrdererOrg).SetModPolicy: added
- Application.ModPolicy: added
- Channel.ModPolicy: added
- Orderer.ModPolicy: added
- Organization.ModPolicy: added
- Policy.ModPolicy: added

[richardfelkl]

@sykesm I can make those SetPolicy and SetPolicies incompatible methods revert back but the modPolicy parameter will be duplicate. And we need to set preference of those. That's why I removed it. Refactoring to this change will be fairly easy.

[sykesm]

@sykesm I can make those SetPolicy and SetPolicies incompatible methods revert back but the modPolicy parameter will be duplicate. And we need to set preference of those. That's why I removed it. Refactoring to this change will be fairly easy.

Okay, thanks. Give me a chance to review what's here before you make any changes.

[richardfelkl]

@sykesm Hello, did you have time to take a look on the code?

[sykesm]

@sykesm Hello, did you have time to take a look on the code?

I have not. It's been several months since I spent any time in this library so I'll need to become reacquainted with it before I can make any meaningful comments.

I hope to get through it before the end of the week.

[sykesm]

Overall, looks good. Style items that I've referenced as "nits" wouldn't hold up a merge by themselves but it would be good to take care of those while addressing the other comments.

The missing tests and the error contents are the larger items that need to be resolved.

After looking through this, I agree with the changes to remove the modification policy arguments from the methods. This is something I'll need to highlight in the release notes when we tag it.

[richardfelkl]

@sykesm I've updated the PR and addressed all of the issues you mentioned.

[sykesm]

woops

[sykesm]

LGTM. Thanks for addressing the comments.