summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrancisco Javier López <fjlopez@gitlab.com>2019-04-04 18:32:02 +0000
committerNick Thomas <nick@gitlab.com>2019-04-04 18:32:02 +0000
commit8a134f4c6505c4f8f3c89e0ae4d4ea2293765be3 (patch)
tree910dce4df7fd61af86a9f282f52fcc72ff056975
parent465f82e32cd1ca7c87ca7553e350af4c52b00805 (diff)
downloadgitlab-ce-8a134f4c6505c4f8f3c89e0ae4d4ea2293765be3.tar.gz
Renamed terminal_specification to channel_specification
We're moving from using terminology related to terminals when we refer to Websockets connections in Workhorse. It's more appropiate a concept like channel.
-rw-r--r--GITLAB_WORKHORSE_VERSION2
-rw-r--r--app/controllers/projects/environments_controller.rb2
-rw-r--r--app/controllers/projects/jobs_controller.rb2
-rw-r--r--app/models/ci/build_runner_session.rb18
-rw-r--r--lib/gitlab/url_helpers.rb16
-rw-r--r--lib/gitlab/workhorse.rb14
-rw-r--r--spec/controllers/projects/environments_controller_spec.rb2
-rw-r--r--spec/controllers/projects/jobs_controller_spec.rb2
-rw-r--r--spec/lib/gitlab/workhorse_spec.rb10
-rw-r--r--spec/models/ci/build_runner_session_spec.rb16
10 files changed, 60 insertions, 24 deletions
diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION
index a2f28f43be3..6d2890793d4 100644
--- a/GITLAB_WORKHORSE_VERSION
+++ b/GITLAB_WORKHORSE_VERSION
@@ -1 +1 @@
-8.4.0
+8.5.0
diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb
index 4fa6cd94ae5..301449cfa90 100644
--- a/app/controllers/projects/environments_controller.rb
+++ b/app/controllers/projects/environments_controller.rb
@@ -117,7 +117,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController
terminal = environment.terminals.try(:first)
if terminal
set_workhorse_internal_api_content_type
- render json: Gitlab::Workhorse.terminal_websocket(terminal)
+ render json: Gitlab::Workhorse.channel_websocket(terminal)
else
render html: 'Not found', status: :not_found
end
diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index 35cc32d3e63..2a4933e7bc2 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -157,7 +157,7 @@ class Projects::JobsController < Projects::ApplicationController
# GET .../terminal.ws : implemented in gitlab-workhorse
def terminal_websocket_authorize
set_workhorse_internal_api_content_type
- render json: Gitlab::Workhorse.terminal_websocket(@build.terminal_specification)
+ render json: Gitlab::Workhorse.channel_websocket(@build.terminal_specification)
end
private
diff --git a/app/models/ci/build_runner_session.rb b/app/models/ci/build_runner_session.rb
index 061eff090f5..80dbb150085 100644
--- a/app/models/ci/build_runner_session.rb
+++ b/app/models/ci/build_runner_session.rb
@@ -6,6 +6,8 @@ module Ci
class BuildRunnerSession < ApplicationRecord
extend Gitlab::Ci::Model
+ TERMINAL_SUBPROTOCOL = 'terminal.gitlab.com'.freeze
+
self.table_name = 'ci_builds_runner_session'
belongs_to :build, class_name: 'Ci::Build', inverse_of: :runner_session
@@ -14,11 +16,21 @@ module Ci
validates :url, url: { protocols: %w(https) }
def terminal_specification
- return {} unless url.present?
+ wss_url = Gitlab::UrlHelpers.as_wss(self.url)
+ return {} unless wss_url.present?
+
+ wss_url = "#{wss_url}/exec"
+ channel_specification(wss_url, TERMINAL_SUBPROTOCOL)
+ end
+
+ private
+
+ def channel_specification(url, subprotocol)
+ return {} if subprotocol.blank? || url.blank?
{
- subprotocols: ['terminal.gitlab.com'].freeze,
- url: "#{url}/exec".sub("https://", "wss://"),
+ subprotocols: Array(subprotocol),
+ url: url,
headers: { Authorization: [authorization.presence] }.compact,
ca_pem: certificate.presence
}
diff --git a/lib/gitlab/url_helpers.rb b/lib/gitlab/url_helpers.rb
new file mode 100644
index 00000000000..883585c196f
--- /dev/null
+++ b/lib/gitlab/url_helpers.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module Gitlab
+ class UrlHelpers
+ WSS_PROTOCOL = "wss".freeze
+ def self.as_wss(url)
+ return unless url.present?
+
+ URI.parse(url).tap do |uri|
+ uri.scheme = WSS_PROTOCOL
+ end.to_s
+ rescue URI::InvalidURIError
+ nil
+ end
+ end
+end
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index 83eabb8d674..533757d2237 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -162,16 +162,16 @@ module Gitlab
]
end
- def terminal_websocket(terminal)
+ def channel_websocket(channel)
details = {
- 'Terminal' => {
- 'Subprotocols' => terminal[:subprotocols],
- 'Url' => terminal[:url],
- 'Header' => terminal[:headers],
- 'MaxSessionTime' => terminal[:max_session_time]
+ 'Channel' => {
+ 'Subprotocols' => channel[:subprotocols],
+ 'Url' => channel[:url],
+ 'Header' => channel[:headers],
+ 'MaxSessionTime' => channel[:max_session_time]
}
}
- details['Terminal']['CAPem'] = terminal[:ca_pem] if terminal.key?(:ca_pem)
+ details['Channel']['CAPem'] = channel[:ca_pem] if channel.key?(:ca_pem)
details
end
diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb
index 2dca2c3976f..43639875265 100644
--- a/spec/controllers/projects/environments_controller_spec.rb
+++ b/spec/controllers/projects/environments_controller_spec.rb
@@ -283,7 +283,7 @@ describe Projects::EnvironmentsController do
.and_return([:fake_terminal])
expect(Gitlab::Workhorse)
- .to receive(:terminal_websocket)
+ .to receive(:channel_websocket)
.with(:fake_terminal)
.and_return(workhorse: :response)
diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb
index d8a331b3cf0..23e4e9806c2 100644
--- a/spec/controllers/projects/jobs_controller_spec.rb
+++ b/spec/controllers/projects/jobs_controller_spec.rb
@@ -989,7 +989,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
context 'and valid id' do
it 'returns the terminal for the job' do
expect(Gitlab::Workhorse)
- .to receive(:terminal_websocket)
+ .to receive(:channel_websocket)
.and_return(workhorse: :response)
get_terminal_websocket(id: job.id)
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index fed7834e2a9..f8ce399287a 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -94,7 +94,7 @@ describe Gitlab::Workhorse do
end
end
- describe '.terminal_websocket' do
+ describe '.channel_websocket' do
def terminal(ca_pem: nil)
out = {
subprotocols: ['foo'],
@@ -108,25 +108,25 @@ describe Gitlab::Workhorse do
def workhorse(ca_pem: nil)
out = {
- 'Terminal' => {
+ 'Channel' => {
'Subprotocols' => ['foo'],
'Url' => 'wss://example.com/terminal.ws',
'Header' => { 'Authorization' => ['Token x'] },
'MaxSessionTime' => 600
}
}
- out['Terminal']['CAPem'] = ca_pem if ca_pem
+ out['Channel']['CAPem'] = ca_pem if ca_pem
out
end
context 'without ca_pem' do
- subject { described_class.terminal_websocket(terminal) }
+ subject { described_class.channel_websocket(terminal) }
it { is_expected.to eq(workhorse) }
end
context 'with ca_pem' do
- subject { described_class.terminal_websocket(terminal(ca_pem: "foo")) }
+ subject { described_class.channel_websocket(terminal(ca_pem: "foo")) }
it { is_expected.to eq(workhorse(ca_pem: "foo")) }
end
diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb
index a52c10019e6..e51fd009f50 100644
--- a/spec/models/ci/build_runner_session_spec.rb
+++ b/spec/models/ci/build_runner_session_spec.rb
@@ -13,25 +13,33 @@ describe Ci::BuildRunnerSession, model: true do
it { is_expected.to validate_presence_of(:url).with_message('must be a valid URL') }
describe '#terminal_specification' do
- let(:terminal_specification) { subject.terminal_specification }
+ let(:specification) { subject.terminal_specification }
+
+ it 'returns terminal.gitlab.com protocol' do
+ expect(specification[:subprotocols]).to eq ['terminal.gitlab.com']
+ end
+
+ it 'returns a wss url' do
+ expect(specification[:url]).to start_with('wss://')
+ end
it 'returns empty hash if no url' do
subject.url = ''
- expect(terminal_specification).to be_empty
+ expect(specification).to be_empty
end
context 'when url is present' do
it 'returns ca_pem nil if empty certificate' do
subject.certificate = ''
- expect(terminal_specification[:ca_pem]).to be_nil
+ expect(specification[:ca_pem]).to be_nil
end
it 'adds Authorization header if authorization is present' do
subject.authorization = 'whatever'
- expect(terminal_specification[:headers]).to include(Authorization: ['whatever'])
+ expect(specification[:headers]).to include(Authorization: ['whatever'])
end
end
end