diff options
| -rw-r--r-- | app/models/application_setting_implementation.rb | 11 | ||||
| -rw-r--r-- | app/services/application_settings/update_service.rb | 9 | ||||
| -rw-r--r-- | app/services/self_monitoring/project/create_service.rb | 32 | ||||
| -rw-r--r-- | locale/gitlab.pot | 6 | ||||
| -rw-r--r-- | spec/services/application_settings/update_service_spec.rb | 48 | ||||
| -rw-r--r-- | spec/services/self_monitoring/project/create_service_spec.rb | 24 | ||||
| -rw-r--r-- | spec/support/shared_examples/application_setting_examples.rb | 62 |
7 files changed, 172 insertions, 20 deletions
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 6efd07a6008..4bb09bf3b53 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -158,9 +158,20 @@ module ApplicationSettingImplementation end def outbound_local_requests_whitelist_raw=(values) + clear_memoization(:outbound_local_requests_whitelist_arrays) + self.outbound_local_requests_whitelist = domain_strings_to_array(values) end + def add_to_outbound_local_requests_whitelist(values_array) + clear_memoization(:outbound_local_requests_whitelist_arrays) + + self.outbound_local_requests_whitelist ||= [] + self.outbound_local_requests_whitelist += values_array + + self.outbound_local_requests_whitelist.uniq! + end + def outbound_local_requests_whitelist_arrays strong_memoize(:outbound_local_requests_whitelist_arrays) do next [[], []] unless self.outbound_local_requests_whitelist diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb index 7eeaf8aade1..471df6e2d0c 100644 --- a/app/services/application_settings/update_service.rb +++ b/app/services/application_settings/update_service.rb @@ -15,6 +15,8 @@ module ApplicationSettings update_terms(@params.delete(:terms)) + add_to_outbound_local_requests_whitelist(@params.delete(:add_to_outbound_local_requests_whitelist)) + if params.key?(:performance_bar_allowed_group_path) params[:performance_bar_allowed_group_id] = performance_bar_allowed_group_id end @@ -32,6 +34,13 @@ module ApplicationSettings params.key?(:usage_ping_enabled) || params.key?(:version_check_enabled) end + def add_to_outbound_local_requests_whitelist(values) + values_array = Array(values).reject(&:empty?) + return if values_array.empty? + + @application_setting.add_to_outbound_local_requests_whitelist(values_array) + end + def update_terms(terms) return unless terms.present? diff --git a/app/services/self_monitoring/project/create_service.rb b/app/services/self_monitoring/project/create_service.rb index e5ef8c15456..8ffd22de127 100644 --- a/app/services/self_monitoring/project/create_service.rb +++ b/app/services/self_monitoring/project/create_service.rb @@ -14,6 +14,7 @@ module SelfMonitoring steps :validate_admins, :create_project, :add_project_members, + :add_to_whitelist, :add_prometheus_manual_configuration def initialize @@ -59,15 +60,29 @@ module SelfMonitoring end end - def add_prometheus_manual_configuration + def add_to_whitelist return success unless prometheus_enabled? return success unless prometheus_listen_address.present? - # TODO: Currently, adding the internal prometheus server as a manual configuration - # is only possible if the setting to allow webhooks and services to connect - # to local network is on. - # https://gitlab.com/gitlab-org/gitlab-ce/issues/44496 will add - # a whitelist that will allow connections to certain ips on the local network. + uri = parse_url(internal_prometheus_listen_address_uri) + return error(_('Prometheus listen_address is not a valid URI')) unless uri + + result = ApplicationSettings::UpdateService.new( + Gitlab::CurrentSettings.current_application_settings, + project_owner, + outbound_local_requests_whitelist: [uri.normalized_host] + ).execute + + if result + success + else + error(_('Could not add prometheus URL to whitelist')) + end + end + + def add_prometheus_manual_configuration + return success unless prometheus_enabled? + return success unless prometheus_listen_address.present? service = project.find_or_initialize_service('prometheus') @@ -79,6 +94,11 @@ module SelfMonitoring success end + def parse_url(uri_string) + Addressable::URI.parse(uri_string) + rescue Addressable::URI::InvalidURIError, TypeError + end + def prometheus_enabled? Gitlab.config.prometheus.enable rescue Settingslogic::MissingSetting diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 709226e686c..f64a9046ecb 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3282,6 +3282,9 @@ msgstr "" msgid "Copy token to clipboard" msgstr "" +msgid "Could not add prometheus URL to whitelist" +msgstr "" + msgid "Could not authorize chat nickname. Try again!" msgstr "" @@ -8732,6 +8735,9 @@ 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 is not a valid URI" +msgstr "" + msgid "PrometheusService|%{exporters} with %{metrics} were found" msgstr "" diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index 33cd1f37ff6..adb5219d691 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -62,6 +62,54 @@ describe ApplicationSettings::UpdateService do end end + describe 'updating outbound_local_requests_whitelist' do + context 'when params is blank' do + let(:params) { {} } + + it 'does not add to whitelist' do + expect { subject.execute }.not_to change { + application_settings.outbound_local_requests_whitelist + } + end + end + + context 'when param add_to_outbound_local_requests_whitelist contains values' do + before do + application_settings.outbound_local_requests_whitelist = ['127.0.0.1'] + end + + let(:params) { { add_to_outbound_local_requests_whitelist: ['example.com', ''] } } + + it 'adds to whitelist' do + expect { subject.execute }.to change { + application_settings.outbound_local_requests_whitelist + } + + expect(application_settings.outbound_local_requests_whitelist).to contain_exactly( + '127.0.0.1', 'example.com' + ) + end + end + + context 'when param outbound_local_requests_whitelist_raw is passed' do + before do + application_settings.outbound_local_requests_whitelist = ['127.0.0.1'] + end + + let(:params) { { outbound_local_requests_whitelist_raw: 'example.com;gitlab.com' } } + + it 'overwrites the existing whitelist' do + expect { subject.execute }.to change { + application_settings.outbound_local_requests_whitelist + } + + expect(application_settings.outbound_local_requests_whitelist).to contain_exactly( + 'example.com', 'gitlab.com' + ) + end + end + end + describe 'performance bar settings' do using RSpec::Parameterized::TableSyntax diff --git a/spec/services/self_monitoring/project/create_service_spec.rb b/spec/services/self_monitoring/project/create_service_spec.rb index d11e27c6d52..a1e7aaf45f2 100644 --- a/spec/services/self_monitoring/project/create_service_spec.rb +++ b/spec/services/self_monitoring/project/create_service_spec.rb @@ -90,19 +90,17 @@ describe SelfMonitoring::Project::CreateService do ) end - # This should pass when https://gitlab.com/gitlab-org/gitlab-ce/issues/44496 - # is complete and the prometheus listen address is added to the whitelist. - # context 'when local requests from hooks and services are not allowed' do - # before do - # allow(ApplicationSetting) - # .to receive(:current) - # .and_return( - # ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: false) - # ) - # end - - # it_behaves_like 'has prometheus service', 'http://localhost:9090' - # end + context 'when local requests from hooks and services are not allowed' do + before do + allow(ApplicationSetting) + .to receive(:current) + .and_return( + ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: false) + ) + end + + it_behaves_like 'has prometheus service', 'http://localhost:9090' + end context 'with non default prometheus address' do before do diff --git a/spec/support/shared_examples/application_setting_examples.rb b/spec/support/shared_examples/application_setting_examples.rb index 0e6a8059376..a43d2a75082 100644 --- a/spec/support/shared_examples/application_setting_examples.rb +++ b/spec/support/shared_examples/application_setting_examples.rb @@ -63,6 +63,19 @@ RSpec.shared_examples 'application settings examples' do context 'outbound_local_requests_whitelist' do it_behaves_like 'string of domains', :outbound_local_requests_whitelist + + it 'clears outbound_local_requests_whitelist_arrays memoization' do + setting.outbound_local_requests_whitelist_raw = 'example.com' + + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], ['example.com'] + ) + + setting.outbound_local_requests_whitelist_raw = 'gitlab.com' + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], ['gitlab.com'] + ) + end end context 'outbound_local_requests_whitelist_arrays' do @@ -78,7 +91,54 @@ RSpec.shared_examples 'application settings examples' do ] domain_whitelist = ['www.example.com', 'example.com', 'subdomain.example.com'] - expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(ip_whitelist, domain_whitelist) + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + ip_whitelist, domain_whitelist + ) + end + end + + context 'add_to_outbound_local_requests_whitelist' do + it 'adds entry to outbound_local_requests_whitelist' do + setting.outbound_local_requests_whitelist = ['example.com'] + + setting.add_to_outbound_local_requests_whitelist( + ['example.com', '127.0.0.1', 'gitlab.com'] + ) + + expect(setting.outbound_local_requests_whitelist).to contain_exactly( + 'example.com', + '127.0.0.1', + 'gitlab.com' + ) + end + + it 'clears outbound_local_requests_whitelist_arrays memoization' do + setting.outbound_local_requests_whitelist = ['example.com'] + + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], + ['example.com'] + ) + + setting.add_to_outbound_local_requests_whitelist( + ['example.com', 'gitlab.com'] + ) + + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], + ['example.com', 'gitlab.com'] + ) + end + + it 'does not raise error with nil' do + setting.outbound_local_requests_whitelist = nil + + setting.add_to_outbound_local_requests_whitelist(['gitlab.com']) + + expect(setting.outbound_local_requests_whitelist).to contain_exactly('gitlab.com') + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], ['gitlab.com'] + ) end it 'does not raise error with nil' do |