Merge branch 'ee-30272-bvl-reject-more-namespaces' into 'master'

Port of "Reject more reserved paths" to EE

See merge request !1778

app/models/concerns/cache_markdown_field.rb

@@ -78,6 +78,9 @@ module CacheMarkdownField
   def cached_html_up_to_date?(markdown_field)
     html_field = cached_markdown_fields.html_field(markdown_field)
 
+    cached = !cached_html_for(markdown_field).nil? && !__send__(markdown_field).nil?
+    return false unless cached
+
     markdown_changed = attribute_changed?(markdown_field) || false
     html_changed = attribute_changed?(html_field) || false
 

app/models/namespace.rb

@@ -34,7 +34,7 @@ class Namespace < ActiveRecord::Base
   validates :path,
     presence: true,
     length: { maximum: 255 },
-    namespace: true
+    dynamic_path: true
 
   validate :nesting_level_allowed
 
@@ -225,6 +225,10 @@ class Namespace < ActiveRecord::Base
     Project.inside_path(full_path)
   end
 
+  def has_parent?
+    parent.present?
+  end
+
   private
 
   def repository_storage_paths

app/models/project.rb

@@ -205,13 +205,14 @@ class Project < ActiveRecord::Base
               message: Gitlab::Regex.project_name_regex_message }
   validates :path,
     presence: true,
-    project_path: true,
+    dynamic_path: true,
     length: { maximum: 255 },
     format: { with: Gitlab::Regex.project_path_regex,
-              message: Gitlab::Regex.project_path_regex_message }
+              message: Gitlab::Regex.project_path_regex_message },
+    uniqueness: { scope: :namespace_id }
+
   validates :namespace, presence: true
   validates :name, uniqueness: { scope: :namespace_id }
-  validates :path, uniqueness: { scope: :namespace_id }
   validates :import_url, addressable_url: true, if: :external_import?
   validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?]
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }

app/models/user.rb

@@ -128,7 +128,7 @@ class User < ActiveRecord::Base
     presence: true,
     numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: Gitlab::Database::MAX_INT_VALUE }
   validates :username,
-    namespace: true,
+    dynamic_path: true,
     presence: true,
     uniqueness: { case_sensitive: false }
 

app/validators/dynamic_path_validator.rb

+# DynamicPathValidator
+#
+# Custom validator for GitLab path values.
+# These paths are assigned to `Namespace` (& `Group` as a subclass) & `Project`
+#
+# Values are checked for formatting and exclusion from a list of reserved path
+# names.
+class DynamicPathValidator < ActiveModel::EachValidator
+  # All routes that appear on the top level must be listed here.
+  # This will make sure that groups cannot be created with these names
+  # as these routes would be masked by the paths already in place.
+  #
+  # Example:
+  #   /api/api-project
+  #
+  #  the path `api` shouldn't be allowed because it would be masked by `api/*`
+  #
+  TOP_LEVEL_ROUTES = %w[
+    -
+    .well-known
+    abuse_reports
+    admin
+    all
+    api
+    assets
+    autocomplete
+    ci
+    dashboard
+    explore
+    files
+    groups
+    health_check
+    help
+    hooks
+    import
+    invites
+    issues
+    jwt
+    koding
+    member
+    merge_requests
+    new
+    notes
+    notification_settings
+    oauth
+    profile
+    projects
+    public
+    repository
+    robots.txt
+    s
+    search
+    sent_notifications
+    services
+    snippets
+    teams
+    u
+    unicorn_test
+    unsubscribes
+    uploads
+    users
+  ].freeze
+
+  # This list should contain all words following `/*namespace_id/:project_id` in
+  # routes that contain a second wildcard.
+  #
+  # Example:
+  #   /*namespace_id/:project_id/badges/*ref/build
+  #
+  # If `badges` was allowed as a project/group name, we would not be able to access the
+  # `badges` route for those projects:
+  #
+  # Consider a namespace with path `foo/bar` and a project called `badges`.
+  # The route to the build badge would then be `/foo/bar/badges/badges/master/build.svg`
+  #
+  # When accessing this path the route would be matched to the `badges` path
+  # with the following params:
+  #   - namespace_id: `foo`
+  #   - project_id: `bar`
+  #   - ref: `badges/master`
+  #
+  # Failing to find the project, this would result in a 404.
+  #
+  # By rejecting `badges` the router can _count_ on the fact that `badges` will
+  # be preceded by the `namespace/project`.
+  WILDCARD_ROUTES = %w[
+    badges
+    blame
+    blob
+    builds
+    commits
+    create
+    create_dir
+    edit
+    environments/folders
+    files
+    find_file
+    gitlab-lfs/objects
+    info/lfs/objects
+    new
+    preview
+    raw
+    refs
+    tree
+    update
+    wikis
+  ].freeze
+
+  # These are all the paths that follow `/groups/*id/ or `/groups/*group_id`
+  # We need to reject these because we have a `/groups/*id` page that is the same
+  # as the `/*id`.
+  #
+  # If we would allow a subgroup to be created with the name `activity` then
+  # this group would not be accessible through `/groups/parent/activity` since
+  # this would map to the activity-page of it's parent.
+  GROUP_ROUTES = %w[
+    activity
+    avatar
+    edit
+    group_members
+    issues
+    labels
+    merge_requests
+    milestones
+    projects
+    subgroups
+    analytics
+    audit_events
+    hooks
+    ldap
+    ldap_group_links
+    notification_setting
+    pipeline_quota
+  ].freeze
+
+  CHILD_ROUTES = (WILDCARD_ROUTES | GROUP_ROUTES).freeze
+
+  def self.without_reserved_wildcard_paths_regex
+    @without_reserved_wildcard_paths_regex ||= regex_excluding_child_paths(WILDCARD_ROUTES)
+  end
+
+  def self.without_reserved_child_paths_regex
+    @without_reserved_child_paths_regex ||= regex_excluding_child_paths(CHILD_ROUTES)
+  end
+
+  # This is used to validate a full path.
+  # It doesn't match paths
+  #   - Starting with one of the top level words
+  #   - Containing one of the child level words in the middle of a path
+  def self.regex_excluding_child_paths(child_routes)
+    reserved_top_level_words = Regexp.union(TOP_LEVEL_ROUTES)
+    not_starting_in_reserved_word = %r{\A/?(?!(#{reserved_top_level_words})(/|\z))}
+
+    reserved_child_level_words = Regexp.union(child_routes)
+    not_containing_reserved_child = %r{(?!\S+/(#{reserved_child_level_words})(/|\z))}
+
+    %r{#{not_starting_in_reserved_word}
+       #{not_containing_reserved_child}
+       #{Gitlab::Regex.full_namespace_regex}}x
+  end
+
+  def self.valid?(path)
+    path =~ Gitlab::Regex.full_namespace_regex && !full_path_reserved?(path)
+  end
+
+  def self.full_path_reserved?(path)
+    path = path.to_s.downcase
+    _project_part, namespace_parts = path.reverse.split('/', 2).map(&:reverse)
+
+    wildcard_reserved?(path) || child_reserved?(namespace_parts)
+  end
+
+  def self.child_reserved?(path)
+    return false unless path
+
+    path !~ without_reserved_child_paths_regex
+  end
+
+  def self.wildcard_reserved?(path)
+    return false unless path
+
+    path !~ without_reserved_wildcard_paths_regex
+  end
+
+  delegate :full_path_reserved?,
+           :child_reserved?,
+           to: :class
+
+  def path_reserved_for_record?(record, value)
+    full_path = record.respond_to?(:full_path) ? record.full_path : value
+
+    # For group paths the entire path cannot contain a reserved child word
+    # The path doesn't contain the last `_project_part` so we need to validate
+    # if the entire path.
+    # Example:
+    #   A *group* with full path `parent/activity` is reserved.
+    #   A *project* with full path `parent/activity` is allowed.
+    if record.is_a? Group
+      child_reserved?(full_path)
+    else
+      full_path_reserved?(full_path)
+    end
+  end
+
+  def validate_each(record, attribute, value)
+    unless value =~ Gitlab::Regex.namespace_regex
+      record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
+      return
+    end
+
+    if path_reserved_for_record?(record, value)
+      record.errors.add(attribute, "#{value} is a reserved name")
+    end
+  end
+end

app/validators/namespace_validator.rb

-# NamespaceValidator
-#
-# Custom validator for GitLab namespace values.
-#
-# Values are checked for formatting and exclusion from a list of reserved path
-# names.
-class NamespaceValidator < ActiveModel::EachValidator
-  RESERVED = %w[
-    .well-known
-    admin
-    all
-    assets
-    ci
-    dashboard
-    files
-    groups
-    help
-    hooks
-    issues
-    merge_requests
-    new
-    notes
-    profile
-    projects
-    public
-    repository
-    robots.txt
-    s
-    search
-    services
-    snippets
-    teams
-    u
-    unsubscribes
-    users
-  ].freeze
-
-  WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree
-                       preview blob blame raw files create_dir find_file
-                       artifacts graphs refs badges].freeze
-
-  STRICT_RESERVED = (RESERVED + WILDCARD_ROUTES).freeze
-
-  def self.valid?(value)
-    !reserved?(value) && follow_format?(value)
-  end
-
-  def self.reserved?(value, strict: false)
-    if strict
-      STRICT_RESERVED.include?(value)
-    else
-      RESERVED.include?(value)
-    end
-  end
-
-  def self.follow_format?(value)
-    value =~ Gitlab::Regex.namespace_regex
-  end
-
-  delegate :reserved?, :follow_format?, to: :class
-
-  def validate_each(record, attribute, value)
-    unless follow_format?(value)
-      record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
-    end
-
-    strict = record.is_a?(Group) && record.parent_id
-
-    if reserved?(value, strict: strict)
-      record.errors.add(attribute, "#{value} is a reserved name")
-    end
-  end
-end

app/validators/project_path_validator.rb

-# ProjectPathValidator
-#
-# Custom validator for GitLab project path values.
-#
-# Values are checked for formatting and exclusion from a list of reserved path
-# names.
-class ProjectPathValidator < ActiveModel::EachValidator
-  # All project routes with wildcard argument must be listed here.
-  # Otherwise it can lead to routing issues when route considered as project name.
-  #
-  # Example:
-  #  /group/project/tree/deploy_keys
-  #
-  #  without tree as reserved name routing can match 'group/project' as group name,
-  #  'tree' as project name and 'deploy_keys' as route.
-  #
-  RESERVED = (NamespaceValidator::STRICT_RESERVED -
-              %w[dashboard help ci admin search notes services assets profile public]).freeze
-
-  def self.valid?(value)
-    !reserved?(value)
-  end
-
-  def self.reserved?(value)
-    RESERVED.include?(value)
-  end
-
-  delegate :reserved?, to: :class
-
-  def validate_each(record, attribute, value)
-    if reserved?(value)
-      record.errors.add(attribute, "#{value} is a reserved name")
-    end
-  end
-end

changelogs/unreleased/30272-bvl-reject-more-namespaces.yml

+---
+title: Improve validation of namespace & project paths
+merge_request: 10413
+author:

db/post_migrate/20170412174900_rename_reserved_dynamic_paths.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RenameReservedDynamicPaths < ActiveRecord::Migration
+  include Gitlab::Database::RenameReservedPathsMigration::V1
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  DISALLOWED_ROOT_PATHS = %w[
+    -
+    abuse_reports
+    api
+    autocomplete
+    explore
+    health_check
+    import
+    invites
+    jwt
+    koding
+    member
+    notification_settings
+    oauth
+    sent_notifications
+    unicorn_test
+    uploads
+    users
+  ]
+
+  DISALLOWED_WILDCARD_PATHS = %w[
+    environments/folders
+    gitlab-lfs/objects
+    info/lfs/objects
+  ]
+
+  DISSALLOWED_GROUP_PATHS = %w[
+    activity
+    avatar
+    group_members
+    labels
+    milestones
+    subgroups
+    analytics
+    audit_events
+    hooks
+    ldap
+    ldap_group_links
+    notification_setting
+    pipeline_quota
+  ]
+
+  def up
+    rename_root_paths(DISALLOWED_ROOT_PATHS)
+    rename_wildcard_paths(DISALLOWED_WILDCARD_PATHS)
+    rename_child_paths(DISSALLOWED_GROUP_PATHS)
+  end
+
+  def down
+    # nothing to do
+  end
+end

doc/development/migration_style_guide.md

@@ -270,3 +270,28 @@ end
 
 When doing so be sure to explicitly set the model's table name so it's not
 derived from the class name or namespace.
+
+### Renaming reserved paths
+
+When a new route for projects is introduced that could conflict with any
+existing records. The path for this records should be renamed, and the
+related data should be moved on disk.
+
+Since we had to do this a few times already, there are now some helpers to help
+with this.
+
+To use this you can include `Gitlab::Database::RenameReservedPathsMigration::V1`
+in your migration. This will provide 3 methods which you can pass one or more
+paths that need to be rejected.
+
+**`rename_root_paths`**: This will rename the path of all _namespaces_ with the
+given name that don't have a `parent_id`.
+
+**`rename_child_paths`**: This will rename the path of all _namespaces_ with the
+given name that have a `parent_id`.
+
+**`rename_wildcard_paths`**: This will rename the path of all _projects_, and all
+_namespaces_ that have a `project_id`.
+
+The `path` column for these rows will be renamed to their previous value followed
+by an integer. For example: `users` would turn into `users0`

lib/constraints/group_url_constrainer.rb

@@ -2,16 +2,8 @@ class GroupUrlConstrainer
   def matches?(request)
     id = request.params[:id]
 
-    return false unless valid?(id)
+    return false unless DynamicPathValidator.valid?(id)
 
     Group.find_by_full_path(id).present?
   end
-
-  private
-
-  def valid?(id)
-    id.split('/').all? do |namespace|
-      NamespaceValidator.valid?(namespace)
-    end
-  end
 end

lib/constraints/project_url_constrainer.rb

@@ -4,9 +4,7 @@ class ProjectUrlConstrainer
     project_path = request.params[:project_id] || request.params[:id]
     full_path = namespace_path + '/' + project_path
 
-    unless ProjectPathValidator.valid?(project_path)
-      return false
-    end
+    return false unless DynamicPathValidator.valid?(full_path)
 
     Project.find_by_full_path(full_path).present?
   end

lib/gitlab/database/migration_helpers.rb

@@ -498,6 +498,29 @@ module Gitlab
 
         columns(table).find { |column| column.name == name }
       end
+
+      # This will replace the first occurance of a string in a column with
+      # the replacement
+      # On postgresql we can use `regexp_replace` for that.
+      # On mysql we find the location of the pattern, and overwrite it
+      # with the replacement
+      def replace_sql(column, pattern, replacement)
+        quoted_pattern = Arel::Nodes::Quoted.new(pattern.to_s)
+        quoted_replacement = Arel::Nodes::Quoted.new(replacement.to_s)
+
+        if Database.mysql?
+          locate = Arel::Nodes::NamedFunction.
+            new('locate', [quoted_pattern, column])
+          insert_in_place = Arel::Nodes::NamedFunction.
+            new('insert', [column, locate, pattern.size, quoted_replacement])
+
+          Arel::Nodes::SqlLiteral.new(insert_in_place.to_sql)
+        else
+          replace = Arel::Nodes::NamedFunction.
+            new("regexp_replace", [column, quoted_pattern, quoted_replacement])
+          Arel::Nodes::SqlLiteral.new(replace.to_sql)
+        end
+      end
     end
   end
 end

lib/gitlab/database/rename_reserved_paths_migration/v1.rb

+# This module can be included in migrations to make it easier to rename paths
+# of `Namespace` & `Project` models certain paths would become `reserved`.
+#
+# If the way things are stored on the filesystem related to namespaces and
+# projects ever changes. Don't update this module, or anything nested in `V1`,
+# since it needs to keep functioning for all migrations using it using the state
+# that the data is in at the time. Instead, create a `V2` module that implements
+# the new way of reserving paths.
+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        def self.included(kls)
+          kls.include(MigrationHelpers)
+        end
+
+        def rename_wildcard_paths(one_or_more_paths)
+          rename_child_paths(one_or_more_paths)
+          paths = Array(one_or_more_paths)
+          RenameProjects.new(paths, self).rename_projects
+        end
+
+        def rename_child_paths(one_or_more_paths)
+          paths = Array(one_or_more_paths)
+          RenameNamespaces.new(paths, self).rename_namespaces(type: :child)
+        end
+
+        def rename_root_paths(paths)
+          paths = Array(paths)
+          RenameNamespaces.new(paths, self).rename_namespaces(type: :top_level)
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb

+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        module MigrationClasses
+          module Routable
+            def full_path
+              if route && route.path.present?
+                @full_path ||= route.path
+              else
+                update_route if persisted?
+
+                build_full_path
+              end
+            end
+
+            def build_full_path
+              if parent && path
+                parent.full_path + '/' + path
+              else
+                path
+              end
+            end
+
+            def update_route
+              prepare_route
+              route.save
+            end
+
+            def prepare_route
+              route || build_route(source: self)
+              route.path = build_full_path
+              @full_path = nil
+            end
+          end
+
+          class Namespace < ActiveRecord::Base
+            include MigrationClasses::Routable
+            self.table_name = 'namespaces'
+            belongs_to :parent,
+                       class_name: "#{MigrationClasses.name}::Namespace"
+            has_one :route, as: :source
+            has_many :children,
+                     class_name: "#{MigrationClasses.name}::Namespace",
+                     foreign_key: :parent_id
+
+            # Overridden to have the correct `source_type` for the `route` relation
+            def self.name
+              'Namespace'
+            end
+          end
+
+          class Route < ActiveRecord::Base
+            self.table_name = 'routes'
+            belongs_to :source, polymorphic: true
+          end
+
+          class Project < ActiveRecord::Base
+            include MigrationClasses::Routable
+            has_one :route, as: :source
+            self.table_name = 'projects'
+
+            def repository_storage_path
+              Gitlab.config.repositories.storages[repository_storage]['path']
+            end
+
+            # Overridden to have the correct `source_type` for the `route` relation
+            def self.name
+              'Project'
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base.rb

+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        class RenameBase
+          attr_reader :paths, :migration
+
+          delegate :update_column_in_batches,
+                   :replace_sql,
+                   to: :migration
+
+          def initialize(paths, migration)
+            @paths = paths
+            @migration = migration
+          end
+
+          def path_patterns
+            @path_patterns ||= paths.map { |path| "%#{path}" }
+          end
+
+          def rename_path_for_routable(routable)
+            old_path = routable.path
+            old_full_path = routable.full_path
+            # Only remove the last occurrence of the path name to get the parent namespace path
+            namespace_path = remove_last_occurrence(old_full_path, old_path)
+            new_path = rename_path(namespace_path, old_path)
+            new_full_path = join_routable_path(namespace_path, new_path)
+
+            # skips callbacks & validations
+            routable.class.where(id: routable).
+              update_all(path: new_path)
+
+            rename_routes(old_full_path, new_full_path)
+
+            [old_full_path, new_full_path]
+          end
+
+          def rename_routes(old_full_path, new_full_path)
+            replace_statement = replace_sql(Route.arel_table[:path],
+                                            old_full_path,
+                                            new_full_path)
+
+            update_column_in_batches(:routes, :path, replace_statement)  do |table, query|
+              query.where(MigrationClasses::Route.arel_table[:path].matches("#{old_full_path}%"))
+            end
+          end
+
+          def rename_path(namespace_path, path_was)
+            counter = 0
+            path = "#{path_was}#{counter}"
+
+            while route_exists?(join_routable_path(namespace_path, path))
+              counter += 1
+              path = "#{path_was}#{counter}"
+            end
+
+            path
+          end
+
+          def remove_last_occurrence(string, pattern)
+            string.reverse.sub(pattern.reverse, "").reverse
+          end
+
+          def join_routable_path(namespace_path, top_level)
+            if namespace_path.present?
+              File.join(namespace_path, top_level)
+            else
+              top_level
+            end
+          end
+
+          def route_exists?(full_path)
+            MigrationClasses::Route.where(Route.arel_table[:path].matches(full_path)).any?
+          end
+
+          def move_pages(old_path, new_path)
+            move_folders(pages_dir, old_path, new_path)
+          end
+
+          def move_uploads(old_path, new_path)
+            return unless file_storage?
+
+            move_folders(uploads_dir, old_path, new_path)
+          end
+
+          def move_folders(directory, old_relative_path, new_relative_path)
+            old_path = File.join(directory, old_relative_path)
+            return unless File.directory?(old_path)
+
+            new_path = File.join(directory, new_relative_path)
+            FileUtils.mv(old_path, new_path)
+          end
+
+          def remove_cached_html_for_projects(project_ids)
+            update_column_in_batches(:projects, :description_html, nil) do |table, query|
+              query.where(table[:id].in(project_ids))
+            end
+
+            update_column_in_batches(:issues, :description_html, nil) do |table, query|
+              query.where(table[:project_id].in(project_ids))
+            end
+
+            update_column_in_batches(:merge_requests, :description_html, nil) do |table, query|
+              query.where(table[:target_project_id].in(project_ids))
+            end
+
+            update_column_in_batches(:notes, :note_html, nil) do |table, query|
+              query.where(table[:project_id].in(project_ids))
+            end
+
+            update_column_in_batches(:milestones, :description_html, nil) do |table, query|
+              query.where(table[:project_id].in(project_ids))
+            end
+          end
+
+          def file_storage?
+            CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
+          end
+
+          def uploads_dir
+            File.join(CarrierWave.root, "uploads")
+          end
+
+          def pages_dir
+            Settings.pages.path
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb

+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        class RenameNamespaces < RenameBase
+          include Gitlab::ShellAdapter
+
+          def rename_namespaces(type:)
+            namespaces_for_paths(type: type).each do |namespace|
+              rename_namespace(namespace)
+            end
+          end
+
+          def namespaces_for_paths(type:)
+            namespaces = case type
+                         when :child
+                           MigrationClasses::Namespace.where.not(parent_id: nil)
+                         when :top_level
+                           MigrationClasses::Namespace.where(parent_id: nil)
+                         end
+            with_paths = MigrationClasses::Route.arel_table[:path].
+                           matches_any(path_patterns)
+            namespaces.joins(:route).where(with_paths)
+          end
+
+          def rename_namespace(namespace)
+            old_full_path, new_full_path = rename_path_for_routable(namespace)
+
+            move_repositories(namespace, old_full_path, new_full_path)
+            move_uploads(old_full_path, new_full_path)
+            move_pages(old_full_path, new_full_path)
+            remove_cached_html_for_projects(projects_for_namespace(namespace).map(&:id))
+          end
+
+          def move_repositories(namespace, old_full_path, new_full_path)
+            repo_paths_for_namespace(namespace).each do |repository_storage_path|
+              # Ensure old directory exists before moving it
+              gitlab_shell.add_namespace(repository_storage_path, old_full_path)
+
+              unless gitlab_shell.mv_namespace(repository_storage_path, old_full_path, new_full_path)
+                message = "Exception moving path #{repository_storage_path} \
+                           from #{old_full_path} to #{new_full_path}"
+                Rails.logger.error message
+              end
+            end
+          end
+
+          def repo_paths_for_namespace(namespace)
+            projects_for_namespace(namespace).distinct.select(:repository_storage).
+              map(&:repository_storage_path)
+          end
+
+          def projects_for_namespace(namespace)
+            namespace_ids = child_ids_for_parent(namespace, ids: [namespace.id])
+            namespace_or_children = MigrationClasses::Project.
+                                      arel_table[:namespace_id].
+                                      in(namespace_ids)
+            MigrationClasses::Project.where(namespace_or_children)
+          end
+
+          def child_ids_for_parent(namespace, ids: [])
+            namespace.children.each do |child|
+              ids << child.id
+              child_ids_for_parent(child, ids: ids) if child.children.any?
+            end
+            ids
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb

+module Gitlab
+  module Database
+    module RenameReservedPathsMigration
+      module V1
+        class RenameProjects < RenameBase
+          include Gitlab::ShellAdapter
+
+          def rename_projects
+            projects_for_paths.each do |project|
+              rename_project(project)
+            end
+
+            remove_cached_html_for_projects(projects_for_paths.map(&:id))
+          end
+
+          def rename_project(project)
+            old_full_path, new_full_path = rename_path_for_routable(project)
+
+            move_repository(project, old_full_path, new_full_path)
+            move_repository(project, "#{old_full_path}.wiki", "#{new_full_path}.wiki")
+            move_uploads(old_full_path, new_full_path)
+            move_pages(old_full_path, new_full_path)
+          end
+
+          def move_repository(project, old_path, new_path)
+            unless gitlab_shell.mv_repository(project.repository_storage_path,
+                                              old_path,
+                                              new_path)
+              Rails.logger.error "Error moving #{old_path} to #{new_path}"
+            end
+          end
+
+          def projects_for_paths
+            return @projects_for_paths if @projects_for_paths
+
+            with_paths = MigrationClasses::Route.arel_table[:path]
+                           .matches_any(path_patterns)
+
+            @projects_for_paths = MigrationClasses::Project.joins(:route).where(with_paths)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/etag_caching/router.rb

@@ -2,31 +2,39 @@ module Gitlab
   module EtagCaching
     class Router
       Route = Struct.new(:regexp, :name)
-
-      RESERVED_WORDS = NamespaceValidator::WILDCARD_ROUTES.map { |word| "/#{word}/" }.join('|')
+      # We enable an ETag for every request matching the regex.
+      # To match a regex the path needs to match the following:
+      #   - Don't contain a reserved word (expect for the words used in the
+      #     regex itself)
+      #   - Ending in `noteable/issue/<id>/notes` for the `issue_notes` route
+      #   - Ending in `issues/id`/rendered_title` for the `issue_title` route
+      USED_IN_ROUTES = %w[noteable issue notes issues rendered_title
+                          commit pipelines merge_requests new].freeze
+      RESERVED_WORDS = DynamicPathValidator::WILDCARD_ROUTES - USED_IN_ROUTES
+      RESERVED_WORDS_REGEX = Regexp.union(*RESERVED_WORDS)
       ROUTES = [
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/noteable/issue/\d+/notes\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/noteable/issue/\d+/notes\z),
           'issue_notes'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/issues/\d+/rendered_title\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/issues/\d+/rendered_title\z),
           'issue_title'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/commit/\S+/pipelines\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/commit/\S+/pipelines\.json\z),
           'commit_pipelines'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/new\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/merge_requests/new\.json\z),
           'new_merge_request_pipelines'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/\d+/pipelines\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/merge_requests/\d+/pipelines\.json\z),
           'merge_request_pipelines'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/pipelines\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/pipelines\.json\z),
           'project_pipelines'
         )
       ].freeze

