Merge branch 'ce-to-ee-2018-11-27' into 'master'

CE upstream - 2018-11-27 21:21 UTC

Closes gitlab-ce#54503

See merge request gitlab-org/gitlab-ee!8613

app/assets/javascripts/merge_request_tabs.js

@@ -217,6 +217,28 @@ export default class MergeRequestTabs {
       }
 
       this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
+    } else if (action === this.currentAction) {
+      // ContentTop is used to handle anything at the top of the page before the main content
+      const mainContentContainer = document.querySelector('.content-wrapper');
+      const tabContentContainer = document.querySelector('.tab-content');
+
+      if (mainContentContainer && tabContentContainer) {
+        const mainContentTop = mainContentContainer.getBoundingClientRect().top;
+        const tabContentTop = tabContentContainer.getBoundingClientRect().top;
+
+        // 51px is the height of the navbar buttons, e.g. `Discussion | Commits | Changes`
+        const scrollDestination = tabContentTop - mainContentTop - 51;
+
+        // scrollBehavior is only available in browsers that support scrollToOptions
+        if ('scrollBehavior' in document.documentElement.style) {
+          window.scrollTo({
+            top: scrollDestination,
+            behavior: 'smooth',
+          });
+        } else {
+          window.scrollTo(0, scrollDestination);
+        }
+      }
     }
   }
 

app/assets/javascripts/pages/projects/commit/show/index.js

@@ -11,6 +11,8 @@ import initDiffNotes from '~/diff_notes/diff_notes_bundle';
 import { fetchCommitMergeRequests } from '~/commit_merge_requests';
 
 document.addEventListener('DOMContentLoaded', () => {
+  const hasPerfBar = document.querySelector('.with-performance-bar');
+  const performanceHeight = hasPerfBar ? 35 : 0;
   new Diff();
   new ZenMode();
   new ShortcutsNavigation();
@@ -18,8 +20,7 @@ document.addEventListener('DOMContentLoaded', () => {
     container: '.js-commit-pipeline-graph',
   }).bindEvents();
   initNotes();
-  const stickyBarPaddingTop = 16;
-  initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight - stickyBarPaddingTop);
+  initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight + performanceHeight);
   $('.commit-info.branches').load(document.querySelector('.js-commit-box').dataset.commitPath);
   fetchCommitMergeRequests();
   initDiffNotes();

app/assets/stylesheets/framework/dropdowns.scss

@@ -802,11 +802,7 @@
 
 @include media-breakpoint-down(xs) {
   .navbar-gitlab {
-    li.header-projects,
-    li.header-groups,
-    li.header-more,
-    li.header-new,
-    li.header-user {
+    li.dropdown {
       position: static;
     }
   }

app/assets/stylesheets/framework/header.scss

@@ -110,6 +110,10 @@
       }
     }
 
+    .navbar-collapse > ul.nav > li:not(.d-none) {
+      margin: 0 2px;
+    }
+
     &.menu-expanded {
       @include media-breakpoint-down(xs) {
         .title-container {
@@ -117,7 +121,7 @@
         }
 
         .navbar-collapse {
-          display: block;
+          display: flex;
         }
       }
     }
@@ -209,7 +213,7 @@
 
       > a {
         will-change: color;
-        margin: 4px 2px;
+        margin: 4px 0;
         padding: 6px 8px;
         height: 32px;
 
@@ -455,14 +459,11 @@
   color: $indigo-900;
   font-weight: $gl-font-weight-bold;
   line-height: 18px;
+  margin: 4px 0 4px 2px;
 
   &:hover {
     background-color: $white-light;
   }
-
-  @include media-breakpoint-down(xs) {
-    margin-top: $gl-padding-4;
-  }
 }
 
 .navbar-nav {
@@ -509,12 +510,7 @@
       margin-right: -10px;
 
       .nav > li:not(.d-none) {
-        display: table-cell !important;
-        width: 25%;
-
-        a {
-          margin-right: 8px;
-        }
+        flex: 1;
       }
     }
   }

app/assets/stylesheets/pages/diff.scss

@@ -10,22 +10,32 @@
       position: -webkit-sticky;
       position: sticky;
       top: 92px;
+      margin-left: -1px;
+      border-left: 1px solid $border-color;
       z-index: 102;
 
+      &.is-commit {
+        top: $header-height + 36px;
+
+        .with-performance-bar & {
+          top: $header-height + 36px + $performance-bar-height;
+        }
+      }
+
       &::before {
         content: '';
         position: absolute;
         top: -1px;
-        left: -10px;
+        left: -11px;
         width: 10px;
         height: calc(100% + 1px);
         background: $white-light;
-        border-right: 1px solid $border-color;
+        pointer-events: none;
       }
-    }
 
