From cd40885e4c6620cade4d9ab2c0b2bcd020f0213f Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Mon, 15 Sep 2025 11:29:26 -1000 Subject: [PATCH] Clean up FF geo_proxy_fetch_direct_to_primary It is hard-coded to true in gitlab-org/gitlab --- internal/command/uploadpack/uploadpack.go | 20 +-- internal/gitlabnet/accessverifier/client.go | 1 - .../gitlabnet/accessverifier/client_test.go | 9 +- .../responses/allowed_with_pull_payload.json | 1 - ...itlab_shell_custom_git_upload_pack_spec.rb | 118 ------------------ 5 files changed, 9 insertions(+), 140 deletions(-) delete mode 100644 spec/gitlab_shell_custom_git_upload_pack_spec.rb diff --git a/internal/command/uploadpack/uploadpack.go b/internal/command/uploadpack/uploadpack.go index 8e9c3436..8d545734 100644 --- a/internal/command/uploadpack/uploadpack.go +++ b/internal/command/uploadpack/uploadpack.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/gitauditevent" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/readwriter" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/shared/accessverifier" - "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/shared/customaction" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/shared/disallowedcommand" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" ) @@ -47,23 +46,14 @@ func (c *Command) Execute(ctx context.Context) (context.Context, error) { ctxWithLogData := context.WithValue(ctx, logDataKey{}, logData) if response.IsCustomAction() { - if response.Payload.Data.GeoProxyFetchDirectToPrimary { - cmd := githttp.PullCommand{ - Config: c.Config, - ReadWriter: c.ReadWriter, - Args: c.Args, - Response: response, - } - - return ctxWithLogData, cmd.Execute(ctx) - } - - customAction := customaction.Command{ + cmd := githttp.PullCommand{ Config: c.Config, ReadWriter: c.ReadWriter, - EOFSent: false, + Args: c.Args, + Response: response, } - return ctxWithLogData, customAction.Execute(ctx, response) + + return ctxWithLogData, cmd.Execute(ctx) } stats, err := c.performGitalyCall(ctx, response) diff --git a/internal/gitlabnet/accessverifier/client.go b/internal/gitlabnet/accessverifier/client.go index ead052fe..6bdf8f4b 100644 --- a/internal/gitlabnet/accessverifier/client.go +++ b/internal/gitlabnet/accessverifier/client.go @@ -54,7 +54,6 @@ type CustomPayloadData struct { UserID string `json:"gl_id,omitempty"` RequestHeaders map[string]string `json:"request_headers"` GeoProxyDirectToPrimary bool `json:"geo_proxy_direct_to_primary"` - GeoProxyFetchDirectToPrimary bool `json:"geo_proxy_fetch_direct_to_primary"` GeoProxyFetchSSHDirectToPrimary bool `json:"geo_proxy_fetch_ssh_direct_to_primary"` GeoProxyPushSSHDirectToPrimary bool `json:"geo_proxy_push_ssh_direct_to_primary"` } diff --git a/internal/gitlabnet/accessverifier/client_test.go b/internal/gitlabnet/accessverifier/client_test.go index 706c8941..dd571c42 100644 --- a/internal/gitlabnet/accessverifier/client_test.go +++ b/internal/gitlabnet/accessverifier/client_test.go @@ -142,11 +142,10 @@ func TestGeoPullGetCustomAction(t *testing.T) { response.Payload = CustomPayload{ Action: "geo_proxy_to_primary", Data: CustomPayloadData{ - APIEndpoints: []string{"geo/proxy_git_ssh/info_refs_upload_pack", "geo/proxy_git_ssh/upload_pack"}, - Username: "custom", - GeoProxyFetchDirectToPrimary: true, - PrimaryRepo: "https://repo/path", - RequestHeaders: map[string]string{"Authorization": "Bearer token"}, + APIEndpoints: []string{"geo/proxy_git_ssh/info_refs_upload_pack", "geo/proxy_git_ssh/upload_pack"}, + Username: "custom", + PrimaryRepo: "https://repo/path", + RequestHeaders: map[string]string{"Authorization": "Bearer token"}, }, } response.StatusCode = 300 diff --git a/internal/testhelper/testdata/testroot/responses/allowed_with_pull_payload.json b/internal/testhelper/testdata/testroot/responses/allowed_with_pull_payload.json index 82a108d8..67143eea 100644 --- a/internal/testhelper/testdata/testroot/responses/allowed_with_pull_payload.json +++ b/internal/testhelper/testdata/testroot/responses/allowed_with_pull_payload.json @@ -30,7 +30,6 @@ ], "gl_username": "custom", "primary_repo": "https://repo/path", - "geo_proxy_fetch_direct_to_primary": true, "request_headers": { "Authorization": "Bearer token" } } }, diff --git a/spec/gitlab_shell_custom_git_upload_pack_spec.rb b/spec/gitlab_shell_custom_git_upload_pack_spec.rb deleted file mode 100644 index 6770344a..00000000 --- a/spec/gitlab_shell_custom_git_upload_pack_spec.rb +++ /dev/null @@ -1,118 +0,0 @@ -require_relative 'spec_helper' - -require 'open3' -require 'json' -require 'base64' - -describe 'Custom bin/gitlab-shell git-upload-pack' do - include_context 'gitlab shell' - - let(:env) { {'SSH_CONNECTION' => 'fake', 'SSH_ORIGINAL_COMMAND' => 'git-upload-pack group/repo' } } - let(:divider) { "remote: ========================================================================\n" } - - before(:context) do - write_config("gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}") - end - - def mock_server(server) - server.mount_proc('/geo/proxy_git_ssh/info_refs_upload_pack') do |req, res| - res.content_type = 'application/json' - res.status = 200 - - res.body = {"result" => "#{Base64.encode64('custom')}"}.to_json - end - - server.mount_proc('/geo/proxy_git_ssh/upload_pack') do |req, res| - res.content_type = 'application/json' - res.status = 200 - - output = JSON.parse(req.body)['output'] - - res.body = {"result" => output}.to_json - end - - server.mount_proc('/api/v4/internal/allowed') do |req, res| - res.content_type = 'application/json' - - key_id = req.query['key_id'] || req.query['username'] - - unless key_id - body = JSON.parse(req.body) - key_id = body['key_id'] || body['username'].to_s - end - - case key_id - when '100', 'someone' then - res.status = 300 - body = { - "gl_id" => "user-100", - "status" => true, - "payload" => { - "action" => "geo_proxy_to_primary", - "data" => { - "api_endpoints" => ["/geo/proxy_git_ssh/info_refs_upload_pack", "/geo/proxy_git_ssh/upload_pack"], - "gl_username" => "custom", - "primary_repo" => "https://repo/path" - }, - }, - "gl_console_messages" => ["console", "message"] - } - res.body = body.to_json - else - res.status = 403 - end - end - end - - describe 'dialog for performing a custom action' do - context 'when API calls perform successfully' do - let(:remote_blank_line) { "remote: \n" } - def verify_successful_call!(cmd) - Open3.popen3(env, cmd) do |stdin, stdout, stderr| - expect(stderr.gets).to eq(remote_blank_line) - expect(stderr.gets).to eq("remote: console\n") - expect(stderr.gets).to eq("remote: message\n") - expect(stderr.gets).to eq(remote_blank_line) - - stdin.puts("0032want 343d70886785dc1f98aaf70f3b4ca87c93a5d0dd\n") - stdin.close - - expect(stdout.gets(6)).to eq("custom") - expect(stdout.flush.read).to eq("0032want 343d70886785dc1f98aaf70f3b4ca87c93a5d0dd\n") - end - end - - context 'when key is provided' do - let(:cmd) { "#{gitlab_shell_path} key-100" } - - it 'custom action is performed' do - verify_successful_call!(cmd) - end - end - - context 'when username is provided' do - let(:cmd) { "#{gitlab_shell_path} username-someone" } - - it 'custom action is performed' do - verify_successful_call!(cmd) - end - end - end - - context 'when API error occurs' do - let(:cmd) { "#{gitlab_shell_path} key-101" } - - it 'custom action is not performed' do - Open3.popen2e(env, cmd) do |stdin, stdout| - expect(stdout.gets).to eq("remote: \n") - expect(stdout.gets).to eq(divider) - expect(stdout.gets).to eq("remote: \n") - expect(stdout.gets).to eq("remote: Internal API error (403)\n") - expect(stdout.gets).to eq("remote: \n") - expect(stdout.gets).to eq(divider) - expect(stdout.gets).to eq("remote: \n") - end - end - end - end -end -- GitLab