Deprecate container_registry_enabled in projects public API

Now that we've migrated to use
ProjectFeature#container_registry_access_level instead of
Project#container_registry_enabled, allow
container_registry_access_level to be modified by the public
projects API.

This allows users to set the visibility of a
container registry independently of the project visibility.

Also update the public API docs.

Deprecate container_registry_enabled in the public API in favor of
container_registry_access_level.

Changelog: deprecated

lib/api/entities/project.rb

@@ -71,6 +71,7 @@ module API
       expose(:pages_access_level) { |project, options| project.project_feature.string_access_level(:pages) }
       expose(:operations_access_level) { |project, options| project.project_feature.string_access_level(:operations) }
       expose(:analytics_access_level) { |project, options| project.project_feature.string_access_level(:analytics) }
+      expose(:container_registry_access_level) { |project, options| project.project_feature.string_access_level(:container_registry) }
 
       expose :emails_disabled
       expose :shared_runners_enabled

lib/api/helpers/projects_helpers.rb

@@ -35,13 +35,14 @@ module API
         optional :pages_access_level, type: String, values: %w(disabled private enabled public), desc: 'Pages access level. One of `disabled`, `private`, `enabled` or `public`'
         optional :operations_access_level, type: String, values: %w(disabled private enabled), desc: 'Operations access level. One of `disabled`, `private` or `enabled`'
         optional :analytics_access_level, type: String, values: %w(disabled private enabled), desc: 'Analytics access level. One of `disabled`, `private` or `enabled`'
+        optional :container_registry_access_level, type: String, values: %w(disabled private enabled), desc: 'Controls visibility of the container registry. One of `disabled`, `private` or `enabled`. `private` will make the container registry accessible only to project members (reporter role and above). `enabled` will make the container registry accessible to everyone who has access to the project. `disabled` will disable the container registry'
 
         optional :emails_disabled, type: Boolean, desc: 'Disable email notifications'
         optional :show_default_award_emojis, type: Boolean, desc: 'Show default award emojis'
         optional :shared_runners_enabled, type: Boolean, desc: 'Flag indication if shared runners are enabled for that project'
         optional :resolve_outdated_diff_discussions, type: Boolean, desc: 'Automatically resolve merge request diffs discussions on lines changed with a push'
         optional :remove_source_branch_after_merge, type: Boolean, desc: 'Remove the source branch by default after merge'
-        optional :container_registry_enabled, type: Boolean, desc: 'Flag indication if the container registry is enabled for that project'
+        optional :container_registry_enabled, type: Boolean, desc: 'Deprecated: Use :container_registry_access_level instead. Flag indication if the container registry is enabled for that project'
         optional :container_expiration_policy_attributes, type: Hash do
           use :optional_container_expiration_policy_params
         end
@@ -124,7 +125,7 @@ module API
           :ci_config_path,
           :ci_default_git_depth,
           :ci_forward_deployment_enabled,
-          :container_registry_enabled,
+          :container_registry_access_level,
           :container_expiration_policy_attributes,
           :default_branch,
           :description,
@@ -169,7 +170,8 @@ module API
           :jobs_enabled,
           :merge_requests_enabled,
           :wiki_enabled,
-          :snippets_enabled
+          :snippets_enabled,
+          :container_registry_enabled
         ]
       end
 

spec/requests/api/project_attributes.yml

@@ -121,7 +121,6 @@ project_feature:
     - project_id
     - requirements_access_level
     - security_and_compliance_access_level
-    - container_registry_access_level
     - updated_at
   computed_attributes:
     - issues_enabled

spec/requests/api/projects_spec.rb

@@ -207,6 +207,18 @@ RSpec.describe API::Projects do
         let(:current_user) { user }
       end
 
+      it 'includes container_registry_access_level', :aggregate_failures do
+        project.project_feature.update!(container_registry_access_level: ProjectFeature::DISABLED)
+
+        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_access_level']).to eq('disabled')
+        expect(project_response['container_registry_enabled']).to eq(false)
+      end
+
       context 'when some projects are in a group' do
         before do
           create(:project, :public, group: create(:group))
@@ -1042,7 +1054,7 @@ RSpec.describe API::Projects do
       expect(response).to have_gitlab_http_status(:bad_request)
     end
 
-    it "assigns attributes to project" do
+    it "assigns attributes to project", :aggregate_failures do
       project = attributes_for(:project, {
         path: 'camelCasePath',
         issues_enabled: false,
@@ -1064,6 +1076,7 @@ RSpec.describe API::Projects do
       }).tap do |attrs|
         attrs[:operations_access_level] = 'disabled'
         attrs[:analytics_access_level] = 'disabled'
+        attrs[:container_registry_access_level] = 'private'
       end
 
       post api('/projects', user), params: project
@@ -1071,7 +1084,10 @@ RSpec.describe API::Projects do
       expect(response).to have_gitlab_http_status(:created)
 
       project.each_pair do |k, v|
-        next if %i[has_external_issue_tracker has_external_wiki issues_enabled merge_requests_enabled wiki_enabled storage_version].include?(k)
+        next if %i[
+          has_external_issue_tracker has_external_wiki issues_enabled merge_requests_enabled wiki_enabled storage_version
+          container_registry_access_level
+        ].include?(k)
 
         expect(json_response[k.to_s]).to eq(v)
       end