-    .with-performance-bar & {
-      top: 127px;
+      .with-performance-bar & {
+        top: 127px;
+      }
     }
 
     a:hover {
@@ -701,15 +711,14 @@
   }
 
   @include media-breakpoint-up(sm) {
-    top: 24px;
+    position: -webkit-sticky;
+    position: sticky;
+    top: $header-height;
     background-color: $white-light;
+    z-index: 200;
 
-    &.diff-files-changed-merge-request {
-      position: sticky;
-      top: 90px;
-      z-index: 200;
-      margin: $gl-padding 0;
-      padding: 0;
+    .with-performance-bar & {
+      top: $header-height + $performance-bar-height;
     }
 
     &.is-stuck {
@@ -734,14 +743,6 @@
   }
 }
 
-@include media-breakpoint-up(sm) {
-  .with-performance-bar {
-    .diff-files-changed.diff-files-changed-merge-request {
-      top: 76px + $performance-bar-height;
-    }
-  }
-}
-
 .diff-file-changes {
   max-width: 560px;
   width: 100%;

app/assets/stylesheets/pages/notes.scss

@@ -393,6 +393,14 @@ $note-form-margin-left: 72px;
     border-top: 1px solid $border-color;
     border-radius: 0;
 
+    @media (min-width: map-get($grid-breakpoints, md)) {
+      top: 91px;
+
+      .with-performance-bar & {
+        top: 126px;
+      }
+    }
+
     &:hover {
       background-color: $gray-light;
     }

app/models/ci/build_trace_chunk.rb

@@ -76,7 +76,7 @@ module Ci
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       raise ArgumentError, 'Chunk size overflow' if CHUNK_SIZE < (offset + new_data.bytesize)
 
-      in_lock(*lock_params) do # Write opetation is atomic
+      in_lock(*lock_params) do # Write operation is atomic
         unsafe_set_data!(data.byteslice(0, offset) + new_data)
       end
 
@@ -100,7 +100,7 @@ module Ci
     end
 
     def persist_data!
-      in_lock(*lock_params) do # Write opetation is atomic
+      in_lock(*lock_params) do # Write operation is atomic
         unsafe_persist_to!(self.class.persistable_store)
       end
     end

app/services/clusters/applications/base_helm_service.rb

@@ -11,6 +11,25 @@ module Clusters
 
       protected
 
+      def log_error(error)
+        meta = {
+          exception: error.class.name,
+          error_code: error.respond_to?(:error_code) ? error.error_code : nil,
+          service: self.class.name,
+          app_id: app.id,
+          project_ids: app.cluster.project_ids,
+          group_ids: app.cluster.group_ids,
+          message: error.message
+        }
+
+        logger.error(meta)
+        Gitlab::Sentry.track_acceptable_exception(error, extra: meta)
+      end
+
+      def logger
+        @logger ||= Gitlab::Kubernetes::Logger.build
+      end
+
       def cluster
         app.cluster
       end

app/services/clusters/applications/check_installation_progress_service.rb

@@ -15,8 +15,7 @@ module Clusters
           check_timeout
         end
       rescue Kubeclient::HttpError => e
-        Rails.logger.error("Kubernetes error: #{e.error_code} #{e.message}")
-        Gitlab::Sentry.track_acceptable_exception(e, extra: { scope: 'kubernetes', app_id: app.id })
+        log_error(e)
         app.make_errored!("Kubernetes error: #{e.error_code}") unless app.errored?
       end
 

app/services/clusters/applications/install_service.rb

@@ -13,12 +13,10 @@ module Clusters
           ClusterWaitForAppInstallationWorker.perform_in(
             ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id)
         rescue Kubeclient::HttpError => e
-          Rails.logger.error("Kubernetes error: #{e.error_code} #{e.message}")
-          Gitlab::Sentry.track_acceptable_exception(e, extra: { scope: 'kubernetes', app_id: app.id })
+          log_error(e)
           app.make_errored!("Kubernetes error: #{e.error_code}")
         rescue StandardError => e
-          Rails.logger.error "Can't start installation process: #{e.class.name} #{e.message}"
-          Gitlab::Sentry.track_acceptable_exception(e, extra: { scope: 'kubernetes', app_id: app.id })
+          log_error(e)
           app.make_errored!("Can't start installation process.")
         end
       end

app/views/layouts/header/_default.html.haml

@@ -71,7 +71,7 @@
               = link_to admin_impersonation_path, class: 'nav-link impersonation-btn', method: :delete, title: _('Stop impersonation'), aria: { label: _('Stop impersonation') }, data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do
                 = icon('user-secret')
           - if header_link?(:sign_in)
-            %li.nav-item.m-auto
+            %li.nav-item
               %div
                 - sign_in_text = allow_signup? ? _('Sign in / Register') : _('Sign in')
                 = link_to sign_in_text, new_session_path(:user, redirect_to_referer: 'yes'), class: 'btn btn-sign-in'

app/views/projects/commit/show.html.haml

@@ -13,7 +13,7 @@
     = render "ci_menu"
   - else
     .block-connector
-  = render "projects/diffs/diffs", diffs: @diffs, environment: @environment
+  = render "projects/diffs/diffs", diffs: @diffs, environment: @environment, is_commit: true
 
   .limited-width-notes
     = render "shared/notes/notes_with_form", :autocomplete => true

app/views/projects/diffs/_diffs.html.haml

@@ -2,9 +2,9 @@
 - show_whitespace_toggle = local_assigns.fetch(:show_whitespace_toggle, true)
 - can_create_note = !@diff_notes_disabled && can?(current_user, :create_note, diffs.project)
 - diff_files = diffs.diff_files
-- merge_request = local_assigns.fetch(:merge_request, false)
+- is_commit = local_assigns.fetch(:is_commit, false)
 
-.content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed{ class: ("diff-files-changed-merge-request" if merge_request) }
+.content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed
   .files-changed-inner
     .inline-parallel-buttons.d-none.d-sm-none.d-md-block
       - if !diffs_expanded? && diff_files.any? { |diff_file| diff_file.collapsed? }
@@ -25,4 +25,4 @@
   = render 'projects/diffs/warning', diff_files: diffs
 
 .files{ data: { can_create_note: can_create_note } }
-  = render partial: 'projects/diffs/file', collection: diff_files, as: :diff_file, locals: { project: diffs.project, environment: environment }
+  = render partial: 'projects/diffs/file', collection: diff_files, as: :diff_file, locals: { project: diffs.project, environment: environment, is_commit: is_commit }

app/views/projects/diffs/_file.html.haml

 - environment = local_assigns.fetch(:environment, nil)
+- is_commit = local_assigns.fetch(:is_commit, false)
 - file_hash = hexdigest(diff_file.file_path)
 - image_diff = diff_file.rich_viewer && diff_file.rich_viewer.partial_name == 'image'
 - image_replaced = diff_file.old_content_sha && diff_file.old_content_sha != diff_file.content_sha
 
 .diff-file.file-holder{ id: file_hash, data: diff_file_html_data(project, diff_file.file_path, diff_file.content_sha) }
-  .js-file-title.file-title-flex-parent
+  .js-file-title.file-title-flex-parent{ class: is_commit ? "is-commit" : "" }
     .file-header-content
       = render "projects/diffs/file_header", diff_file: diff_file, url: "##{file_hash}"
 

app/views/projects/merge_requests/show.html.haml

@@ -21,7 +21,7 @@
       window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget')}
 
       window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
-      window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests', anchor: 'troubleshooting')}';
+      window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/index.md', anchor: 'troubleshooting')}';
 
     #js-vue-mr-widget.mr-widget
 

app/views/projects/new.html.haml

@@ -4,10 +4,10 @@
 - header_title  "Projects", dashboard_projects_path
 - active_tab = local_assigns.fetch(:active_tab, 'blank')
 
-.project-edit-container
+.project-edit-container.prepend-top-default
   .project-edit-errors
     = render 'projects/errors'
-  .row.prepend-top-default
+  .row
     .col-lg-3.profile-settings-sidebar
       %h4.prepend-top-0
         = _('New project')

changelogs/unreleased/52276-jump-to-top-in-merge-request.yml

+---
+title: Allow user to scroll to top of tab on MR page
+merge_request:
+author:
+type: added

changelogs/unreleased/53763-fix-encrypt-columns-data-loss.yml

+---
+title: Correctly handle data-loss scenarios when encrypting columns
+merge_request: 23306
+author:
+type: fixed

changelogs/unreleased/53874-navbar-lowres.yml

+---
+title: "Fix overlapping navbar separator and overflowing navbar dropdown on small displays"
+merge_request: 23126
+author: Thomas Pathier
+type: fix

changelogs/unreleased/lock-trace-writes.yml

+---
+title: Lock writes to trace stream
+merge_request:
+author:
+type: fixed

config/initializers/attr_encrypted_no_db_connection.rb

 module AttrEncrypted
   module Adapters
     module ActiveRecord
-      module DBConnectionQuerier
+      module GitlabMonkeyPatches
+        # Prevent attr_encrypted from defining virtual accessors for encryption
+        # data when the code and schema are out of sync. See this issue for more
+        # details: https://github.com/attr-encrypted/attr_encrypted/issues/332
+        def attribute_instance_methods_as_symbols_available?
+          false
+        end
+
+        # Prevent attr_encrypted from checking out a database connection
+        # indefinitely. The result of this method is only used when the former
+        # is true, but it is called unconditionally, so there is still value to
+        # ensuring the connection is released
         def attribute_instance_methods_as_symbols
           # Use with_connection so the connection doesn't stay pinned to the thread.
           connected = ::ActiveRecord::Base.connection_pool.with_connection(&:active?) rescue false
@@ -15,7 +26,16 @@ module AttrEncrypted
           end
         end
       end
-      prepend DBConnectionQuerier
     end
   end
 end
+
+# As of v3.1.0, the attr_encrypted gem defines the AttrEncrypted and
+# AttrEncrypted::Adapters::ActiveRecord modules, and uses "extend" to mix them
+# into the ActiveRecord::Base class. This intervention overrides utility methods
+# defined by attr_encrypted to fix two bugs, as detailed above.
+#
+# The methods are used here: https://github.com/attr-encrypted/attr_encrypted/blob/3.1.0/lib/attr_encrypted.rb#L145-158
+ActiveSupport.on_load(:active_record) do
+  extend AttrEncrypted::Adapters::ActiveRecord::GitlabMonkeyPatches
+end

doc/administration/high_availability/gitlab.md

@@ -122,7 +122,7 @@ need some extra configuration.
     gitlab_rails['db_key_base'] = 'bf2e47b68d6cafaef1d767e628b619365becf27571e10f196f98dc85e7771042b9203199d39aff91fcb6837c8ed83f2a912b278da50999bb11a2fbc0fba52964'
     ```
 
-1. Run `touch /etc/gitlab/skip-auto-migrations` to prevent database migrations
+1. Run `touch /etc/gitlab/skip-auto-reconfigure` to prevent database migrations
    from running on upgrade. Only the primary GitLab application server should
    handle migrations.
 

doc/administration/high_availability/redis.md

@@ -338,7 +338,7 @@ The prerequisites for a HA Redis setup are the following:
 1. To prevent reconfigure from running automatically on upgrade, run:
 
     ```
-    sudo touch /etc/gitlab/skip-auto-migrations
+    sudo touch /etc/gitlab/skip-auto-reconfigure
     ```
 
 1. [Reconfigure Omnibus GitLab][reconfigure] for the changes to take effect.
@@ -462,7 +462,7 @@ multiple machines with the Sentinel daemon.
 1. To prevent database migrations from running on upgrade, run:
 
     ```
-    sudo touch /etc/gitlab/skip-auto-migrations
+    sudo touch /etc/gitlab/skip-auto-reconfigure
     ```
 
     Only the primary GitLab application server should handle migrations.

doc/administration/logs.md

@@ -126,6 +126,25 @@ It contains information about [integrations](../user/project/integrations/projec
 {"severity":"INFO","time":"2018-09-06T17:15:16.365Z","service_class":"JiraService","project_id":3,"project_path":"namespace2/project2","message":"Successfully posted","client_url":"http://jira.example.net"}
 ```
 
+## `kubernetes.log`
+
+Introduced in GitLab 11.6. This file lives in
+`/var/log/gitlab/gitlab-rails/kubernetes.log` for Omnibus GitLab
+packages or in `/home/git/gitlab/log/kubernetes.log` for
+installations from source.
+
+It logs information related to the Kubernetes Integration including errors
+during installing cluster applications on your GitLab managed Kubernetes
+clusters.
+
+Each line contains a JSON line that can be ingested by Elasticsearch, Splunk,
+etc. For example:
+
+```json
+{"severity":"ERROR","time":"2018-11-23T15:14:54.652Z","exception":"Kubeclient::HttpError","error_code":401,"service":"Clusters::Applications::CheckInstallationProgressService","app_id":14,"project_ids":[1],"group_ids":[],"message":"Unauthorized"}
+{"severity":"ERROR","time":"2018-11-23T15:42:11.647Z","exception":"Kubeclient::HttpError","error_code":null,"service":"Clusters::Applications::InstallService","app_id":2,"project_ids":[19],"group_ids":[],"message":"SSL_connect returned=1 errno=0 state=error: certificate verify failed (unable to get local issuer certificate)"}
+```
+
 ## `githost.log`
 
 This file lives in `/var/log/gitlab/gitlab-rails/githost.log` for

doc/development/db_dump.md

@@ -13,7 +13,7 @@ large database imports.
 ```
 # On STAGING
 echo "postgresql['checkpoint_segments'] = 64" | sudo tee -a /etc/gitlab/gitlab.rb
-sudo touch /etc/gitlab/skip-auto-migrations
+sudo touch /etc/gitlab/skip-auto-reconfigure
 sudo gitlab-ctl reconfigure
 sudo gitlab-ctl stop unicorn
 sudo gitlab-ctl stop sidekiq

doc/development/logging.md

@@ -75,7 +75,7 @@ To create a new file:
           module Import
             class Logger < ::Gitlab::JsonLogger
               def self.file_name_noext
-                'importer_json'
+                'importer'
               end
             end
            end
@@ -105,7 +105,7 @@ To create a new file:
 
     ```ruby
     # GOOD
-    logger.info("Unable to create project", project_id: project.id)
+    logger.info(message: "Unable to create project", project_id: project.id)
     ```
 
 1. Be sure to create a common base structure of your log messages. For example,
@@ -118,13 +118,13 @@ To create a new file:
 
     ```ruby
     # BAD
-    logger.info("Import error", error: 1)
-    logger.info("Import error", error: "I/O failure")
+    logger.info(message: "Import error", error: 1)
+    logger.info(message: "Import error", error: "I/O failure")
     ```
 
     ```ruby
     # GOOD
-    logger.info("Import error", error_code: 1, error: "I/O failure")
+    logger.info(message: "Import error", error_code: 1, error: "I/O failure")
     ```
 
 ## Additional steps with new log files

doc/integration/github.md

-# Integrate your server with GitHub
+# Integrate your GitLab instance with GitHub
 
-Import projects from GitHub and login to your GitLab instance with your GitHub account.
+You can integrate your GitLab instance with GitHub.com as well as GitHub Enterprise to enable users to import projects from GitHub and/or to login to your GitLab instance with your GitHub account.
 
-To enable the GitHub OmniAuth provider you must register your application with GitHub.
-GitHub will generate an application ID and secret key for you to use.
+## Enabling GitHub OAuth
 
-1.  Sign in to GitHub.
+To enable GitHub OmniAuth provider, you must use GitHub's credentials for your GitLab instance.
+To get the credentials (a pair of Client ID and Client Secret), you must register an application as an OAuth App on GitHub.
 
-1.  Navigate to your individual user settings or an organization's settings, depending on how you want the application registered. It does not matter if the application is registered as an individual or an organization - that is entirely up to you.
+1.  Sign in to GitHub.
 
-1.  Select "OAuth applications" in the left menu.
+1.  Navigate to your individual user or organization settings, depending on how you want the application registered. It does not matter if the application is registered as an individual or an organization - that is entirely up to you.
 
-1.  If you already have applications listed, switch to the "Developer applications" tab.
+    - For individual accounts, select **Developer settings** from the left menu, then select **OAuth Apps**.
+    - For organization accounts, directly select **OAuth Apps** from the left menu.
 
-1.  Select "Register new application".
+1.  Select **Register an application** (if you don't have any OAuth App) or **New OAuth App** (if you already have OAuth Apps).
+    ![Register OAuth App](img/github_app_entry.png)
 
 1.  Provide the required details.
     - Application name: This can be anything. Consider something like `<Organization>'s GitLab` or `<Your Name>'s GitLab` or something else descriptive.
-    - Homepage URL: The URL to your GitLab installation. 'https://gitlab.company.com'
+    - Homepage URL: the URL to your GitLab installation. e.g., `https://gitlab.company.com`
     - Application description: Fill this in if you wish.
-    - Authorization callback URL is 'http(s)://${YOUR_DOMAIN}'. Please make sure the port is included if your GitLab instance is not configured on default port.
-1.  Select "Register application".
+    - Authorization callback URL: `http(s)://${YOUR_DOMAIN}`. Please make sure the port is included if your GitLab instance is not configured on default port.
+    ![Register OAuth App](img/github_register_app.png)
+
+1.  Select **Register application**.
 
-1.  You should now see a Client ID and Client Secret near the top right of the page (see screenshot).
+1.  You should now see a pair of **Client ID** and **Client Secret** near the top right of the page (see screenshot).
     Keep this page open as you continue configuration.
     ![GitHub app](img/github_app.png)
 
@@ -97,9 +101,9 @@ GitHub will generate an application ID and secret key for you to use.
 
     __Replace `https://github.example.com/` with your GitHub URL.__
 
-1.  Change 'YOUR_APP_ID' to the client ID from the GitHub application page from step 7.
+1.  Change `YOUR_APP_ID` to the Client ID from the GitHub application page from step 6.
 
-1.  Change 'YOUR_APP_SECRET' to the client secret from the GitHub application page  from step 7.
+1.  Change `YOUR_APP_SECRET` to the Client Secret from the GitHub application page from step 6.
 
 1.  Save the configuration file.
 

doc/user/project/pages/introduction.md

@@ -446,7 +446,9 @@ See also: [GitLab Pages from A to Z: Part 1 - Static sites and GitLab Pages doma
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/33422) in GitLab 11.5.
 
 NOTE: **Note:**
-GitLab Pages access control is not activated on GitLab.com.
+GitLab Pages access control is not activated on GitLab.com. You can check its
+progress on the
+[infrastructure issue tracker](https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/5576).
 
 You can enable Pages access control on your project, so that only
 [members of your project](../../permissions.md#project-members-permissions)

lib/api/api.rb

@@ -57,6 +57,10 @@ module API
       rack_response({ 'message' => '404 Not found' }.to_json, 404)
     end
 
+    rescue_from ::Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError do
+      rack_response({ 'message' => '409 Conflict: Resource lock' }.to_json, 409)
+    end
+
     rescue_from UploadedFile::InvalidPathError do |e|
       rack_response({ 'message' => e.message }.to_json, 400)
     end

lib/gitlab/background_migration/encrypt_columns.rb

@@ -17,6 +17,12 @@ module Gitlab
     class EncryptColumns
       def perform(model, attributes, from, to)
         model = model.constantize if model.is_a?(String)
+
+        # If sidekiq hasn't undergone a restart, its idea of what columns are
+        # present may be inaccurate, so ensure this is as fresh as possible
+        model.reset_column_information
+        model.define_attribute_methods
+
         attributes = expand_attributes(model, Array(attributes).map(&:to_sym))
 
         model.transaction do
@@ -41,6 +47,14 @@ module Gitlab
           raise "Couldn't determine encrypted column for #{klass}##{attribute}" if
             crypt_column_name.nil?
 
+          raise "#{klass} source column: #{attribute} is missing" unless
+            klass.column_names.include?(attribute.to_s)
+
+          # Running the migration without the destination column being present
+          # leads to data loss
+          raise "#{klass} destination column: #{crypt_column_name} is missing" unless
+            klass.column_names.include?(crypt_column_name.to_s)
+
           [attribute, crypt_column_name]
         end
 

lib/gitlab/ci/trace.rb

@@ -3,9 +3,11 @@
 module Gitlab
   module Ci
     class Trace
-      include ExclusiveLeaseGuard
+      include ::Gitlab::ExclusiveLeaseHelpers
 
-      LEASE_TIMEOUT = 1.hour
+      LOCK_TTL = 1.minute
+      LOCK_RETRIES = 2
+      LOCK_SLEEP = 0.001.seconds
 
       ArchiveError = Class.new(StandardError)
       AlreadyArchivedError = Class.new(StandardError)
@@ -82,24 +84,10 @@ module Gitlab
         stream&.close
       end
 
-      def write(mode)
-        stream = Gitlab::Ci::Trace::Stream.new do
-          if trace_artifact
-            raise AlreadyArchivedError, 'Could not write to the archived trace'
-          elsif current_path
-            File.open(current_path, mode)
-          elsif Feature.enabled?('ci_enable_live_trace')
-            Gitlab::Ci::Trace::ChunkedIO.new(job)
-          else
-            File.open(ensure_path, mode)
-          end
+      def write(mode, &blk)
+        in_write_lock do
+          unsafe_write!(mode, &blk)
         end
-
-        yield(stream).tap do
-          job.touch if job.needs_touch?
-        end
-      ensure
-        stream&.close
       end
 
       def erase!
@@ -117,13 +105,33 @@ module Gitlab
       end
 
       def archive!
-        try_obtain_lease do
+        in_write_lock do
           unsafe_archive!
         end
       end
 
       private
 
+      def unsafe_write!(mode, &blk)
+        stream = Gitlab::Ci::Trace::Stream.new do
+          if trace_artifact
+            raise AlreadyArchivedError, 'Could not write to the archived trace'
+          elsif current_path
+            File.open(current_path, mode)
+          elsif Feature.enabled?('ci_enable_live_trace')
+            Gitlab::Ci::Trace::ChunkedIO.new(job)
+          else
+            File.open(ensure_path, mode)
+          end
+        end
+
+        yield(stream).tap do
+          job.touch if job.needs_touch?
+        end
+      ensure
+        stream&.close
+      end
+
       def unsafe_archive!
         raise AlreadyArchivedError, 'Could not archive again' if trace_artifact
         raise ArchiveError, 'Job is not finished yet' unless job.complete?
@@ -146,6 +154,11 @@ module Gitlab
         end
       end
 
+      def in_write_lock(&blk)
+        lock_key = "trace:write:lock:#{job.id}"
+        in_lock(lock_key, ttl: LOCK_TTL, retries: LOCK_RETRIES, sleep_sec: LOCK_SLEEP, &blk)
+      end
+
       def archive_stream!(stream)
         clone_file!(stream, JobArtifactUploader.workhorse_upload_path) do |clone_path|
           create_build_trace!(job, clone_path)
@@ -226,16 +239,6 @@ module Gitlab
       def trace_artifact
         job.job_artifacts_trace
       end
-
-      # For ExclusiveLeaseGuard concern
-      def lease_key
-        @lease_key ||= "trace:archive:#{job.id}"
-      end
-
-      # For ExclusiveLeaseGuard concern
-      def lease_timeout
-        LEASE_TIMEOUT
-      end
     end
   end
 end

lib/gitlab/ci/trace/stream.rb

@@ -43,19 +43,14 @@ module Gitlab
         def append(data, offset)
           data = data.force_encoding(Encoding::BINARY)
 
-          stream.truncate(offset)
-          stream.seek(0, IO::SEEK_END)
+          stream.seek(offset, IO::SEEK_SET)
           stream.write(data)
+          stream.truncate(offset + data.bytesize)
           stream.flush()
         end
 
         def set(data)
-          data = data.force_encoding(Encoding::BINARY)
-
-          stream.seek(0, IO::SEEK_SET)
-          stream.write(data)
-          stream.truncate(data.bytesize)
-          stream.flush()
+          append(data, 0)
         end
 
         def raw(last_lines: nil)

lib/gitlab/exclusive_lease_helpers.rb

@@ -12,6 +12,8 @@ module Gitlab
     #       because it holds the connection until all `retries` is consumed.
     #       This could potentially eat up all connection pools.
     def in_lock(key, ttl: 1.minute, retries: 10, sleep_sec: 0.01.seconds)
+      raise ArgumentError, 'Key needs to be specified' unless key
+
       lease = Gitlab::ExclusiveLease.new(key, timeout: ttl)
 
       until uuid = lease.try_obtain

lib/gitlab/kubernetes/logger.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Kubernetes
+    class Logger < ::Gitlab::JsonLogger
+      def self.file_name_noext
+        'kubernetes'
+      end
+    end
+  end
+end

package.json

@@ -148,6 +148,6 @@
     "karma-webpack": "^4.0.0-beta.0",
     "nodemon": "^1.18.4",
     "prettier": "1.15.2",
-    "webpack-dev-server": "^3.1.8"
+    "webpack-dev-server": "^3.1.10"
   }
 }

spec/initializers/attr_encrypted_no_db_connection_spec.rb

+require 'spec_helper'
+
+describe 'GitLab monkey-patches to AttrEncrypted' do
+  describe '#attribute_instance_methods_as_symbols_available?' do
+    it 'returns false' do
+      expect(ActiveRecord::Base.__send__(:attribute_instance_methods_as_symbols_available?)).to be_falsy
+    end
+
+    it 'does not define virtual attributes' do
+      klass = Class.new(ActiveRecord::Base) do
+        # We need some sort of table to work on
+        self.table_name = 'projects'
+
+        attr_encrypted :foo
+      end
+
+      instance = klass.new
+
+      aggregate_failures do
+        %w[
+          encrypted_foo encrypted_foo=
+          encrypted_foo_iv encrypted_foo_iv=
+          encrypted_foo_salt encrypted_foo_salt=
+        ].each do |method_name|
+          expect(instance).not_to respond_to(method_name)
+        end
+      end
+    end
+  end
+end

spec/javascripts/merge_request_tabs_spec.js

@@ -239,4 +239,38 @@ describe('MergeRequestTabs', function() {
       expect($('.content-wrapper')).toContainElement('.container-limited');
     });
   });
