diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb
index 3fedd5bfb29b6d1671c3961aa7808079a2c15b99..72f59693adc0545533cd8de9a6ac3fe37471b035 100644
--- a/app/controllers/health_controller.rb
+++ b/app/controllers/health_controller.rb
@@ -18,9 +18,8 @@ def readiness
end
def liveness
- results = CHECKS.map { |check| [check.name, check.liveness] }
-
- render_check_results(results)
+ # This should never be called; it should be intercepted by LivenessHealthCheck middleware
+ head :not_found
end
def storage_check
diff --git a/changelogs/unreleased/sh-simplify-liveness-check.yml b/changelogs/unreleased/sh-simplify-liveness-check.yml
new file mode 100644
index 0000000000000000000000000000000000000000..7eb108a1300b92a980285b58ee3d57b93d4e5367
--- /dev/null
+++ b/changelogs/unreleased/sh-simplify-liveness-check.yml
@@ -0,0 +1,5 @@
+---
+title: Simplify /-/liveness check to avoid connecting to the database
+merge_request:
+author:
+type: changed
diff --git a/config/application.rb b/config/application.rb
index b9d4f6765e3e0af1d163b404f834583ee35e15a0..ae4ff5dd7c97ce4bd2152345c24708cdbd473afb 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -154,6 +154,10 @@ class Application < Rails::Application
config.action_view.sanitized_allowed_protocols = %w(smb)
+ # This middleware needs to precede ActiveRecord::QueryCache and other middlewares that
+ # connect to the database.
+ config.middleware.insert_after "Rails::Rack::Logger", "Gitlab::Middleware::LivenessHealthCheck"
+
config.middleware.insert_after Warden::Manager, Rack::Attack
# Allow access to GitLab API from other domains
diff --git a/config/routes.rb b/config/routes.rb
index e0a9139b1b4515987f44d5690021b93caf98dacd..6bf0335a9239b65230d59ec92a824d406a29492e 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -46,7 +46,7 @@
get 'health_check(/:checks)' => 'health_check#index', as: :health_check
scope path: '-' do
- get 'liveness' => 'health#liveness'
+ get 'liveness' => 'health#liveness' # Intercepted via Gitlab::Middleware::LivenessHealthCheck
get 'readiness' => 'health#readiness'
post 'storage_check' => 'health#storage_check'
resources :metrics, only: [:index]
diff --git a/doc/user/admin_area/monitoring/health_check.md b/doc/user/admin_area/monitoring/health_check.md
index 843fb4ce26b7ab9726376d80abf9e39ce8cde36d..958962fbfbc2596cb638888c6ecc0e78c1a3468e 100644
--- a/doc/user/admin_area/monitoring/health_check.md
+++ b/doc/user/admin_area/monitoring/health_check.md
@@ -27,7 +27,7 @@ With default whitelist settings, the probes can be accessed from localhost:
- `http://localhost/-/readiness`
- `http://localhost/-/liveness`
-which will then provide a report of system health in JSON format.
+The readiness endpoint will provide a report of system health in JSON format.
Readiness example output:
@@ -57,29 +57,12 @@ Readiness example output:
}
```
-Liveness example output:
+The liveness endpoint only checks whether the application server is running. It does
+not verify the database or other services are running. A successful response with return
+a 200 status code with the following message:
```
-{
- "fs_shards_check" : {
- "status" : "ok"
- },
- "cache_check" : {
- "status" : "ok"
- },
- "db_check" : {
- "status" : "ok"
- },
- "redis_check" : {
- "status" : "ok"
- },
- "queues_check" : {
- "status" : "ok"
- },
- "shared_state_check" : {
- "status" : "ok"
- }
-}
+GitLab is alive
```
## Status
diff --git a/lib/gitlab/middleware/liveness_health_check.rb b/lib/gitlab/middleware/liveness_health_check.rb
new file mode 100644
index 0000000000000000000000000000000000000000..404c506a08e45522fc18becfc0d108de22e29992
--- /dev/null
+++ b/lib/gitlab/middleware/liveness_health_check.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+# This middleware provides a health check that does not hit the database. Its purpose
+# is to notify the prober that the application server is handling requests, but a 200
+# response does not signify that the database or other services are ready.
+#
+# See https://thisdata.com/blog/making-a-rails-health-check-that-doesnt-hit-the-database/ for
+# more details.
+
+module Gitlab
+ module Middleware
+ class LivenessHealthCheck
+ # This can't be frozen because Rails::Rack::Logger wraps the body
+ # rubocop:disable Style/MutableConstant
+ OK_RESPONSE = [200, { 'Content-Type' => 'text/plain' }, ["GitLab is alive"]]
+ EMPTY_RESPONSE = [404, { 'Content-Type' => 'text/plain' }, [""]]
+ # rubocop:enable Style/MutableConstant
+ LIVENESS_PATH = '/-/liveness'
+
+ def initialize(app)
+ @app = app
+ end
+
+ def call(env)
+ return @app.call(env) unless env['PATH_INFO'] == LIVENESS_PATH
+
+ request = Rack::Request.new(env)
+
+ return OK_RESPONSE if client_ip_whitelisted?(request)
+
+ EMPTY_RESPONSE
+ end
+
+ def client_ip_whitelisted?(request)
+ ip_whitelist.any? { |e| e.include?(request.ip) }
+ end
+
+ def ip_whitelist
+ @ip_whitelist ||= Settings.monitoring.ip_whitelist.map(&IPAddr.method(:new))
+ end
+ end
+ end
+end
diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb
index d800ad7c1876cae71045fc3e5cd5518595dd83c3..789b35dc3deae3a1a9f20c0c16f8b13aee34f329 100644
--- a/spec/controllers/health_controller_spec.rb
+++ b/spec/controllers/health_controller_spec.rb
@@ -109,55 +109,4 @@
end
end
end
-
- describe '#liveness' do
- shared_context 'endpoint responding with liveness data' do
- subject { get :liveness }
-
- it 'responds with liveness checks data' do
- subject
-
- expect(json_response['db_check']['status']).to eq('ok')
- expect(json_response['cache_check']['status']).to eq('ok')
- expect(json_response['queues_check']['status']).to eq('ok')
- expect(json_response['shared_state_check']['status']).to eq('ok')
- end
- end
-
- context 'accessed from whitelisted ip' do
- before do
- allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
- end
-
- it_behaves_like 'endpoint responding with liveness data'
- end
-
- context 'accessed from not whitelisted ip' do
- before do
- allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
- end
-
- it 'responds with resource not found' do
- get :liveness
-
- expect(response.status).to eq(404)
- end
-
- context 'accessed with valid token' do
- context 'token passed in request header' do
- before do
- request.headers['TOKEN'] = token
- end
-
- it_behaves_like 'endpoint responding with liveness data'
- end
-
- context 'token passed as URL param' do
- it_behaves_like 'endpoint responding with liveness data' do
- subject { get :liveness, token: token }
- end
- end
- end
- end
- end
end
diff --git a/spec/lib/gitlab/middleware/liveness_health_check_spec.rb b/spec/lib/gitlab/middleware/liveness_health_check_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..3dee13b77705f022d7cca96bf0f4962429f0788e
--- /dev/null
+++ b/spec/lib/gitlab/middleware/liveness_health_check_spec.rb
@@ -0,0 +1,57 @@
+require 'spec_helper'
+
+describe Gitlab::Middleware::LivenessHealthCheck do
+ let(:app) { double(:app) }
+ let(:middleware) { described_class.new(app) }
+ let(:env) { {} }
+
+ describe '#call' do
+ context 'outside IP' do
+ before do
+ env['REMOTE_ADDR'] = '8.8.8.8'
+ end
+
+ it 'returns a 404' do
+ env['PATH_INFO'] = described_class::LIVENESS_PATH
+
+ response = middleware.call(env)
+
+ expect(response[0]).to eq(404)
+ end
+
+ it 'forwards the call for other paths' do
+ env['PATH_INFO'] = '/'
+
+ expect(app).to receive(:call)
+
+ middleware.call(env)
+ end
+ end
+
+ context 'whitelisted IP' do
+ before do
+ env['REMOTE_ADDR'] = '127.0.0.1'
+ end
+
+ it 'returns 200 response when endpoint is hit' do
+ env['PATH_INFO'] = described_class::LIVENESS_PATH
+
+ expect(app).not_to receive(:call)
+
+ response = middleware.call(env)
+
+ expect(response[0]).to eq(200)
+ expect(response[1]).to eq({ 'Content-Type' => 'text/plain' })
+ expect(response[2]).to eq(['GitLab is alive'])
+ end
+
+ it 'forwards the call for other paths' do
+ env['PATH_INFO'] = '/-/readiness'
+
+ expect(app).to receive(:call)
+
+ middleware.call(env)
+ end
+ end
+ end
+end