Merge branch 'bvl-codeowners-root-dir' into 'master'

Allow matching only the repo-root for CODEOWNERS

Closes #8593

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

ee/changelogs/unreleased/bvl-codeowners-root-dir.yml

+---
+title: Allow matching only the repo-root for CODEOWNERS
+merge_request: 8708
+author:
+type: fixed

ee/lib/gitlab/code_owners/file.rb

@@ -16,6 +16,8 @@ module Gitlab
       end
 
       def owners_for_path(path)
+        path = "/#{path}" unless path.start_with?('/')
+
         matching_pattern = parsed_data.keys.reverse.detect do |pattern|
           path_matches?(pattern, path)
         end
@@ -57,35 +59,25 @@ module Gitlab
         # Replace all whitespace preceded by a \ with a regular whitespace
         pattern = pattern.gsub(/\\\s+/, ' ')
 
-        if pattern.starts_with?('/')
-          # Remove the leading slash when only matching root directory as the
-          # paths that we will be matching will always be passed in starting
-          # from the root of the repsitory.
-          pattern = pattern.sub(%r{\A/}, '')
-        elsif !pattern.starts_with?('*')
-          # If the pattern is a regular match, prepend it with ** so we match
-          # nested in every directory
-          pattern = "**#{pattern}"
+        return '/**/*' if pattern == '*'
+
+        unless pattern.starts_with?('/')
+          pattern = "/**/#{pattern}"
+        end
+
+        if pattern.end_with?('/')
+          pattern = "#{pattern}**/*"
         end
 
         pattern
       end
 
       def path_matches?(pattern, path)
-        flags = ::File::FNM_DOTMATCH
-
-        if pattern.ends_with?('/*')
-          # Then the pattern ends in a wildcard, we only want to go one level deep
-          # setting `::File::FNM_PATHNAME` makes the `*` not match directory
-          # separators
-          flags |= ::File::FNM_PATHNAME
-          ::File.fnmatch?(pattern, path, flags)
-        else
-          # Replace a pattern ending with `/` to `/*` to match everything within
-          # that directory
-          nested_pattern = pattern.sub(%r{/\z}, '/*')
-          ::File.fnmatch?(nested_pattern, path, flags)
-        end
+        # `FNM_DOTMATCH` makes sure we also match files starting with a `.`
+        # `FNM_PATHNAME` makes sure ** matches path separators
+        flags = ::File::FNM_DOTMATCH | ::File::FNM_PATHNAME
+
+        ::File.fnmatch?(pattern, path, flags)
       end
     end
   end

ee/spec/lib/gitlab/code_owners/file_spec.rb

@@ -17,15 +17,15 @@ describe Gitlab::CodeOwners::File do
   describe '#parsed_data' do
     it 'parses all the required lines' do
       expected_patterns = [
-        '*', '**#file_with_pound.rb', '*.rb', '**CODEOWNERS', '**LICENSE', 'docs/',
-        'docs/*', 'config/', '**lib/', '**path with spaces/'
+        '/**/*', '/**/#file_with_pound.rb', '/**/*.rb', '/**/CODEOWNERS', '/**/LICENSE', '/docs/**/*',
+        '/docs/*', '/config/**/*', '/**/lib/**/*', '/**/path with spaces/**/*'
       ]
       expect(file.parsed_data.keys)
         .to contain_exactly(*expected_patterns)
     end
 
     it 'allows usernames and emails' do
-      expect(file.parsed_data['**LICENSE']).to include('legal', 'janedoe@gitlab.com')
+      expect(file.parsed_data['/**/LICENSE']).to include('legal', 'janedoe@gitlab.com')
     end
 
     context 'when there are entries that do not look like user references' do
@@ -34,7 +34,7 @@ describe Gitlab::CodeOwners::File do
       end
 
       it 'ignores the entries' do
-        expect(file.parsed_data['**a-path/']).to include('username', 'email@gitlab.org')
+        expect(file.parsed_data['/**/a-path/**/*']).to include('username', 'email@gitlab.org')
       end
     end
   end
@@ -160,5 +160,40 @@ describe Gitlab::CodeOwners::File do
         expect(owners).not_to include('username', 'and-email@lookalikes.com')
       end
     end
+
+    context 'a glob on the root directory' do
+      let(:file_content) do
+        '/* @user-1 @user-2'
+      end
+
+      it 'matches files in the root directory' do
+        owners = file.owners_for_path('README.md')
+
+        expect(owners).to include('user-1', 'user-2')
+      end
+
+      it 'does not match nested files' do
+        owners = file.owners_for_path('nested/path/README.md')
+
+        expect(owners).to be_nil
+      end
+    end
+
+    context 'partial matches' do
+      let(:file_content) do
+        'foo/* @user-1 @user-2'
+      end
+
+      it 'does not match a file in a folder that looks the same' do
+        owners = file.owners_for_path('fufoo/bar')
+
+        expect(owners).to be_nil
+      end
+
+      it 'matches the file in any folder' do
+        expect(file.owners_for_path('baz/foo/bar')).to include('user-1', 'user-2')
+        expect(file.owners_for_path('/foo/bar')).to include('user-1', 'user-2')
+      end
+    end
   end
 end