Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FAB-17515] Support configuring BlockValidation policy for orderer group #697

Merged
merged 1 commit into from
Feb 25, 2020

Conversation

wlahti
Copy link
Contributor

@wlahti wlahti commented Feb 19, 2020

Type of change

  • Bug fix

Description

Before we used to hardcode the BlockValidation policy to an ImplicitMetaAnyPolicy and we also did not enforce that the BlockValidation policy existed.

Related issues

FAB-17515

@wlahti wlahti requested a review from a team as a code owner February 19, 2020 19:22
@@ -109,6 +109,12 @@ func addImplicitMetaPolicyDefaults(cg *cb.ConfigGroup) {
addPolicy(cg, policies.ImplicitMetaAnyPolicy(channelconfig.WritersPolicyKey), channelconfig.AdminsPolicyKey)
}

// addOrdererImplicitMetaPolicyDefaults adds the orderer's Readers/Writers/Admins/BlockValidation policies, with Any/Any/Majority/Any rules respectively.
func addOrdererImplicitMetaPolicyDefaults(cg *cb.ConfigGroup) {
addPolicy(cg, policies.ImplicitMetaAnyPolicy(BlockValidationPolicyKey), channelconfig.AdminsPolicyKey)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't use addPolicy here because it uses the same key name as the sub policy. We can just copy the removed Policies map addition below here

@wlahti wlahti force-pushed the FAB-17515-release-1.4 branch 2 times, most recently from df54c54 to cfe86cc Compare February 19, 2020 22:42
@wlahti wlahti requested a review from a team as a code owner February 19, 2020 22:42
@wlahti wlahti force-pushed the FAB-17515-release-1.4 branch from cfe86cc to 0257000 Compare February 19, 2020 22:45
Expect(cg.Policies["SamplePolicy"]).NotTo(BeNil())
Expect(cg.Policies["BlockValidation"]).NotTo(BeNil())
Expect(cg.Policies["SamplePolicy"].Policy).To(Equal(expectedAdminPolicy))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you convert this inline with protoutil.MarshalOrPanic and remove lines 194-203?

Expect(cg.Policies["SamplePolicy"].Policy).To(Equal(&cb.Policy{
				Type: int32(cb.Policy_IMPLICIT_META),
				Value: protoutil.MarshalOrPanic(&cb.ImplicitMetaPolicy{
					SubPolicy: "Admins",
					Rule:      cb.ImplicitMetaPolicy_ANY,
				}),
			}))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@wlahti wlahti force-pushed the FAB-17515-release-1.4 branch from 0257000 to af1efde Compare February 24, 2020 19:41
@sykesm
Copy link
Contributor

sykesm commented Feb 24, 2020

Opened https://jira.hyperledger.org/browse/FAB-17531 for the UT failure.

@sykesm
Copy link
Contributor

sykesm commented Feb 24, 2020

Looks like the latest failure is similar to what was fixed in #704

2020-02-24T22:12:03.4410585Z panic: runtime error: invalid memory address or nil pointer dereference
2020-02-24T22:12:03.4410971Z [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x139bcfa]
2020-02-24T22:12:03.4411205Z 
2020-02-24T22:12:03.4411363Z goroutine 571 [running]:
2020-02-24T22:12:03.4411776Z github.com/hyperledger/fabric/orderer/common/cluster_test.(*deliverServer).addExpectPullAssert.func1(0xc0004b8540, 0xc00016d806, 0x9)
2020-02-24T22:12:03.4412321Z 	/home/vsts/work/1/go/src/github.com/hyperledger/fabric/orderer/common/cluster/deliver_test.go:262 +0x14a
2020-02-24T22:12:03.4412906Z github.com/hyperledger/fabric/orderer/common/cluster_test.(*deliverServer).Deliver(0xc0004b92c0, 0x17a8ec0, 0xc00030e400, 0x178c4e0, 0xc0004b92c0)
2020-02-24T22:12:03.4413482Z 	/home/vsts/work/1/go/src/github.com/hyperledger/fabric/orderer/common/cluster/deliver_test.go:171 +0x19a
2020-02-24T22:12:03.4414062Z github.com/hyperledger/fabric/protos/orderer._AtomicBroadcast_Deliver_Handler(0x15d5660, 0xc0004b92c0, 0x17a5920, 0xc0005124d0, 0x2b8c178, 0xc0003b0300)
2020-02-24T22:12:03.4414627Z 	/home/vsts/work/1/go/src/github.com/hyperledger/fabric/protos/orderer/ab.pb.go:754 +0xce
2020-02-24T22:12:03.4415227Z github.com/hyperledger/fabric/vendor/google.golang.org/grpc.(*Server).processStreamingRPC(0xc000173c80, 0x17aa780, 0xc0005e9c80, 0xc0003b0300, 0xc0002303c0, 0x20fb1e0, 0x0, 0x0, 0x0)
2020-02-24T22:12:03.4415872Z 	/home/vsts/work/1/go/src/github.com/hyperledger/fabric/vendor/google.golang.org/grpc/server.go:1124 +0x1178
2020-02-24T22:12:03.4416431Z github.com/hyperledger/fabric/vendor/google.golang.org/grpc.(*Server).handleStream(0xc000173c80, 0x17aa780, 0xc0005e9c80, 0xc0003b0300, 0x0)
2020-02-24T22:12:03.4416991Z 	/home/vsts/work/1/go/src/github.com/hyperledger/fabric/vendor/google.golang.org/grpc/server.go:1212 +0x12e3
2020-02-24T22:12:03.4417687Z github.com/hyperledger/fabric/vendor/google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc00003e650, 0xc000173c80, 0x17aa780, 0xc0005e9c80, 0xc0003b0300)
2020-02-24T22:12:03.4418270Z 	/home/vsts/work/1/go/src/github.com/hyperledger/fabric/vendor/google.golang.org/grpc/server.go:686 +0xad
2020-02-24T22:12:03.4418767Z created by github.com/hyperledger/fabric/vendor/google.golang.org/grpc.(*Server).serveStreams.func1
2020-02-24T22:12:03.4419406Z 	/home/vsts/work/1/go/src/github.com/hyperledger/fabric/vendor/google.golang.org/grpc/server.go:684 +0xb9
2020-02-24T22:12:03.4420003Z FAIL	github.com/hyperledger/fabric/orderer/common/cluster	2.172s

Before we used to hardcode the BlockValidation policy to an ImplicitMetaAnyPolicy.

Signed-off-by: Danny Cao <dcao@us.ibm.com>
Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
@wlahti wlahti force-pushed the FAB-17515-release-1.4 branch from af1efde to 1f98861 Compare February 25, 2020 13:03
Comment on lines +112 to +120
// addOrdererImplicitMetaPolicyDefaults adds the orderer's Readers/Writers/Admins/BlockValidation policies, with Any/Any/Majority/Any rules respectively.
func addOrdererImplicitMetaPolicyDefaults(cg *cb.ConfigGroup) {
cg.Policies[BlockValidationPolicyKey] = &cb.ConfigPolicy{
Policy: policies.ImplicitMetaAnyPolicy(channelconfig.WritersPolicyKey).Value(),
ModPolicy: channelconfig.AdminsPolicyKey,
}
addImplicitMetaPolicyDefaults(cg)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some sort of assertion for this that I'm misssing? Maybe in an existing test?

@sykesm sykesm merged commit 87df051 into hyperledger:release-1.4 Feb 25, 2020
@wlahti wlahti deleted the FAB-17515-release-1.4 branch March 25, 2020 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants