Skip to content

Commit bb6c2fe

Browse files
CPM handle 404 response gracefully with user-friendly log (#17052) (#17098)
Starting from es-output 12.0.2, a 404 response is treated as an error. Previously, central pipeline management considered 404 as an empty pipeline, not an error. This commit restores the expected behavior by handling 404 gracefully and logs a user-friendly message. It also removes the redundant cache of pipeline in CPM Fixes: #17035 (cherry picked from commit e896cd7) Co-authored-by: kaisecheng <[email protected]>
1 parent d934419 commit bb6c2fe

File tree

2 files changed

+28
-32
lines changed

2 files changed

+28
-32
lines changed

x-pack/lib/config_management/elasticsearch_source.rb

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,20 +59,19 @@ def get_pipeline_fetcher(es_version)
5959
def pipeline_configs
6060
logger.trace("Fetch remote config pipeline", :pipeline_ids => pipeline_ids)
6161

62-
begin
63-
license_check(true)
64-
rescue LogStash::LicenseChecker::LicenseError => e
65-
if @cached_pipelines.nil?
66-
raise e
67-
else
68-
return @cached_pipelines
69-
end
70-
end
62+
license_check(true)
7163
es_version = get_es_version
7264
fetcher = get_pipeline_fetcher(es_version)
73-
fetcher.fetch_config(es_version, pipeline_ids, client)
7465

75-
@cached_pipelines = fetcher.get_pipeline_ids.collect do |pid|
66+
begin
67+
fetcher.fetch_config(es_version, pipeline_ids, client)
68+
rescue LogStash::Outputs::ElasticSearch::HttpClient::Pool::BadResponseCodeError => e
69+
# es-output 12.0.2 throws 404 as error, but we want to handle it as empty config
70+
return [] if e.response_code == 404
71+
raise e
72+
end
73+
74+
fetcher.get_pipeline_ids.collect do |pid|
7675
get_pipeline(pid, fetcher)
7776
end.compact
7877
end

x-pack/spec/config_management/elasticsearch_source_spec.rb

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -103,23 +103,7 @@
103103
"status" => 400}
104104
}
105105

106-
let(:elasticsearch_8_err_response) {
107-
{"error" =>
108-
{"root_cause" =>
109-
[{"type" => "index_not_found_exception",
110-
"reason" => "no such index [.logstash]",
111-
"resource.type" => "index_expression",
112-
"resource.id" => ".logstash",
113-
"index_uuid" => "_na_",
114-
"index" => ".logstash"}],
115-
"type" => "index_not_found_exception",
116-
"reason" => "no such index [.logstash]",
117-
"resource.type" => "index_expression",
118-
"resource.id" => ".logstash",
119-
"index_uuid" => "_na_",
120-
"index" => ".logstash"},
121-
"status" => 404}
122-
}
106+
let(:elasticsearch_8_err_response) { {"error" => "Incorrect HTTP method for uri", "status" => 405} }
123107

124108
before do
125109
extension.additionals_settings(system_settings)
@@ -466,13 +450,13 @@
466450
}]
467451
}.to_json
468452
end
469-
let(:es_path) { ".logstash/_mget" }
453+
let(:legacy_api) { ".logstash/_mget" }
470454
let(:request_body_string) { LogStash::Json.dump({ "docs" => [{ "_id" => pipeline_id }] }) }
471455

472456
before do
473457
allow(mock_client).to receive(:get).with(system_indices_url_regex).and_return(LogStash::Json.load(elasticsearch_response))
474458
allow(mock_client).to receive(:get).with("/").and_return(es_version_response)
475-
allow(mock_client).to receive(:post).with(es_path, {}, request_body_string).and_return(LogStash::Json.load(elasticsearch_7_9_response))
459+
allow(mock_client).to receive(:post).with(legacy_api, {}, request_body_string).and_return(LogStash::Json.load(elasticsearch_7_9_response))
476460
allow(mock_license_client).to receive(:get).with('_xpack').and_return(valid_xpack_response)
477461
allow(mock_license_client).to receive(:get).with('/').and_return(cluster_info(LOGSTASH_VERSION))
478462

@@ -493,7 +477,7 @@
493477
before :each do
494478
expect_any_instance_of(described_class).to receive(:build_client).and_return(mock_client)
495479
allow_any_instance_of(described_class).to receive(:logger).and_return(logger_stub)
496-
allow(mock_client).to receive(:post).with(es_path, {}, request_body_string).and_return(LogStash::Json.load(elasticsearch_7_9_response))
480+
allow(mock_client).to receive(:post).with(legacy_api, {}, request_body_string).and_return(LogStash::Json.load(elasticsearch_7_9_response))
497481
end
498482

499483
let(:config) { "input { generator {} } filter { mutate {} } output { }" }
@@ -734,6 +718,7 @@
734718

735719
describe "when exception occur" do
736720
let(:elasticsearch_response) { "" }
721+
let(:bad_response_404) { LogStash::Outputs::ElasticSearch::HttpClient::Pool::BadResponseCodeError.new(404, "url", "request_body", "response_body") }
737722

738723
before do
739724
expect_any_instance_of(described_class).to receive(:build_client).and_return(mock_client)
@@ -747,9 +732,21 @@
747732

748733
it "raises the exception upstream in [7.9]" do
749734
allow(mock_client).to receive(:get).with("/").and_return(es_version_7_9_response)
750-
expect(mock_client).to receive(:post).with(es_path, {}, request_body_string).and_raise("Something bad")
735+
expect(mock_client).to receive(:post).with(legacy_api, {}, request_body_string).and_raise("Something bad")
751736
expect { subject.pipeline_configs }.to raise_error /Something bad/
752737
end
738+
739+
it "returns empty pipeline when ES client raise BadResponseCodeError in [8]" do
740+
allow(mock_client).to receive(:get).with("/").and_return(es_version_response)
741+
expect(mock_client).to receive(:get).with(system_indices_url_regex).and_raise(bad_response_404)
742+
expect(subject.pipeline_configs).to be_empty
743+
end
744+
745+
it "returns empty pipeline when ES client raise BadResponseCodeError in [7.9]" do
746+
allow(mock_client).to receive(:get).with("/").and_return(es_version_7_9_response)
747+
expect(mock_client).to receive(:post).with(legacy_api, {}, request_body_string).and_raise(bad_response_404)
748+
expect(subject.pipeline_configs).to be_empty
749+
end
753750
end
754751
end
755752

0 commit comments

Comments
 (0)