diff options
33 files changed, 177 insertions, 174 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cedfa60b3e..5d3cd42a95d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,25 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 9.5.5 (2017-09-18) + +- [SECURITY] Upgrade mail and nokogiri gems due to security issues. !13662 (Markus Koller) +- [FIXED] Fix division by zero error in blame age mapping. !13803 (Jeff Stubler) +- [FIXED] Fix problems sanitizing URLs with empty passwords. !14083 +- [FIXED] Fix a wrong `X-Gitlab-Event` header when testing webhooks. !14108 +- [FIXED] Fixes the 500 errors caused by a race condition in GPG's tmp directory handling. !14194 (Alexis Reigel) +- [FIXED] Fix Pipeline Triggers to show triggered label and predefined variables (e.g. CI_PIPELINE_TRIGGERED). !14244 +- [FIXED] Fix project feature being deleted when updating project with invalid visibility level. +- [FIXED] Fix new navigation wrapping and causing height to grow. +- [FIXED] Fix buttons with different height in merge request widget. +- [FIXED] Normalize styles for empty state combo button. +- [FIXED] Fix broken svg in jobs dropdown for success status. +- [FIXED] Improve migrations using triggers. +- [FIXED] Disable GitLab Project Import Button if source disabled. +- [CHANGED] Update the GPG verification semantics: A GPG signature must additionally match the committer in order to be verified. !13771 (Alexis Reigel) +- [OTHER] Fix repository equality check and avoid fetching ref if the commit is already available. This affects merge request creation performance. !13685 +- [OTHER] Update documentation for confidential issue. !14117 + ## 9.5.4 (2017-09-06) - [SECURITY] Upgrade mail and nokogiri gems due to security issues. !13662 (Markus Koller) diff --git a/app/assets/javascripts/commit/image_file.js b/app/assets/javascripts/commit/image_file.js index 17d14dc1e79..bb33e9eb783 100644 --- a/app/assets/javascripts/commit/image_file.js +++ b/app/assets/javascripts/commit/image_file.js @@ -134,8 +134,9 @@ width: maxWidth + 1, height: maxHeight + 2 }); + // Set swipeBar left position to match image frame $swipeBar.css({ - left: 0 + left: 1 }); wrapPadding = parseInt($swipeWrap.css('right').replace('px', ''), 10); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 052c005a2e8..46d31e41ada 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -727,6 +727,12 @@ ul.notes { border-bottom-left-radius: 0; } + .btn { + svg path { + fill: $gray-darkest; + } + } + .btn.discussion-create-issue-btn { margin-left: -4px; border-radius: 0; @@ -741,10 +747,6 @@ ul.notes { border: 0; } } - - .new-issue-for-discussion path { - fill: $gray-darkest; - } } } @@ -817,16 +819,6 @@ ul.notes { vertical-align: middle; } -.discussion-next-btn { - svg { - margin: 0; - - path { - fill: $gray-darkest; - } - } -} - // Merge request notes in diffs .diff-file { // Diff is inline diff --git a/app/models/event.rb b/app/models/event.rb index 0b1f053a7e6..0997b056c6a 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,7 +1,7 @@ class Event < ActiveRecord::Base include Sortable include IgnorableColumn - default_scope { reorder(nil).where.not(author_id: nil) } + default_scope { reorder(nil) } CREATED = 1 UPDATED = 2 @@ -77,6 +77,12 @@ class Event < ActiveRecord::Base scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) } + # Authors are required as they're used to display who pushed data. + # + # We're just validating the presence of the ID here as foreign key constraints + # should ensure the ID points to a valid user. + validates :author_id, presence: true + self.inheritance_column = 'action' # "data" will be removed in 10.0 but it may be possible that JOINs happen that diff --git a/app/models/key.rb b/app/models/key.rb index 4fa6cac2fd0..0c41e34d969 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -4,8 +4,6 @@ class Key < ActiveRecord::Base include Gitlab::CurrentSettings include Sortable - LAST_USED_AT_REFRESH_TIME = 1.day.to_i - belongs_to :user before_validation :generate_fingerprint @@ -54,10 +52,7 @@ class Key < ActiveRecord::Base end def update_last_used_at - lease = Gitlab::ExclusiveLease.new("key_update_last_used_at:#{id}", timeout: LAST_USED_AT_REFRESH_TIME) - return unless lease.try_obtain - - UseKeyWorker.perform_async(id) + Keys::LastUsedService.new(self).execute end def add_to_shell diff --git a/app/models/push_event.rb b/app/models/push_event.rb index 708513c7861..83ce9014094 100644 --- a/app/models/push_event.rb +++ b/app/models/push_event.rb @@ -3,12 +3,6 @@ class PushEvent < Event # different "action" value. validate :validate_push_action - # Authors are required as they're used to display who pushed data. - # - # We're just validating the presence of the ID here as foreign key constraints - # should ensure the ID points to a valid user. - validates :author_id, presence: true - # The project is required to build links to commits, commit ranges, etc. # # We're just validating the presence of the ID here as foreign key constraints diff --git a/app/services/keys/last_used_service.rb b/app/services/keys/last_used_service.rb new file mode 100644 index 00000000000..066f3246158 --- /dev/null +++ b/app/services/keys/last_used_service.rb @@ -0,0 +1,33 @@ +module Keys + class LastUsedService + TIMEOUT = 1.day.to_i + + attr_reader :key + + # key - The Key for which to update the last used timestamp. + def initialize(key) + @key = key + end + + def execute + # We _only_ want to update last_used_at and not also updated_at (which + # would be updated when using #touch). + key.update_column(:last_used_at, Time.zone.now) if update? + end + + def update? + last_used = key.last_used_at + + return false if last_used && (Time.zone.now - last_used) <= TIMEOUT + + !!redis_lease.try_obtain + end + + private + + def redis_lease + Gitlab::ExclusiveLease + .new("key_update_last_used_at:#{key.id}", timeout: TIMEOUT) + end + end +end diff --git a/app/views/discussions/_new_issue_for_all_discussions.html.haml b/app/views/discussions/_new_issue_for_all_discussions.html.haml index cab346fb514..50dd5864195 100644 --- a/app/views/discussions/_new_issue_for_all_discussions.html.haml +++ b/app/views/discussions/_new_issue_for_all_discussions.html.haml @@ -1,6 +1,8 @@ - if merge_request.discussions_can_be_resolved_by?(current_user) && can?(current_user, :create_issue, @project) .btn-group{ role: "group", "v-if" => "unresolvedDiscussionCount > 0" } - .btn.btn-default.discussion-create-issue-btn.has-tooltip{ title: "Resolve all discussions in new issue", - "aria-label" => "Resolve all discussions in a new issue", - "data-container" => "body" } - = link_to custom_icon('icon_mr_issue'), new_project_issue_path(@project, merge_request_to_resolve_discussions_of: merge_request.iid), title: "Resolve all discussions in new issue", class: 'new-issue-for-discussion' + = link_to custom_icon('icon_mr_issue'), + new_project_issue_path(@project, merge_request_to_resolve_discussions_of: merge_request.iid), + title: 'Resolve all discussions in new issue', + aria: { label: 'Resolve all discussions in new issue' }, + data: { container: 'body' }, + class: 'new-issue-for-discussion btn btn-default discussion-create-issue-btn has-tooltip' diff --git a/app/views/discussions/_new_issue_for_discussion.html.haml b/app/views/discussions/_new_issue_for_discussion.html.haml index a9bc317b8f8..2bfe118c608 100644 --- a/app/views/discussions/_new_issue_for_discussion.html.haml +++ b/app/views/discussions/_new_issue_for_discussion.html.haml @@ -2,7 +2,9 @@ %new-issue-for-discussion-btn{ ":discussion-id" => "'#{discussion.id}'", "inline-template" => true } .btn-group{ role: "group", "v-if" => "showButton" } - .btn.btn-default.discussion-create-issue-btn.has-tooltip{ title: "Resolve this discussion in a new issue", - "aria-label" => "Resolve this discussion in a new issue", - "data-container" => "body" } - = link_to custom_icon('icon_mr_issue'), new_project_issue_path(@project, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id), title: "Resolve this discussion in a new issue", class: 'new-issue-for-discussion' + = link_to custom_icon('icon_mr_issue'), + new_project_issue_path(@project, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id), + title: 'Resolve this discussion in a new issue', + aria: { label: 'Resolve this discussion in a new issue' }, + data: { container: 'body' }, + class: 'new-issue-for-discussion btn btn-default discussion-create-issue-btn has-tooltip' diff --git a/app/workers/use_key_worker.rb b/app/workers/use_key_worker.rb deleted file mode 100644 index c9d382cc5d6..00000000000 --- a/app/workers/use_key_worker.rb +++ /dev/null @@ -1,13 +0,0 @@ -class UseKeyWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue - - def perform(key_id) - key = Key.find(key_id) - key.touch(:last_used_at) - rescue ActiveRecord::RecordNotFound - Rails.logger.error("UseKeyWorker: couldn't find key with ID=#{key_id}, skipping job") - - false - end -end diff --git a/changelogs/unreleased/35441-fix-division-by-zero.yml b/changelogs/unreleased/35441-fix-division-by-zero.yml deleted file mode 100644 index 335b2d40494..00000000000 --- a/changelogs/unreleased/35441-fix-division-by-zero.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix division by zero error in blame age mapping -merge_request: 13803 -author: Jeff Stubler -type: fixed diff --git a/changelogs/unreleased/36638-select-project-to-create-issue-button-is-disconnected-from-dropdown-button.yml b/changelogs/unreleased/36638-select-project-to-create-issue-button-is-disconnected-from-dropdown-button.yml deleted file mode 100644 index 76c9c905590..00000000000 --- a/changelogs/unreleased/36638-select-project-to-create-issue-button-is-disconnected-from-dropdown-button.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Normalize styles for empty state combo button -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/36821-fix-new-nav-wrapping-caret-and-increasing-height.yml b/changelogs/unreleased/36821-fix-new-nav-wrapping-caret-and-increasing-height.yml deleted file mode 100644 index 54c7a8c8788..00000000000 --- a/changelogs/unreleased/36821-fix-new-nav-wrapping-caret-and-increasing-height.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix new navigation wrapping and causing height to grow -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/36882-disable-gitlab-project-import-button-if-source-disabled.yml b/changelogs/unreleased/36882-disable-gitlab-project-import-button-if-source-disabled.yml deleted file mode 100644 index a06c84c30e6..00000000000 --- a/changelogs/unreleased/36882-disable-gitlab-project-import-button-if-source-disabled.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Disable GitLab Project Import Button if source disabled -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml b/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml deleted file mode 100644 index d6d21ac4c51..00000000000 --- a/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix a wrong `X-Gitlab-Event` header when testing webhooks -merge_request: 14108 -author: -type: fixed diff --git a/changelogs/unreleased/37331-button-MR-widget.yml b/changelogs/unreleased/37331-button-MR-widget.yml deleted file mode 100644 index 59bc1bd201e..00000000000 --- a/changelogs/unreleased/37331-button-MR-widget.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix buttons with different height in merge request widget -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/37406-success-status-icon.yml b/changelogs/unreleased/37406-success-status-icon.yml deleted file mode 100644 index faac947f188..00000000000 --- a/changelogs/unreleased/37406-success-status-icon.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix broken svg in jobs dropdown for success status -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/38049-fix-resolve-in-new-issue-btn.yml b/changelogs/unreleased/38049-fix-resolve-in-new-issue-btn.yml new file mode 100644 index 00000000000..a904c656f4f --- /dev/null +++ b/changelogs/unreleased/38049-fix-resolve-in-new-issue-btn.yml @@ -0,0 +1,5 @@ +--- +title: Fix the "resolve discussion in a new issue" button +merge_request: 14357 +author: +type: fixed diff --git a/changelogs/unreleased/check-trigger-permissions.yml b/changelogs/unreleased/check-trigger-permissions.yml deleted file mode 100644 index e0809cea9bf..00000000000 --- a/changelogs/unreleased/check-trigger-permissions.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve migrations using triggers -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/docs-confidential-issue.yml b/changelogs/unreleased/docs-confidential-issue.yml deleted file mode 100644 index 841970ef4cf..00000000000 --- a/changelogs/unreleased/docs-confidential-issue.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Update documentation for confidential issue -merge_request: 14117 -author: -type: other diff --git a/changelogs/unreleased/events-redundant-where.yml b/changelogs/unreleased/events-redundant-where.yml new file mode 100644 index 00000000000..a118a2e33e1 --- /dev/null +++ b/changelogs/unreleased/events-redundant-where.yml @@ -0,0 +1,5 @@ +--- +title: Remove redundant WHERE from event queries +merge_request: +author: +type: other diff --git a/changelogs/unreleased/feature-gpg-verification-status.yml b/changelogs/unreleased/feature-gpg-verification-status.yml deleted file mode 100644 index 7518fafcdb8..00000000000 --- a/changelogs/unreleased/feature-gpg-verification-status.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: 'Update the GPG verification semantics: A GPG signature must additionally match - the committer in order to be verified' -merge_request: 13771 -author: Alexis Reigel -type: changed diff --git a/changelogs/unreleased/fix-gpg-tmp-dir-removal-race-condition.yml b/changelogs/unreleased/fix-gpg-tmp-dir-removal-race-condition.yml deleted file mode 100644 index e75f188913f..00000000000 --- a/changelogs/unreleased/fix-gpg-tmp-dir-removal-race-condition.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fixes the 500 errors caused by a race condition in GPG's tmp directory handling -merge_request: 14194 -author: Alexis Reigel -type: fixed diff --git a/changelogs/unreleased/fix-image-diff-swipe-handle.yml b/changelogs/unreleased/fix-image-diff-swipe-handle.yml new file mode 100644 index 00000000000..a4e0c2e8465 --- /dev/null +++ b/changelogs/unreleased/fix-image-diff-swipe-handle.yml @@ -0,0 +1,5 @@ +--- +title: Fix image diff swipe handle offset to correctly align with the frame +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml b/changelogs/unreleased/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml deleted file mode 100644 index 8aae0f6f5b6..00000000000 --- a/changelogs/unreleased/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Fix Pipeline Triggers to show triggered label and predefined variables (e.g. - CI_PIPELINE_TRIGGERED) -merge_request: 14244 -author: -type: fixed diff --git a/changelogs/unreleased/issue_37640.yml b/changelogs/unreleased/issue_37640.yml deleted file mode 100644 index d806ed64bed..00000000000 --- a/changelogs/unreleased/issue_37640.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Fix project feature being deleted when updating project with invalid visibility - level -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/perf-slow-issuable.yml b/changelogs/unreleased/perf-slow-issuable.yml deleted file mode 100644 index 29d15be1401..00000000000 --- a/changelogs/unreleased/perf-slow-issuable.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Fix repository equality check and avoid fetching ref if the commit is already - available. This affects merge request creation performance -merge_request: 13685 -author: -type: other diff --git a/changelogs/unreleased/remove-use-key-worker.yml b/changelogs/unreleased/remove-use-key-worker.yml new file mode 100644 index 00000000000..a39bcae66bc --- /dev/null +++ b/changelogs/unreleased/remove-use-key-worker.yml @@ -0,0 +1,5 @@ +--- +title: Stop using Sidekiq for updating Key#last_used_at +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/url-sanitizer-fixes.yml b/changelogs/unreleased/url-sanitizer-fixes.yml deleted file mode 100644 index 769036c829c..00000000000 --- a/changelogs/unreleased/url-sanitizer-fixes.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix problems sanitizing URLs with empty passwords -merge_request: 14083 -author: -type: fixed diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 24c001362c6..d169c38a693 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -38,7 +38,6 @@ - [invalid_gpg_signature_update, 2] - [create_gpg_signature, 2] - [upload_checksum, 1] - - [use_key, 1] - [repository_fork, 1] - [repository_import, 1] - [project_service, 1] diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index dbc4aba8547..8eabc4ca72f 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -37,30 +37,17 @@ describe Key, :mailer do end describe "#update_last_used_at" do - let(:key) { create(:key) } - - context 'when key was not updated during the last day' do - before do - allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) - .and_return('000000') - end - - it 'enqueues a UseKeyWorker job' do - expect(UseKeyWorker).to receive(:perform_async).with(key.id) - key.update_last_used_at - end - end + it 'updates the last used timestamp' do + key = build(:key) + service = double(:service) + + expect(Keys::LastUsedService).to receive(:new) + .with(key) + .and_return(service) - context 'when key was updated during the last day' do - before do - allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) - .and_return(false) - end + expect(service).to receive(:execute) - it 'does not enqueue a UseKeyWorker job' do - expect(UseKeyWorker).not_to receive(:perform_async) - key.update_last_used_at - end + key.update_last_used_at end end end diff --git a/spec/services/keys/last_used_service_spec.rb b/spec/services/keys/last_used_service_spec.rb new file mode 100644 index 00000000000..edaace293ae --- /dev/null +++ b/spec/services/keys/last_used_service_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +describe Keys::LastUsedService do + describe '#execute', :clean_gitlab_redis_shared_state do + it 'updates the key when it has not been used recently' do + key = create(:key, last_used_at: 1.year.ago) + time = Time.zone.now + + Timecop.freeze(time) { described_class.new(key).execute } + + expect(key.last_used_at).to eq(time) + end + + it 'does not update the key when it has been used recently' do + time = 1.minute.ago + key = create(:key, last_used_at: time) + + described_class.new(key).execute + + expect(key.last_used_at).to eq(time) + end + + it 'does not update the updated_at field' do + # Since a lot of these updates could happen in parallel for different keys + # we want these updates to be as lightweight as possible, hence we want to + # make sure we _only_ update last_used_at and not always updated_at. + key = create(:key, last_used_at: 1.year.ago) + + recorder = ActiveRecord::QueryRecorder.new do + described_class.new(key).execute + end + + expect(recorder.count).to eq(1) + expect(recorder.log[0]).not_to include('updated_at') + end + end + + describe '#update?', :clean_gitlab_redis_shared_state do + it 'returns true when no last used timestamp is present' do + key = build(:key, last_used_at: nil) + service = described_class.new(key) + + expect(service.update?).to eq(true) + end + + it 'returns true when the key needs to be updated' do + key = build(:key, last_used_at: 1.year.ago) + service = described_class.new(key) + + expect(service.update?).to eq(true) + end + + it 'returns false when a lease has already been obtained' do + key = build(:key, last_used_at: 1.year.ago) + service = described_class.new(key) + + expect(service.update?).to eq(true) + expect(service.update?).to eq(false) + end + + it 'returns false when the key does not yet need to be updated' do + key = build(:key, last_used_at: 1.minute.ago) + service = described_class.new(key) + + expect(service.update?).to eq(false) + end + end +end diff --git a/spec/workers/use_key_worker_spec.rb b/spec/workers/use_key_worker_spec.rb deleted file mode 100644 index e50c788b82a..00000000000 --- a/spec/workers/use_key_worker_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe UseKeyWorker do - describe "#perform" do - it "updates the key's last_used_at attribute to the current time when it exists" do - worker = described_class.new - key = create(:key) - current_time = Time.zone.now - - Timecop.freeze(current_time) do - expect { worker.perform(key.id) } - .to change { key.reload.last_used_at }.from(nil).to be_like_time(current_time) - end - end - - it "returns false and skips the job when the key doesn't exist" do - worker = described_class.new - key = create(:key) - - expect(worker.perform(key.id + 1)).to eq false - end - end -end |