@@ -1083,6 +1099,18 @@ RSpec.describe API::Projects do
       expect(project.project_feature.wiki_access_level).to eq(ProjectFeature::DISABLED)
       expect(project.operations_access_level).to eq(ProjectFeature::DISABLED)
       expect(project.project_feature.analytics_access_level).to eq(ProjectFeature::DISABLED)
+      expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::PRIVATE)
+    end
+
+    it 'assigns container_registry_enabled to project', :aggregate_failures do
+      project = attributes_for(:project, { container_registry_enabled: true })
+
+      post api('/projects', user), params: project
+
+      expect(response).to have_gitlab_http_status(:created)
+      expect(json_response['container_registry_enabled']).to eq(true)
+      expect(json_response['container_registry_access_level']).to eq('enabled')
+      expect(Project.find_by(path: project[:path]).container_registry_access_level).to eq(ProjectFeature::ENABLED)
     end
 
     it 'creates a project using a template' do
@@ -1340,6 +1368,14 @@ RSpec.describe API::Projects do
       expect(json_response.map { |project| project['id'] }).to contain_exactly(public_project.id)
     end
 
+    it 'includes container_registry_access_level', :aggregate_failures do
+      get api("/users/#{user4.id}/projects/", user)
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response).to be_an Array
+      expect(json_response.first.keys).to include('container_registry_access_level')
+    end
+
     context 'and using id_after' do
       let!(:another_public_project) { create(:project, :public, name: 'another_public_project', creator_id: user4.id, namespace: user4.namespace) }
 
@@ -1649,6 +1685,59 @@ RSpec.describe API::Projects do
 
       expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy
     end
+
+    context 'container_registry_enabled' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:container_registry_enabled, :container_registry_access_level) do
+        true  | ProjectFeature::ENABLED
+        false | ProjectFeature::DISABLED
+      end
+
+      with_them do
+        it 'setting container_registry_enabled also sets container_registry_access_level', :aggregate_failures do
+          project_attributes = attributes_for(:project).tap do |attrs|
+            attrs[:container_registry_enabled] = container_registry_enabled
+          end
+
+          post api("/projects/user/#{user.id}", admin), params: project_attributes
+
+          project = Project.find_by(path: project_attributes[:path])
+          expect(response).to have_gitlab_http_status(:created)
+          expect(json_response['container_registry_access_level']).to eq(ProjectFeature.str_from_access_level(container_registry_access_level))
+          expect(json_response['container_registry_enabled']).to eq(container_registry_enabled)
+          expect(project.container_registry_access_level).to eq(container_registry_access_level)
+          expect(project.container_registry_enabled).to eq(container_registry_enabled)
+        end
+      end
+    end
+
+    context 'container_registry_access_level' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:container_registry_access_level, :container_registry_enabled) do
+        'enabled'  | true
+        'private'  | true
+        'disabled' | false
+      end
+
+      with_them do
+        it 'setting container_registry_access_level also sets container_registry_enabled', :aggregate_failures do
+          project_attributes = attributes_for(:project).tap do |attrs|
+            attrs[:container_registry_access_level] = container_registry_access_level
+          end
+
+          post api("/projects/user/#{user.id}", admin), params: project_attributes
+
+          project = Project.find_by(path: project_attributes[:path])
+          expect(response).to have_gitlab_http_status(:created)
+          expect(json_response['container_registry_access_level']).to eq(container_registry_access_level)
+          expect(json_response['container_registry_enabled']).to eq(container_registry_enabled)
+          expect(project.container_registry_access_level).to eq(ProjectFeature.access_level_from_str(container_registry_access_level))
+          expect(project.container_registry_enabled).to eq(container_registry_enabled)
+        end
+      end
+    end
   end
 
   describe "POST /projects/:id/uploads/authorize" do
@@ -2034,6 +2123,7 @@ RSpec.describe API::Projects do
         expect(json_response['jobs_enabled']).to be_present
         expect(json_response['snippets_enabled']).to be_present
         expect(json_response['container_registry_enabled']).to be_present
+        expect(json_response['container_registry_access_level']).to be_present
         expect(json_response['created_at']).to be_present
         expect(json_response['last_activity_at']).to be_present
         expect(json_response['shared_runners_enabled']).to be_present
@@ -2125,6 +2215,7 @@ RSpec.describe API::Projects do
         expect(json_response['resolve_outdated_diff_discussions']).to eq(project.resolve_outdated_diff_discussions)
         expect(json_response['remove_source_branch_after_merge']).to be_truthy
         expect(json_response['container_registry_enabled']).to be_present
+        expect(json_response['container_registry_access_level']).to be_present
         expect(json_response['created_at']).to be_present
         expect(json_response['last_activity_at']).to be_present
         expect(json_response['shared_runners_enabled']).to be_present
@@ -2951,6 +3042,14 @@ RSpec.describe API::Projects do
       end
     end
 
+    it 'sets container_registry_access_level', :aggregate_failures do
+      put api("/projects/#{project.id}", user), params: { container_registry_access_level: 'private' }
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response['container_registry_access_level']).to eq('private')
+      expect(Project.find_by(path: project[:path]).container_registry_access_level).to eq(ProjectFeature::PRIVATE)
+    end
+
     it 'returns 400 when nothing sent' do
       project_param = {}