lib/gitlab/regex.rb

@@ -22,6 +22,10 @@ module Gitlab
       @namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
     end
 
+    def full_namespace_regex
+      @full_namespace_regex ||= %r{\A#{FULL_NAMESPACE_REGEX_STR}\z}
+    end
+
     def namespace_route_regex
       @namespace_route_regex ||= /#{NAMESPACE_REGEX_STR}/.freeze
     end

spec/lib/constraints/group_url_constrainer_spec.rb

@@ -17,6 +17,13 @@ describe GroupUrlConstrainer, lib: true do
       it { expect(subject.matches?(request)).to be_truthy }
     end
 
+    context 'valid request for nested group with reserved top level name' do
+      let!(:nested_group) { create(:group, path: 'api', parent: group) }
+      let!(:request) { build_request('gitlab/api') }
+
+      it { expect(subject.matches?(request)).to be_truthy }
+    end
+
     context 'invalid request' do
       let(:request) { build_request('foo') }
 

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -726,4 +726,37 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
       expect(model.column_for(:users, :kittens)).to be_nil
     end
   end
+
+  describe '#replace_sql' do
+    context 'using postgres' do
+      before do
+        allow(Gitlab::Database).to receive(:mysql?).and_return(false)
+      end
+
+      it 'builds the sql with correct functions' do
+        expect(model.replace_sql(Arel::Table.new(:users)[:first_name], "Alice", "Eve").to_s).
+          to include('regexp_replace')
+      end
+    end
+
+    context 'using mysql' do
+      before do
+        allow(Gitlab::Database).to receive(:mysql?).and_return(true)
+      end
+
+      it 'builds the sql with the correct functions' do
+        expect(model.replace_sql(Arel::Table.new(:users)[:first_name], "Alice", "Eve").to_s).
+          to include('locate', 'insert')
+      end
+    end
+
+    describe 'results' do
+      let!(:user) { create(:user, name: 'Kathy Alice Aliceson') }
+
+      it 'replaces the correct part of the string' do
+        model.update_column_in_batches(:users, :name, model.replace_sql(Arel::Table.new(:users)[:name], 'Alice', 'Eve'))
+        expect(user.reload.name).to eq('Kathy Eve Aliceson')
+      end
+    end
+  end
 end

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase do
+  let(:migration) { FakeRenameReservedPathMigrationV1.new }
+  let(:subject) { described_class.new(['the-path'], migration) }
+
+  before do
+    allow(migration).to receive(:say)
+  end
+
+  def migration_namespace(namespace)
+    Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::
+      Namespace.find(namespace.id)
+  end
+
+  def migration_project(project)
+    Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::
+      Project.find(project.id)
+  end
+
+  describe "#remove_last_ocurrence" do
+    it "removes only the last occurance of a string" do
+      input = "this/is/a-word-to-replace/namespace/with/a-word-to-replace"
+
+      expect(subject.remove_last_occurrence(input, "a-word-to-replace"))
+        .to eq("this/is/a-word-to-replace/namespace/with/")
+    end
+  end
+
+  describe '#remove_cached_html_for_projects' do
+    let(:project) { create(:empty_project, description_html: 'Project description') }
+
+    it 'removes description_html from projects' do
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(project.reload.description_html).to be_nil
+    end
+
+    it 'removes issue descriptions' do
+      issue = create(:issue, project: project, description_html: 'Issue description')
+
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(issue.reload.description_html).to be_nil
+    end
+
+    it 'removes merge request descriptions' do
+      merge_request = create(:merge_request,
+                             source_project: project,
+                             target_project: project,
+                             description_html: 'MergeRequest description')
+
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(merge_request.reload.description_html).to be_nil
+    end
+
+    it 'removes note html' do
+      note = create(:note,
+                    project: project,
+                    noteable: create(:issue, project: project),
+                    note_html: 'note description')
+
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(note.reload.note_html).to be_nil
+    end
+
+    it 'removes milestone description' do
+      milestone = create(:milestone,
+                    project: project,
+                    description_html: 'milestone description')
+
+      subject.remove_cached_html_for_projects([project.id])
+
+      expect(milestone.reload.description_html).to be_nil
+    end
+  end
+
+  describe '#rename_path_for_routable' do
+    context 'for namespaces' do
+      let(:namespace) { create(:namespace, path: 'the-path') }
+      it "renames namespaces called the-path" do
+        subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(namespace.reload.path).to eq("the-path0")
+      end
+
+      it "renames the route to the namespace" do
+        subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(Namespace.find(namespace.id).full_path).to eq("the-path0")
+      end
+
+      it "renames the route for projects of the namespace" do
+        project = create(:project, path: "project-path", namespace: namespace)
+
+        subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(project.route.reload.path).to eq("the-path0/project-path")
+      end
+
+      it 'returns the old & the new path' do
+        old_path, new_path = subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(old_path).to eq('the-path')
+        expect(new_path).to eq('the-path0')
+      end
+
+      context "the-path namespace -> subgroup -> the-path0 project" do
+        it "updates the route of the project correctly" do
+          subgroup = create(:group, path: "subgroup", parent: namespace)
+          project = create(:project, path: "the-path0", namespace: subgroup)
+
+          subject.rename_path_for_routable(migration_namespace(namespace))
+
+          expect(project.route.reload.path).to eq("the-path0/subgroup/the-path0")
+        end
+      end
+    end
+
+    context 'for projects' do
+      let(:parent) { create(:namespace, path: 'the-parent') }
+      let(:project) { create(:empty_project, path: 'the-path', namespace: parent) }
+
+      it 'renames the project called `the-path`' do
+        subject.rename_path_for_routable(migration_project(project))
+
+        expect(project.reload.path).to eq('the-path0')
+      end
+
+      it 'renames the route for the project' do
+        subject.rename_path_for_routable(project)
+
+        expect(project.reload.route.path).to eq('the-parent/the-path0')
+      end
+
+      it 'returns the old & new path' do
+        old_path, new_path = subject.rename_path_for_routable(migration_project(project))
+
+        expect(old_path).to eq('the-parent/the-path')
+        expect(new_path).to eq('the-parent/the-path0')
+      end
+    end
+  end
+
+  describe '#move_pages' do
+    it 'moves the pages directory' do
+      expect(subject).to receive(:move_folders)
+                           .with(TestEnv.pages_path, 'old-path', 'new-path')
+
+      subject.move_pages('old-path', 'new-path')
+    end
+  end
+
+  describe "#move_uploads" do
+    let(:test_dir) { File.join(Rails.root, 'tmp', 'tests', 'rename_reserved_paths') }
+    let(:uploads_dir) { File.join(test_dir, 'public', 'uploads') }
+
+    it 'moves subdirectories in the uploads folder' do
+      expect(subject).to receive(:uploads_dir).and_return(uploads_dir)
+      expect(subject).to receive(:move_folders).with(uploads_dir, 'old_path', 'new_path')
+
+      subject.move_uploads('old_path', 'new_path')
+    end
+
+    it "doesn't move uploads when they are stored in object storage" do
+      expect(subject).to receive(:file_storage?).and_return(false)
+      expect(subject).not_to receive(:move_folders)
+
+      subject.move_uploads('old_path', 'new_path')
+    end
+  end
+
+  describe '#move_folders' do
+    let(:test_dir) { File.join(Rails.root, 'tmp', 'tests', 'rename_reserved_paths') }
+    let(:uploads_dir) { File.join(test_dir, 'public', 'uploads') }
+
+    before do
+      FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+      FileUtils.mkdir_p(uploads_dir)
+      allow(subject).to receive(:uploads_dir).and_return(uploads_dir)
+    end
+
+    it 'moves a folder with files' do
+      source = File.join(uploads_dir, 'parent-group', 'sub-group')
+      FileUtils.mkdir_p(source)
+      destination = File.join(uploads_dir, 'parent-group', 'moved-group')
+      FileUtils.touch(File.join(source, 'test.txt'))
+      expected_file = File.join(destination, 'test.txt')
+
+      subject.move_folders(uploads_dir, File.join('parent-group', 'sub-group'), File.join('parent-group', 'moved-group'))
+
+      expect(File.exist?(expected_file)).to be(true)
+    end
+  end
+end

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces do
+  let(:migration) { FakeRenameReservedPathMigrationV1.new }
+  let(:subject) { described_class.new(['the-path'], migration) }
+
+  before do
+    allow(migration).to receive(:say)
+  end
+
+  def migration_namespace(namespace)
+    Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::
+      Namespace.find(namespace.id)
+  end
+
+  describe '#namespaces_for_paths' do
+    context 'nested namespaces' do
+      let(:subject) { described_class.new(['parent/the-Path'], migration) }
+
+      it 'includes the namespace' do
+        parent = create(:namespace, path: 'parent')
+        child = create(:namespace, path: 'the-path', parent: parent)
+
+        found_ids = subject.namespaces_for_paths(type: :child).
+                      map(&:id)
+        expect(found_ids).to contain_exactly(child.id)
+      end
+    end
+
+    context 'for child namespaces' do
+      it 'only returns child namespaces with the correct path' do
+        _root_namespace = create(:namespace, path: 'THE-path')
+        _other_path = create(:namespace,
+                             path: 'other',
+                             parent: create(:namespace))
+        namespace = create(:namespace,
+                           path: 'the-path',
+                           parent: create(:namespace))
+
+        found_ids = subject.namespaces_for_paths(type: :child).
+                      map(&:id)
+        expect(found_ids).to contain_exactly(namespace.id)
+      end
+    end
+
+    context 'for top levelnamespaces' do
+      it 'only returns child namespaces with the correct path' do
+        root_namespace = create(:namespace, path: 'the-path')
+        _other_path = create(:namespace, path: 'other')
+        _child_namespace = create(:namespace,
+                                  path: 'the-path',
+                                  parent: create(:namespace))
+
+        found_ids = subject.namespaces_for_paths(type: :top_level).
+                      map(&:id)
+        expect(found_ids).to contain_exactly(root_namespace.id)
+      end
+    end
+  end
+
+  describe '#move_repositories' do
+    let(:namespace) { create(:group, name: 'hello-group') }
+    it 'moves a project for a namespace' do
+      create(:project, namespace: namespace, path: 'hello-project')
+      expected_path = File.join(TestEnv.repos_path, 'bye-group', 'hello-project.git')
+
+      subject.move_repositories(namespace, 'hello-group', 'bye-group')
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+
+    it 'moves a namespace in a subdirectory correctly' do
+      child_namespace = create(:group, name: 'sub-group', parent: namespace)
+      create(:project, namespace: child_namespace, path: 'hello-project')
+
+      expected_path = File.join(TestEnv.repos_path, 'hello-group', 'renamed-sub-group', 'hello-project.git')
+
+      subject.move_repositories(child_namespace, 'hello-group/sub-group', 'hello-group/renamed-sub-group')
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+
+    it 'moves a parent namespace with subdirectories' do
+      child_namespace = create(:group, name: 'sub-group', parent: namespace)
+      create(:project, namespace: child_namespace, path: 'hello-project')
+      expected_path = File.join(TestEnv.repos_path, 'renamed-group', 'sub-group', 'hello-project.git')
+
+      subject.move_repositories(child_namespace, 'hello-group', 'renamed-group')
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+  end
+
+  describe "#child_ids_for_parent" do
+    it "collects child ids for all levels" do
+      parent = create(:namespace)
+      first_child = create(:namespace, parent: parent)
+      second_child = create(:namespace, parent: parent)
+      third_child = create(:namespace, parent: second_child)
+      all_ids = [parent.id, first_child.id, second_child.id, third_child.id]
+
+      collected_ids = subject.child_ids_for_parent(parent, ids: [parent.id])
+
+      expect(collected_ids).to contain_exactly(*all_ids)
+    end
+  end
+
+  describe "#rename_namespace" do
+    let(:namespace) { create(:namespace, path: 'the-path') }
+
+    it 'renames paths & routes for the namespace' do
+      expect(subject).to receive(:rename_path_for_routable).
+                           with(namespace).
+                           and_call_original
+
+      subject.rename_namespace(namespace)
+
+      expect(namespace.reload.path).to eq('the-path0')
+    end
+
+    it "moves the the repository for a project in the namespace" do
+      create(:project, namespace: namespace, path: "the-path-project")
+      expected_repo = File.join(TestEnv.repos_path, "the-path0", "the-path-project.git")
+
+      subject.rename_namespace(namespace)
+
+      expect(File.directory?(expected_repo)).to be(true)
+    end
+
+    it "moves the uploads for the namespace" do
+      expect(subject).to receive(:move_uploads).with("the-path", "the-path0")
+
+      subject.rename_namespace(namespace)
+    end
+
+    it "moves the pages for the namespace" do
+      expect(subject).to receive(:move_pages).with("the-path", "the-path0")
+
+      subject.rename_namespace(namespace)
+    end
+
+    it 'invalidates the markdown cache of related projects' do
+      project = create(:empty_project, namespace: namespace, path: "the-path-project")
+
+      expect(subject).to receive(:remove_cached_html_for_projects).with([project.id])
+
+      subject.rename_namespace(namespace)
+    end
+  end
+
+  describe '#rename_namespaces' do
+    let!(:top_level_namespace) { create(:namespace, path: 'the-path') }
+    let!(:child_namespace) do
+      create(:namespace, path: 'the-path', parent: create(:namespace))
+    end
+
+    it 'renames top level namespaces the namespace' do
+      expect(subject).to receive(:rename_namespace).
+                           with(migration_namespace(top_level_namespace))
+
+      subject.rename_namespaces(type: :top_level)
+    end
+
+    it 'renames child namespaces' do
+      expect(subject).to receive(:rename_namespace).
+                           with(migration_namespace(child_namespace))
+
+      subject.rename_namespaces(type: :child)
+    end
+  end
+end

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects do
+  let(:migration) { FakeRenameReservedPathMigrationV1.new }
+  let(:subject) { described_class.new(['the-path'], migration) }
+
+  before do
+    allow(migration).to receive(:say)
+  end
+
+  describe '#projects_for_paths' do
+    it 'searches using nested paths' do
+      namespace = create(:namespace, path: 'hello')
+      project = create(:empty_project, path: 'THE-path', namespace: namespace)
+
+      result_ids = described_class.new(['Hello/the-path'], migration).
+                     projects_for_paths.map(&:id)
+
+      expect(result_ids).to contain_exactly(project.id)
+    end
+
+    it 'includes the correct projects' do
+      project = create(:empty_project, path: 'THE-path')
+      _other_project = create(:empty_project)
+
+      result_ids = subject.projects_for_paths.map(&:id)
+
+      expect(result_ids).to contain_exactly(project.id)
+    end
+  end
+
+  describe '#rename_projects' do
+    let!(:projects) { create_list(:empty_project, 2, path: 'the-path') }
+
+    it 'renames each project' do
+      expect(subject).to receive(:rename_project).twice
+
+      subject.rename_projects
+    end
+
+    it 'invalidates the markdown cache of related projects' do
+      expect(subject).to receive(:remove_cached_html_for_projects).
+                           with(projects.map(&:id))
+
+      subject.rename_projects
+    end
+  end
+
+  describe '#rename_project' do
+    let(:project) do
+      create(:empty_project,
+             path: 'the-path',
+             namespace: create(:namespace, path: 'known-parent' ))
+    end
+
+    it 'renames path & route for the project' do
+      expect(subject).to receive(:rename_path_for_routable).
+                           with(project).
+                           and_call_original
+
+      subject.rename_project(project)
+
+      expect(project.reload.path).to eq('the-path0')
+    end
+
+    it 'moves the wiki & the repo' do
+      expect(subject).to receive(:move_repository).
+                           with(project, 'known-parent/the-path.wiki', 'known-parent/the-path0.wiki')
+      expect(subject).to receive(:move_repository).
+                           with(project, 'known-parent/the-path', 'known-parent/the-path0')
+
+      subject.rename_project(project)
+    end
+
+    it 'moves uploads' do
+      expect(subject).to receive(:move_uploads).
+                           with('known-parent/the-path', 'known-parent/the-path0')
+
+      subject.rename_project(project)
+    end
+
+    it 'moves pages' do
+      expect(subject).to receive(:move_pages).
+                           with('known-parent/the-path', 'known-parent/the-path0')
+
+      subject.rename_project(project)
+    end
+  end
+
+  describe '#move_repository' do
+    let(:known_parent) { create(:namespace, path: 'known-parent') }
+    let(:project) { create(:project, path: 'the-path', namespace: known_parent) }
+
+    it 'moves the repository for a project' do
+      expected_path = File.join(TestEnv.repos_path, 'known-parent', 'new-repo.git')
+
+      subject.move_repository(project, 'known-parent/the-path', 'known-parent/new-repo')
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+  end
+end

spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb

+require 'spec_helper'
+
+shared_examples 'renames child namespaces' do |type|
+  it 'renames namespaces' do
+    rename_namespaces = double
+    expect(described_class::RenameNamespaces).
+      to receive(:new).with(['first-path', 'second-path'], subject).
+           and_return(rename_namespaces)
+    expect(rename_namespaces).to receive(:rename_namespaces).
+                                   with(type: :child)
+
+    subject.rename_wildcard_paths(['first-path', 'second-path'])
+  end
+end
+
+describe Gitlab::Database::RenameReservedPathsMigration::V1 do
+  let(:subject) { FakeRenameReservedPathMigrationV1.new }
+
+  before do
+    allow(subject).to receive(:say)
+  end
+
+  describe '#rename_child_paths' do
+    it_behaves_like 'renames child namespaces'
+  end
+
+  describe '#rename_wildcard_paths' do
+    it_behaves_like 'renames child namespaces'
+
+    it 'should rename projects' do
+      rename_projects = double
+      expect(described_class::RenameProjects).
+        to receive(:new).with(['the-path'], subject).
+             and_return(rename_projects)
+
+      expect(rename_projects).to receive(:rename_projects)
+
+      subject.rename_wildcard_paths(['the-path'])
+    end
+  end
+
+  describe '#rename_root_paths' do
+    it 'should rename namespaces' do
+      rename_namespaces = double
+      expect(described_class::RenameNamespaces).
+        to receive(:new).with(['the-path'], subject).
+             and_return(rename_namespaces)
+      expect(rename_namespaces).to receive(:rename_namespaces).
+                           with(type: :top_level)
+
+      subject.rename_root_paths('the-path')
+    end
+  end
+end

spec/lib/gitlab/regex_spec.rb

@@ -45,8 +45,8 @@ describe Gitlab::Regex, lib: true do
     it { is_expected.not_to match('foo-') }
   end
 
-  describe 'FULL_NAMESPACE_REGEX_STR' do
-    subject { %r{\A#{Gitlab::Regex::FULL_NAMESPACE_REGEX_STR}\z} }
+  describe '.full_namespace_regex' do
+    subject { described_class.full_namespace_regex }
 
     it { is_expected.to match('gitlab.org') }
     it { is_expected.to match('gitlab.org/gitlab-git') }

spec/models/concerns/cache_markdown_field_spec.rb

@@ -170,6 +170,12 @@ describe CacheMarkdownField do
 
       is_expected.to be_truthy
     end
+
+    it 'returns false if the markdown field is set but the html is not' do
+      thing.foo_html = nil
+
+      is_expected.to be_falsy
+    end
   end
 
   describe '#refresh_markdown_cache!' do

spec/models/group_spec.rb

@@ -57,6 +57,32 @@ describe Group, models: true do
     it { is_expected.not_to validate_presence_of :owner }
     it { is_expected.to validate_presence_of :two_factor_grace_period }
     it { is_expected.to validate_numericality_of(:two_factor_grace_period).is_greater_than_or_equal_to(0) }
+
+    describe 'path validation' do
+      it 'rejects paths reserved on the root namespace when the group has no parent' do
+        group = build(:group, path: 'api')
+
+        expect(group).not_to be_valid
+      end
+
+      it 'allows root paths when the group has a parent' do
+        group = build(:group, path: 'api', parent: create(:group))
+
+        expect(group).to be_valid
+      end
+
+      it 'rejects any wildcard paths when not a top level group' do
+        group = build(:group, path: 'tree', parent: create(:group))
+
+        expect(group).not_to be_valid
+      end
+
+      it 'rejects reserved group paths' do
+        group = build(:group, path: 'activity', parent: create(:group))
+
+        expect(group).not_to be_valid
+      end
+    end
   end
 
   describe '.visible_to_user' do

spec/models/namespace_spec.rb

@@ -34,6 +34,13 @@ describe Namespace, models: true do
         let(:group) { build(:group, :nested, path: 'tree') }
 
         it { expect(group).not_to be_valid }
+
+        it 'rejects nested paths' do
+          parent = create(:group, :nested, path: 'environments')
+          namespace = build(:project, path: 'folders', namespace: parent)
+
+          expect(namespace).not_to be_valid
+        end
       end
 
       context 'top-level group' do
@@ -47,6 +54,7 @@ describe Namespace, models: true do
   describe "Respond to" do
     it { is_expected.to respond_to(:human_name) }
     it { is_expected.to respond_to(:to_param) }
+    it { is_expected.to respond_to(:has_parent?) }
   end
 
   describe '#to_param' do

spec/models/project_spec.rb

@@ -280,6 +280,34 @@ describe Project, models: true do
         expect(new_project.errors.full_messages.first).to eq('The project is still being deleted. Please try again later.')
       end
     end
+
+    describe 'path validation' do
+      it 'allows paths reserved on the root namespace' do
+        project = build(:project, path: 'api')
+
+        expect(project).to be_valid
+      end
+
+      it 'rejects paths reserved on another level' do
+        project = build(:project, path: 'tree')
+
+        expect(project).not_to be_valid
+      end
+
+      it 'rejects nested paths' do
+        parent = create(:group, :nested, path: 'environments')
+        project = build(:project, path: 'folders', namespace: parent)
+
+        expect(project).not_to be_valid
+      end
+
+      it 'allows a reserved group name' do
+        parent = create(:group)
+        project = build(:project, path: 'avatar', namespace: parent)
+
+        expect(project).to be_valid
+      end
+    end
   end
 
   describe 'default_scope' do

spec/models/user_spec.rb

@@ -102,6 +102,18 @@ describe User, models: true do
         expect(user.errors.values).to eq [['dashboard is a reserved name']]
       end
 
+      it 'allows child names' do
+        user = build(:user, username: 'avatar')
+
+        expect(user).to be_valid
+      end
+
+      it 'allows wildcard names' do
+        user = build(:user, username: 'blob')
+
+        expect(user).to be_valid
+      end
+
       it 'validates uniqueness' do
         expect(subject).to validate_uniqueness_of(:username).case_insensitive
       end

spec/support/fake_migration_classes.rb

+class FakeRenameReservedPathMigrationV1 < ActiveRecord::Migration
+  include Gitlab::Database::RenameReservedPathsMigration::V1
+end

spec/validators/dynamic_path_validator_spec.rb

+require 'spec_helper'
+
+describe DynamicPathValidator do
+  let(:validator) { described_class.new(attributes: [:path]) }
+
+  # Pass in a full path to remove the format segment:
+  # `/ci/lint(.:format)` -> `/ci/lint`
+  def without_format(path)
+    path.split('(', 2)[0]
+  end
+
+  # Pass in a full path and get the last segment before a wildcard
+  # That's not a parameter
+  # `/*namespace_id/:project_id/builds/artifacts/*ref_name_and_path`
+  #    -> 'builds/artifacts'
+  def path_before_wildcard(path)
+    path = path.gsub(STARTING_WITH_NAMESPACE, "")
+    path_segments = path.split('/').reject(&:empty?)
+    wildcard_index = path_segments.index { |segment| parameter?(segment) }
+
+    segments_before_wildcard = path_segments[0..wildcard_index - 1]
+
+    segments_before_wildcard.join('/')
+  end
+
+  def parameter?(segment)
+    segment =~ /[*:]/
+  end
+
+  # If the path is reserved. Then no conflicting paths can# be created for any
+  # route using this reserved word.
+  #
+  # Both `builds/artifacts` & `build` are covered by reserving the word
+  # `build`
+  def wildcards_include?(path)
+    described_class::WILDCARD_ROUTES.include?(path) ||
+      described_class::WILDCARD_ROUTES.include?(path.split('/').first)
+  end
+
+  def failure_message(missing_words, constant_name, migration_helper)
+    missing_words = Array(missing_words)
+    <<-MSG
+      Found new routes that could cause conflicts with existing namespaced routes
+      for groups or projects.
+
+      Add <#{missing_words.join(', ')}> to `DynamicPathValidator::#{constant_name}
+      to make sure no projects or namespaces can be created with those paths.
+
+      To rename any existing records with those paths you can use the
+      `Gitlab::Database::RenameReservedpathsMigration::<VERSION>.#{migration_helper}`
+      migration helper.
+
+      Make sure to make a note of the renamed records in the release blog post.
+
+    MSG
+  end
+
+  let(:all_routes) do
+    Rails.application.routes.routes.routes.
+      map { |r| r.path.spec.to_s }
+  end
+
+  let(:routes_without_format) { all_routes.map { |path| without_format(path) } }
+
+  # Routes not starting with `/:` or `/*`
+  # all routes not starting with a param
+  let(:routes_not_starting_in_wildcard) { routes_without_format.select { |p| p !~ %r{^/[:*]} } }
+
+  let(:top_level_words) do
+    routes_not_starting_in_wildcard.map do |route|
+      route.split('/')[1]
+    end.compact.uniq
+  end
+
+  # All routes that start with a namespaced path, that have 1 or more
+  # path-segments before having another wildcard parameter.
+  # - Starting with paths:
+  #   - `/*namespace_id/:project_id/`
+  #   - `/*namespace_id/:id/`
+  # - Followed by one or more path-parts not starting with `:` or `*`
+  # - Followed by a path-part that includes a wildcard parameter `*`
+  # At the time of writing these routes match: http://rubular.com/r/Rv2pDE5Dvw
+  STARTING_WITH_NAMESPACE = %r{^/\*namespace_id/:(project_)?id}
+  NON_PARAM_PARTS = %r{[^:*][a-z\-_/]*}
+  ANY_OTHER_PATH_PART = %r{[a-z\-_/:]*}
+  WILDCARD_SEGMENT = %r{\*}
+  let(:namespaced_wildcard_routes) do
+    routes_without_format.select do |p|
+      p =~ %r{#{STARTING_WITH_NAMESPACE}/#{NON_PARAM_PARTS}/#{ANY_OTHER_PATH_PART}#{WILDCARD_SEGMENT}}
+    end
+  end
+
+  # This will return all paths that are used in a namespaced route
+  # before another wildcard path:
+  #
+  # /*namespace_id/:project_id/builds/artifacts/*ref_name_and_path
+  # /*namespace_id/:project_id/info/lfs/objects/*oid
+  # /*namespace_id/:project_id/commits/*id
+  # /*namespace_id/:project_id/builds/:build_id/artifacts/file/*path
+  # -> ['builds/artifacts', 'info/lfs/objects', 'commits', 'artifacts/file']
+  let(:all_wildcard_paths) do
+    namespaced_wildcard_routes.map do |route|
+      path_before_wildcard(route)
+    end.uniq
+  end
+
+  STARTING_WITH_GROUP = %r{^/groups/\*(group_)?id/}
+  let(:group_routes) do
+    routes_without_format.select do |path|
+      path =~ STARTING_WITH_GROUP
+    end
+  end
+
+  let(:paths_after_group_id) do
+    group_routes.map do |route|
+      route.gsub(STARTING_WITH_GROUP, '').split('/').first
+    end.uniq
+  end
+
+  describe 'TOP_LEVEL_ROUTES' do
+    it 'includes all the top level namespaces' do
+      failure_block = lambda do
+        missing_words = top_level_words - described_class::TOP_LEVEL_ROUTES
+        failure_message(missing_words, 'TOP_LEVEL_ROUTES', 'rename_root_paths')
+      end
+
+      expect(described_class::TOP_LEVEL_ROUTES)
+        .to include(*top_level_words), failure_block
+    end
+  end
+
+  describe 'GROUP_ROUTES' do
+    it "don't contain a second wildcard" do
+      failure_block = lambda do
+        missing_words = paths_after_group_id - described_class::GROUP_ROUTES
+        failure_message(missing_words, 'GROUP_ROUTES', 'rename_child_paths')
+      end
+
+      expect(described_class::GROUP_ROUTES)
+        .to include(*paths_after_group_id), failure_block
+    end
+  end
+
+  describe 'WILDCARD_ROUTES' do
+    it 'includes all paths that can be used after a namespace/project path' do
+      aggregate_failures do
+        all_wildcard_paths.each do |path|
+          expect(wildcards_include?(path))
+            .to be(true), failure_message(path, 'WILDCARD_ROUTES', 'rename_wildcard_paths')
+        end
+      end
+    end
+  end
+
+  describe '.without_reserved_wildcard_paths_regex' do
+    subject { described_class.without_reserved_wildcard_paths_regex }
+
+    it 'rejects paths starting with a reserved top level' do
+      expect(subject).not_to match('dashboard/hello/world')
+      expect(subject).not_to match('dashboard')
+    end
+
+    it 'matches valid paths with a toplevel word in a different place' do
+      expect(subject).to match('parent/dashboard/project-path')
+    end
+
+    it 'rejects paths containing a wildcard reserved word' do
+      expect(subject).not_to match('hello/edit')
+      expect(subject).not_to match('hello/edit/in-the-middle')
+      expect(subject).not_to match('foo/bar1/refs/master/logs_tree')
+    end
+
+    it 'matches valid paths' do
+      expect(subject).to match('parent/child/project-path')
+    end
+  end
+
+  describe '.regex_excluding_child_paths' do
+    let(:subject) { described_class.without_reserved_child_paths_regex }
+
+    it 'rejects paths containing a child reserved word' do
+      expect(subject).not_to match('hello/group_members')
+      expect(subject).not_to match('hello/activity/in-the-middle')
+      expect(subject).not_to match('foo/bar1/refs/master/logs_tree')
+    end
+
+    it 'allows a child path on the top level' do
+      expect(subject).to match('activity/foo')
+      expect(subject).to match('avatar')
+    end
+  end
+
+  describe ".valid?" do
+    it 'is not case sensitive' do
+      expect(described_class.valid?("Users")).to be_falsey
+    end
+
+    it "isn't valid when the top level is reserved" do
+      test_path = 'u/should-be-a/reserved-word'
+
+      expect(described_class.valid?(test_path)).to be_falsey
+    end
+
+    it "isn't valid if any of the path segments is reserved" do
+      test_path = 'the-wildcard/wikis/is-not-allowed'
+
+      expect(described_class.valid?(test_path)).to be_falsey
+    end
+
+    it "is valid if the path doesn't contain reserved words" do
+      test_path = 'there-are/no-wildcards/in-this-path'
+
+      expect(described_class.valid?(test_path)).to be_truthy
+    end
+
+    it 'allows allows a child path on the last spot' do
+      test_path = 'there/can-be-a/project-called/labels'
+
+      expect(described_class.valid?(test_path)).to be_truthy
+    end
+
+    it 'rejects a child path somewhere else' do
+      test_path = 'there/can-be-no/labels/group'
+
+      expect(described_class.valid?(test_path)).to be_falsey
+    end
+
+    it 'rejects paths that are in an incorrect format' do
+      test_path = 'incorrect/format.git'
+
+      expect(described_class.valid?(test_path)).to be_falsey
+    end
+  end
+
+  describe '#path_reserved_for_record?' do
+    it 'reserves a sub-group named activity' do
+      group = build(:group, :nested, path: 'activity')
+
+      expect(validator.path_reserved_for_record?(group, 'activity')).to be_truthy
+    end
+
+    it "doesn't reserve a project called activity" do
+      project = build(:project, path: 'activity')
+
+      expect(validator.path_reserved_for_record?(project, 'activity')).to be_falsey
+    end
+  end
+
+  describe '#validates_each' do
+    it 'adds a message when the path is not in the correct format' do
+      group = build(:group)
+
+      validator.validate_each(group, :path, "Path with spaces, and comma's!")
+
+      expect(group.errors[:path]).to include(Gitlab::Regex.namespace_regex_message)
+    end
+
+    it 'adds a message when the path is not in the correct format' do
+      group = build(:group, path: 'users')
+
+      validator.validate_each(group, :path, 'users')
+
+      expect(group.errors[:path]).to include('users is a reserved name')
+    end
+  end
+end