diff --git a/internal/command/receivepack/receivepack.go b/internal/command/receivepack/receivepack.go index 2c75eb4d1db30bfd41f258f47b976020ad07ecfe..e39dfbdd376022e1797d6069a466cac672787c5e 100644 --- a/internal/command/receivepack/receivepack.go +++ b/internal/command/receivepack/receivepack.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command/githttp" "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" ) @@ -38,26 +37,13 @@ func (c *Command) Execute(ctx context.Context) (context.Context, error) { )) if response.IsCustomAction() { - // When `geo_proxy_direct_to_primary` feature flag is enabled, a Git over HTTP direct request - // to primary repo is performed instead of proxying the request through Gitlab Rails. - // After the feature flag is enabled by default and removed, - // custom action functionality will be removed along with it. - if response.Payload.Data.GeoProxyDirectToPrimary { - cmd := githttp.PushCommand{ - Config: c.Config, - ReadWriter: c.ReadWriter, - Response: response, - } - - return ctxWithLogData, cmd.Execute(ctx) - } - - customAction := customaction.Command{ + cmd := githttp.PushCommand{ Config: c.Config, ReadWriter: c.ReadWriter, - EOFSent: true, + Response: response, } - return ctxWithLogData, customAction.Execute(ctx, response) + + return ctxWithLogData, cmd.Execute(ctx) } err = c.performGitalyCall(ctx, response) diff --git a/internal/command/receivepack/receivepack_test.go b/internal/command/receivepack/receivepack_test.go index 78561d365d74f38f7b3c81ec6ca479def92ba4d8..16cf830e7dbc42aa76a6e11651e9caa758905c1b 100644 --- a/internal/command/receivepack/receivepack_test.go +++ b/internal/command/receivepack/receivepack_test.go @@ -38,14 +38,6 @@ func TestForbiddenAccess(t *testing.T) { require.Equal(t, "Disallowed by API call", err.Error()) } -func TestCustomReceivePack(t *testing.T) { - cmd, output := setup(t, "1", requesthandlers.BuildAllowedWithCustomActionsHandlers(t)) - - _, err := cmd.Execute(context.Background()) - require.NoError(t, err) - require.Equal(t, "customoutput", output.String()) -} - func setup(t *testing.T, keyId string, requests []testserver.TestRequestHandler) (*Command, *bytes.Buffer) { url := testserver.StartSocketHttpServer(t, requests) diff --git a/internal/command/uploadpack/uploadpack.go b/internal/command/uploadpack/uploadpack.go index f4d89bbee63e0c952c34b3dc761338be249b4b4c..2167fa66275e5223796f3c68b8ed869e8fafce02 100644 --- a/internal/command/uploadpack/uploadpack.go +++ b/internal/command/uploadpack/uploadpack.go @@ -9,7 +9,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" ) @@ -39,22 +38,13 @@ func (c *Command) Execute(ctx context.Context) (context.Context, error) { ctxWithLogData := context.WithValue(ctx, "logData", logData) if response.IsCustomAction() { - if response.Payload.Data.GeoProxyFetchDirectToPrimary { - cmd := githttp.PullCommand{ - Config: c.Config, - ReadWriter: c.ReadWriter, - Response: response, - } - - return ctxWithLogData, cmd.Execute(ctx) - } - - customAction := customaction.Command{ + cmd := githttp.PullCommand{ Config: c.Config, ReadWriter: c.ReadWriter, - EOFSent: false, + 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 2cd14b88267c7753919a2247e964518c3f55c183..4818beeac91ded76b6d3b4a7012e36aef367de07 100644 --- a/internal/gitlabnet/accessverifier/client.go +++ b/internal/gitlabnet/accessverifier/client.go @@ -48,8 +48,6 @@ type CustomPayloadData struct { PrimaryRepo string `json:"primary_repo"` 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"` GeoProxyFetchDirectToPrimaryWithOptions bool `json:"geo_proxy_fetch_direct_to_primary_with_options"` } diff --git a/internal/gitlabnet/accessverifier/client_test.go b/internal/gitlabnet/accessverifier/client_test.go index 4e9cc4d55d79b711dc4d7b7ee3701b6af5b62108..7ba197bdf8617c2b452f3d5b3f67f0c9aeffb957 100644 --- a/internal/gitlabnet/accessverifier/client_test.go +++ b/internal/gitlabnet/accessverifier/client_test.go @@ -112,7 +112,6 @@ func TestGeoPushGetCustomAction(t *testing.T) { Action: "geo_proxy_to_primary", Data: CustomPayloadData{ ApiEndpoints: []string{"geo/proxy_git_ssh/info_refs_receive_pack", "geo/proxy_git_ssh/receive_pack"}, - GeoProxyDirectToPrimary: true, RequestHeaders: map[string]string{"Authorization": "Bearer token"}, Username: "custom", PrimaryRepo: "https://repo/path", @@ -143,7 +142,6 @@ func TestGeoPullGetCustomAction(t *testing.T) { 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"}, }, diff --git a/spec/gitlab_shell_custom_git_receive_pack_spec.rb b/spec/gitlab_shell_custom_git_receive_pack_spec.rb deleted file mode 100644 index 1dce4bbab8193e33416cfcb248ca0c36e6873a67..0000000000000000000000000000000000000000 --- a/spec/gitlab_shell_custom_git_receive_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-receive-pack' do - include_context 'gitlab shell' - - let(:env) { {'SSH_CONNECTION' => 'fake', 'SSH_ORIGINAL_COMMAND' => 'git-receive-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_receive_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/receive_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_receive_pack", "/geo/proxy_git_ssh/receive_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("0009input") - stdin.close - - expect(stdout.gets(6)).to eq("custom") - expect(stdout.flush.read).to eq("0009input") - 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 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 6770344a5c21bbab651a74583198d7c11f0694e1..0000000000000000000000000000000000000000 --- 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