Pass gitaly part of /allowed response as raw json straight to Gitaly requests
The following discussion from !300 (merged) should be addressed:
-
@nick.thomas started a discussion: (+1 comment) @igor.drozdov @reprazent so here we decode the
Gitaly
response from/internal/allowed
, only to pack it up again into anSSHReceivePackRequest
protobuf message. We have a TODO in the Ruby code:# TODO: instead of building from pieces here in gitlab-shell, build the # entire gitaly_request in gitlab-ce and pass on as-is here.
This Go code works, but it's a lot more painful to do this kind of thing in Go than it is to do it in Ruby, and we run a risk - if a new field is added to the internal API response, it's easy to forget to add it here too, whereas in ruby, it would be passed straight through. In short, we're embedding knowledge about Gitaly into gitlab-shell that wasn't previously there.
I'm wondering what you think about it. Perhaps we can use
json.RawMessage
in a few places here to get raw JSON blobs that we can pass down to the point where we want to build apb.SSHReceivePackRequest
, to avoid some of the deserialization here? Should we address this TODO now and start sending a pre-constructedpb.SSHReceivePackRequest
alongside the existing data, and have gitlab-shell-go only consult that? Or do you think we're OK as-is and this can be deferred to a follow-up issue?I think I'd be tempted to start sending that pre-constructed
pb.SSHReceivePackRequest
now, but it does mean linking this MR to changes in gitlab-ce. We did the same for thetoken
request header in an earlier MR.
As I've mentioned in the discussion above, we might want to implement it after we've migrated the features from Ruby