+
+  describe('tabShown', function() {
+    const mainContent = document.createElement('div');
+    const tabContent = document.createElement('div');
+
+    beforeEach(function() {
+      spyOn(mainContent, 'getBoundingClientRect').and.returnValue({ top: 10 });
+      spyOn(tabContent, 'getBoundingClientRect').and.returnValue({ top: 100 });
+      spyOn(document, 'querySelector').and.callFake(function(selector) {
+        return selector === '.content-wrapper' ? mainContent : tabContent;
+      });
+      this.class.currentAction = 'commits';
+    });
+
+    it('calls window scrollTo with options if document has scrollBehavior', function() {
+      document.documentElement.style.scrollBehavior = '';
+
+      spyOn(window, 'scrollTo');
+
+      this.class.tabShown('commits', 'foobar');
+
+      expect(window.scrollTo.calls.first().args[0]).toEqual({ top: 39, behavior: 'smooth' });
+    });
+
+    it('calls window scrollTo with two args if document does not have scrollBehavior', function() {
+      spyOnProperty(document.documentElement, 'style', 'get').and.returnValue({});
+
+      spyOn(window, 'scrollTo');
+
+      this.class.tabShown('commits', 'foobar');
+
+      expect(window.scrollTo.calls.first().args).toEqual([0, 39]);
+    });
+  });
 });

