Skip to content

Add test to validate route-to-primary gRPC metadata

Stan Hu requested to merge sh-validate-grpc-metadata into master

What does this MR do and why?

!119765 (merged) introduced a regression that caused GitLab Rails to drop the route-to-primary gRPC metadata, which caused any changes pushed to Gitaly Cluster repositories to fail intermittently: gitlab-com/gl-infra/production#14259 (closed).

To ensure this behavior is maintained, we add a GRPC interceptor in the /api/v4/internal/base request test and ensure ensuing gRPC requests have this metadata. Note that we can only test this with project and snippet repositories because the Git change access checks appear to invoke gRPC calls, while Wiki repositories don't appear to invoke any gRPC calls.

How to test locally

  1. Run bundle exec rspec spec/requests/api/internal/base_spec.rb. Tests should pass.
  2. On this branch, cherry-pick changes in !119765 (merged). There are some minor conflicts to resolve.
  3. Run bundle exec rspec spec/requests/api/internal/base_spec.rb. Tests should fail:
Failures:

  1) API::Internal::Base POST /internal/allowed access granted git push with personal snippet behaves like sets hook env and routes to primary sets env in RequestStore and routes gRPC messages to primary
     Failure/Error: expect(interceptor.route_to_primary_received?).to be_truthy

       expected: truthy value
            got: false
     Shared Example Group: "sets hook env and routes to primary" called from ./spec/requests/api/internal/base_spec.rb:632
     # ./spec/requests/api/internal/base_spec.rb:572:in `block (5 levels) in <top (required)>'
     # ./spec/requests/api/internal/base_spec.rb:509:in `block (5 levels) in <top (required)>'
     # ./spec/requests/api/internal/base_spec.rb:509:in `block (4 levels) in <top (required)>'
     # ./spec/spec_helper.rb:423:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:18:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:415:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:411:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:61:in `with_raw_context'
     # ./spec/spec_helper.rb:411:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:378:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/with_request_store.rb:17:in `enabling_request_store'
     # ./lib/gitlab/with_request_store.rb:10:in `with_request_store'
     # ./spec/spec_helper.rb:378:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:242:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
     # ./spec/support/redis.rb:17:in `block (3 levels) in <main>'
     # ./spec/support/redis.rb:17:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:108:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:62:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:108:in `block (2 levels) in <main>'

  2) API::Internal::Base POST /internal/allowed access granted git push with project snippet behaves like sets hook env and routes to primary sets env in RequestStore and routes gRPC messages to primary
     Failure/Error: expect(interceptor.route_to_primary_received?).to be_truthy

       expected: truthy value
            got: false
     Shared Example Group: "sets hook env and routes to primary" called from ./spec/requests/api/internal/base_spec.rb:664
     # ./spec/requests/api/internal/base_spec.rb:572:in `block (5 levels) in <top (required)>'
     # ./spec/requests/api/internal/base_spec.rb:509:in `block (5 levels) in <top (required)>'
     # ./spec/requests/api/internal/base_spec.rb:509:in `block (4 levels) in <top (required)>'
     # ./spec/spec_helper.rb:423:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:18:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:415:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:411:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:61:in `with_raw_context'
     # ./spec/spec_helper.rb:411:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:378:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/with_request_store.rb:17:in `enabling_request_store'
     # ./lib/gitlab/with_request_store.rb:10:in `with_request_store'
     # ./spec/spec_helper.rb:378:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:242:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
     # ./spec/support/redis.rb:17:in `block (3 levels) in <main>'
     # ./spec/support/redis.rb:17:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:108:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:62:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:108:in `block (2 levels) in <main>'

Finished in 1 minute 28.8 seconds (files took 14.16 seconds to load)
146 examples, 2 failures

Failed examples:

rspec './spec/requests/api/internal/base_spec.rb[1:7:1:3:2:1]' # API::Internal::Base POST /internal/allowed access granted git push with personal snippet behaves like sets hook env and routes to primary sets env in RequestStore and routes gRPC messages to primary
rspec './spec/requests/api/internal/base_spec.rb[1:7:1:5:2:1]' # API::Internal::Base POST /internal/allowed access granted git push with project snippet behaves like sets hook env and routes to primary sets env in RequestStore and routes gRPC messages to primary

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Stan Hu

Merge request reports