Merge branch '335703-cleanup' into 'master'

Remove container registry migration phase 1 feature flags

See merge request gitlab-org/gitlab!81821

app/services/auth/container_registry_authentication_service.rb

@@ -59,12 +59,7 @@ module Auth
       token.expire_time = token_expire_at
 
       token[:access] = names.map do |name|
-        {
-          type: type,
-          name: name,
-          actions: actions,
-          migration_eligible: type == 'repository' ? migration_eligible(repository_path: name) : nil
-        }.compact
+        { type: type, name: name, actions: actions }
       end
 
       token.encoded
@@ -136,12 +131,7 @@ module Auth
       #
       ensure_container_repository!(path, authorized_actions)
 
-      {
-        type: type,
-        name: path.to_s,
-        actions: authorized_actions,
-        migration_eligible: self.class.migration_eligible(project: requested_project)
-      }.compact
+      { type: type, name: path.to_s, actions: authorized_actions }
     end
 
     def actively_importing?(actions, path)
@@ -153,28 +143,6 @@ module Auth
       container_repository.migration_importing?
     end
 
-    def self.migration_eligible(project: nil, repository_path: nil)
-      return unless Feature.enabled?(:container_registry_migration_phase1)
-
-      # project has precedence over repository_path. If only the latter is provided, we find the corresponding Project.
-      unless project
-        return unless repository_path
-
-        project = ContainerRegistry::Path.new(repository_path).repository_project
-      end
-
-      # The migration process will start by allowing only specific test and gitlab-org projects using the
-      # `container_registry_migration_phase1_allow` FF. We'll then move on to a percentage rollout using this same FF.
-      # To remove the risk of impacting enterprise customers that rely heavily on the registry during the percentage
-      # rollout, we'll add their top-level group/namespace to the `container_registry_migration_phase1_deny` FF. Later,
-      # we'll remove them manually from this deny list, and their new repositories will become eligible.
-      Feature.disabled?(:container_registry_migration_phase1_deny, project.root_ancestor) &&
-        Feature.enabled?(:container_registry_migration_phase1_allow, project)
-    rescue ContainerRegistry::Path::InvalidRegistryPathError => ex
-      Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ex, **Gitlab::ApplicationContext.current)
-      false
-    end
-
     ##
     # Because we do not have two way communication with registry yet,
     # we create a container repository image resource when push to the

config/feature_flags/development/container_registry_migration_phase1.yml

----
-name: container_registry_migration_phase1
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63907
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335703
-milestone: '14.1'
-type: development
-group: group::package
-default_enabled: false

config/feature_flags/development/container_registry_migration_phase1_allow.yml

----
-name: container_registry_migration_phase1_allow
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63907
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335705
-milestone: '14.1'
-type: development
-group: group::package
-default_enabled: false

config/feature_flags/development/container_registry_migration_phase1_deny.yml

----
-name: container_registry_migration_phase1_deny
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63907
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335706
-milestone: '14.1'
-type: development
-group: group::package
-default_enabled: false

spec/services/auth/container_registry_authentication_service_spec.rb

@@ -6,143 +6,4 @@ RSpec.describe Auth::ContainerRegistryAuthenticationService do
   include AdminModeHelper
 
   it_behaves_like 'a container registry auth service'