spec/lib/gitlab/background_migration/encrypt_columns_spec.rb

@@ -65,5 +65,30 @@ describe Gitlab::BackgroundMigration::EncryptColumns, :migration, schema: 201809
 
       expect(hook).to have_attributes(values)
     end
+
+    it 'reloads the model column information' do
+      expect(model).to receive(:reset_column_information).and_call_original
+      expect(model).to receive(:define_attribute_methods).and_call_original
+
+      subject.perform(model, [:token, :url], 1, 1)
+    end
+
+    it 'fails if a source column is not present' do
+      columns = model.columns.reject { |c| c.name == 'url' }
+      allow(model).to receive(:columns) { columns }
+
+      expect do
+        subject.perform(model, [:token, :url], 1, 1)
+      end.to raise_error(/source column: url is missing/)
+    end
+
+    it 'fails if a destination column is not present' do
+      columns = model.columns.reject { |c| c.name == 'encrypted_url' }
+      allow(model).to receive(:columns) { columns }
+
+      expect do
+        subject.perform(model, [:token, :url], 1, 1)
+      end.to raise_error(/destination column: encrypted_url is missing/)
+    end
   end
 end

spec/lib/gitlab/ci/trace/stream_spec.rb

@@ -257,7 +257,8 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
         let!(:last_result) { stream.html_with_state }
 
         before do
-          stream.append("5678", 4)
+          data_stream.seek(4, IO::SEEK_SET)
+          data_stream.write("5678")
           stream.seek(0)
         end
 
@@ -271,25 +272,29 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
     end
 
     context 'when stream is StringIO' do
+      let(:data_stream) do
+        StringIO.new("1234")
+      end
+
       let(:stream) do
-        described_class.new do
-          StringIO.new("1234")
-        end
+        described_class.new { data_stream }
       end
 
       it_behaves_like 'html_with_states'
     end
 
     context 'when stream is ChunkedIO' do
-      let(:stream) do
-        described_class.new do
-          Gitlab::Ci::Trace::ChunkedIO.new(build).tap do |chunked_io|
-            chunked_io.write("1234")
-            chunked_io.seek(0, IO::SEEK_SET)
-          end
+      let(:data_stream) do
+        Gitlab::Ci::Trace::ChunkedIO.new(build).tap do |chunked_io|
+          chunked_io.write("1234")
+          chunked_io.seek(0, IO::SEEK_SET)
         end
       end
 
+      let(:stream) do
+        described_class.new { data_stream }
+      end
+
       it_behaves_like 'html_with_states'
     end
   end

spec/lib/gitlab/exclusive_lease_helpers_spec.rb

@@ -11,6 +11,14 @@ describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state do
 
     let(:options) { {} }
 
+    context 'when unique key is not set' do
+      let(:unique_key) { }
+
+      it 'raises an error' do
+        expect { subject }.to raise_error ArgumentError
+      end
+    end
+
     context 'when the lease is not obtained yet' do
       before do
         stub_exclusive_lease(unique_key, 'uuid')

spec/requests/api/runner_spec.rb

@@ -835,6 +835,18 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
           expect(job.trace.raw).to eq 'BUILD TRACE UPDATED'
           expect(job.job_artifacts_trace.open.read).to eq 'BUILD TRACE UPDATED'
         end
+
+        context 'when concurrent update of trace is happening' do
+          before do
+            job.trace.write('wb') do
+              update_job(state: 'success', trace: 'BUILD TRACE UPDATED')
+            end
+          end
+
+          it 'returns that operation conflicts' do
+            expect(response.status).to eq(409)
+          end
+        end
       end
 
       context 'when no trace is given' do
@@ -1027,6 +1039,18 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
           end
         end
 
+        context 'when concurrent update of trace is happening' do
+          before do
+            job.trace.write('wb') do
+              patch_the_trace
+            end
+          end
+
+          it 'returns that operation conflicts' do
+            expect(response.status).to eq(409)
+          end
+        end
+
         context 'when the job is canceled' do
           before do
             job.cancel

spec/services/clusters/applications/check_installation_progress_service_spec.rb

@@ -105,6 +105,12 @@ describe Clusters::Applications::CheckInstallationProgressService do
         expect(application).to be_errored
         expect(application.status_reason).to eq('Kubernetes error: 401')
       end
+
+      it 'should log error' do
+        expect(service.send(:logger)).to receive(:error)
+
+        service.execute
+      end
     end
   end
 end

spec/services/clusters/applications/install_service_spec.rb

@@ -33,8 +33,9 @@ describe Clusters::Applications::InstallService do
     end
 
     context 'when k8s cluster communication fails' do
+      let(:error) { Kubeclient::HttpError.new(500, 'system failure', nil) }
+
       before do
-        error = Kubeclient::HttpError.new(500, 'system failure', nil)
         expect(helm_client).to receive(:install).with(install_command).and_raise(error)
       end
 
@@ -44,18 +45,81 @@ describe Clusters::Applications::InstallService do
         expect(application).to be_errored
         expect(application.status_reason).to match('Kubernetes error: 500')
       end
+
+      it 'logs errors' do
+        expect(service.send(:logger)).to receive(:error).with(
+          {
+            exception: 'Kubeclient::HttpError',
+            message: 'system failure',
+            service: 'Clusters::Applications::InstallService',
+            app_id: application.id,
+            project_ids: application.cluster.project_ids,
+            group_ids: [],
+            error_code: 500
+          }
+        )
+
+        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
+          error,
+          extra: {
+            exception: 'Kubeclient::HttpError',
+            message: 'system failure',
+            service: 'Clusters::Applications::InstallService',
+            app_id: application.id,
+            project_ids: application.cluster.project_ids,
+            group_ids: [],
+            error_code: 500
+          }
+        )
+
+        service.execute
+      end
     end
 
-    context 'when application cannot be persisted' do
+    context 'a non kubernetes error happens' do
       let(:application) { create(:clusters_applications_helm, :scheduled) }
+      let(:error) { StandardError.new("something bad happened") }
+
+      before do
+        expect(application).to receive(:make_installing!).once.and_raise(error)
+      end
 
       it 'make the application errored' do
-        expect(application).to receive(:make_installing!).once.and_raise(ActiveRecord::RecordInvalid)
         expect(helm_client).not_to receive(:install)
 
         service.execute
 
         expect(application).to be_errored
+        expect(application.status_reason).to eq("Can't start installation process.")
+      end
+
+      it 'logs errors' do
+        expect(service.send(:logger)).to receive(:error).with(
+          {
+            exception: 'StandardError',
+            error_code: nil,
+            message: 'something bad happened',
+            service: 'Clusters::Applications::InstallService',
+            app_id: application.id,
+            project_ids: application.cluster.projects.pluck(:id),
+            group_ids: []
+          }
+        )
+
+        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
+          error,
+          extra: {
+            exception: 'StandardError',
+            error_code: nil,
+            message: 'something bad happened',
+            service: 'Clusters::Applications::InstallService',
+            app_id: application.id,
+            project_ids: application.cluster.projects.pluck(:id),
+            group_ids: []
+          }
+        )
+
+        service.execute
       end
     end
   end

spec/support/shared_examples/ci_trace_shared_examples.rb

@@ -272,16 +272,11 @@ shared_examples_for 'common trace features' do
           include ExclusiveLeaseHelpers
 
           before do
-            stub_exclusive_lease_taken("trace:archive:#{trace.job.id}", timeout: 1.hour)
+            stub_exclusive_lease_taken("trace:write:lock:#{trace.job.id}", timeout: 1.minute)
           end
 
           it 'blocks concurrent archiving' do
-            expect(Rails.logger).to receive(:error).with('Cannot obtain an exclusive lease. There must be another instance already in execution.')
-
-            subject
-
-            build.reload
-            expect(build.job_artifacts_trace).to be_nil
+            expect { subject }.to raise_error(::Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError)
           end
         end
       end

spec/workers/stuck_ci_jobs_worker_spec.rb

@@ -5,7 +5,7 @@ describe StuckCiJobsWorker do
 
   let!(:runner) { create :ci_runner }
   let!(:job) { create :ci_build, runner: runner }
-  let(:trace_lease_key) { "trace:archive:#{job.id}" }
+  let(:trace_lease_key) { "trace:write:lock:#{job.id}" }
   let(:trace_lease_uuid) { SecureRandom.uuid }
   let(:worker_lease_key) { StuckCiJobsWorker::EXCLUSIVE_LEASE_KEY }
   let(:worker_lease_uuid) { SecureRandom.uuid }