diff --git a/changelogs/unreleased/bvl-free-unused-names.yml b/changelogs/unreleased/bvl-free-unused-names.yml new file mode 100644 index 0000000000000000000000000000000000000000..53acb95e5bb46f7e3298e160ef567d79c251467f --- /dev/null +++ b/changelogs/unreleased/bvl-free-unused-names.yml @@ -0,0 +1,5 @@ +--- +title: Free up some top level words, reject top level groups named like files in the + public folder +merge_request: 12932 +author: diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 60a32d5d5eaa2d4f065de18e50c1ceb2677cf4b8..894bd5efae5e88093ad8c61209193979b101f2a7 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -14,42 +14,42 @@ module Gitlab TOP_LEVEL_ROUTES = %w[ - .well-known + 404.html + 422.html + 500.html + 502.html + 503.html abuse_reports admin - all api + apple-touch-icon-precomposed.png + apple-touch-icon.png assets autocomplete ci dashboard + deploy.html explore + favicon.ico 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 + slash-command-logo.png snippets - teams u unicorn_test unsubscribes diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index 1eea710c80ba1f965c72040c73e5c0ca7347200d..c38bbb64fc3f60140634e8c2d426a52398eabfbb 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -36,9 +36,12 @@ 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 = "" + if missing_words.any? + message += <<-MISSING Found new routes that could cause conflicts with existing namespaced routes for groups or projects. @@ -51,7 +54,18 @@ describe Gitlab::PathRegex, lib: true do Make sure to make a note of the renamed records in the release blog post. - MSG + MISSING + end + + if additional_words.any? + message += <<-ADDITIONAL + Why are <#{additional_words.join(', ')}> in `#{constant_name}`? + If they are really required, update these specs to reflect that. + + ADDITIONAL + end + + message end let(:all_routes) do @@ -68,9 +82,23 @@ 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 + ee_top_level_words + files_in_public + end + + let(:ee_top_level_words) do + ['unsubscribes'] + 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 @@ -115,18 +143,29 @@ describe Gitlab::PathRegex, lib: true do let(:paths_after_group_id) do group_routes.map do |route| route.gsub(STARTING_WITH_GROUP, '').split('/').first - end.uniq + end.uniq + ee_paths_after_group_id + end + + let(:ee_paths_after_group_id) do + %w(analytics + ldap + ldap_group_links + notification_setting + audit_events + pipeline_quota hooks) 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') + 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,11 +173,13 @@ 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') + additional_words = described_class::GROUP_ROUTES - paths_after_group_id + failure_message('GROUP_ROUTES', 'rename_child_paths', + missing_words: missing_words, additional_words: additional_words) end expect(described_class::GROUP_ROUTES) - .to include(*paths_after_group_id), failure_block + .to contain_exactly(*paths_after_group_id), failure_block end end @@ -147,7 +188,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