Adjust `PathRegex` to validate files in the `public` directory

And reports when too many words are rejected.

1 files changed, 39 insertions, 7 deletions

diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb
index 1eea710c80b..37c67db8217 100644
--- a/spec/lib/gitlab/path_regex_spec.rb
+++ b/spec/lib/gitlab/path_regex_spec.rb
@@ -36,9 +36,10 @@ describe Gitlab::PathRegex, lib: true do
       described_class::PROJECT_WILDCARD_ROUTES.include?(path.split('/').first)
   end
 
-  def failure_message(missing_words, constant_name, migration_helper)
+  def failure_message(constant_name, migration_helper, missing_words:, additional_words: [])
     missing_words = Array(missing_words)
-    <<-MSG
+    additional_words = Array(additional_words)
+    message = <<-MSG
       Found new routes that could cause conflicts with existing namespaced routes
       for groups or projects.
 
@@ -52,6 +53,18 @@ describe Gitlab::PathRegex, lib: true do
       Make sure to make a note of the renamed records in the release blog post.
 
     MSG
+
+    if additional_words.any?
+      additional_message = <<-ADDITIONAL
+      Why are <#{additional_words.join(', ')}> in `#{constant_name}`?
+      If they are really required, update these specs to reflect that.
+
+      ADDITIONAL
+
+      message = [message, additional_message].join
+    end
+
+    message
   end
 
   let(:all_routes) do
@@ -68,9 +81,26 @@ describe Gitlab::PathRegex, lib: true do
   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|
+    words = routes_not_starting_in_wildcard.map do |route|
       route.split('/')[1]
     end.compact.uniq
+
+    words += files_in_public
+    words + additional_top_level_words
+  end
+
+  let(:additional_top_level_words) do
+    # Required to keep the uploads safe, remove after
+    # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12917 gets merged
+    ['system']
+  end
+
+  let(:files_in_public) do
+    git = Gitlab.config.git.bin_path
+    `cd #{Rails.root} && #{git} ls-files public`
+      .split("\n")
+      .map { |entry| entry.gsub('public/', '') }
+      .uniq
   end
 
   # All routes that start with a namespaced path, that have 1 or more
@@ -122,11 +152,13 @@ describe Gitlab::PathRegex, lib: true 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')
+        additional_words = described_class::TOP_LEVEL_ROUTES - top_level_words
+        failure_message('TOP_LEVEL_ROUTES', 'rename_root_paths',
+                        missing_words: missing_words, additional_words: additional_words)
       end
 
       expect(described_class::TOP_LEVEL_ROUTES)
-        .to include(*top_level_words), failure_block
+        .to contain_exactly(*top_level_words), failure_block
     end
   end
 
@@ -134,7 +166,7 @@ describe Gitlab::PathRegex, lib: true 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')
+        failure_message('GROUP_ROUTES', 'rename_child_paths', missing_words: missing_words)
       end
 
       expect(described_class::GROUP_ROUTES)
@@ -147,7 +179,7 @@ describe Gitlab::PathRegex, lib: true do
       aggregate_failures do
         all_wildcard_paths.each do |path|
           expect(wildcards_include?(path))
-            .to be(true), failure_message(path, 'PROJECT_WILDCARD_ROUTES', 'rename_wildcard_paths')
+            .to be(true), failure_message('PROJECT_WILDCARD_ROUTES', 'rename_wildcard_paths', missing_words: path)
         end
       end
     end