diff options
author | Rémy Coutable <remy@rymai.me> | 2016-07-18 15:31:42 +0200 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-07-18 15:31:42 +0200 |
commit | 5f79494d623e0797bdfaa3cb19783c4fa1464de8 (patch) | |
tree | dad94a66d4d9e81dbe892b3a97a54e5629ae43ea | |
parent | 119368fb96d5a867d2bf62d6079852d7098de428 (diff) | |
parent | 4e898b9b5e1a782d226212ba6227a8ddcb5177ae (diff) | |
download | gitlab-ce-5f79494d623e0797bdfaa3cb19783c4fa1464de8.tar.gz |
Merge remote-tracking branch 'origin/master' into 8-10-stable
24 files changed, 387 insertions, 191 deletions
diff --git a/CHANGELOG b/CHANGELOG index 94f212324f7..fcfc2d7aa30 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ v 8.10.0 (unreleased) - Align flash messages with left side of page content !4959 (winniehell) - Display tooltip for "Copy to Clipboard" button !5164 (winniehell) - Use default cursor for table header of project files !5165 (winniehell) + - Store when and yaml variables in builds table - Display last commit of deleted branch in push events !4699 (winniehell) - Escape file extension when parsing search results !5141 (winniehell) - Apply the trusted_proxies config to the rack request object for use with rack_attack @@ -91,6 +92,7 @@ v 8.10.0 (unreleased) - Handle custom Git hook result in GitLab UI - Allow to access Container Registry for Public and Internal projects - Allow '?', or '&' for label names + - Support redirected blobs for Container Registry integration - Fix importer for GitHub Pull Requests when a branch was reused across Pull Requests - Add date when user joined the team on the member page - Fix 404 redirect after validation fails importing a GitLab project @@ -111,6 +113,7 @@ v 8.10.0 (unreleased) - Fix creating group with space in group path - Create Todos for Issue author when assign or mention himself (Katarzyna Kobierska) - Limit the number of retries on error to 3 for exporting projects + - Allow empty repositories on project import/export v 8.9.6 - Fix importing of events under notes for GitLab projects. !5154 @@ -119,6 +122,7 @@ v 8.9.6 - Fix broken migration in MySQL. !5005 - Overwrite Host and X-Forwarded-Host headers in NGINX !5213 - Keeps issue number when importing from Gitlab.com + - Add Pending tab for Builds (Katarzyna Kobierska, Urszula Budziszewska) v 8.9.7 (unreleased) - Fix import_data wrongly saved as a result of an invalid import_url diff --git a/app/controllers/admin/builds_controller.rb b/app/controllers/admin/builds_controller.rb index 0db91eaaf2e..88f3c0e2fd4 100644 --- a/app/controllers/admin/builds_controller.rb +++ b/app/controllers/admin/builds_controller.rb @@ -5,8 +5,10 @@ class Admin::BuildsController < Admin::ApplicationController @builds = @all_builds.order('created_at DESC') @builds = case @scope + when 'pending' + @builds.pending.reverse_order when 'running' - @builds.running_or_pending.reverse_order + @builds.running.reverse_order when 'finished' @builds.finished else diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index ef3051d7519..d7513d75f01 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -10,8 +10,10 @@ class Projects::BuildsController < Projects::ApplicationController @builds = @all_builds.order('created_at DESC') @builds = case @scope + when 'pending' + @builds.pending.reverse_order when 'running' - @builds.running_or_pending.reverse_order + @builds.running.reverse_order when 'finished' @builds.finished else diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b24527247e0..ffac3a22efc 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -5,6 +5,7 @@ module Ci belongs_to :erased_by, class_name: 'User' serialize :options + serialize :yaml_variables validates :coverage, numericality: true, allow_blank: true validates_presence_of :ref @@ -52,6 +53,8 @@ module Ci new_build.stage = build.stage new_build.stage_idx = build.stage_idx new_build.trigger_request = build.trigger_request + new_build.yaml_variables = build.yaml_variables + new_build.when = build.when new_build.user = user new_build.environment = build.environment new_build.save @@ -118,7 +121,12 @@ module Ci end def variables - predefined_variables + yaml_variables + project_variables + trigger_variables + variables = [] + variables += predefined_variables + variables += yaml_variables if yaml_variables + variables += project_variables + variables += trigger_variables + variables end def merge_request @@ -395,30 +403,6 @@ module Ci self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil) end - def yaml_variables - global_yaml_variables + job_yaml_variables - end - - def global_yaml_variables - if pipeline.config_processor - pipeline.config_processor.global_variables.map do |key, value| - { key: key, value: value, public: true } - end - else - [] - end - end - - def job_yaml_variables - if pipeline.config_processor - pipeline.config_processor.job_variables(name).map do |key, value| - { key: key, value: value, public: true } - end - else - [] - end - end - def project_variables project.variables.map do |variable| { key: variable.key, value: variable.value, public: false } diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb index 2dcb052d274..3b21f0acb96 100644 --- a/app/services/ci/create_builds_service.rb +++ b/app/services/ci/create_builds_service.rb @@ -36,7 +36,9 @@ module Ci :allow_failure, :stage, :stage_idx, - :environment) + :environment, + :when, + :yaml_variables) build_attrs.merge!(pipeline: @pipeline, ref: @pipeline.ref, diff --git a/app/views/admin/builds/index.html.haml b/app/views/admin/builds/index.html.haml index 9ea3cca0ecb..3d77634d8fa 100644 --- a/app/views/admin/builds/index.html.haml +++ b/app/views/admin/builds/index.html.haml @@ -10,15 +10,20 @@ All %span.badge.js-totalbuilds-count= @all_builds.count(:id) + %li{class: ('active' if @scope == 'pending')} + = link_to admin_builds_path(scope: :pending) do + Pending + %span.badge= number_with_delimiter(@all_builds.pending.count(:id)) + %li{class: ('active' if @scope == 'running')} = link_to admin_builds_path(scope: :running) do Running - %span.badge.js-running-count= number_with_delimiter(@all_builds.running_or_pending.count(:id)) + %span.badge= number_with_delimiter(@all_builds.running.count(:id)) %li{class: ('active' if @scope == 'finished')} = link_to admin_builds_path(scope: :finished) do Finished - %span.badge.js-running-count= number_with_delimiter(@all_builds.finished.count(:id)) + %span.badge= number_with_delimiter(@all_builds.finished.count(:id)) .nav-controls - if @all_builds.running_or_pending.any? diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml index 85c31dfd918..2af625f69cd 100644 --- a/app/views/projects/builds/index.html.haml +++ b/app/views/projects/builds/index.html.haml @@ -11,17 +11,22 @@ %span.badge.js-totalbuilds-count = number_with_delimiter(@all_builds.count(:id)) + %li{class: ('active' if @scope == 'pending')} + = link_to project_builds_path(@project, scope: :pending) do + Pending + %span.badge + = number_with_delimiter(@all_builds.pending.count(:id)) %li{class: ('active' if @scope == 'running')} = link_to project_builds_path(@project, scope: :running) do Running - %span.badge.js-running-count - = number_with_delimiter(@all_builds.running_or_pending.count(:id)) + %span.badge + = number_with_delimiter(@all_builds.running.count(:id)) %li{class: ('active' if @scope == 'finished')} = link_to project_builds_path(@project, scope: :finished) do Finished - %span.badge.js-running-count + %span.badge = number_with_delimiter(@all_builds.finished.count(:id)) .nav-controls diff --git a/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb b/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb index e09caa0e6d7..7c991c6d998 100644 --- a/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb +++ b/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb @@ -1,6 +1,8 @@ class AddIndexForPipelineUserId < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + def change add_concurrent_index :ci_commits, :user_id end diff --git a/db/migrate/20160716115710_add_when_and_yaml_variables_to_ci_builds.rb b/db/migrate/20160716115710_add_when_and_yaml_variables_to_ci_builds.rb new file mode 100644 index 00000000000..3e084023a65 --- /dev/null +++ b/db/migrate/20160716115710_add_when_and_yaml_variables_to_ci_builds.rb @@ -0,0 +1,8 @@ +class AddWhenAndYamlVariablesToCiBuilds < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def change + add_column :ci_builds, :when, :string + add_column :ci_builds, :yaml_variables, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 48ecdc6ba1b..d250d65fe21 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160715134306) do +ActiveRecord::Schema.define(version: 20160716115710) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -168,6 +168,8 @@ ActiveRecord::Schema.define(version: 20160715134306) do t.string "environment" t.datetime "artifacts_expire_at" t.integer "artifacts_size" + t.string "when" + t.text "yaml_variables" end add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 01ef13df57a..a48dc542b14 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -31,28 +31,34 @@ module Ci raise ValidationError, e.message end - def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil) - builds.select do |build| - build[:stage] == stage && - process?(build[:only], build[:except], ref, tag, trigger_request) + def jobs_for_ref(ref, tag = false, trigger_request = nil) + @jobs.select do |_, job| + process?(job[:only], job[:except], ref, tag, trigger_request) end end - def builds - @jobs.map do |name, job| - build_job(name, job) + def jobs_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil) + jobs_for_ref(ref, tag, trigger_request).select do |_, job| + job[:stage] == stage end end - def global_variables - @variables + def builds_for_ref(ref, tag = false, trigger_request = nil) + jobs_for_ref(ref, tag, trigger_request).map do |name, job| + build_job(name, job) + end end - def job_variables(name) - job = @jobs[name.to_sym] - return [] unless job + def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil) + jobs_for_stage_and_ref(stage, ref, tag, trigger_request).map do |name, job| + build_job(name, job) + end + end - job[:variables] || [] + def builds + @jobs.map do |name, job| + build_job(name, job) + end end private @@ -95,11 +101,10 @@ module Ci commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"), tag_list: job[:tags] || [], name: name, - only: job[:only], - except: job[:except], allow_failure: job[:allow_failure] || false, when: job[:when] || 'on_success', environment: job[:environment], + yaml_variables: yaml_variables(name), options: { image: job[:image] || @image, services: job[:services] || @services, @@ -111,6 +116,24 @@ module Ci } end + def yaml_variables(name) + variables = global_variables.merge(job_variables(name)) + variables.map do |key, value| + { key: key, value: value, public: true } + end + end + + def global_variables + @variables || {} + end + + def job_variables(name) + job = @jobs[name.to_sym] + return {} unless job + + job[:variables] || {} + end + def validate! @jobs.each do |name, job| validate_job!(name, job) diff --git a/lib/container_registry/client.rb b/lib/container_registry/client.rb index 42232b7129d..2edddb84fc3 100644 --- a/lib/container_registry/client.rb +++ b/lib/container_registry/client.rb @@ -7,62 +7,91 @@ module ContainerRegistry MANIFEST_VERSION = 'application/vnd.docker.distribution.manifest.v2+json' + # Taken from: FaradayMiddleware::FollowRedirects + REDIRECT_CODES = Set.new [301, 302, 303, 307] + def initialize(base_uri, options = {}) @base_uri = base_uri - @faraday = Faraday.new(@base_uri) do |conn| - initialize_connection(conn, options) - end + @options = options end def repository_tags(name) - response_body @faraday.get("/v2/#{name}/tags/list") + response_body faraday.get("/v2/#{name}/tags/list") end def repository_manifest(name, reference) - response_body @faraday.get("/v2/#{name}/manifests/#{reference}") + response_body faraday.get("/v2/#{name}/manifests/#{reference}") end def repository_tag_digest(name, reference) - response = @faraday.head("/v2/#{name}/manifests/#{reference}") + response = faraday.head("/v2/#{name}/manifests/#{reference}") response.headers['docker-content-digest'] if response.success? end def delete_repository_tag(name, reference) - @faraday.delete("/v2/#{name}/manifests/#{reference}").success? + faraday.delete("/v2/#{name}/manifests/#{reference}").success? end def blob(name, digest, type = nil) - headers = {} - headers['Accept'] = type if type - response_body @faraday.get("/v2/#{name}/blobs/#{digest}", nil, headers) + type ||= 'application/octet-stream' + response_body faraday_blob.get("/v2/#{name}/blobs/#{digest}", nil, 'Accept' => type), allow_redirect: true end def delete_blob(name, digest) - @faraday.delete("/v2/#{name}/blobs/#{digest}").success? + faraday.delete("/v2/#{name}/blobs/#{digest}").success? end - + private - + def initialize_connection(conn, options) conn.request :json + + if options[:user] && options[:password] + conn.request(:basic_auth, options[:user].to_s, options[:password].to_s) + elsif options[:token] + conn.request(:authorization, :bearer, options[:token].to_s) + end + + conn.adapter :net_http + end + + def accept_manifest(conn) conn.headers['Accept'] = MANIFEST_VERSION conn.response :json, content_type: 'application/json' conn.response :json, content_type: 'application/vnd.docker.distribution.manifest.v1+prettyjws' conn.response :json, content_type: 'application/vnd.docker.distribution.manifest.v1+json' conn.response :json, content_type: 'application/vnd.docker.distribution.manifest.v2+json' + end - if options[:user] && options[:password] - conn.request(:basic_auth, options[:user].to_s, options[:password].to_s) - elsif options[:token] - conn.request(:authorization, :bearer, options[:token].to_s) + def response_body(response, allow_redirect: false) + if allow_redirect && REDIRECT_CODES.include?(response.status) + response = redirect_response(response.headers['location']) end - conn.adapter :net_http + response.body if response && response.success? + end + + def redirect_response(location) + return unless location + + # We explicitly remove authorization token + faraday_blob.get(location) do |req| + req['Authorization'] = '' + end end - def response_body(response) - response.body if response.success? + def faraday + @faraday ||= Faraday.new(@base_uri) do |conn| + initialize_connection(conn, @options) + accept_manifest(conn) + end + end + + def faraday_blob + @faraday_blob ||= Faraday.new(@base_uri) do |conn| + initialize_connection(conn, @options) + end end end end diff --git a/lib/container_registry/tag.rb b/lib/container_registry/tag.rb index 708d01b95a1..59040199920 100644 --- a/lib/container_registry/tag.rb +++ b/lib/container_registry/tag.rb @@ -53,7 +53,7 @@ module ContainerRegistry def config return unless config_blob - @config ||= ContainerRegistry::Config.new(self, config_blob) + @config ||= ContainerRegistry::Config.new(self, config_blob) if config_blob.data end def created_at diff --git a/lib/gitlab/import_export/importer.rb b/lib/gitlab/import_export/importer.rb index 8f66f48cbfe..6b69a653f12 100644 --- a/lib/gitlab/import_export/importer.rb +++ b/lib/gitlab/import_export/importer.rb @@ -44,8 +44,7 @@ module Gitlab def wiki_restorer Gitlab::ImportExport::RepoRestorer.new(path_to_bundle: wiki_repo_path, shared: @shared, - project: ProjectWiki.new(project_tree.restored_project), - wiki: true) + project: ProjectWiki.new(project_tree.restored_project)) end def uploads_restorer diff --git a/lib/gitlab/import_export/repo_restorer.rb b/lib/gitlab/import_export/repo_restorer.rb index 546dae4d122..f84de652a57 100644 --- a/lib/gitlab/import_export/repo_restorer.rb +++ b/lib/gitlab/import_export/repo_restorer.rb @@ -3,15 +3,14 @@ module Gitlab class RepoRestorer include Gitlab::ImportExport::CommandLineUtil - def initialize(project:, shared:, path_to_bundle:, wiki: false) + def initialize(project:, shared:, path_to_bundle:) @project = project @path_to_bundle = path_to_bundle @shared = shared - @wiki = wiki end def restore - return wiki? unless File.exist?(@path_to_bundle) + return true unless File.exist?(@path_to_bundle) FileUtils.mkdir_p(path_to_repo) @@ -30,10 +29,6 @@ module Gitlab def path_to_repo @project.repository.path_to_repo end - - def wiki? - @wiki - end end end end diff --git a/lib/gitlab/import_export/repo_saver.rb b/lib/gitlab/import_export/repo_saver.rb index cce43fe994b..331e14021e6 100644 --- a/lib/gitlab/import_export/repo_saver.rb +++ b/lib/gitlab/import_export/repo_saver.rb @@ -11,7 +11,7 @@ module Gitlab end def save - return false if @project.empty_repo? + return true if @project.empty_repo? # it's ok to have no repo @full_path = File.join(@shared.export_path, ImportExport.project_bundle_filename) bundle_to_disk diff --git a/lib/gitlab/import_export/wiki_repo_saver.rb b/lib/gitlab/import_export/wiki_repo_saver.rb index 1eedae39f8a..6107420e4dd 100644 --- a/lib/gitlab/import_export/wiki_repo_saver.rb +++ b/lib/gitlab/import_export/wiki_repo_saver.rb @@ -4,6 +4,7 @@ module Gitlab def save @wiki = ProjectWiki.new(@project) return true unless wiki_repository_exists? # it's okay to have no Wiki + bundle_to_disk(File.join(@shared.export_path, project_filename)) end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index fe05a0cfc00..5fb671df570 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -15,6 +15,11 @@ FactoryGirl.define do services: ["postgres"] } end + yaml_variables do + [ + { key: :DB_NAME, value: 'postgres', public: true } + ] + end pipeline factory: :ci_pipeline diff --git a/spec/features/admin/admin_builds_spec.rb b/spec/features/admin/admin_builds_spec.rb index a6198389f04..e177059d959 100644 --- a/spec/features/admin/admin_builds_spec.rb +++ b/spec/features/admin/admin_builds_spec.rb @@ -36,12 +36,45 @@ describe 'Admin Builds' do end end + context 'Pending tab' do + context 'when have pending builds' do + it 'shows pending builds' do + build1 = create(:ci_build, pipeline: pipeline, status: :pending) + build2 = create(:ci_build, pipeline: pipeline, status: :running) + build3 = create(:ci_build, pipeline: pipeline, status: :success) + build4 = create(:ci_build, pipeline: pipeline, status: :failed) + + visit admin_builds_path(scope: :pending) + + expect(page).to have_selector('.nav-links li.active', text: 'Pending') + expect(page.find('.build-link')).to have_content(build1.id) + expect(page.find('.build-link')).not_to have_content(build2.id) + expect(page.find('.build-link')).not_to have_content(build3.id) + expect(page.find('.build-link')).not_to have_content(build4.id) + expect(page).to have_link 'Cancel all' + end + end + + context 'when have no builds pending' do + it 'shows a message' do + create(:ci_build, pipeline: pipeline, status: :success) + + visit admin_builds_path(scope: :pending) + + expect(page).to have_selector('.nav-links li.active', text: 'Pending') + expect(page).to have_content 'No builds to show' + expect(page).not_to have_link 'Cancel all' + end + end + end + context 'Running tab' do context 'when have running builds' do it 'shows running builds' do - build1 = create(:ci_build, pipeline: pipeline, status: :pending) + build1 = create(:ci_build, pipeline: pipeline, status: :running) build2 = create(:ci_build, pipeline: pipeline, status: :success) build3 = create(:ci_build, pipeline: pipeline, status: :failed) + build4 = create(:ci_build, pipeline: pipeline, status: :pending) visit admin_builds_path(scope: :running) @@ -49,6 +82,7 @@ describe 'Admin Builds' do expect(page.find('.build-link')).to have_content(build1.id) expect(page.find('.build-link')).not_to have_content(build2.id) expect(page.find('.build-link')).not_to have_content(build3.id) + expect(page.find('.build-link')).not_to have_content(build4.id) expect(page).to have_link 'Cancel all' end end diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index 16832c297ac..cab3dc1d167 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -13,17 +13,33 @@ describe "Builds" do end describe "GET /:project/builds" do + context "Pending scope" do + before do + visit namespace_project_builds_path(@project.namespace, @project, scope: :pending) + end + + it "shows Pending tab builds" do + expect(page).to have_link 'Cancel running' + expect(page).to have_selector('.nav-links li.active', text: 'Pending') + expect(page).to have_content @build.short_sha + expect(page).to have_content @build.ref + expect(page).to have_content @build.name + end + end + context "Running scope" do before do @build.run! visit namespace_project_builds_path(@project.namespace, @project, scope: :running) end - it { expect(page).to have_selector('.nav-links li.active', text: 'Running') } - it { expect(page).to have_link 'Cancel running' } - it { expect(page).to have_content @build.short_sha } - it { expect(page).to have_content @build.ref } - it { expect(page).to have_content @build.name } + it "shows Running tab builds" do + expect(page).to have_selector('.nav-links li.active', text: 'Running') + expect(page).to have_link 'Cancel running' + expect(page).to have_content @build.short_sha + expect(page).to have_content @build.ref + expect(page).to have_content @build.name + end end context "Finished scope" do @@ -32,9 +48,11 @@ describe "Builds" do visit namespace_project_builds_path(@project.namespace, @project, scope: :finished) end - it { expect(page).to have_selector('.nav-links li.active', text: 'Finished') } - it { expect(page).to have_content 'No builds to show' } - it { expect(page).to have_link 'Cancel running' } + it "shows Finished tab builds" do + expect(page).to have_selector('.nav-links li.active', text: 'Finished') + expect(page).to have_content 'No builds to show' + expect(page).to have_link 'Cancel running' + end end context "All builds" do @@ -43,11 +61,13 @@ describe "Builds" do visit namespace_project_builds_path(@project.namespace, @project) end - it { expect(page).to have_selector('.nav-links li.active', text: 'All') } - it { expect(page).to have_content @build.short_sha } - it { expect(page).to have_content @build.ref } - it { expect(page).to have_content @build.name } - it { expect(page).not_to have_link 'Cancel running' } + it "shows All tab builds" do + expect(page).to have_selector('.nav-links li.active', text: 'All') + expect(page).to have_content @build.short_sha + expect(page).to have_content @build.ref + expect(page).to have_content @build.name + expect(page).not_to have_link 'Cancel running' + end end end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index bcbf409c8b0..ad6587b4c25 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -19,15 +19,14 @@ module Ci expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({ stage: "test", stage_idx: 1, - except: nil, name: :rspec, - only: nil, commands: "pwd\nrspec", tag_list: [], options: {}, allow_failure: false, when: "on_success", environment: nil, + yaml_variables: [] }) end @@ -432,11 +431,9 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ - except: nil, stage: "test", stage_idx: 1, name: :rspec, - only: nil, commands: "pwd\nrspec", tag_list: [], options: { @@ -446,6 +443,7 @@ module Ci allow_failure: false, when: "on_success", environment: nil, + yaml_variables: [] }) end @@ -461,11 +459,9 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ - except: nil, stage: "test", stage_idx: 1, name: :rspec, - only: nil, commands: "pwd\nrspec", tag_list: [], options: { @@ -475,101 +471,126 @@ module Ci allow_failure: false, when: "on_success", environment: nil, + yaml_variables: [] }) end end describe 'Variables' do - context 'when global variables are defined' do - it 'returns global variables' do - variables = { - VAR1: 'value1', - VAR2: 'value2', - } + let(:config_processor) { GitlabCiYamlProcessor.new(YAML.dump(config), path) } - config = YAML.dump({ + subject { config_processor.builds.first[:yaml_variables] } + + context 'when global variables are defined' do + let(:variables) do + { VAR1: 'value1', VAR2: 'value2' } + end + let(:config) do + { variables: variables, before_script: ['pwd'], rspec: { script: 'rspec' } - }) + } + end - config_processor = GitlabCiYamlProcessor.new(config, path) + it 'returns global variables' do + expect(subject).to contain_exactly( + { key: :VAR1, value: 'value1', public: true }, + { key: :VAR2, value: 'value2', public: true } + ) + end + end + + context 'when job and global variables are defined' do + let(:global_variables) do + { VAR1: 'global1', VAR3: 'global3' } + end + let(:job_variables) do + { VAR1: 'value1', VAR2: 'value2' } + end + let(:config) do + { + before_script: ['pwd'], + variables: global_variables, + rspec: { script: 'rspec', variables: job_variables } + } + end - expect(config_processor.global_variables).to eq(variables) + it 'returns all unique variables' do + expect(subject).to contain_exactly( + { key: :VAR3, value: 'global3', public: true }, + { key: :VAR1, value: 'value1', public: true }, + { key: :VAR2, value: 'value2', public: true } + ) end end context 'when job variables are defined' do - context 'when syntax is correct' do - it 'returns job variables' do - variables = { - KEY1: 'value1', - SOME_KEY_2: 'value2' - } + let(:config) do + { + before_script: ['pwd'], + rspec: { script: 'rspec', variables: variables } + } + end + + context 'when also global variables are defined' do - config = YAML.dump( - { before_script: ['pwd'], - rspec: { - variables: variables, - script: 'rspec' } - }) + end - config_processor = GitlabCiYamlProcessor.new(config, path) + context 'when syntax is correct' do + let(:variables) do + { VAR1: 'value1', VAR2: 'value2' } + end - expect(config_processor.job_variables(:rspec)).to eq variables + it 'returns job variables' do + expect(subject).to contain_exactly( + { key: :VAR1, value: 'value1', public: true }, + { key: :VAR2, value: 'value2', public: true } + ) end end context 'when syntax is incorrect' do context 'when variables defined but invalid' do - it 'raises error' do - variables = [:KEY1, 'value1', :KEY2, 'value2'] - - config = YAML.dump( - { before_script: ['pwd'], - rspec: { - variables: variables, - script: 'rspec' } - }) + let(:variables) do + [ :VAR1, 'value1', :VAR2, 'value2' ] + end - expect { GitlabCiYamlProcessor.new(config, path) } + it 'raises error' do + expect { subject } .to raise_error(GitlabCiYamlProcessor::ValidationError, - /job: variables should be a map/) + /job: variables should be a map/) end end context 'when variables key defined but value not specified' do - it 'returns empty array' do - config = YAML.dump( - { before_script: ['pwd'], - rspec: { - variables: nil, - script: 'rspec' } - }) - - config_processor = GitlabCiYamlProcessor.new(config, path) + let(:variables) do + nil + end + it 'returns empty array' do ## # When variables config is empty, we assume this is a valid # configuration, see issue #18775 # - expect(config_processor.job_variables(:rspec)) - .to be_an_instance_of(Array).and be_empty + expect(subject).to be_an_instance_of(Array) + expect(subject).to be_empty end end end end context 'when job variables are not defined' do - it 'returns empty array' do - config = YAML.dump({ + let(:config) do + { before_script: ['pwd'], rspec: { script: 'rspec' } - }) - - config_processor = GitlabCiYamlProcessor.new(config, path) + } + end - expect(config_processor.job_variables(:rspec)).to eq [] + it 'returns empty array' do + expect(subject).to be_an_instance_of(Array) + expect(subject).to be_empty end end end @@ -681,11 +702,9 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ - except: nil, stage: "test", stage_idx: 1, name: :rspec, - only: nil, commands: "pwd\nrspec", tag_list: [], options: { @@ -701,6 +720,7 @@ module Ci when: "on_success", allow_failure: false, environment: nil, + yaml_variables: [] }) end @@ -819,17 +839,16 @@ module Ci it "doesn't create jobs that start with dot" do expect(subject.size).to eq(1) expect(subject.first).to eq({ - except: nil, stage: "test", stage_idx: 1, name: :normal_job, - only: nil, commands: "test", tag_list: [], options: {}, when: "on_success", allow_failure: false, environment: nil, + yaml_variables: [] }) end end @@ -865,30 +884,28 @@ module Ci it "is correctly supported for jobs" do expect(subject.size).to eq(2) expect(subject.first).to eq({ - except: nil, stage: "build", stage_idx: 0, name: :job1, - only: nil, commands: "execute-script-for-job", tag_list: [], options: {}, when: "on_success", allow_failure: false, environment: nil, + yaml_variables: [] }) expect(subject.second).to eq({ - except: nil, stage: "build", stage_idx: 0, name: :job2, - only: nil, commands: "execute-script-for-job", tag_list: [], options: {}, when: "on_success", allow_failure: false, environment: nil, + yaml_variables: [] }) end end diff --git a/spec/lib/container_registry/blob_spec.rb b/spec/lib/container_registry/blob_spec.rb index 4d8cb787dde..bbacdc67ebd 100644 --- a/spec/lib/container_registry/blob_spec.rb +++ b/spec/lib/container_registry/blob_spec.rb @@ -9,8 +9,9 @@ describe ContainerRegistry::Blob do 'size' => 1000 } end + let(:token) { 'authorization-token' } - let(:registry) { ContainerRegistry::Registry.new('http://example.com') } + let(:registry) { ContainerRegistry::Registry.new('http://example.com', token: token) } let(:repository) { registry.repository('group/test') } let(:blob) { repository.blob(config) } @@ -58,4 +59,53 @@ describe ContainerRegistry::Blob do it { is_expected.to be_truthy } end + + context '#data' do + let(:data) { '{"key":"value"}' } + + subject { blob.data } + + context 'when locally stored' do + before do + stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:0123456789012345'). + to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: data) + end + + it { is_expected.to eq(data) } + end + + context 'when externally stored' do + before do + stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:0123456789012345'). + with(headers: { 'Authorization' => "bearer #{token}" }). + to_return( + status: 307, + headers: { 'Location' => location }) + end + + context 'for a valid address' do + let(:location) { 'http://external.com/blob/file' } + + before do + stub_request(:get, location). + with(headers: { 'Authorization' => nil }). + to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: data) + end + + it { is_expected.to eq(data) } + end + + context 'for invalid file' do + let(:location) { 'file:///etc/passwd' } + + it { expect{ subject }.to raise_error(ArgumentError, 'invalid address') } + end + end + end end diff --git a/spec/lib/container_registry/tag_spec.rb b/spec/lib/container_registry/tag_spec.rb index c7324c2bf77..c5e31ae82b6 100644 --- a/spec/lib/container_registry/tag_spec.rb +++ b/spec/lib/container_registry/tag_spec.rb @@ -77,24 +77,47 @@ describe ContainerRegistry::Tag do end context 'config processing' do - before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). - with(headers: { 'Accept' => 'application/octet-stream' }). - to_return( - status: 200, - body: File.read(Rails.root + 'spec/fixtures/container_registry/config_blob.json')) - end + shared_examples 'a processable' do + context '#config' do + subject { tag.config } - context '#config' do - subject { tag.config } + it { is_expected.not_to be_nil } + end + + context '#created_at' do + subject { tag.created_at } - it { is_expected.not_to be_nil } + it { is_expected.not_to be_nil } + end end - context '#created_at' do - subject { tag.created_at } + context 'when locally stored' do + before do + stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). + with(headers: { 'Accept' => 'application/octet-stream' }). + to_return( + status: 200, + body: File.read(Rails.root + 'spec/fixtures/container_registry/config_blob.json')) + end + + it_behaves_like 'a processable' + end - it { is_expected.not_to be_nil } + context 'when externally stored' do + before do + stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). + with(headers: { 'Accept' => 'application/octet-stream' }). + to_return( + status: 307, + headers: { 'Location' => 'http://external.com/blob/file' }) + + stub_request(:get, 'http://external.com/blob/file'). + to_return( + status: 200, + body: File.read(Rails.root + 'spec/fixtures/container_registry/config_blob.json')) + end + + it_behaves_like 'a processable' end end end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index e8171788872..481416319dd 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -208,7 +208,7 @@ describe Ci::Build, models: true do end before do - build.update_attributes(stage: 'stage') + build.update_attributes(stage: 'stage', yaml_variables: yaml_variables) end it { is_expected.to eq(predefined_variables + yaml_variables) } @@ -260,22 +260,6 @@ describe Ci::Build, models: true do it { is_expected.to eq(predefined_variables + predefined_trigger_variable + yaml_variables + secure_variables + trigger_variables) } end - - context 'when job variables are defined' do - ## - # Job-level variables are defined in gitlab_ci.yml fixture - # - context 'when job variables are unique' do - let(:build) { create(:ci_build, name: 'staging') } - - it 'includes job variables' do - expect(subject).to include( - { key: :KEY1, value: 'value1', public: true }, - { key: :KEY2, value: 'value2', public: true } - ) - end - end - end end end end |