diff options
author | Jarka Kadlecova <jarka@gitlab.com> | 2017-09-18 08:30:27 +0300 |
---|---|---|
committer | Jarka Kadlecova <jarka@gitlab.com> | 2017-09-18 08:30:27 +0300 |
commit | 7bc31e138b9c9f72d978b802f17d75e33554a5b2 (patch) | |
tree | e9927407dcaa83aad0a526cf8d6d7c5a2d86069c | |
parent | e1836b8fb3ee021cbbef96f3a8b00323b1f0d6e2 (diff) | |
parent | 00fe6307ab89443b3fbca8ee42df3b02c1b5cbd8 (diff) |
Merge branch '10-0-stable-prepare-rc4' into '10-0-stable'
Prepare 10.0 RC4 release
See merge request gitlab-org/gitlab-ce!14310
22 files changed, 279 insertions, 31 deletions
diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index 5ffa67a1220..2f7717760ec 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -328,7 +328,7 @@ border-bottom: 1px solid $border-color; transition: padding $sidebar-transition-duration; text-align: center; - margin-top: $header-height; + margin-top: $new-navbar-height; .container-fluid { position: relative; diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 871c76fbad3..ae85fdb720a 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -453,6 +453,10 @@ module Ci .fabricate! end + def latest_builds_with_artifacts + @latest_builds_with_artifacts ||= builds.latest.with_artifacts + end + private def ci_yaml_from_repo diff --git a/app/models/environment.rb b/app/models/environment.rb index 9b05f8b1cd5..44e39e21442 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -6,7 +6,10 @@ class Environment < ActiveRecord::Base belongs_to :project, required: true, validate: true - has_many :deployments, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :deployments, + -> (env) { where(project_id: env.project_id) }, + dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_one :last_deployment, -> { order('deployments.id DESC') }, class_name: 'Deployment' before_validation :nullify_external_url diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 4a9a23fea1f..e279d8dd8c5 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -231,6 +231,13 @@ class Namespace < ActiveRecord::Base end def force_share_with_group_lock_on_descendants - descendants.update_all(share_with_group_lock: true) + return unless Group.supports_nested_groups? + + # We can't use `descendants.update_all` since Rails will throw away the WITH + # RECURSIVE statement. We also can't use WHERE EXISTS since we can't use + # different table aliases, hence we're just using WHERE IN. Since we have a + # maximum of 20 nested groups this should be fine. + Namespace.where(id: descendants.select(:id)) + .update_all(share_with_group_lock: true) end end diff --git a/app/models/project.rb b/app/models/project.rb index ff5638dd155..add272cc5ce 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1163,6 +1163,23 @@ class Project < ActiveRecord::Base pipelines.order(id: :desc).find_by(sha: sha, ref: ref) end + def latest_successful_pipeline_for_default_branch + if defined?(@latest_successful_pipeline_for_default_branch) + return @latest_successful_pipeline_for_default_branch + end + + @latest_successful_pipeline_for_default_branch = + pipelines.latest_successful_for(default_branch) + end + + def latest_successful_pipeline_for(ref = nil) + if ref && ref != default_branch + pipelines.latest_successful_for(ref) + else + latest_successful_pipeline_for_default_branch + end + end + def enable_ci project_feature.update_attribute(:builds_access_level, ProjectFeature::ENABLED) end diff --git a/app/views/projects/buttons/_download.html.haml b/app/views/projects/buttons/_download.html.haml index 883922dbf04..9d85e027ac9 100644 --- a/app/views/projects/buttons/_download.html.haml +++ b/app/views/projects/buttons/_download.html.haml @@ -1,4 +1,4 @@ -- pipeline = local_assigns.fetch(:pipeline) { project.pipelines.latest_successful_for(ref) } +- pipeline = local_assigns.fetch(:pipeline) { project.latest_successful_pipeline_for(ref) } - if !project.empty_repo? && can?(current_user, :download_code, project) .project-action-button.dropdown.inline> @@ -26,18 +26,16 @@ %i.fa.fa-download %span= _('Download tar') - - if pipeline - - artifacts = pipeline.builds.latest.with_artifacts - - if artifacts.any? - %li.dropdown-header Artifacts - - unless pipeline.latest? - - latest_pipeline = project.pipeline_for(ref) - %li - .unclickable= ci_status_for_statuseable(latest_pipeline) - %li.dropdown-header Previous Artifacts - - artifacts.each do |job| - %li - = link_to latest_succeeded_project_artifacts_path(project, "#{ref}/download", job: job.name), rel: 'nofollow', download: '' do - %i.fa.fa-download - %span - #{ s_('DownloadArtifacts|Download') } '#{job.name}' + - if pipeline && pipeline.latest_builds_with_artifacts.any? + %li.dropdown-header Artifacts + - unless pipeline.latest? + - latest_pipeline = project.pipeline_for(ref) + %li + .unclickable= ci_status_for_statuseable(latest_pipeline) + %li.dropdown-header Previous Artifacts + - pipeline.latest_builds_with_artifacts.each do |job| + %li + = link_to latest_succeeded_project_artifacts_path(project, "#{ref}/download", job: job.name), rel: 'nofollow', download: '' do + %i.fa.fa-download + %span + #{s_('DownloadArtifacts|Download')} '#{job.name}' diff --git a/app/views/projects/runners/_form.html.haml b/app/views/projects/runners/_form.html.haml index ac8e15a48b2..ceae53abfff 100644 --- a/app/views/projects/runners/_form.html.haml +++ b/app/views/projects/runners/_form.html.haml @@ -11,7 +11,7 @@ .col-sm-10 .checkbox = f.check_box :access_level, {}, 'ref_protected', 'not_protected' - %span.light This runner will only run on pipelines trigged on protected branches + %span.light This runner will only run on pipelines triggered on protected branches .form-group = label :run_untagged, 'Run untagged jobs', class: 'control-label' .col-sm-10 diff --git a/changelogs/unreleased/ci-environment-status-performance.yml b/changelogs/unreleased/ci-environment-status-performance.yml new file mode 100644 index 00000000000..8812733b5a7 --- /dev/null +++ b/changelogs/unreleased/ci-environment-status-performance.yml @@ -0,0 +1,5 @@ +--- +title: Constrain environment deployments to project IDs +merge_request: +author: +type: other diff --git a/changelogs/unreleased/disallow-null-values-for-environments-project-id.yml b/changelogs/unreleased/disallow-null-values-for-environments-project-id.yml new file mode 100644 index 00000000000..f4a956e6724 --- /dev/null +++ b/changelogs/unreleased/disallow-null-values-for-environments-project-id.yml @@ -0,0 +1,5 @@ +--- +title: "Disallow NULL values for environments.project_id" +merge_request: +author: +type: other diff --git a/changelogs/unreleased/memoize-the-latest-builds-of-a-pipeline.yml b/changelogs/unreleased/memoize-the-latest-builds-of-a-pipeline.yml new file mode 100644 index 00000000000..5a7cd42b888 --- /dev/null +++ b/changelogs/unreleased/memoize-the-latest-builds-of-a-pipeline.yml @@ -0,0 +1,5 @@ +--- +title: "Memoize the latest builds of a pipeline on a project's homepage" +merge_request: +author: +type: other diff --git a/changelogs/unreleased/projects-controller-show.yml b/changelogs/unreleased/projects-controller-show.yml new file mode 100644 index 00000000000..25f4a72710b --- /dev/null +++ b/changelogs/unreleased/projects-controller-show.yml @@ -0,0 +1,5 @@ +--- +title: Memoize pipelines for project download buttons +merge_request: +author: +type: other diff --git a/db/migrate/20170913131410_environments_project_id_not_null.rb b/db/migrate/20170913131410_environments_project_id_not_null.rb new file mode 100644 index 00000000000..d5404f8ede9 --- /dev/null +++ b/db/migrate/20170913131410_environments_project_id_not_null.rb @@ -0,0 +1,16 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class EnvironmentsProjectIdNotNull < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + change_column_null :environments, :project_id, false + end + + def down + change_column_null :environments, :project_id, true + end +end diff --git a/db/migrate/20170914135630_add_index_for_recent_push_events.rb b/db/migrate/20170914135630_add_index_for_recent_push_events.rb new file mode 100644 index 00000000000..99f593b0465 --- /dev/null +++ b/db/migrate/20170914135630_add_index_for_recent_push_events.rb @@ -0,0 +1,40 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddIndexForRecentPushEvents < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index_if_not_present( + :merge_requests, + [:source_project_id, :source_branch] + ) + + remove_concurrent_index_if_present(:merge_requests, :source_project_id) + end + + def down + add_concurrent_index_if_not_present(:merge_requests, :source_project_id) + + remove_concurrent_index_if_present( + :merge_requests, + [:source_project_id, :source_branch] + ) + end + + def add_concurrent_index_if_not_present(table, columns) + return if index_exists?(table, columns) + + add_concurrent_index(table, columns) + end + + def remove_concurrent_index_if_present(table, columns) + return unless index_exists?(table, columns) + + remove_concurrent_index(table, columns) + end +end diff --git a/db/schema.rb b/db/schema.rb index 2149f5ad23d..2d8c33591f0 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: 20170905112933) do +ActiveRecord::Schema.define(version: 20170914135630) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -520,7 +520,7 @@ ActiveRecord::Schema.define(version: 20170905112933) do add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree create_table "environments", force: :cascade do |t| - t.integer "project_id" + t.integer "project_id", null: false t.string "name", null: false t.datetime "created_at" t.datetime "updated_at" @@ -892,7 +892,7 @@ ActiveRecord::Schema.define(version: 20170905112933) do add_index "merge_requests", ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id", using: :btree add_index "merge_requests", ["milestone_id"], name: "index_merge_requests_on_milestone_id", using: :btree add_index "merge_requests", ["source_branch"], name: "index_merge_requests_on_source_branch", using: :btree - add_index "merge_requests", ["source_project_id"], name: "index_merge_requests_on_source_project_id", using: :btree + add_index "merge_requests", ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_id_and_source_branch", using: :btree add_index "merge_requests", ["target_branch"], name: "index_merge_requests_on_target_branch", using: :btree add_index "merge_requests", ["target_project_id", "iid"], name: "index_merge_requests_on_target_project_id_and_iid", unique: true, using: :btree add_index "merge_requests", ["title"], name: "index_merge_requests_on_title", using: :btree diff --git a/doc/administration/reply_by_email.md b/doc/administration/reply_by_email.md index e99a7ee29cc..1304476e678 100644 --- a/doc/administration/reply_by_email.md +++ b/doc/administration/reply_by_email.md @@ -77,6 +77,33 @@ and use [an application password](https://support.google.com/mail/answer/185833) To set up a basic Postfix mail server with IMAP access on Ubuntu, follow the [Postfix setup documentation](reply_by_email_postfix_setup.md). +### Security Concerns + +**WARNING:** Be careful when choosing the domain used for receiving incoming +email. + +For the sake of example, suppose your top-level company domain is `hooli.com`. +All employees in your company have an email address at that domain via Google +Apps, and your company's private Slack instance requires a valid `@hooli.com` +email address in order to sign up. + +If you also host a public-facing GitLab instance at `hooli.com` and set your +incoming email domain to `hooli.com`, an attacker could abuse the "Create new +issue by email" feature by using a project's unique address as the email when +signing up for Slack, which would send a confirmation email, which would create +a new issue on the project owned by the attacker, allowing them to click the +confirmation link and validate their account on your company's private Slack +instance. + +We recommend receiving incoming email on a subdomain, such as +`incoming.hooli.com`, and ensuring that you do not employ any services that +authenticate solely based on access to an email domain such as `*.hooli.com.` +Alternatively, use a dedicated domain for GitLab email communications such as +`hooli-gitlab.com`. + +See GitLab issue [#30366](https://gitlab.com/gitlab-org/gitlab-ce/issues/30366) +for a real-world example of this exploit. + ### Omnibus package installations 1. Find the `incoming_email` section in `/etc/gitlab/gitlab.rb`, enable the @@ -141,7 +168,7 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow the # The IDLE command timeout. gitlab_rails['incoming_email_idle_timeout'] = 60 ``` - + ```ruby # Configuration for Microsoft Exchange mail server w/ IMAP enabled, assumes mailbox incoming@exchange.example.com gitlab_rails['incoming_email_enabled'] = true @@ -253,7 +280,7 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow the # The IDLE command timeout. idle_timeout: 60 ``` - + ```yaml # Configuration for Microsoft Exchange mail server w/ IMAP enabled, assumes mailbox incoming@exchange.example.com incoming_email: diff --git a/doc/user/search/index.md b/doc/user/search/index.md index bcc3625f908..2b23c494dc4 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -31,8 +31,8 @@ on the search field on the top-right of your screen: If you want to search for issues present in a specific project, navigate to a project's **Issues** tab, and click on the field **Search or filter results...**. It will -display a dropdown menu, from which you can add filters per author, assignee, milestone, label, -and weight. When done, press **Enter** on your keyboard to filter the issues. +display a dropdown menu, from which you can add filters per author, assignee, milestone, +label, weight, and 'my-reaction' (based on your emoji votes). When done, press **Enter** on your keyboard to filter the issues. ![filter issues in a project](img/issue_search_filter.png) diff --git a/lib/api/users.rb b/lib/api/users.rb index 1825c90a23b..bdebda58d3f 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -88,7 +88,7 @@ module API user = User.find_by(id: params[:id]) not_found!('User') unless user && can?(current_user, :read_user, user) - opts = current_user&.admin? ? { with: Entities::UserWithAdmin } : {} + opts = current_user&.admin? ? { with: Entities::UserWithAdmin } : { with: Entities::User } present user, opts end diff --git a/lib/gitlab/database/read_only_relation.rb b/lib/gitlab/database/read_only_relation.rb new file mode 100644 index 00000000000..4571ad122ce --- /dev/null +++ b/lib/gitlab/database/read_only_relation.rb @@ -0,0 +1,16 @@ +module Gitlab + module Database + # Module that can be injected into a ActiveRecord::Relation to make it + # read-only. + module ReadOnlyRelation + [:delete, :delete_all, :update, :update_all].each do |method| + define_method(method) do |*args| + raise( + ActiveRecord::ReadOnlyRecord, + "This relation is marked as read-only" + ) + end + end + end + end +end diff --git a/lib/gitlab/group_hierarchy.rb b/lib/gitlab/group_hierarchy.rb index 5a31e56cb30..635f52131f9 100644 --- a/lib/gitlab/group_hierarchy.rb +++ b/lib/gitlab/group_hierarchy.rb @@ -22,7 +22,7 @@ module Gitlab def base_and_ancestors return ancestors_base unless Group.supports_nested_groups? - base_and_ancestors_cte.apply_to(model.all) + read_only(base_and_ancestors_cte.apply_to(model.all)) end # Returns a relation that includes the descendants_base set of groups @@ -30,7 +30,7 @@ module Gitlab def base_and_descendants return descendants_base unless Group.supports_nested_groups? - base_and_descendants_cte.apply_to(model.all) + read_only(base_and_descendants_cte.apply_to(model.all)) end # Returns a relation that includes the base groups, their ancestors, @@ -67,11 +67,13 @@ module Gitlab union = SQL::Union.new([model.unscoped.from(ancestors_table), model.unscoped.from(descendants_table)]) - model + relation = model .unscoped .with .recursive(ancestors.to_arel, descendants.to_arel) .from("(#{union.to_sql}) #{model.table_name}") + + read_only(relation) end private @@ -107,5 +109,12 @@ module Gitlab def groups_table model.arel_table end + + def read_only(relation) + # relations using a CTE are not safe to use with update_all as it will + # throw away the CTE, hence we mark them as read-only. + relation.extend(Gitlab::Database::ReadOnlyRelation) + relation + end end end diff --git a/spec/lib/gitlab/group_hierarchy_spec.rb b/spec/lib/gitlab/group_hierarchy_spec.rb index 08010c2d0e2..8dc83a6db7f 100644 --- a/spec/lib/gitlab/group_hierarchy_spec.rb +++ b/spec/lib/gitlab/group_hierarchy_spec.rb @@ -23,6 +23,11 @@ describe Gitlab::GroupHierarchy, :postgresql do expect(relation).to include(parent, child1, child2) end + + it 'does not allow the use of #update_all' do + expect { relation.update_all(share_with_group_lock: false) } + .to raise_error(ActiveRecord::ReadOnlyRecord) + end end describe '#base_and_descendants' do @@ -43,6 +48,11 @@ describe Gitlab::GroupHierarchy, :postgresql do expect(relation).to include(parent, child1, child2) end + + it 'does not allow the use of #update_all' do + expect { relation.update_all(share_with_group_lock: false) } + .to raise_error(ActiveRecord::ReadOnlyRecord) + end end describe '#all_groups' do @@ -73,5 +83,10 @@ describe Gitlab::GroupHierarchy, :postgresql do expect(relation).to include(child2) end + + it 'does not allow the use of #update_all' do + expect { relation.update_all(share_with_group_lock: false) } + .to raise_error(ActiveRecord::ReadOnlyRecord) + end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 95da97b7bc5..77f0be6b120 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1439,4 +1439,24 @@ describe Ci::Pipeline, :mailer do it_behaves_like 'not sending any notification' end end + + describe '#latest_builds_with_artifacts' do + let!(:pipeline) { create(:ci_pipeline, :success) } + + let!(:build) do + create(:ci_build, :success, :artifacts, pipeline: pipeline) + end + + it 'returns the latest builds' do + expect(pipeline.latest_builds_with_artifacts).to eq([build]) + end + + it 'memoizes the returned relation' do + query_count = ActiveRecord::QueryRecorder + .new { 2.times { pipeline.latest_builds_with_artifacts.to_a } } + .count + + expect(query_count).to eq(1) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 48fc77423ff..78226c6c3fa 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2682,4 +2682,60 @@ describe Project do end end end + + describe '#latest_successful_builds_for' do + let(:project) { build(:project) } + + before do + allow(project).to receive(:default_branch).and_return('master') + end + + context 'without a ref' do + it 'returns a pipeline for the default branch' do + expect(project) + .to receive(:latest_successful_pipeline_for_default_branch) + + project.latest_successful_pipeline_for + end + end + + context 'with the ref set to the default branch' do + it 'returns a pipeline for the default branch' do + expect(project) + .to receive(:latest_successful_pipeline_for_default_branch) + + project.latest_successful_pipeline_for(project.default_branch) + end + end + + context 'with a ref that is not the default branch' do + it 'returns the latest successful pipeline for the given ref' do + expect(project.pipelines).to receive(:latest_successful_for).with('foo') + + project.latest_successful_pipeline_for('foo') + end + end + end + + describe '#latest_successful_pipeline_for_default_branch' do + let(:project) { build(:project) } + + before do + allow(project).to receive(:default_branch).and_return('master') + end + + it 'memoizes and returns the latest successful pipeline for the default branch' do + pipeline = double(:pipeline) + + expect(project.pipelines).to receive(:latest_successful_for) + .with(project.default_branch) + .and_return(pipeline) + .once + + 2.times do + expect(project.latest_successful_pipeline_for_default_branch) + .to eq(pipeline) + end + end + end end |