Merge branch 'kerrizor/remove-sectional-codeowners-feature-flag' into 'master'

Remove sectional_codeowners feature flag

See merge request gitlab-org/gitlab!42389

doc/user/project/code_owners.md

@@ -157,12 +157,8 @@ file.md @group-x @group-x/subgroup-y
 
 ### Code Owners Sections **(PREMIUM)**
 
-> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12137) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.2.
-> - It's deployed behind a feature flag, enabled by default.
-> - It's enabled on GitLab.com.
-> - It can be enabled or disabled per-project.
-> - It's recommended for production use.
-> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-code-owner-sections). **(CORE ONLY)**
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12137) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.2 behind a feature flag, enabled by default.
+> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42389) in GitLab 13.4. 
 
 Code Owner rules can be grouped into named sections. This allows for better
 organization of broader categories of Code Owner rules to be applied.
@@ -224,28 +220,6 @@ the rules for "Groups" and "Documentation" sections:
 
 ![MR widget - Sectional Code Owners](img/sectional_code_owners_v13.2.png)
 
-#### Enable or disable Code Owner Sections **(CORE ONLY)**
-
-Sections is under development but ready for production use.
-It is deployed behind a feature flag that is **enabled by default**.
-[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
-can opt to disable it for your instance.
-
-To disable it:
-
-```ruby
-Feature.disable(:sectional_codeowners)
-```
-
-To enable it:
-
-```ruby
-Feature.enable(:sectional_codeowners)
-```
-
-CAUTION: **Caution:**
-Disabling Sections will **not** refresh Code Owner Approval Rules on existing merge requests.
-
 ## Example `CODEOWNERS` file
 
 ```plaintext

ee/lib/gitlab/code_owners/file.rb

@@ -16,35 +16,25 @@ module Gitlab
         @parsed_data ||= get_parsed_data
       end
 
+      # Since an otherwise "empty" CODEOWNERS file will still return a default
+      #   section of "codeowners", a la
+      #
+      #   {"codeowners"=>{}}
+      #
+      #   ...we must cycle through all the actual values parsed into each
+      #   section to determine if the file is empty or not.
+      #
       def empty?
-        parsed_data.empty?
-      end
-
-      def entry_for_path(path)
-        path = "/#{path}" unless path.start_with?('/')
-
-        if sectional_codeowners?
-          sectional_entry_for_path(path)
-        else
-          non_sectional_entry_for_path(path)
-        end
+        parsed_data.values.all?(&:empty?)
       end
 
       def path
         @blob&.path
       end
 
-      private
-
-      def non_sectional_entry_for_path(path)
-        matching_pattern = parsed_data.keys.reverse.detect do |pattern|
-          path_matches?(pattern, path)
-        end
-
-        matching_pattern ? [parsed_data[matching_pattern].dup] : []
-      end
+      def entry_for_path(path)
+        path = "/#{path}" unless path.start_with?('/')
 
-      def sectional_entry_for_path(path)
         matches = []
 
         parsed_data.each do |_, section_entries|
@@ -67,28 +57,6 @@ module Gitlab
       end
 
       def get_parsed_data
-        if sectional_codeowners?
-          get_parsed_sectional_data
-        else
-          get_parsed_non_sectional_data
-        end
-      end
-
-      def get_parsed_non_sectional_data
-        parsed = {}
-
-        data.lines.each do |line|
-          line = line.strip
-
-          next if skip?(line) || line.match?(SECTION_HEADER_REGEX)
-
-          extract_entry_and_populate_parsed_data(line, parsed)
-        end
-
-        parsed
-      end
-
-      def get_parsed_sectional_data
         parsed_sectional_data = {}
         canonical_section_name = ::Gitlab::CodeOwners::Entry::DEFAULT_SECTION
 
@@ -168,12 +136,6 @@ module Gitlab
 
         ::File.fnmatch?(pattern, path, flags)
       end
-
-      def sectional_codeowners?
-        strong_memoize(:sectional_codeowners_check) do
-          Feature.enabled?(:sectional_codeowners, @project, default_enabled: true)
-        end
-      end
     end
   end
 end

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

@@ -14,51 +14,30 @@ RSpec.describe Gitlab::CodeOwners::File do
 
   subject(:file) { described_class.new(blob) }
 
-  before do
-    stub_feature_flags(sectional_codeowners: false)
-  end
-
   describe '#parsed_data' do
     def owner_line(pattern)
-      file.parsed_data[pattern].owner_line
-    end
-
-    it 'parses all the required lines' do
-      expected_patterns = [
-        '/**/*', '/**/#file_with_pound.rb', '/**/*.rb', '/**/CODEOWNERS', '/**/LICENSE', '/docs/**/*',
-        '/docs/*', '/config/**/*', '/**/lib/**/*', '/**/path with spaces/**/*'
-      ]
-
-      expect(file.parsed_data.keys)
-        .to contain_exactly(*expected_patterns)
+      file.parsed_data["codeowners"][pattern].owner_line
     end
 
-    it 'allows usernames and emails' do
-      expect(owner_line('/**/LICENSE')).to include('legal', 'janedoe@gitlab.com')
-    end
+    context "when CODEOWNERS file contains no sections" do
+      it 'parses all the required lines' do
+        expected_patterns = [
+          '/**/*', '/**/#file_with_pound.rb', '/**/*.rb', '/**/CODEOWNERS', '/**/LICENSE', '/docs/**/*',
+          '/docs/*', '/config/**/*', '/**/lib/**/*', '/**/path with spaces/**/*'
+        ]
 
-    context "when CODEOWNERS file contains multiple sections" do
-      let(:file_content) do
-        File.read(Rails.root.join("ee", "spec", "fixtures", "sectional_codeowners_example"))
+        expect(file.parsed_data["codeowners"].keys)
+          .to contain_exactly(*expected_patterns)
       end
 
-      let(:patterns) { ["[Documentation]", "[Database]"] }
-      let(:paths) { ["/**/[Documentation]", "/**/[Database]"] }
-
-      it "skips section headers when parsing" do
-        expect(file.parsed_data.keys).not_to include(*paths)
-        expect(file.parsed_data.values.any? { |e| patterns.include?(e.pattern) }).to be_falsey
-        expect(file.parsed_data.values.any? { |e| e.owner_line.blank? }).to be_falsey
+      it 'allows usernames and emails' do
+        expect(owner_line('/**/LICENSE')).to include('legal', 'janedoe@gitlab.com')
       end
     end
 
-    context "when feature flag `:sectional_codeowners` is enabled" do
+    context "when handling a sectional codeowners file" do
       using RSpec::Parameterized::TableSyntax
 
-      before do
-        stub_feature_flags(sectional_codeowners: true)
-      end
-
       shared_examples_for "creates expected parsed sectional results" do
         it "is a hash sorted by sections without duplicates" do
           data = file.parsed_data
@@ -107,12 +86,6 @@ RSpec.describe Gitlab::CodeOwners::File do
         end
       end
 
-      it "passes the call to #get_parsed_sectional_data" do
-        expect(file).to receive(:get_parsed_sectional_data)
-
-        file.parsed_data
-      end
-
       it "populates a hash with a single default section" do
         data = file.parsed_data
 
@@ -316,40 +289,16 @@ RSpec.describe Gitlab::CodeOwners::File do
       end
     end
 
-    context "when feature flag `:sectional_codeowners` is enabled" do
-      before do
-        stub_feature_flags(sectional_codeowners: true)
-      end
-
-      context "when CODEOWNERS file contains no sections" do
-        it_behaves_like "returns expected matches"
-      end
-
-      context "when CODEOWNERS file contains multiple sections" do
-        let(:file_content) do
-          File.read(Rails.root.join("ee", "spec", "fixtures", "sectional_codeowners_example"))
-        end
-
-        it_behaves_like "returns expected matches"
-      end
+    context "when CODEOWNERS file contains no sections" do
+      it_behaves_like "returns expected matches"
     end
 
-    context "when feature flag `:sectional_codeowners` is disabled" do
-      before do
-        stub_feature_flags(sectional_codeowners: false)
-      end
-
-      context "when CODEOWNERS file contains no sections" do
-        it_behaves_like "returns expected matches"
+    context "when CODEOWNERS file contains multiple sections" do
+      let(:file_content) do
+        File.read(Rails.root.join("ee", "spec", "fixtures", "sectional_codeowners_example"))
       end
 
-      context "when CODEOWNERS file contains multiple sections" do
-        let(:file_content) do
-          File.read(Rails.root.join("ee", "spec", "fixtures", "sectional_codeowners_example"))
-        end
-
-        it_behaves_like "returns expected matches"
-      end
+      it_behaves_like "returns expected matches"
     end
   end
 end

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

@@ -39,10 +39,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do
     let(:expected_entry) { Gitlab::CodeOwners::Entry.new('docs/CODEOWNERS', '@owner-1 owner2@gitlab.org @owner-3 @documentation-owner') }
     let(:first_entry) { loader.entries.first }
 
-    before do
-      stub_feature_flags(sectional_codeowners: false)
-    end
-
     it 'returns entries for the matched line' do
       expect(loader.entries).to contain_exactly(expected_entry)
     end
@@ -134,15 +130,11 @@ RSpec.describe Gitlab::CodeOwners::Loader do
       end
     end
 
-    context "when sectional_codeowners is disabled" do
-      before do
-        stub_feature_flags(sectional_codeowners: false)
-      end
-
+    context "non-sectional codeowners_content" do
       it_behaves_like "returns users for passed path"
     end
 
-    context "when sectional_codeowners is enabled" do
+    context "when codeowners_content contains sections" do
       let(:codeowner_content) do
         <<~CODEOWNERS
         [Documentation]
@@ -153,10 +145,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do
         CODEOWNERS
       end
 
-      before do
-        stub_feature_flags(sectional_codeowners: true)
-      end
-
       it_behaves_like "returns users for passed path"
     end
   end
@@ -178,10 +166,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do
   end
 
   describe '#empty_code_owners?' do
-    before do
-      stub_feature_flags(sectional_codeowners: false)
-    end
-
     context 'when file does not exist' do
       let(:codeowner_blob) { nil }