Set Correct policy while channel update #114
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nidhi-singh02
force-pushed
the
updateChannel
branch
from
eb84812
to
662028f
Compare
Signed-off-by: n0s09by <[email protected]>
nidhi-singh02
force-pushed
the
updateChannel
branch
from
662028f
to
3bb082c
Compare
arsulegai
reviewed
Dec 26, 2023
| * @return HashMap with role and the configuration policy | ||
| */ | ||
| @Override | ||
| public HashMap<String, Configtx.ConfigPolicy> getDefaultRolePolicy(String orgMSPId) { |
There was a problem hiding this comment.
[nitpick] What do you think?
Looks like this can be a channel utility function and it can be moved to a different class.
There was a problem hiding this comment.
Good suggestion, created a class for channel utility and moved the functions to this class so it can be used anywhere.
arsulegai
reviewed
Dec 26, 2023
|
|
||
| return applicationPoliciesMap; | ||
| } | ||
|
|
||
| private ConfigPolicy setNewOrgPolicy(String newOrgName, String policyTarget) { |
There was a problem hiding this comment.
[nitpick] setNewOrgPolicy setTypeOnePolicy is not used and can be deleted.
There was a problem hiding this comment.
Sure, Addressed.
Signed-off-by: n0s09by <[email protected]>
| import org.hyperledger.fabric.protos.common.Policies; | ||
|
|
||
| @Slf4j | ||
| public class FabricChannelUtil { |
There was a problem hiding this comment.
[nitpick] @UtilityClass
Comment on lines
104
to
142
| case FabricClientConstants.CHANNEL_CONFIG_POLICY_TYPE_ADMINS: | ||
| mspRole = | ||
| MspPrincipal.MSPRole.newBuilder() | ||
| .setMspIdentifier(orgMSPId) | ||
| .setRole(MspPrincipal.MSPRole.MSPRoleType.ADMIN) | ||
| .build(); | ||
| mspPrincipal = | ||
| MspPrincipal.MSPPrincipal.newBuilder() | ||
| .setPrincipal(mspRole.toByteString()) | ||
| .setPrincipalClassification(MspPrincipal.MSPPrincipal.Classification.ROLE) | ||
| .build(); | ||
| mspPrincipals.add(mspPrincipal); | ||
| break; | ||
| case FabricClientConstants.CHANNEL_CONFIG_POLICY_TYPE_WRITERS: | ||
| // any member who is an admin can write | ||
| mspRole = | ||
| MspPrincipal.MSPRole.newBuilder() | ||
| .setMspIdentifier(orgMSPId) | ||
| .setRole(MspPrincipal.MSPRole.MSPRoleType.ADMIN) | ||
| .build(); | ||
| mspPrincipal = | ||
| MspPrincipal.MSPPrincipal.newBuilder() | ||
| .setPrincipal(mspRole.toByteString()) | ||
| .setPrincipalClassification(MspPrincipal.MSPPrincipal.Classification.ROLE) | ||
| .build(); | ||
| mspPrincipals.add(mspPrincipal); | ||
| // any client can also write | ||
| mspRole = | ||
| MspPrincipal.MSPRole.newBuilder() | ||
| .setMspIdentifier(orgMSPId) | ||
| .setRole(MspPrincipal.MSPRole.MSPRoleType.CLIENT) | ||
| .build(); | ||
| mspPrincipal = | ||
| MspPrincipal.MSPPrincipal.newBuilder() | ||
| .setPrincipal(mspRole.toByteString()) | ||
| .setPrincipalClassification(MspPrincipal.MSPPrincipal.Classification.ROLE) | ||
| .build(); | ||
| mspPrincipals.add(mspPrincipal); | ||
| break; |
There was a problem hiding this comment.
There are multiple fragments of duplicate code here. We can simplify this as
switch (policyFor) {
case FabricClientConstants.CHANNEL_CONFIG_POLICY_TYPE_ADMINS:
mspPrincipals.add(createPrincipal(orgMSPId, MspPrincipal.MSPRole.MSPRoleType.ADMIN));
break;
case FabricClientConstants.CHANNEL_CONFIG_POLICY_TYPE_WRITERS:
mspPrincipals.add(createPrincipal(orgMSPId, MspPrincipal.MSPRole.MSPRoleType.ADMIN));
mspPrincipals.add(createPrincipal(orgMSPId, MspPrincipal.MSPRole.MSPRoleType.CLIENT));
break;
.
.
// Other case statements
createPrinciple would look like
private static MspPrincipal.MSPPrincipal createPrincipal(String mspId, MspPrincipal.MSPRole.MSPRoleType roleType) {
MspPrincipal.MSPRole mspRole = MspPrincipal.MSPRole.newBuilder()
.setMspIdentifier(mspId)
.setRole(roleType)
.build();
return MspPrincipal.MSPPrincipal.newBuilder()
.setPrincipal(mspRole.toByteString())
.setPrincipalClassification(MspPrincipal.MSPPrincipal.Classification.ROLE)
.build();
}
There was a problem hiding this comment.
Good suggestion, Incorporated changes.
| import org.hyperledger.fabric.protos.common.MspPrincipal; | ||
| import org.hyperledger.fabric.protos.common.Policies; | ||
|
|
||
| @Slf4j |
There was a problem hiding this comment.
[nitpick] you can safely remove this since nothing is being logged in this class, or can introduce log statements if required.
There was a problem hiding this comment.
Addressed, removed it.
| @@ -44,25 +39,27 @@ public ConfigGroup buildWriteset(ConfigGroup readset, NewOrgParamsDTO organizati | |||
| // Get existing organizations in the channel and set with as objects and their | |||
| // version to prevent deletion or modification | |||
| // Omitting existing groups results in their deletion. | |||
| Map<String, ConfigGroup> organizations = new HashMap<>(); | |||
| Map<String, ConfigGroup> existingOrganizations = new HashMap<>(); | |||
arsulegai
approved these changes
Dec 27, 2023
Signed-off-by: n0s09by <[email protected]>
Thanks Arun, Open comments has been addressed. |
nithin-pankaj
approved these changes
Dec 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The policy while channel update for the newly added organization is not correct.