Merge branch '204839-move-update-logic-to-service' into 'master'

Move container registry info update logic to a separate service

See merge request gitlab-org/gitlab!36065

app/services/update_container_registry_info_service.rb

+# frozen_string_literal: true
+
+class UpdateContainerRegistryInfoService
+  def execute
+    registry_config = Gitlab.config.registry
+    return unless registry_config.enabled && registry_config.api_url.presence
+
+    # registry_info will query the /v2 route of the registry API. This route
+    # requires authentication, but not authorization (the response has no body,
+    # only headers that show the version of the registry). There might be no
+    # associated user when running this (e.g. from a rake task or a cron job),
+    # so we need to generate a valid JWT token with no access permissions to
+    # authenticate as a trusted client.
+    token = Auth::ContainerRegistryAuthenticationService.access_token([], [])
+    client = ContainerRegistry::Client.new(registry_config.api_url, token: token)
+    info = client.registry_info
+
+    Gitlab::CurrentSettings.update!(
+      container_registry_vendor: info[:vendor] || '',
+      container_registry_version: info[:version] || '',
+      container_registry_features: info[:features] || []
+    )
+  end
+end

lib/tasks/gitlab/container_registry.rake

@@ -15,21 +15,7 @@ namespace :gitlab do
 
       warn_user_is_not_gitlab
 
-      url = registry_config.api_url
-      # registry_info will query the /v2 route of the registry API. This route
-      # requires authentication, but not authorization (the response has no body,
-      # only headers that show the version of the registry). There is no
-      # associated user when running this rake, so we need to generate a valid
-      # JWT token with no access permissions to authenticate as a trusted client.
-      token = Auth::ContainerRegistryAuthenticationService.access_token([], [])
-      client = ContainerRegistry::Client.new(url, token: token)
-      info = client.registry_info
-
-      Gitlab::CurrentSettings.update!(
-        container_registry_vendor: info[:vendor] || '',
-        container_registry_version: info[:version] || '',
-        container_registry_features: info[:features] || []
-      )
+      UpdateContainerRegistryInfoService.new.execute
     end
   end
 end

spec/services/update_container_registry_info_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe UpdateContainerRegistryInfoService do
+  let_it_be(:application_settings) { Gitlab::CurrentSettings }
+  let_it_be(:api_url) { 'http://registry.gitlab' }
+
+  describe '#execute' do
+    before do
+      stub_access_token
+      stub_container_registry_config(enabled: true, api_url: api_url)
+    end
+
+    subject { described_class.new.execute }
+
+    shared_examples 'invalid config' do
+      it 'does not update the application settings' do
+        expect(application_settings).not_to receive(:update!)
+
+        subject
+      end
+
+      it 'does not raise an error' do
+        expect { subject }.not_to raise_error
+      end
+    end
+
+    context 'when container registry is disabled' do
+      before do
+        stub_container_registry_config(enabled: false)
+      end
+
+      it_behaves_like 'invalid config'
+    end
+
+    context 'when container registry api_url is blank' do
+      before do
+        stub_container_registry_config(api_url: '')
+      end
+
+      it_behaves_like 'invalid config'
+    end
+
+    context 'when creating a registry client instance' do
+      let(:token) { 'foo' }
+      let(:client) { ContainerRegistry::Client.new(api_url, token: token) }
+
+      before do
+        stub_registry_info({})
+      end
+
+      it 'uses a token with no access permissions' do
+        expect(Auth::ContainerRegistryAuthenticationService)
+          .to receive(:access_token).with([], []).and_return(token)
+        expect(ContainerRegistry::Client)
+          .to receive(:new).with(api_url, token: token).and_return(client)
+
+        subject
+      end
+    end
+
+    context 'when unabled to detect the container registry type' do
+      it 'sets the application settings to their defaults' do
+        stub_registry_info({})
+
+        subject
+
+        application_settings.reload
+        expect(application_settings.container_registry_vendor).to be_blank
+        expect(application_settings.container_registry_version).to be_blank
+        expect(application_settings.container_registry_features).to eq([])
+      end
+    end
+
+    context 'when able to detect the container registry type' do
+      context 'when using the GitLab container registry' do
+        it 'updates application settings accordingly' do
+          stub_registry_info(vendor: 'gitlab', version: '2.9.1-gitlab', features: %w[a,b,c])
+
+          subject
+
+          application_settings.reload
+          expect(application_settings.container_registry_vendor).to eq('gitlab')
+          expect(application_settings.container_registry_version).to eq('2.9.1-gitlab')
+          expect(application_settings.container_registry_features).to eq(%w[a,b,c])
+        end
+      end
+
+      context 'when using a third-party container registry' do
+        it 'updates application settings accordingly' do
+          stub_registry_info(vendor: 'other', version: nil, features: nil)
+
+          subject
+
+          application_settings.reload
+          expect(application_settings.container_registry_vendor).to eq('other')
+          expect(application_settings.container_registry_version).to be_blank
+          expect(application_settings.container_registry_features).to eq([])
+        end
+      end
+    end
+  end
+
+  def stub_access_token
+    allow(Auth::ContainerRegistryAuthenticationService)
+      .to receive(:access_token).with([], []).and_return('foo')
+  end
+
+  def stub_registry_info(output)
+    allow_next_instance_of(ContainerRegistry::Client) do |client|
+      allow(client).to receive(:registry_info).and_return(output)
+    end
+  end
+end

spec/tasks/gitlab/container_registry_rake_spec.rb

@@ -3,24 +3,18 @@
 require 'rake_helper'
 
 RSpec.describe 'gitlab:container_registry namespace rake tasks' do
-  let_it_be(:application_settings) { Gitlab::CurrentSettings }
   let_it_be(:api_url) { 'http://registry.gitlab' }
 
   before :all do
     Rake.application.rake_require 'tasks/gitlab/container_registry'
   end
 
-  describe 'configure' do
-    before do
-      stub_access_token
-      stub_container_registry_config(enabled: true, api_url: api_url)
-    end
-
+  describe '#configure' do
     subject { run_rake_task('gitlab:container_registry:configure') }
 
     shared_examples 'invalid config' do
-      it 'does not update the application settings' do
-        expect(application_settings).not_to receive(:update!)
+      it 'does not call UpdateContainerRegistryInfoService' do
+        expect_any_instance_of(UpdateContainerRegistryInfoService).not_to receive(:execute)
 
         subject
       end
@@ -30,7 +24,7 @@ RSpec.describe 'gitlab:container_registry namespace rake tasks' do
       end
 
       it 'prints a warning message' do
-        expect { subject }.to output(/Registry is not enabled or registry api url is not present./).to_stdout
+        expect { subject }.to output("Registry is not enabled or registry api url is not present.\n").to_stdout
       end
     end
 
@@ -50,74 +44,18 @@ RSpec.describe 'gitlab:container_registry namespace rake tasks' do
       it_behaves_like 'invalid config'
     end
 
-    context 'when creating a registry client instance' do
-      let(:token) { 'foo' }
-      let(:client) { ContainerRegistry::Client.new(api_url, token: token) }
-
+    context 'when container registry is enabled and api_url is not blank' do
       before do
-        stub_registry_info({})
-      end
-
-      it 'uses a token with no access permissions' do
-        expect(Auth::ContainerRegistryAuthenticationService)
-          .to receive(:access_token).with([], []).and_return(token)
-        expect(ContainerRegistry::Client)
-          .to receive(:new).with(api_url, token: token).and_return(client)
-
-        run_rake_task('gitlab:container_registry:configure')
+        stub_container_registry_config(enabled: true, api_url: api_url)
       end
-    end
-
-    context 'when unabled to detect the container registry type' do
-      it 'fails and raises an error message' do
-        stub_registry_info({})
-
-        run_rake_task('gitlab:container_registry:configure')
-
-        application_settings.reload
-        expect(application_settings.container_registry_vendor).to be_blank
-        expect(application_settings.container_registry_version).to be_blank
-        expect(application_settings.container_registry_features).to eq([])
-      end
-    end
 
-    context 'when able to detect the container registry type' do
-      context 'when using the GitLab container registry' do
-        it 'updates application settings accordingly' do
-          stub_registry_info(vendor: 'gitlab', version: '2.9.1-gitlab', features: %w[a,b,c])
-
-          run_rake_task('gitlab:container_registry:configure')
-
-          application_settings.reload
-          expect(application_settings.container_registry_vendor).to eq('gitlab')
-          expect(application_settings.container_registry_version).to eq('2.9.1-gitlab')
-          expect(application_settings.container_registry_features).to eq(%w[a,b,c])
+      it 'calls UpdateContainerRegistryInfoService' do
+        expect_next_instance_of(UpdateContainerRegistryInfoService) do |service|
+          expect(service).to receive(:execute)
         end
-      end
 
-      context 'when using a third-party container registry' do
-        it 'updates application settings accordingly' do
-          stub_registry_info(vendor: 'other', version: nil, features: nil)
-
-          run_rake_task('gitlab:container_registry:configure')
-
-          application_settings.reload
-          expect(application_settings.container_registry_vendor).to eq('other')
-          expect(application_settings.container_registry_version).to be_blank
-          expect(application_settings.container_registry_features).to eq([])
-        end
+        subject
       end
     end
   end
-
-  def stub_access_token
-    allow(Auth::ContainerRegistryAuthenticationService)
-      .to receive(:access_token).with([], []).and_return('foo')
-  end
-
-  def stub_registry_info(output)
-    allow_next_instance_of(ContainerRegistry::Client) do |client|
-      allow(client).to receive(:registry_info).and_return(output)
-    end
-  end
 end