Merge branch 'tc-geo-read-only-to-ce' into 'master'

Create idea of read-only database

Closes #2855, #3398, #3399, and #3505

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

app/controllers/admin/application_controller.rb

@@ -3,18 +3,23 @@
 # Automatically sets the layout and ensures an administrator is logged in
 class Admin::ApplicationController < ApplicationController
   before_action :authenticate_admin!
-  before_action :display_geo_information
+  before_action :display_read_only_information
   layout 'admin'
   def authenticate_admin!
     render_404 unless current_user.admin?
   end
-  def display_geo_information
-    return unless Gitlab::Geo.secondary?
-    return unless Gitlab::Geo.primary_node_configured?
-    primary_node = view_context.link_to('primary node', Gitlab::Geo.primary_node.url)
-    flash.now[:notice] = "You are on a secondary (read-only) Geo node. If you want to make any changes, you must visit the #{primary_node}.".html_safe
+  def display_read_only_information
+    return unless Gitlab::Database.read_only?
+    flash.now[:notice] = read_only_message
+  end
+
+  private
+
+  # Overridden in EE
+  def read_only_message
+    _('You are on a read-only GitLab instance.')
   end
 end

app/controllers/boards/issues_controller.rb

@@ -12,7 +12,7 @@ class IssuesController < Boards::ApplicationController
     def index
       issues = Boards::Issues::ListService.new(board_parent, current_user, filter_params).execute
       issues = issues.page(params[:page]).per(params[:per] || 20)
-      make_sure_position_is_set(issues) unless Gitlab::Geo.secondary?
+      make_sure_position_is_set(issues) if Gitlab::Database.read_write?
       issues = issues.preload(:project,
                               :milestone,
                               :assignees,

app/controllers/projects/lfs_api_controller.rb

@@ -4,6 +4,8 @@ class Projects::LfsApiController < Projects::GitHttpClientController
   include GitlabRoutingHelper
   include LfsRequest
+  prepend ::EE::Projects::LfsApiController
+
   skip_before_action :lfs_check_access!, only: [:deprecated]
   before_action :lfs_check_batch_operation!, only: [:batch]
@@ -96,14 +98,19 @@ def upload_actions(object)
   end
   def lfs_check_batch_operation!
-    if upload_request? && Gitlab::Geo.secondary?
+    if upload_request? && Gitlab::Database.read_only?
       render(
         json: {
-          message: "You cannot write to a secondary GitLab Geo instance. Please use #{geo_primary_default_url_to_repo(project)} instead."
+          message: lfs_read_only_message
         },
-        content_type: "application/vnd.git-lfs+json",
+        content_type: 'application/vnd.git-lfs+json',
         status: 403
       )
     end
   end
+
+  # Overridden in EE
+  def lfs_read_only_message
+    _('You cannot write to this read-only GitLab instance.')
+  end
 end

app/controllers/projects/merge_requests/application_controller.rb

@@ -15,7 +15,7 @@ def merge_request
   # Make sure merge requests created before 8.0
   # have head file in refs/merge-requests/
   def ensure_ref_fetched
-    @merge_request.ensure_ref_fetched
+    @merge_request.ensure_ref_fetched if Gitlab::Database.read_write?
   end
   def merge_request_params

app/controllers/sessions_controller.rb

@@ -9,9 +9,7 @@ class SessionsController < Devise::SessionsController
   prepend_before_action :check_initial_setup, only: [:new]
   prepend_before_action :authenticate_with_two_factor,
     if: :two_factor_enabled?, only: [:create]
-  prepend_before_action :store_redirect_path, only: [:new]
-  before_action :gitlab_geo_login, only: [:new]
-  before_action :gitlab_geo_logout, only: [:destroy]
+  prepend_before_action :store_redirect_uri, only: [:new]
   before_action :auto_sign_in_with_provider, only: [:new]
   before_action :load_recaptcha
@@ -88,7 +86,11 @@ def find_user
     end
   end
-  def store_redirect_path
+  def stored_redirect_uri
+    @redirect_to ||= stored_location_for(:redirect)
+  end
+
+  def store_redirect_uri
     redirect_uri =
       if request.referer.present? && (params['redirect_to_referer'] == 'yes')
         URI(request.referer)
@@ -98,40 +100,22 @@ def store_redirect_path
     # Prevent a 'you are already signed in' message directly after signing:
     # we should never redirect to '/users/sign_in' after signing in successfully.
-    if redirect_uri.path == new_user_session_path
-      return true
-    elsif redirect_uri.host == Gitlab.config.gitlab.host && redirect_uri.port == Gitlab.config.gitlab.port
-      redirect_to = redirect_uri.to_s
-    elsif Gitlab::Geo.geo_node?(host: redirect_uri.host, port: redirect_uri.port)
-      redirect_to = redirect_uri.to_s
-    end
+    return true if redirect_uri.path == new_user_session_path
+
+    redirect_to = redirect_uri.to_s if redirect_allowed_to?(redirect_uri)
     @redirect_to = redirect_to
     store_location_for(:redirect, redirect_to)
   end
-  def two_factor_enabled?
-    find_user.try(:two_factor_enabled?)
-  end
-
-  def gitlab_geo_login
-    return unless Gitlab::Geo.secondary?
-    return if signed_in?
-
-    oauth = Gitlab::Geo::OauthSession.new
-
-    # share full url with primary node by oauth state
-    user_return_to = URI.join(root_url, session[:user_return_to].to_s).to_s
-    oauth.return_to = @redirect_to || user_return_to
-
-    redirect_to oauth_geo_auth_url(state: oauth.generate_oauth_state)
+  # Overridden in EE
+  def redirect_allowed_to?(uri)
+    uri.host == Gitlab.config.gitlab.host &&
+      uri.port == Gitlab.config.gitlab.port
   end
-  def gitlab_geo_logout
-    return unless Gitlab::Geo.secondary?
-
-    oauth = Gitlab::Geo::OauthSession.new(access_token: session[:access_token])
-    @geo_logout_state = oauth.generate_logout_state
+  def two_factor_enabled?
+    find_user&.two_factor_enabled?
   end
   def auto_sign_in_with_provider

app/models/concerns/cache_markdown_field.rb

@@ -59,7 +59,7 @@ def banzai_render_context(field)
   # Update every column in a row if any one is invalidated, as we only store
   # one version per row
-  def refresh_markdown_cache!(do_update: false)
+  def refresh_markdown_cache
     options = { skip_project_check: skip_project_check? }
     updates = cached_markdown_fields.markdown_fields.map do |markdown_field|
@@ -71,8 +71,14 @@ def refresh_markdown_cache!(do_update: false)
     updates['cached_markdown_version'] = CacheMarkdownField::CACHE_VERSION
     updates.each {|html_field, data| write_attribute(html_field, data) }
-    update_columns(updates) if persisted? && do_update
+  end
+
+  def refresh_markdown_cache!
+    updates = refresh_markdown_cache
+
+    return unless persisted? && Gitlab::Database.read_write?
+    update_columns(updates)
   end
   def cached_html_up_to_date?(markdown_field)
@@ -124,8 +130,8 @@ def attributes
     end
     # Using before_update here conflicts with elasticsearch-model somehow
-    before_create :refresh_markdown_cache!, if: :invalidated_markdown_cache?
-    before_update :refresh_markdown_cache!, if: :invalidated_markdown_cache?
+    before_create :refresh_markdown_cache, if: :invalidated_markdown_cache?
+    before_update :refresh_markdown_cache, if: :invalidated_markdown_cache?
   end
   class_methods do

app/models/concerns/routable.rb

@@ -156,7 +156,7 @@ def build_full_name
   end
   def update_route
-    return if Gitlab::Geo.secondary?
+    return if Gitlab::Database.read_only?
     prepare_route
     route.save

app/models/concerns/token_authenticatable.rb

@@ -43,15 +43,17 @@ def add_authentication_token_field(token_field)
         write_attribute(token_field, token) if token
       end
+      # Returns a token, but only saves when the database is in read & write mode
       define_method("ensure_#{token_field}!") do
         send("reset_#{token_field}!") if read_attribute(token_field).blank? # rubocop:disable GitlabSecurity/PublicSend
         read_attribute(token_field)
       end
+      # Resets the token, but only saves when the database is in read & write mode
       define_method("reset_#{token_field}!") do
         write_new_token(token_field)
-        save!
+        save! if Gitlab::Database.read_write?
       end
     end
   end

app/models/merge_request.rb

@@ -498,7 +498,7 @@ def reload_diff(current_user = nil)
   end
   def check_if_can_be_merged
-    return unless unchecked? && !Gitlab::Geo.secondary?
+    return unless unchecked? && Gitlab::Database.read_write?
     can_be_merged =
       !broken? && project.repository.can_be_merged?(diff_head_sha, target_branch)

app/models/project.rb

@@ -811,7 +811,7 @@ def external_issue_tracker
   end
   def cache_has_external_issue_tracker
-    update_column(:has_external_issue_tracker, services.external_issue_trackers.any?)
+    update_column(:has_external_issue_tracker, services.external_issue_trackers.any?) if Gitlab::Database.read_write?
   end
   def has_wiki?
@@ -831,7 +831,7 @@ def external_wiki
   end
   def cache_has_external_wiki
-    update_column(:has_external_wiki, services.external_wikis.any?)
+    update_column(:has_external_wiki, services.external_wikis.any?) if Gitlab::Database.read_write?
   end
   def find_or_initialize_services(exceptions: [])

app/models/user.rb

@@ -477,6 +477,14 @@ def recently_sent_password_reset?
     reset_password_sent_at.present? && reset_password_sent_at >= 1.minute.ago
   end
+  def remember_me!
+    super if ::Gitlab::Database.read_write?
+  end
+
+  def forget_me!
+    super if ::Gitlab::Database.read_write?
+  end
+
   def disable_two_factor!
     transaction do
       update_attributes(

app/services/keys/last_used_service.rb

 module Keys
   class LastUsedService
-    prepend ::EE::Keys::LastUsedService
-
     TIMEOUT = 1.day.to_i
     attr_reader :key
@@ -18,6 +16,8 @@ def execute
     end
     def update?
+      return false if ::Gitlab::Database.read_only?
+
       last_used = key.last_used_at
       return false if last_used && (Time.zone.now - last_used) <= TIMEOUT

app/services/users/activity_service.rb

@@ -14,7 +14,7 @@ def execute
     private
     def record_activity
-      Gitlab::UserActivities.record(@author.id) unless Gitlab::Geo.secondary?
+      Gitlab::UserActivities.record(@author.id) if Gitlab::Database.read_write?
       Rails.logger.debug("Recorded activity: #{@activity} for User ID: #{@author.id} (username: #{@author.username})")
     end

app/views/admin/geo_nodes/index.html.haml

@@ -84,7 +84,7 @@
                 %p
                   .js-health
-          - unless Gitlab::Geo.secondary?
+          - if Gitlab::Database.read_write?
             .node-actions
               - if Gitlab::Geo.license_allows?
                 - if node.missing_oauth_application?

changelogs/unreleased-ee/tc-geo-read-only-to-ce.yml

+---
+title: Create idea of read-only database and add method to check for it
+merge_request: 2954
+author:
+type: changed

config/application.rb

@@ -174,8 +174,8 @@ class Application < Rails::Application
     ENV['GITLAB_PATH_OUTSIDE_HOOK'] = ENV['PATH']
     ENV['GIT_TERMINAL_PROMPT'] = '0'
-    # Gitlab Geo Middleware support
-    config.middleware.insert_after ActionDispatch::Flash, 'Gitlab::Middleware::ReadonlyGeo'
+    # Gitlab Read-only middleware support
+    config.middleware.insert_after ActionDispatch::Flash, 'Gitlab::Middleware::ReadOnly'
     config.generators do |g|
       g.factory_girl false

doc/development/geo.md

@@ -78,19 +78,20 @@ to see if there are changes since the last time the log was checked
 and will handle repository updates, deletes, changes & renames.
-## Readonly
+## Read-only
 All **Secondary** nodes are read-only.
-We have a Rails Middleware that filters any potentially writing operations
-and prevent user from trying to update the database and getting a 500 error
-(see `Gitlab::Middleware::ReadonlyGeo`).
-Database will already be read-only in a replicated setup, so we don't need to
-take any extra step for that.
-We do use our feature toggle `.secondary?` to coordinate Git operations and do
-the correct authorization (denying writing on any secondary node).
+The general principle of a [read-only database](verifying_database_capabilities.md#read-only-database)
+applies to all Geo secondary nodes. So `Gitlab::Database.read_only?`
+will always return `true` on a secondary node.
+When some write actions are not allowed, because the node is a
+secondary, consider the `Gitlab::Database.read_only?` or `Gitlab::Database.read_write?`
+guard, instead of `Gitlab::Geo.secondary?`.
+Database itself will already be read-only in a replicated setup, so we
+don't need to take any extra step for that.
 ## File Transfers

doc/development/verifying_database_capabilities.md

@@ -24,3 +24,15 @@ else
   run_query
 end
 ```
+
+# Read-only database
+
+The database can be used in read-only mode. In this case we have to
+make sure all GET requests don't attempt any write operations to the
+database. If one of those requests wants to write to the database, it needs
+to be wrapped in a `Gitlab::Database.read_only?` or `Gitlab::Database.read_write?`
+guard, to make sure it doesn't for read-only databases.
+
+We have a Rails Middleware that filters any potentially writing
+operations (the CUD operations of CRUD) and prevent the user from trying
+to update the database and getting a 500 error (see `Gitlab::Middleware::ReadOnly`).

ee/app/controllers/ee/admin/application_controller.rb

+module EE
+  module Admin
+    module ApplicationController
+      def read_only_message
+        raise NotImplementedError unless defined?(super)
+
+        return super unless Gitlab::Geo.secondary_with_primary?
+
+        link_to_primary_node = view_context.link_to('primary node', Gitlab::Geo.primary_node.url)
+        (_('You are on a read-only GitLab instance. If you want to make any changes, you must visit the %{link_to_primary_node}.') % { link_to_primary_node: link_to_primary_node }).html_safe
+      end
+    end
+  end
+end

ee/app/controllers/ee/projects/lfs_api_controller.rb

+module EE
+  module Projects
+    module LfsApiController
+      def lfs_read_only_message
+        raise NotImplementedError unless defined?(super)
+
+        return super unless ::Gitlab::Geo.secondary_with_primary?
+
+        (_('You cannot write to a read-only secondary GitLab Geo instance. Please use %{link_to_primary_node} instead.') % { link_to_primary_node: geo_primary_default_url_to_repo(project) }).html_safe
+      end
+    end
+  end
+end