Merge branch '32353-project-connect-prometheus' into 'master'

Connect self-monitoring project to internal Prometheus

See merge request gitlab-org/gitlab!19369

app/models/project_services/prometheus_service.rb

@@ -7,8 +7,15 @@ class PrometheusService < MonitoringService
   prop_accessor :api_url
   boolean_accessor :manual_configuration
 
+  # We need to allow the self-monitoring project to connect to the internal
+  # Prometheus instance.
+  # Since the internal Prometheus instance is usually a localhost URL, we need
+  # to allow localhost URLs when the following conditions are true:
+  # 1. project is the self-monitoring project.
+  # 2. api_url is the internal Prometheus URL.
   with_options presence: true, if: :manual_configuration? do
-    validates :api_url, public_url: true
+    validates :api_url, public_url: true, unless: proc { |object| object.allow_local_api_url? }
+    validates :api_url, url: true, if: proc { |object| object.allow_local_api_url? }
   end
 
   before_save :synchronize_service_state
@@ -82,12 +89,28 @@ class PrometheusService < MonitoringService
     project.clusters.enabled.any? { |cluster| cluster.application_prometheus_available? }
   end
 
+  def allow_local_api_url?
+    self_monitoring_project? && internal_prometheus_url?
+  end
+
   private
 
+  def self_monitoring_project?
+    project && project.id == current_settings.instance_administration_project_id
+  end
+
+  def internal_prometheus_url?
+    api_url.present? && api_url == ::Gitlab::Prometheus::Internal.uri
+  end
+
   def should_return_client?
     api_url.present? && manual_configuration? && active? && valid?
   end
 
+  def current_settings
+    Gitlab::CurrentSettings.current_application_settings
+  end
+
   def synchronize_service_state
     self.active = prometheus_available? || manual_configuration?
 

lib/gitlab/database_importers/self_monitoring/project/create_service.rb

@@ -21,7 +21,6 @@ module Gitlab
             :create_project,
             :save_project_id,
             :add_group_members,
-            :add_to_whitelist,
             :add_prometheus_manual_configuration
 
           def initialize
@@ -126,28 +125,6 @@ module Gitlab
             end
           end
 
-          def add_to_whitelist(result)
-            return success(result) unless prometheus_enabled?
-            return success(result) unless prometheus_listen_address.present?
-
-            uri = parse_url(internal_prometheus_listen_address_uri)
-            return error(_('Prometheus listen_address in config/gitlab.yml is not a valid URI')) unless uri
-
-            application_settings.add_to_outbound_local_requests_whitelist([uri.normalized_host])
-            response = application_settings.save
-
-            if response
-              # Expire the Gitlab::CurrentSettings cache after updating the whitelist.
-              # This happens automatically in an after_commit hook, but in migrations,
-              # the after_commit hook only runs at the end of the migration.
-              Gitlab::CurrentSettings.expire_current_application_settings
-              success(result)
-            else
-              log_error("Could not add prometheus URL to whitelist, errors: %{errors}" % { errors: application_settings.errors.full_messages })
-              error(_('Could not add prometheus URL to whitelist'))
-            end
-          end
-
           def add_prometheus_manual_configuration(result)
             return success(result) unless prometheus_enabled?
             return success(result) unless prometheus_listen_address.present?

locale/gitlab.pot

@@ -4632,9 +4632,6 @@ msgstr ""
 msgid "Could not add admins as members"
 msgstr ""
 
-msgid "Could not add prometheus URL to whitelist"
-msgstr ""
-
 msgid "Could not authorize chat nickname. Try again!"
 msgstr ""
 
@@ -13292,9 +13289,6 @@ msgstr ""
 msgid "ProjectsNew|Want to house several dependent projects under the same namespace? %{link_start}Create a group.%{link_end}"
 msgstr ""
 
-msgid "Prometheus listen_address in config/gitlab.yml is not a valid URI"
-msgstr ""
-
 msgid "PrometheusAlerts|%{count} alerts applied"
 msgstr ""
 

spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb

@@ -164,15 +164,6 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do
         end
 
         it_behaves_like 'has prometheus service', 'http://localhost:9090'
-
-        it 'does not overwrite the existing whitelist' do
-          application_setting.outbound_local_requests_whitelist = ['example.com']
-
-          expect(result[:status]).to eq(:success)
-          expect(application_setting.outbound_local_requests_whitelist).to contain_exactly(
-            'example.com', 'localhost'
-          )
-        end
       end
 
       context 'with non default prometheus address' do

spec/models/project_services/prometheus_service_spec.rb

@@ -65,6 +65,37 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
           end
         end
       end
+
+      context 'with self-monitoring project and internal Prometheus' do
+        before do
+          service.api_url = 'http://localhost:9090'
+
+          stub_application_setting(instance_administration_project_id: project.id)
+          stub_config(prometheus: { enable: true, listen_address: 'localhost:9090' })
+        end
+
+        it 'allows self-monitoring project to connect to internal Prometheus' do
+          aggregate_failures do
+            ['127.0.0.1', '192.168.2.3'].each do |url|
+              allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([Addrinfo.tcp(url, 80)])
+
+              expect(service.can_query?).to be true
+            end
+          end
+        end
+
+        it 'does not allow self-monitoring project to connect to other local URLs' do
+          service.api_url = 'http://localhost:8000'
+
+          aggregate_failures do
+            ['127.0.0.1', '192.168.2.3'].each do |url|
+              allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([Addrinfo.tcp(url, 80)])
+
+              expect(service.can_query?).to be false
+            end
+          end
+        end
+      end
     end
   end