Put changes behind feature flag

Put the reading of project_features.container_registry_access_level
instead of projects.container_registry_enabled behind a feature flag
called read_container_registry_access_level.

app/models/container_repository.rb

@@ -24,8 +24,15 @@ class ContainerRepository < ApplicationRecord
   scope :for_group_and_its_subgroups, ->(group) do
     project_scope = Project
       .for_group_and_its_subgroups(group)
-      .with_feature_enabled(:container_registry)
-      .select(:id)
+
+    project_scope =
+      if Feature.enabled?(:read_container_registry_access_level, group, default_enabled: :yaml)
+        project_scope.with_feature_enabled(:container_registry)
+      else
+        project_scope.with_container_registry
+      end
+
+    project_scope = project_scope.select(:id)
 
     joins("INNER JOIN (#{project_scope.to_sql}) projects on projects.id=container_repositories.project_id")
   end

app/models/project.rb

@@ -406,9 +406,8 @@ class Project < ApplicationRecord
     :wiki_access_level, :snippets_access_level, :builds_access_level,
     :repository_access_level, :pages_access_level, :metrics_dashboard_access_level, :analytics_access_level,
     :operations_enabled?, :operations_access_level, :security_and_compliance_access_level,
-    :container_registry_access_level, :container_registry_enabled?,
+    :container_registry_access_level,
     to: :project_feature, allow_nil: true
-  alias_method :container_registry_enabled, :container_registry_enabled?
   delegate :show_default_award_emojis, :show_default_award_emojis=,
     :show_default_award_emojis?,
     to: :project_setting, allow_nil: true
@@ -547,6 +546,7 @@ class Project < ApplicationRecord
   scope :include_project_feature, -> { includes(:project_feature) }
   scope :with_service, ->(service) { joins(service).eager_load(service) }
   scope :with_shared_runners, -> { where(shared_runners_enabled: true) }
+  scope :with_container_registry, -> { where(container_registry_enabled: true) }
   scope :inside_path, ->(path) do
     # We need routes alias rs for JOIN so it does not conflict with
     # includes(:route) which we use in ProjectsFinder.
@@ -2612,6 +2612,15 @@ class Project < ApplicationRecord
     !!read_attribute(:merge_requests_author_approval)
   end
 
+  def container_registry_enabled
+    if Feature.enabled?(:read_container_registry_access_level, self.namespace, default_enabled: :yaml)
+      project_feature.container_registry_enabled?
+    else
+      read_attribute(:container_registry_enabled)
+    end
+  end
+  alias_method :container_registry_enabled?, :container_registry_enabled
+
   private
 
   def set_container_registry_access_level

app/policies/project_policy.rb

@@ -51,7 +51,11 @@ class ProjectPolicy < BasePolicy
 
   desc "Container registry is disabled"
   condition(:container_registry_disabled, scope: :subject) do
-    !access_allowed_to?(:container_registry)
+    if ::Feature.enabled?(:read_container_registry_access_level, @subject&.namespace, default_enabled: :yaml)
+      !access_allowed_to?(:container_registry)
+    else
+      !project.container_registry_enabled
+    end
   end
 
   desc "Project has an external wiki"

config/feature_flags/development/read_container_registry_access_level.yml

+---
+name: read_container_registry_access_level
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55071
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332751
+milestone: '14.0'
+type: development
+group: group::package
+default_enabled: false

lib/api/entities/project.rb

@@ -53,7 +53,13 @@ module API
       expose(:wiki_enabled) { |project, options| project.feature_available?(:wiki, options[:current_user]) }
       expose(:jobs_enabled) { |project, options| project.feature_available?(:builds, options[:current_user]) }
       expose(:snippets_enabled) { |project, options| project.feature_available?(:snippets, options[:current_user]) }
-      expose(:container_registry_enabled) { |project, options| project.feature_available?(:container_registry, options[:current_user]) }
+      expose(:container_registry_enabled) do |project, options|
+        if ::Feature.enabled?(:read_container_registry_access_level, project.namespace, default_enabled: :yaml)
+          project.feature_available?(:container_registry, options[:current_user])
+        else
+          project.read_attribute(:container_registry_enabled)
+        end
+      end
       expose :service_desk_enabled
       expose :service_desk_address
 

spec/models/container_repository_spec.rb

@@ -331,6 +331,40 @@ RSpec.describe ContainerRepository do
 
       it { is_expected.to eq([]) }
     end
+
+    context 'with read_container_registry_access_level disabled' do
+      before do
+        stub_feature_flags(read_container_registry_access_level: false)
+      end
+
+      context 'in a group' do
+        let(:test_group) { group }
+
+        it { is_expected.to contain_exactly(repository) }
+      end
+
+      context 'with a subgroup' do
+        let(:test_group) { create(:group) }
+        let(:another_project) { create(:project, path: 'test', group: test_group) }
+
+        let(:another_repository) do
+          create(:container_repository, name: 'my_image', project: another_project)
+        end
+
+        before do
+          group.parent = test_group
+          group.save!
+        end
+
+        it { is_expected.to contain_exactly(repository, another_repository) }
+      end
+
+      context 'group without container_repositories' do
+        let(:test_group) { create(:group) }
+
+        it { is_expected.to eq([]) }
+      end
+    end
   end
 
   describe '.search_by_name' do

