• Patrick Steinhardt's avatar
    gitaly: Fix access checks with transactions and quarantine environments · edab619a
    Patrick Steinhardt authored
    In order to check whether certain operations are allowed to be executed
    by a user, Gitaly POSTs to the `/internal/allowed` endpoint. The request
    includes information about what change the user wants to perform, but it
    also contains information about the environment the change is currently
    performed in.
    
    When a user performs a push, git will first store all pushed objects
    into a quarantine environment. This is a separate temporary directory
    containing all new objects such that if the push gets rejected, new
    objects will not persist in the repository. The crux is that in order to
    inspect these new objects, git needs to be told that such a quarantine
    environment exists. This is why Gitaly sends information about this
    quarantine environment to `/internal/allowed`, so that we can again
    relay this information back to Gitaly when we want to inspect newly
    pushed objects to determine whether they're allowed or not.
    
    While it's a leaky abstraction, it has worked fine until now. But with
    transactions, that's not true anymore: when multiple Gitaly nodes take
    part in a push, then they'll all generate a randomly named quarantine
    environment. But as only the primary node will inject its info into the
    request, we are not able to acces quarantine environments of secondary
    nodes. If we now route accessor requests to any of the secondary Gitaly
    nodes with the quarantine environment of the primary, then the request
    will fail as git cannot find quarantined objects.
    
    To fix this, Gitaly has recently grown a new GRPC header which allows us
    to force-route requests to the primary via 1102b0b67 (praefect:
    Implement force-routing to primary for node-manager router, 2021-02-03)
    and 4d877d7d5 (praefect: Implement force-routing to primary for per-repo
    router, 2021-02-03). So let's set that header if we know that we're
    being executed via a hook, which is the only case where a quarantine
    environment may exist.
    edab619a
gitaly_client_spec.rb 18.9 KB