diff options
| -rw-r--r-- | app/assets/javascripts/pages/projects/jobs/terminal/index.js | 3 | ||||
| -rw-r--r-- | app/controllers/projects/jobs_controller.rb | 22 | ||||
| -rw-r--r-- | app/models/ci/build.rb | 14 | ||||
| -rw-r--r-- | app/models/ci/build_runner_session.rb | 25 | ||||
| -rw-r--r-- | app/policies/ci/build_policy.rb | 6 | ||||
| -rw-r--r-- | app/services/ci/register_job_service.rb | 4 | ||||
| -rw-r--r-- | app/views/projects/jobs/_sidebar.html.haml | 4 | ||||
| -rw-r--r-- | app/views/projects/jobs/terminal.html.haml | 11 | ||||
| -rw-r--r-- | changelogs/unreleased/fj-web-terminal-ci-build.yml | 5 | ||||
| -rw-r--r-- | config/routes/project.rb | 2 | ||||
| -rw-r--r-- | db/migrate/20180613081317_create_ci_builds_runner_session.rb | 21 | ||||
| -rw-r--r-- | db/schema.rb | 10 | ||||
| -rw-r--r-- | lib/api/entities.rb | 1 | ||||
| -rw-r--r-- | lib/api/runner.rb | 13 | ||||
| -rw-r--r-- | spec/controllers/projects/jobs_controller_spec.rb | 101 | ||||
| -rw-r--r-- | spec/factories/ci/builds.rb | 6 | ||||
| -rw-r--r-- | spec/models/ci/build_runner_session_spec.rb | 36 | ||||
| -rw-r--r-- | spec/models/ci/build_spec.rb | 50 | ||||
| -rw-r--r-- | spec/services/ci/register_job_service_spec.rb | 17 | ||||
| -rw-r--r-- | spec/services/ci/retry_build_service_spec.rb | 2 |
20 files changed, 344 insertions, 9 deletions
diff --git a/app/assets/javascripts/pages/projects/jobs/terminal/index.js b/app/assets/javascripts/pages/projects/jobs/terminal/index.js new file mode 100644 index 00000000000..7129e24cee1 --- /dev/null +++ b/app/assets/javascripts/pages/projects/jobs/terminal/index.js @@ -0,0 +1,3 @@ +import initTerminal from '~/terminal/'; + +document.addEventListener('DOMContentLoaded', initTerminal); diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 02cac862c3d..e69faae754a 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -2,11 +2,12 @@ class Projects::JobsController < Projects::ApplicationController include SendFileUpload before_action :build, except: [:index, :cancel_all] - before_action :authorize_read_build!, - only: [:index, :show, :status, :raw, :trace] + before_action :authorize_read_build! before_action :authorize_update_build!, except: [:index, :show, :status, :raw, :trace, :cancel_all, :erase] before_action :authorize_erase_build!, only: [:erase] + before_action :authorize_use_build_terminal!, only: [:terminal, :terminal_workhorse_authorize] + before_action :verify_api_request!, only: :terminal_websocket_authorize layout 'project' @@ -134,6 +135,15 @@ class Projects::JobsController < Projects::ApplicationController end end + def terminal + end + + # 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) + end + private def authorize_update_build! @@ -144,6 +154,14 @@ class Projects::JobsController < Projects::ApplicationController return access_denied! unless can?(current_user, :erase_build, build) end + def authorize_use_build_terminal! + return access_denied! unless can?(current_user, :create_build_terminal, build) + end + + def verify_api_request! + Gitlab::Workhorse.verify_api_request!(request.headers) + end + def raw_send_params { type: 'text/plain; charset=utf-8', disposition: 'inline' } end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 41446946a5e..bf93a2caf72 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -27,7 +27,13 @@ module Ci has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id has_one :metadata, class_name: 'Ci::BuildMetadata' + has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build + + accepts_nested_attributes_for :runner_session + delegate :timeout, to: :metadata, prefix: true, allow_nil: true + delegate :url, to: :runner_session, prefix: true, allow_nil: true + delegate :terminal_specification, to: :runner_session, allow_nil: true delegate :gitlab_deploy_token, to: :project ## @@ -174,6 +180,10 @@ module Ci after_transition pending: :running do |build| build.ensure_metadata.update_timeout_state end + + after_transition running: any do |build| + Ci::BuildRunnerSession.where(build: build).delete_all + end end def ensure_metadata @@ -584,6 +594,10 @@ module Ci super(options).merge(when: read_attribute(:when)) end + def has_terminal? + running? && runner_session_url.present? + end + private def update_artifacts_size diff --git a/app/models/ci/build_runner_session.rb b/app/models/ci/build_runner_session.rb new file mode 100644 index 00000000000..6f3be31d8e1 --- /dev/null +++ b/app/models/ci/build_runner_session.rb @@ -0,0 +1,25 @@ +module Ci + # The purpose of this class is to store Build related runner session. + # Data will be removed after transitioning from running to any state. + class BuildRunnerSession < ActiveRecord::Base + extend Gitlab::Ci::Model + + self.table_name = 'ci_builds_runner_session' + + belongs_to :build, class_name: 'Ci::Build', inverse_of: :runner_session + + validates :build, presence: true + validates :url, url: { protocols: %w(https) } + + def terminal_specification + return {} unless url.present? + + { + subprotocols: ['terminal.gitlab.com'].freeze, + url: "#{url}/exec".sub("https://", "wss://"), + headers: { Authorization: authorization.presence }.compact, + ca_pem: certificate.presence + } + end + end +end diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 1c0cc7425ec..75c7e529902 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -18,6 +18,10 @@ module Ci @subject.project.branch_allows_collaboration?(@user, @subject.ref) end + condition(:terminal, scope: :subject) do + @subject.has_terminal? + end + rule { protected_ref }.policy do prevent :update_build prevent :erase_build @@ -29,5 +33,7 @@ module Ci enable :update_build enable :update_commit_status end + + rule { can?(:update_build) & terminal }.enable :create_build_terminal end end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index c0dce45e2e7..6eb1c4f52de 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -13,7 +13,7 @@ module Ci @runner = runner end - def execute + def execute(params = {}) builds = if runner.instance_type? builds_for_shared_runner @@ -41,6 +41,8 @@ module Ci # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method. begin build.runner_id = runner.id + build.runner_session_attributes = params[:session] if params[:session].present? + build.run! register_success(build) diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml index 8d890d19278..b88fe47726d 100644 --- a/app/views/projects/jobs/_sidebar.html.haml +++ b/app/views/projects/jobs/_sidebar.html.haml @@ -1,6 +1,10 @@ %aside.right-sidebar.right-sidebar-expanded.build-sidebar.js-build-sidebar.js-right-sidebar{ data: { "offset-top" => "101", "spy" => "affix" } } .sidebar-container .blocks-container + - if can?(current_user, :create_build_terminal, @build) + .block + = link_to terminal_project_job_path(@project, @build), class: 'terminal-button pull-right btn visible-md-block visible-lg-block', title: 'Terminal' do + Terminal #js-details-block-vue{ data: { can_user_retry: can?(current_user, :update_build, @build) && @build.retryable? } } diff --git a/app/views/projects/jobs/terminal.html.haml b/app/views/projects/jobs/terminal.html.haml new file mode 100644 index 00000000000..efea666a4d9 --- /dev/null +++ b/app/views/projects/jobs/terminal.html.haml @@ -0,0 +1,11 @@ +- @no_container = true +- add_to_breadcrumbs 'Jobs', project_jobs_path(@project) +- add_to_breadcrumbs "##{@build.id}", project_job_path(@project, @build) +- breadcrumb_title 'Terminal' +- page_title 'Terminal', "#{@build.name} (##{@build.id})", 'Jobs' + +- content_for :page_specific_javascripts do + = stylesheet_link_tag "xterm/xterm" + +.terminal-container{ class: container_class } + #terminal{ data: { project_path: terminal_project_job_path(@project, @build, format: :ws) } } diff --git a/changelogs/unreleased/fj-web-terminal-ci-build.yml b/changelogs/unreleased/fj-web-terminal-ci-build.yml new file mode 100644 index 00000000000..c3608d4203b --- /dev/null +++ b/changelogs/unreleased/fj-web-terminal-ci-build.yml @@ -0,0 +1,5 @@ +--- +title: Add Web Terminal for Ci Builds +merge_request: +author: Vicky Chijwani +type: added diff --git a/config/routes/project.rb b/config/routes/project.rb index 2ebf84f2ecf..5057e937941 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -279,6 +279,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do post :erase get :trace, defaults: { format: 'json' } get :raw + get :terminal + get '/terminal.ws/authorize', to: 'jobs#terminal_websocket_authorize', constraints: { format: nil } end resource :artifacts, only: [] do diff --git a/db/migrate/20180613081317_create_ci_builds_runner_session.rb b/db/migrate/20180613081317_create_ci_builds_runner_session.rb new file mode 100644 index 00000000000..e550c07b9ab --- /dev/null +++ b/db/migrate/20180613081317_create_ci_builds_runner_session.rb @@ -0,0 +1,21 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateCiBuildsRunnerSession < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def change + create_table :ci_builds_runner_session, id: :bigserial do |t| + t.integer :build_id, null: false + t.string :url, null: false + t.string :certificate + t.string :authorization + + t.foreign_key :ci_builds, column: :build_id, on_delete: :cascade + t.index :build_id, unique: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 9a4e3fe5555..c9aaf80f059 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -358,6 +358,15 @@ ActiveRecord::Schema.define(version: 20180629191052) do add_index "ci_builds_metadata", ["build_id"], name: "index_ci_builds_metadata_on_build_id", unique: true, using: :btree add_index "ci_builds_metadata", ["project_id"], name: "index_ci_builds_metadata_on_project_id", using: :btree + create_table "ci_builds_runner_session", id: :bigserial, force: :cascade do |t| + t.integer "build_id", null: false + t.string "url", null: false + t.string "certificate" + t.string "authorization" + end + + add_index "ci_builds_runner_session", ["build_id"], name: "index_ci_builds_runner_session_on_build_id", unique: true, using: :btree + create_table "ci_group_variables", force: :cascade do |t| t.string "key", null: false t.text "value" @@ -2191,6 +2200,7 @@ ActiveRecord::Schema.define(version: 20180629191052) do add_foreign_key "ci_builds", "projects", name: "fk_befce0568a", on_delete: :cascade add_foreign_key "ci_builds_metadata", "ci_builds", column: "build_id", on_delete: :cascade add_foreign_key "ci_builds_metadata", "projects", on_delete: :cascade + add_foreign_key "ci_builds_runner_session", "ci_builds", column: "build_id", on_delete: :cascade add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade add_foreign_key "ci_job_artifacts", "ci_builds", column: "job_id", on_delete: :cascade add_foreign_key "ci_job_artifacts", "projects", on_delete: :cascade diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 930b5ef37a3..3a6e707fd5b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1203,6 +1203,7 @@ module API class RunnerInfo < Grape::Entity expose :metadata_timeout, as: :timeout + expose :runner_session_url end class Step < Grape::Entity diff --git a/lib/api/runner.rb b/lib/api/runner.rb index b4b984f7b8f..d0cc0945a5f 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -81,6 +81,11 @@ module API requires :token, type: String, desc: %q(Runner's authentication token) optional :last_update, type: String, desc: %q(Runner's queue last_update token) optional :info, type: Hash, desc: %q(Runner's metadata) + optional :session, type: Hash, desc: %q(Runner's session data) do + optional :url, type: String, desc: %q(Session's url) + optional :certificate, type: String, desc: %q(Session's certificate) + optional :authorization, type: String, desc: %q(Session's authorization) + end end post '/request' do authenticate_runner! @@ -90,14 +95,16 @@ module API break no_content! end - if current_runner.runner_queue_value_latest?(params[:last_update]) - header 'X-GitLab-Last-Update', params[:last_update] + runner_params = declared_params(include_missing: false) + + if current_runner.runner_queue_value_latest?(runner_params[:last_update]) + header 'X-GitLab-Last-Update', runner_params[:last_update] Gitlab::Metrics.add_event(:build_not_found_cached) break no_content! end new_update = current_runner.ensure_runner_queue_value - result = ::Ci::RegisterJobService.new(current_runner).execute + result = ::Ci::RegisterJobService.new(current_runner).execute(runner_params) if result.valid? if result.build diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index b10421b8f26..e6332a10265 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -562,4 +562,105 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do end end end + + describe 'GET #terminal' do + before do + project.add_developer(user) + sign_in(user) + end + + context 'when job exists' do + context 'and it has a terminal' do + let!(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) } + + it 'has a job' do + get_terminal(id: job.id) + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:build).id).to eq(job.id) + end + end + + context 'and does not have a terminal' do + let!(:job) { create(:ci_build, :running, pipeline: pipeline) } + + it 'returns not_found' do + get_terminal(id: job.id) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when job does not exist' do + it 'renders not_found' do + get_terminal(id: 1234) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + def get_terminal(**extra_params) + params = { + namespace_id: project.namespace.to_param, + project_id: project + } + + get :terminal, params.merge(extra_params) + end + end + + describe 'GET #terminal_websocket_authorize' do + let!(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) } + + before do + project.add_developer(user) + sign_in(user) + end + + 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 terminal for the job' do + expect(Gitlab::Workhorse) + .to receive(:terminal_websocket) + .and_return(workhorse: :response) + + get_terminal_websocket(id: job.id) + + 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('{"workhorse":"response"}') + end + end + + context 'and invalid id' do + it 'returns 404' do + get_terminal_websocket(id: 1234) + + 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 { get_terminal_websocket(id: job.id) }.to raise_error(JWT::DecodeError) + end + end + + def get_terminal_websocket(**extra_params) + params = { + namespace_id: project.namespace.to_param, + project_id: project + } + + get :terminal_websocket_authorize, params.merge(extra_params) + end + end end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 4acc008ed38..83cb4750741 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -248,5 +248,11 @@ FactoryBot.define do failed failure_reason 2 end + + trait :with_runner_session do + after(:build) do |build| + build.build_runner_session(url: 'ws://localhost') + end + end end end diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb new file mode 100644 index 00000000000..7183957aa50 --- /dev/null +++ b/spec/models/ci/build_runner_session_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe Ci::BuildRunnerSession, model: true do + let!(:build) { create(:ci_build, :with_runner_session) } + + subject { build.runner_session } + + it { is_expected.to belong_to(:build) } + + it { is_expected.to validate_presence_of(:build) } + 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 } + + it 'returns empty hash if no url' do + subject.url = '' + + expect(terminal_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 + end + + it 'adds Authorization header if authorization is present' do + subject.authorization = 'whatever' + + expect(terminal_specification[:headers]).to include(Authorization: 'whatever') + end + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 6758adc59eb..0da1234ee3b 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -19,6 +19,7 @@ describe Ci::Build do it { is_expected.to belong_to(:erased_by) } it { is_expected.to have_many(:deployments) } it { is_expected.to have_many(:trace_sections)} + it { is_expected.to have_one(:runner_session)} it { is_expected.to validate_presence_of(:ref) } it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } @@ -42,6 +43,20 @@ describe Ci::Build do end end + describe 'status' do + context 'when transitioning to any state from running' do + it 'removes runner_session' do + %w(success drop cancel).each do |event| + build = FactoryBot.create(:ci_build, :running, :with_runner_session, pipeline: pipeline) + + build.fire_events!(event) + + expect(build.reload.runner_session).to be_nil + end + end + end + end + describe '.manual_actions' do let!(:manual_but_created) { create(:ci_build, :manual, status: :created, pipeline: pipeline) } let!(:manual_but_succeeded) { create(:ci_build, :manual, status: :success, pipeline: pipeline) } @@ -2605,4 +2620,39 @@ describe Ci::Build do end end end + + describe '#has_terminal?' do + let(:states) { described_class.state_machines[:status].states.keys - [:running] } + + subject { build.has_terminal? } + + it 'returns true if the build is running and it has a runner_session_url' do + build.build_runner_session(url: 'whatever') + build.status = :running + + expect(subject).to be_truthy + end + + context 'returns false' do + it 'when runner_session_url is empty' do + build.status = :running + + expect(subject).to be_falsey + end + + context 'unless the build is running' do + before do + build.build_runner_session(url: 'whatever') + end + + it do + states.each do |state| + build.status = state + + is_expected.to be_falsey + end + end + end + end + end end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 3816bd0deb5..dbb5e33bbdc 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -548,8 +548,21 @@ module Ci end end - def execute(runner) - described_class.new(runner).execute.build + context 'when runner_session params are' do + it 'present sets runner session configuration in the build' do + runner_session_params = { session: { 'url' => 'https://example.com' } } + + expect(execute(specific_runner, runner_session_params).runner_session.attributes) + .to include(runner_session_params[:session]) + end + + it 'not present it does not configure the runner session' do + expect(execute(specific_runner).runner_session).to be_nil + end + end + + def execute(runner, params = {}) + described_class.new(runner).execute(params).build end end end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index e1cb7ed8110..decb5d22f59 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -32,7 +32,7 @@ describe Ci::RetryBuildService do runner_id tag_taggings taggings tags trigger_request_id user_id auto_canceled_by_id retried failure_reason artifacts_file_store artifacts_metadata_store - metadata trace_chunks].freeze + metadata runner_session trace_chunks].freeze shared_examples 'build duplication' do let(:another_pipeline) { create(:ci_empty_pipeline, project: project) } |