spec/models/project_spec.rb

@@ -655,6 +655,14 @@ RSpec.describe Project, factory_default: :keep do
     it { is_expected.to delegate_method(:allow_editing_commit_messages?).to(:project_setting) }
     it { is_expected.to delegate_method(:container_registry_enabled?).to(:project_feature) }
     it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) }
+
+    context 'when read_container_registry_access_level is disabled' do
+      before do
+        stub_feature_flags(read_container_registry_access_level: false)
+      end
+
+      it { is_expected.not_to delegate_method(:container_registry_enabled?).to(:project_feature) }
+    end
   end
 
   describe 'reference methods' do
@@ -2323,6 +2331,20 @@ RSpec.describe Project, factory_default: :keep do
       expect(project.container_registry_enabled).to eq(false)
       expect(project.container_registry_enabled?).to eq(false)
     end
+
+    context 'with read_container_registry_access_level disabled' do
+      before do
+        stub_feature_flags(read_container_registry_access_level: false)
+      end
+
+      it 'reads project.container_registry_enabled' do
+        project.update_column(:container_registry_enabled, true)
+        project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED)
+
+        expect(project.container_registry_enabled).to eq(true)
+        expect(project.container_registry_enabled?).to eq(true)
+      end
+    end
   end
 
   describe '#has_container_registry_tags?' do

spec/policies/project_policy_spec.rb

@@ -1527,5 +1527,72 @@ RSpec.describe ProjectPolicy do
         end
       end
     end
+
+    context 'with read_container_registry_access_level disabled' do
+      before do
+        stub_feature_flags(read_container_registry_access_level: false)
+      end
+
+      where(:project_visibility, :container_registry_enabled, :role, :allowed) do
+        :public   | true   | :maintainer | true
+        :public   | true   | :developer  | true
+        :public   | true   | :reporter   | true
+        :public   | true   | :guest      | true
+        :public   | true   | :anonymous  | true
+        :public   | false  | :maintainer | false
+        :public   | false  | :developer  | false
+        :public   | false  | :reporter   | false
+        :public   | false  | :guest      | false
+        :public   | false  | :anonymous  | false
+        :internal | true   | :maintainer | true
+        :internal | true   | :developer  | true
+        :internal | true   | :reporter   | true
+        :internal | true   | :guest      | true
+        :internal | true   | :anonymous  | false
+        :internal | false  | :maintainer | false
+        :internal | false  | :developer  | false
+        :internal | false  | :reporter   | false
+        :internal | false  | :guest      | false
+        :internal | false  | :anonymous  | false
+        :private  | true   | :maintainer | true
+        :private  | true   | :developer  | true
+        :private  | true   | :reporter   | true
+        :private  | true   | :guest      | false
+        :private  | true   | :anonymous  | false
+        :private  | false  | :maintainer | false
+        :private  | false  | :developer  | false
+        :private  | false  | :reporter   | false
+        :private  | false  | :guest      | false
+        :private  | false  | :anonymous  | false
+      end
+
+      with_them do
+        let(:current_user) { send(role) }
+        let(:project) { send("#{project_visibility}_project") }
+
+        it 'allows/disallows the abilities based on container_registry_enabled' do
+          project.update_column(:container_registry_enabled, container_registry_enabled)
+
+          if allowed
+            expect_allowed(*permissions_abilities(role))
+          else
+            expect_disallowed(*permissions_abilities(role))
+          end
+        end
+
+        def permissions_abilities(role)
+          case role
+          when :maintainer
+            maintainer_operations_permissions
+          when :developer
+            developer_operations_permissions
+          when :reporter, :guest, :anonymous
+            guest_operations_permissions
+          else
+            raise "Unknown role #{role}"
+          end
+        end
+      end
+    end
   end
 end

spec/requests/api/projects_spec.rb

@@ -185,7 +185,8 @@ RSpec.describe API::Projects do
       end
 
       it 'includes correct value of container_registry_enabled', :aggregate_failures do
-        project.project_feature.update!(container_registry_access_level: 0)
+        project.update_column(:container_registry_enabled, true)
+        project.project_feature.update!(container_registry_access_level: ProjectFeature::DISABLED)
 
         get api('/projects', user)
         project_response = json_response.find { |p| p['id'] == project.id }
@@ -195,6 +196,20 @@ RSpec.describe API::Projects do
         expect(project_response['container_registry_enabled']).to eq(false)
       end
 
+      it 'reads projects.container_registry_enabled when read_container_registry_access_level is disabled' do
+        stub_feature_flags(read_container_registry_access_level: false)
+
+        project.project_feature.update!(container_registry_access_level: ProjectFeature::DISABLED)
+        project.update_column(:container_registry_enabled, true)
+
+        get api('/projects', user)
+        project_response = json_response.find { |p| p['id'] == project.id }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to be_an Array
+        expect(project_response['container_registry_enabled']).to eq(true)
+      end
+
       it 'includes project topics' do
         get api('/projects', user)