feat(iam): add iam bindings to crds #191
Conversation
|
Is there something I can do for those failed checks? |
internal/controller/suite_test.go
Outdated
| // try to get access to `Gsrv` thru `*testutil.Server`, that's on first field of `pstest.Server` | ||
| rs := reflect.ValueOf(psServer).Elem() | ||
|
|
||
| // *testutil.Server | ||
| tsrv := rs.Field(0) | ||
| tsrv = reflect.NewAt(tsrv.Type(), unsafe.Pointer(tsrv.UnsafeAddr())).Elem() | ||
|
|
||
| // *grpc.Server is found from the exported field `Gsrv` | ||
| gsrv := reflect.Indirect(tsrv).FieldByName("Gsrv").Interface().(*grpc.Server) |
There was a problem hiding this comment.
Thank you for the great work! I really understand why the reflection is needed and hope pstest package exports the internal *grpc.Server.
If the integration test is broken due to the change of pstest structure, it may be difficult to investigate the root cause. I'd like to suggest to extract this part to a separated function and write a unit test against the function. It ensures that the reflection works as we expect.
For example,
package pubsubtest
func GetInternalGrpcServer(psServer *pstest.Server) *grpc.Server {
//...
}
func TestGetInternalGrpcServer(t *testing.T) {
psServer := pstest.NewServer()
gSrv := GetInternalGrpcServer(psServer)
//...
}
|
It seems due to the permissions of forked repository. |
The fork should be public repo. Should I add write access to someone / something? |
Is there interest to add IAM bindings into the Topic and Subscription specs? I was thinking to add them as part of the topics and subs, rather than own separate CRD. But I suppose that would work as well.
The testing part is not ideal unfortunately. I did not find any good way to register additional servers into
pstest.Server, that hides thegrpc.Serverbehind unexposed variable. I had to hack theIAMPolicyServerinto it thru reflection, and it randomly fails if the registration happens aftergrpc.Server.Serve()...