-
-  context 'when in migration mode' do
-    include_context 'container registry auth service context'
-
-    let_it_be(:current_user) { create(:user) }
-    let_it_be(:project) { create(:project) }
-
-    before do
-      project.add_developer(current_user)
-    end
-
-    shared_examples 'a modified token with migration eligibility' do |eligible|
-      it_behaves_like 'a valid token'
-      it { expect(payload['access']).to include(include('migration_eligible' => eligible)) }
-    end
-
-    shared_examples 'a modified token' do
-      context 'with a non eligible root ancestor and project' do
-        before do
-          stub_feature_flags(container_registry_migration_phase1_deny: project.root_ancestor)
-          stub_feature_flags(container_registry_migration_phase1_allow: false)
-        end
-
-        it_behaves_like 'a modified token with migration eligibility', false
-      end
-
-      context 'with a non eligible root ancestor and eligible project' do
-        before do
-          stub_feature_flags(container_registry_migration_phase1_deny: false)
-          stub_feature_flags(container_registry_migration_phase1_deny: project.root_ancestor)
-          stub_feature_flags(container_registry_migration_phase1_allow: project)
-        end
-
-        it_behaves_like 'a modified token with migration eligibility', false
-      end
-
-      context 'with an eligible root ancestor and non eligible project' do
-        before do
-          stub_feature_flags(container_registry_migration_phase1_deny: false)
-          stub_feature_flags(container_registry_migration_phase1_allow: false)
-        end
-
-        it_behaves_like 'a modified token with migration eligibility', false
-      end
-
-      context 'with an eligible root ancestor and project' do
-        before do
-          stub_feature_flags(container_registry_migration_phase1_deny: false)
-          stub_feature_flags(container_registry_migration_phase1_allow: project)
-        end
-
-        it_behaves_like 'a modified token with migration eligibility', true
-      end
-    end
-
-    context 'with pull action' do
-      let(:current_params) do
-        { scopes: ["repository:#{project.full_path}:pull"] }
-      end
-
-      it_behaves_like 'a modified token'
-    end
-
-    context 'with push action' do
-      let(:current_params) do
-        { scopes: ["repository:#{project.full_path}:push"] }
-      end
-
-      it_behaves_like 'a modified token'
-    end
-
-    context 'with multiple actions' do
-      let(:current_params) do
-        { scopes: ["repository:#{project.full_path}:pull,push,delete"] }
-      end
-
-      it_behaves_like 'a modified token'
-    end
-
-    describe '#access_token' do
-      let(:token) { described_class.access_token(%w[push], [project.full_path]) }
-
-      subject { { token: token } }
-
-      it_behaves_like 'a modified token'
-    end
-
-    context 'with a project with a path with trailing underscore' do
-      let(:bad_project) { create(:project) }
-
-      before do
-        bad_project.update!(path: bad_project.path + '_')
-        bad_project.add_developer(current_user)
-      end
-
-      describe '#full_access_token' do
-        let(:token) { described_class.full_access_token(bad_project.full_path) }
-        let(:access) do
-          [{ 'type' => 'repository',
-             'name' => bad_project.full_path,
-             'actions' => ['*'],
-             'migration_eligible' => false }]
-        end
-
-        subject { { token: token } }
-
-        it 'logs an exception and returns a valid access token' do
-          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
-
-          expect(token).to be_present
-          expect(payload).to be_a(Hash)
-          expect(payload).to include('access' => access)
-        end
-      end
-    end
-  end
-
-  context 'when not in migration mode' do
-    include_context 'container registry auth service context'
-
-    let_it_be(:project) { create(:project) }
-
-    before do
-      stub_feature_flags(container_registry_migration_phase1: false)
-    end
-
-    shared_examples 'an unmodified token' do
-      it_behaves_like 'a valid token'
-      it { expect(payload['access']).not_to include(have_key('migration_eligible')) }
-    end
-
-    describe '#access_token' do
-      let(:token) { described_class.access_token(%w[push], [project.full_path]) }
-
-      subject { { token: token } }
-
-      it_behaves_like 'an unmodified token'
-    end
-  end
 end

spec/support/shared_examples/services/container_registry_auth_service_shared_examples.rb

@@ -69,10 +69,6 @@ RSpec.shared_examples 'a browsable' do
 end
 
 RSpec.shared_examples 'an accessible' do
-  before do
-    stub_feature_flags(container_registry_migration_phase1: false)
-  end
-
   let(:access) do
     [{ 'type' => 'repository',
        'name' => project.full_path,
@@ -161,10 +157,6 @@ end
 RSpec.shared_examples 'a container registry auth service' do
   include_context 'container registry auth service context'
 
-  before do
-    stub_feature_flags(container_registry_migration_phase1: false)
-  end
-
   describe '.full_access_token' do
     let_it_be(:project) { create(:project) }