Commit 25cbb003 authored by Dmitry Gruzd's avatar Dmitry Gruzd Committed by Nick Thomas

Prohibit automatic elasticsearch indexes creation

This change disables "Index all project" button if ES
cluster doesn't have index created and shows a warning on
the ES integration page
parent 5446ec9d
---
title: 'Elasticsearch: when index is absent warn users and disable index button'
merge_request: 25254
author:
type: fixed
...@@ -4,7 +4,7 @@ class Admin::ElasticsearchController < Admin::ApplicationController ...@@ -4,7 +4,7 @@ class Admin::ElasticsearchController < Admin::ApplicationController
before_action :check_elasticsearch_web_indexing_feature_flag! before_action :check_elasticsearch_web_indexing_feature_flag!
def check_elasticsearch_web_indexing_feature_flag! def check_elasticsearch_web_indexing_feature_flag!
render_404 unless Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) render_404 unless Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && Gitlab::Elastic::Helper.index_exists?
end end
# POST # POST
......
...@@ -22,14 +22,19 @@ ...@@ -22,14 +22,19 @@
.sub-section .sub-section
.form-group .form-group
.form-check .form-check
= f.check_box :elasticsearch_indexing, class: 'form-check-input', data: { qa_selector: 'indexing_checkbox' } - indexing_enabled = Gitlab::CurrentSettings.elasticsearch_indexing?
- index_exists = Gitlab::Elastic::Helper.index_exists? rescue false
- disable_indexing_checkbox = !indexing_enabled && !index_exists # Disable this checkbox only when we haven't enabled indexing and we don't have an index
= f.check_box :elasticsearch_indexing, class: 'form-check-input', data: { qa_selector: 'indexing_checkbox' }, disabled: disable_indexing_checkbox
= f.label :elasticsearch_indexing, class: 'form-check-label' do = f.label :elasticsearch_indexing, class: 'form-check-label' do
Elasticsearch indexing Elasticsearch indexing
- unless index_exists
- if Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && Gitlab::CurrentSettings.elasticsearch_indexing? .form-text.text-warning
= _('Please create an index before enabling indexing')
- if Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && indexing_enabled
.form-text .form-text
= link_to _('Index all projects'), admin_elasticsearch_enqueue_index_path, = link_to _('Index all projects'), admin_elasticsearch_enqueue_index_path,
class: 'btn btn-success', method: :post class: ['btn', 'btn-success', ('disabled' unless index_exists)], method: :post
.form-group .form-group
.form-check .form-check
......
...@@ -56,6 +56,14 @@ module Gitlab ...@@ -56,6 +56,14 @@ module Gitlab
Project.__elasticsearch__.version(version).delete_index! Project.__elasticsearch__.version(version).delete_index!
end end
def self.index_exists?(version = ::Elastic::MultiVersionUtil::TARGET_VERSION)
proxy = Project.__elasticsearch__.version(version)
client = proxy.client
index_name = proxy.index_name
client.indices.exists? index: index_name # rubocop:disable CodeReuse/ActiveRecord
end
# Calls Elasticsearch refresh API to ensure data is searchable # Calls Elasticsearch refresh API to ensure data is searchable
# immediately. # immediately.
# https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-refresh.html # https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-refresh.html
......
...@@ -11,6 +11,7 @@ describe Admin::ElasticsearchController do ...@@ -11,6 +11,7 @@ describe Admin::ElasticsearchController do
end end
it 'starts indexing' do it 'starts indexing' do
expect(Gitlab::Elastic::Helper).to(receive(:index_exists?)).and_return(true)
expect_next_instance_of(::Elastic::IndexProjectsService) do |service| expect_next_instance_of(::Elastic::IndexProjectsService) do |service|
expect(service).to receive(:execute) expect(service).to receive(:execute)
end end
...@@ -21,6 +22,20 @@ describe Admin::ElasticsearchController do ...@@ -21,6 +22,20 @@ describe Admin::ElasticsearchController do
expect(response).to redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings') expect(response).to redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
end end
context 'without an index' do
before do
allow(Gitlab::Elastic::Helper).to(receive(:index_exists?)).and_return(false)
end
it 'does nothing and returns 404' do
expect(::Elastic::IndexProjectsService).not_to receive(:new)
post :enqueue_index
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when feature is disabled' do context 'when feature is disabled' do
it 'does nothing and returns 404' do it 'does nothing and returns 404' do
stub_feature_flags(elasticsearch_web_indexing: false) stub_feature_flags(elasticsearch_web_indexing: false)
......
...@@ -9,6 +9,7 @@ describe 'Admin updates EE-only settings' do ...@@ -9,6 +9,7 @@ describe 'Admin updates EE-only settings' do
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
sign_in(create(:admin)) sign_in(create(:admin))
allow(License).to receive(:feature_available?).and_return(true) allow(License).to receive(:feature_available?).and_return(true)
allow(Gitlab::Elastic::Helper).to receive(:index_exists?).and_return(true)
end end
context 'Geo settings' do context 'Geo settings' do
......
# frozen_string_literal: true
require 'fast_spec_helper'
describe Gitlab::Elastic::Helper do
describe '.index_exists' do
it 'returns correct values' do
described_class.create_empty_index
expect(described_class.index_exists?).to eq(true)
described_class.delete_index
expect(described_class.index_exists?).to eq(false)
end
end
end
...@@ -12,6 +12,57 @@ describe 'admin/application_settings/_elasticsearch_form' do ...@@ -12,6 +12,57 @@ describe 'admin/application_settings/_elasticsearch_form' do
allow(view).to receive(:expanded) { true } allow(view).to receive(:expanded) { true }
end end
context 'es index dependent' do
before do
allow(Gitlab::CurrentSettings).to(receive(:elasticsearch_indexing?)).and_return(es_indexing)
allow(Gitlab::Elastic::Helper).to(receive(:index_exists?)).and_return(index_exists)
end
let(:warning_msg) { 'create an index before enabling indexing' }
let(:button_text) { 'Index all projects' }
let(:application_setting) { build(:application_setting, elasticsearch_indexing: es_indexing) }
context 'when elasticsearch index does not exist with indexing enabled' do
let(:es_indexing) { true }
let(:index_exists) { false }
it 'shows a warning and disables a button' do
render
expect(rendered).to have_content(warning_msg)
expect(rendered).to have_css('a.btn-success.disabled', text: button_text)
expect(rendered).to have_css('#application_setting_elasticsearch_indexing')
expect(rendered).not_to have_css('#application_setting_elasticsearch_indexing[disabled="disabled"]')
end
end
context 'when elasticsearch index does not exist with indexing disabled' do
let(:es_indexing) { false }
let(:index_exists) { false }
it 'shows a warning and disables a checkbox and hides an indexing button' do
render
expect(rendered).to have_content(warning_msg)
expect(rendered).not_to have_css('a.btn-success', text: button_text)
expect(rendered).to have_css('#application_setting_elasticsearch_indexing[disabled="disabled"]')
end
end
context 'when elasticsearch index exists' do
let(:es_indexing) { true }
let(:index_exists) { true }
it 'shows non-disabled index button without a warning' do
render
expect(rendered).to have_css('a.btn-success', text: button_text)
expect(rendered).not_to have_css('a.btn-success.disabled', text: button_text)
expect(rendered).not_to have_content(warning_msg)
end
end
end
context 'when elasticsearch_aws_secret_access_key is not set' do context 'when elasticsearch_aws_secret_access_key is not set' do
let(:application_setting) { build(:application_setting) } let(:application_setting) { build(:application_setting) }
......
...@@ -14047,6 +14047,9 @@ msgstr "" ...@@ -14047,6 +14047,9 @@ msgstr ""
msgid "Please create a username with only alphanumeric characters." msgid "Please create a username with only alphanumeric characters."
msgstr "" msgstr ""
msgid "Please create an index before enabling indexing"
msgstr ""
msgid "Please enable and migrate to hashed storage to avoid security issues and ensure data integrity. %{migrate_link}" msgid "Please enable and migrate to hashed storage to avoid security issues and ensure data integrity. %{migrate_link}"
msgstr "" msgstr ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment