Commit 3f5e3903 authored by Francisco Javier López's avatar Francisco Javier López Committed by Nick Thomas

Added new endpoint to proxy websocket job requests

This commit adds a new endpoints to the job's controller. This
endpoint will be used to proxy websockets requests to services running
inside build containers.
parent 119f9509
......@@ -208,3 +208,5 @@ class Projects::JobsController < Projects::ApplicationController
'attachment'
end
end
Projects::JobsController.prepend(EE::Projects::JobsController)
......@@ -37,3 +37,5 @@ module Ci
end
end
end
Ci::BuildRunnerSession.prepend(EE::Ci::BuildRunnerSession)
# frozen_string_literal: true
module EE
module Projects
module JobsController
extend ActiveSupport::Concern
prepended do
before_action :authorize_create_proxy_build!, only: :proxy_websocket_authorize
before_action :verify_proxy_request!, only: :proxy_websocket_authorize
end
def proxy_websocket_authorize
render json: proxy_websocket_service(build_service_specification)
end
private
def authorize_create_proxy_build!
return access_denied! unless can?(current_user, :create_build_service_proxy, build)
end
def verify_proxy_request!
::Gitlab::Workhorse.verify_api_request!(request.headers)
set_workhorse_internal_api_content_type
end
# This method provides the information to Workhorse
# about the service we want to proxy to.
# For security reasons, in case this operation is started by JS,
# it's important to use only sourced GitLab JS code
def proxy_websocket_service(service)
service[:url] = ::Gitlab::UrlHelpers.as_wss(service[:url])
::Gitlab::Workhorse.channel_websocket(service)
end
def build_service_specification
build.service_specification(service: params['service'],
port: params['port'],
path: params['path'],
subprotocols: proxy_subprotocol)
end
def proxy_subprotocol
# This will allow to reuse the same subprotocol set
# in the original websocket connection
request.headers['HTTP_SEC_WEBSOCKET_PROTOCOL'].presence || ::Ci::BuildRunnerSession::TERMINAL_SUBPROTOCOL
end
end
end
end
......@@ -18,6 +18,7 @@ module EE
prepended do
after_save :stick_build_if_status_changed
delegate :service_specification, to: :runner_session, allow_nil: true
has_many :sourced_pipelines,
class_name: ::Ci::Sources::Pipeline,
......
# frozen_string_literal: true
module EE
module Ci
module BuildRunnerSession
extend ActiveSupport::Concern
DEFAULT_SERVICE_NAME = 'build'.freeze
DEFAULT_PORT_NAME = 'default_port'.freeze
def service_specification(service: nil, path: nil, port: nil, subprotocols: nil)
return {} unless url.present?
port = port.presence || DEFAULT_PORT_NAME
service = service.presence || DEFAULT_SERVICE_NAME
url = "#{self.url}/proxy/#{service}/#{port}/#{path}"
subprotocols = subprotocols.presence || ::Ci::BuildRunnerSession::TERMINAL_SUBPROTOCOL
channel_specification(url, subprotocols)
end
end
end
end
......@@ -28,14 +28,20 @@ class WebIdeTerminal
terminal_project_job_path(project, build, format: :ws)
end
def proxy_websocket_path
proxy_project_job_path(project, build, format: :ws)
end
private
def web_ide_terminal_route_generator(action)
url_for(action: action,
controller: 'projects/web_ide_terminals',
namespace_id: project.namespace.to_param,
project_id: project.to_param,
id: build.id,
only_path: true)
def web_ide_terminal_route_generator(action, options = {})
options.reverse_merge!(action: action,
controller: 'projects/web_ide_terminals',
namespace_id: project.namespace.to_param,
project_id: project.to_param,
id: build.id,
only_path: true)
url_for(options)
end
end
......@@ -26,6 +26,11 @@ module EE
rule { can?(:update_web_ide_terminal) & terminal }.policy do
enable :create_build_terminal
enable :create_build_service_proxy
end
rule { ~can?(:build_service_proxy_enabled) }.policy do
prevent :create_build_service_proxy
end
private
......
......@@ -192,6 +192,10 @@ module EE
@subject.feature_available?(:web_ide_terminal)
end
condition(:build_service_proxy_enabled) do
::Feature.enabled?(:build_service_proxy, @subject)
end
rule { web_ide_terminal_available & can?(:create_pipeline) & can?(:maintainer_access) }.enable :create_web_ide_terminal
# Design abilities could also be prevented in the issue policy.
......@@ -201,6 +205,8 @@ module EE
prevent :create_design
prevent :destroy_design
end
rule { build_service_proxy_enabled }.enable :build_service_proxy_enabled
end
end
end
......
......@@ -7,4 +7,5 @@ class WebIdeTerminalEntity < Grape::Entity
expose :cancel_path
expose :retry_path
expose :terminal_path
expose :proxy_websocket_path, if: ->(_) { Feature.enabled?(:build_service_proxy) }
end
---
title: Proxy websocket requests to build services
merge_request: 9723
author:
type: added
......@@ -47,6 +47,13 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
get :download
end
end
resources :jobs, only: [], constraints: { id: /\d+/ } do
member do
get '/proxy.ws/authorize', to: 'jobs#proxy_websocket_authorize', constraints: { format: nil }
get :proxy
end
end
end
namespace :settings do
......
# frozen_string_literal: true
require 'spec_helper'
describe Projects::JobsController do
let(:owner) { create(:owner) }
let(:admin) { create(:admin) }
let(:maintainer) { create(:user) }
let(:developer) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
let(:project) { create(:project, :private, :repository, namespace: owner.namespace) }
let(:pipeline) { create(:ci_pipeline, project: project, source: :webide, config_source: :webide_source, user: user) }
let(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline, user: user) }
let(:user) { maintainer }
let(:extra_params) { { id: job.id } }
before do
stub_licensed_features(web_ide_terminal: true)
stub_feature_flags(build_service_proxy: true)
allow(job).to receive(:has_terminal?).and_return(true)
project.add_maintainer(maintainer)
project.add_developer(developer)
project.add_reporter(reporter)
project.add_guest(guest)
sign_in(user)
end
shared_examples 'proxy access rights' do
before do
allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
make_request
end
context 'with admin' do
let(:user) { admin }
it 'returns 200' do
expect(response).to have_gitlab_http_status(200)
end
end
context 'with owner' do
let(:user) { owner }
it 'returns 200' do
expect(response).to have_gitlab_http_status(200)
end
end
context 'with maintainer' do
let(:user) { maintainer }
it 'returns 200' do
expect(response).to have_gitlab_http_status(200)
end
end
context 'with developer' do
let(:user) { developer }
it 'returns 404' do
expect(response).to have_gitlab_http_status(404)
end
end
context 'with reporter' do
let(:user) { reporter }
it 'returns 404' do
expect(response).to have_gitlab_http_status(404)
end
end
context 'with guest' do
let(:user) { guest }
it 'returns 404' do
expect(response).to have_gitlab_http_status(404)
end
end
context 'with non member' do
let(:user) { create(:user) }
it 'returns 404' do
expect(response).to have_gitlab_http_status(404)
end
end
end
shared_examples 'when pipeline is not from a webide source' do
context 'with admin' do
let(:user) { admin }
let(:pipeline) { create(:ci_pipeline, project: project, source: :chat, user: user) }
before do
allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
make_request
end
it 'returns 404' do
expect(response).to have_gitlab_http_status(404)
end
end
end
shared_examples 'validates workhorse signature' do
context 'with valid workhorse signature' do
before do
allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
end
context 'and valid id' do
it 'returns the proxy data for the service running in the job' do
make_request
expect(response).to have_gitlab_http_status(200)
expect(response.headers["Content-Type"]).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
expect(response.body).to eq(expected_data)
end
end
context 'and invalid id' do
let(:extra_params) { { id: 1234 } }
it 'returns 404' do
make_request
expect(response).to have_gitlab_http_status(404)
end
end
end
context 'with invalid workhorse signature' do
it 'aborts with an exception' do
allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_raise(JWT::DecodeError)
expect { make_request }.to raise_error(JWT::DecodeError)
end
end
end
shared_examples 'feature flag "build_service_proxy" is disabled' do
let(:user) { admin }
it 'returns 404' do
allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
stub_feature_flags(build_service_proxy: false)
make_request
expect(response).to have_gitlab_http_status(404)
end
end
describe 'GET #proxy_websocket_authorize' do
let(:path) { :proxy_websocket_authorize }
let(:render_method) { :channel_websocket }
let(:expected_data) do
{
'Channel' => {
'Subprotocols' => ["terminal.gitlab.com"],
'Url' => 'wss://localhost/proxy/build/default_port/',
'Header' => {
'Authorization' => [nil]
},
'MaxSessionTime' => nil,
'CAPem' => nil
}
}.to_json
end
it_behaves_like 'proxy access rights'
it_behaves_like 'when pipeline is not from a webide source'
it_behaves_like 'validates workhorse signature'
it_behaves_like 'feature flag "build_service_proxy" is disabled'
it 'converts the url scheme into wss' do
allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil)
expect(job.runner_session_url).to start_with('https://')
expect(Gitlab::Workhorse).to receive(:channel_websocket).with(a_hash_including(url: "wss://localhost/proxy/build/default_port/"))
make_request
end
end
def make_request
params = {
namespace_id: project.namespace.to_param,
project_id: project
}
get path, params: params.merge(extra_params)
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Ci::BuildRunnerSession, model: true do
let!(:build) { create(:ci_build, :with_runner_session) }
subject { build.runner_session }
describe '#service_specification' do
let(:service) { 'foo'}
let(:port) { 80 }
let(:path) { 'path' }
let(:subprotocols) { nil }
let(:specification) { subject.service_specification(service: service, port: port, path: path, subprotocols: subprotocols) }
it 'returns service proxy url' do
expect(specification[:url]).to eq "https://localhost/proxy/#{service}/#{port}/#{path}"
end
it 'returns default service proxy websocket subprotocol' do
expect(specification[:subprotocols]).to eq %w[terminal.gitlab.com]
end
it 'returns empty hash if no url' do
subject.url = ''
expect(specification).to be_empty
end
context 'when port is not present' do
let(:port) { nil }
it 'uses the default port name' do
expect(specification[:url]).to eq "https://localhost/proxy/#{service}/default_port/#{path}"
end
end
context 'when the service is not present' do
let(:service) { '' }
it 'uses the service name "build" as default' do
expect(specification[:url]).to eq "https://localhost/proxy/build/#{port}/#{path}"
end
end
context 'when url is present' do
it 'returns ca_pem nil if empty certificate' do
subject.certificate = ''
expect(specification[:ca_pem]).to be_nil
end
it 'adds Authorization header if authorization is present' do
subject.authorization = 'foobar'
expect(specification[:headers]).to include(Authorization: ['foobar'])
end
end
context 'when subprotocol is present' do
let(:subprotocols) { 'foobar' }
it 'returns the new subprotocol' do
expect(specification[:subprotocols]).to eq [subprotocols]
end
end
end
end
......@@ -22,4 +22,8 @@ describe WebIdeTerminal do
it 'returns the terminal_path of the build' do
expect(subject.terminal_path).to end_with("/jobs/#{build.id}/terminal.ws")
end
it 'returns the proxy_websocket_path of the build' do
expect(subject.proxy_websocket_path).to end_with("/jobs/#{build.id}/proxy.ws")
end
end
......@@ -34,7 +34,7 @@ describe Ci::BuildPolicy do
end
describe 'manage a web ide terminal' do
let(:build_permissions) { %i[read_web_ide_terminal create_build_terminal update_web_ide_terminal] }
let(:build_permissions) { %i[read_web_ide_terminal create_build_terminal update_web_ide_terminal create_build_service_proxy] }
set(:maintainer) { create(:user) }
let(:owner) { create(:owner) }
let(:admin) { create(:admin) }
......@@ -79,7 +79,7 @@ describe Ci::BuildPolicy do
context 'when build is not from a webide pipeline' do
let(:pipeline) { create(:ci_empty_pipeline, project: project, source: :chat) }
it { expect_disallowed(:read_web_ide_terminal, :update_web_ide_terminal) }
it { expect_disallowed(:read_web_ide_terminal, :update_web_ide_terminal, :create_build_service_proxy) }
end
context 'when build has no runner terminal' do
......@@ -88,7 +88,15 @@ describe Ci::BuildPolicy do
end
it { expect_allowed(:read_web_ide_terminal, :update_web_ide_terminal) }
it { expect_disallowed(:create_build_terminal) }
it { expect_disallowed(:create_build_terminal, :create_build_service_proxy) }
end
context 'feature flag "build_service_proxy" is disabled' do
before do
stub_feature_flags(build_service_proxy: false)
end
it { expect_disallowed(:create_build_service_proxy) }
end
end
......
......@@ -14,4 +14,20 @@ describe WebIdeTerminalEntity do
it { is_expected.to have_key(:cancel_path) }
it { is_expected.to have_key(:retry_path) }
it { is_expected.to have_key(:terminal_path) }
context 'when feature flag build_service_proxy is enabled' do
before do
stub_feature_flags(build_service_proxy: true)
end
it { is_expected.to have_key(:proxy_websocket_path) }
end
context 'when feature flag build_service_proxy is disabled' do
before do
stub_feature_flags(build_service_proxy: false)
end
it { is_expected.not_to have_key(:proxy_websocket_path) }
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment