Fix setting share_with_group_lockfix-share-with-group-lock-update

Prior to this commit running
Namespace#force_share_with_group_lock_on_descendants would result in
updating _all_ namespaces in the namespaces table, not just the
descendants. This is the result of ActiveRecord::Relation#update_all not
taking into account the CTE. To work around this we use the CTE query as
a sub-query instead of directly calling #update_all.

To prevent this from happening the relations returned by
Gitlab::GroupHierarchy are now marked as read-only, resulting in an
error being raised when methods such as #update_all are used.

Fortunately on GitLab.com our statement timeouts appear to have
prevented this query from actually doing any damage other than causing
a very large amount of dead tuples.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/37916

2 files changed, 28 insertions, 3 deletions

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