Commit 25d241ae authored by Sean McGivern's avatar Sean McGivern

Merge branch '33949-remove-healthcheck-access-token' into 'master'

Remove the need to use health check token by adding ability to whitelist hosts

Closes #33949

See merge request !12612
parents b39c9837 063f03b9
module RequiresHealthToken module RequiresWhitelistedMonitoringClient
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
before_action :validate_health_check_access! before_action :validate_ip_whitelisted_or_valid_token!
end end
private private
def validate_health_check_access! def validate_ip_whitelisted_or_valid_token!
render_404 unless token_valid? render_404 unless client_ip_whitelisted? || valid_token?
end end
def token_valid? def client_ip_whitelisted?
ip_whitelist.any? { |e| e.include?(Gitlab::RequestContext.client_ip) }
end
def ip_whitelist
@ip_whitelist ||= Settings.monitoring.ip_whitelist.map(&IPAddr.method(:new))
end
def valid_token?
token = params[:token].presence || request.headers['TOKEN'] token = params[:token].presence || request.headers['TOKEN']
token.present? && token.present? &&
ActiveSupport::SecurityUtils.variable_size_secure_compare( ActiveSupport::SecurityUtils.variable_size_secure_compare(
......
class HealthCheckController < HealthCheck::HealthCheckController class HealthCheckController < HealthCheck::HealthCheckController
include RequiresHealthToken include RequiresWhitelistedMonitoringClient
end end
class HealthController < ActionController::Base class HealthController < ActionController::Base
protect_from_forgery with: :exception protect_from_forgery with: :exception
include RequiresHealthToken include RequiresWhitelistedMonitoringClient
CHECKS = [ CHECKS = [
Gitlab::HealthChecks::DbCheck, Gitlab::HealthChecks::DbCheck,
......
class MetricsController < ActionController::Base class MetricsController < ActionController::Base
include RequiresHealthToken include RequiresWhitelistedMonitoringClient
protect_from_forgery with: :exception protect_from_forgery with: :exception
before_action :validate_prometheus_metrics before_action :validate_prometheus_metrics
def index def index
render text: metrics_service.metrics_text, content_type: 'text/plain; verssion=0.0.4' render text: metrics_service.metrics_text, content_type: 'text/plain; version=0.0.4'
end end
private private
......
---
title: Deprecate Healthcheck Access Token in favor of IP whitelist
merge_request:
author:
...@@ -539,10 +539,15 @@ production: &base ...@@ -539,10 +539,15 @@ production: &base
# enabled: true # enabled: true
# host: localhost # host: localhost
# port: 3808 # port: 3808
prometheus:
## Monitoring
# Built in monitoring settings
monitoring:
# Time between sampling of unicorn socket metrics, in seconds # Time between sampling of unicorn socket metrics, in seconds
# unicorn_sampler_interval: 10 # unicorn_sampler_interval: 10
# IP whitelist to access monitoring endpoints
ip_whitelist:
- 127.0.0.0/8
# #
# 5. Extra customization # 5. Extra customization
......
...@@ -494,10 +494,11 @@ Settings.webpack.dev_server['host'] ||= 'localhost' ...@@ -494,10 +494,11 @@ Settings.webpack.dev_server['host'] ||= 'localhost'
Settings.webpack.dev_server['port'] ||= 3808 Settings.webpack.dev_server['port'] ||= 3808
# #
# Prometheus metrics settings # Monitoring settings
# #
Settings['prometheus'] ||= Settingslogic.new({}) Settings['monitoring'] ||= Settingslogic.new({})
Settings.prometheus['unicorn_sampler_interval'] ||= 10 Settings.monitoring['ip_whitelist'] ||= ['127.0.0.1/8']
Settings.monitoring['unicorn_sampler_interval'] ||= 10
# #
# Testing settings # Testing settings
......
...@@ -119,7 +119,7 @@ def instrument_classes(instrumentation) ...@@ -119,7 +119,7 @@ def instrument_classes(instrumentation)
end end
# rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/AbcSize
Gitlab::Metrics::UnicornSampler.initialize_instance(Settings.prometheus.unicorn_sampler_interval).start Gitlab::Metrics::UnicornSampler.initialize_instance(Settings.monitoring.unicorn_sampler_interval).start
Gitlab::Application.configure do |config| Gitlab::Application.configure do |config|
# 0 should be Sentry to catch errors in this middleware # 0 should be Sentry to catch errors in this middleware
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
- The `health_check` endpoint was [introduced][ce-3888] in GitLab 8.8 and will - The `health_check` endpoint was [introduced][ce-3888] in GitLab 8.8 and will
be deprecated in GitLab 9.1. Read more in the [old behavior](#old-behavior) be deprecated in GitLab 9.1. Read more in the [old behavior](#old-behavior)
section. section.
- [Access token](#access-token) has been deprecated in GitLab 9.4
in favor of [IP Whitelist](#ip-whitelist)
GitLab provides liveness and readiness probes to indicate service health and GitLab provides liveness and readiness probes to indicate service health and
reachability to required services. These probes report on the status of the reachability to required services. These probes report on the status of the
...@@ -12,7 +14,19 @@ database connection, Redis connection, and access to the filesystem. These ...@@ -12,7 +14,19 @@ database connection, Redis connection, and access to the filesystem. These
endpoints [can be provided to schedulers like Kubernetes][kubernetes] to hold endpoints [can be provided to schedulers like Kubernetes][kubernetes] to hold
traffic until the system is ready or restart the container as needed. traffic until the system is ready or restart the container as needed.
## Access Token ## IP Whitelist
To access monitoring resources the client IP needs to be included in the whitelist.
To add or remove hosts or IP ranges from the list you can edit `gitlab.rb` or `gitlab.yml`.
Example whitelist configuration:
```yaml
monitoring:
ip_whitelist:
- 127.0.0.0/8 # by default only local IPs are allowed to access monitoring resources
```
## Access Token (Deprecated)
An access token needs to be provided while accessing the probe endpoints. The current An access token needs to be provided while accessing the probe endpoints. The current
accepted token can be found under the **Admin area ➔ Monitoring ➔ Health check** accepted token can be found under the **Admin area ➔ Monitoring ➔ Health check**
...@@ -47,10 +61,10 @@ which will then provide a report of system health in JSON format: ...@@ -47,10 +61,10 @@ which will then provide a report of system health in JSON format:
## Using the Endpoint ## Using the Endpoint
Once you have the access token, the probes can be accessed: With default whitelist settings, the probes can be accessed from localhost:
- `https://gitlab.example.com/-/readiness?token=ACCESS_TOKEN` - `http://localhost/-/readiness`
- `https://gitlab.example.com/-/liveness?token=ACCESS_TOKEN` - `http://localhost/-/liveness`
## Status ## Status
...@@ -71,8 +85,8 @@ the database connection, the state of the database migrations, and the ability t ...@@ -71,8 +85,8 @@ the database connection, the state of the database migrations, and the ability t
and access the cache. This endpoint can be provided to uptime monitoring services like and access the cache. This endpoint can be provided to uptime monitoring services like
[Pingdom][pingdom], [Nagios][nagios-health], and [NewRelic][newrelic-health]. [Pingdom][pingdom], [Nagios][nagios-health], and [NewRelic][newrelic-health].
Once you have the [access token](#access-token), health information can be Once you have the [access token](#access-token) or your client IP is [whitelisted](#ip-whitelist),
retrieved as plain text, JSON, or XML using the `health_check` endpoint: health information can be retrieved as plain text, JSON, or XML using the `health_check` endpoint:
- `https://gitlab.example.com/health_check?token=ACCESS_TOKEN` - `https://gitlab.example.com/health_check?token=ACCESS_TOKEN`
- `https://gitlab.example.com/health_check.json?token=ACCESS_TOKEN` - `https://gitlab.example.com/health_check.json?token=ACCESS_TOKEN`
......
...@@ -3,52 +3,79 @@ require 'spec_helper' ...@@ -3,52 +3,79 @@ require 'spec_helper'
describe HealthCheckController do describe HealthCheckController do
include StubENV include StubENV
let(:token) { current_application_settings.health_check_access_token }
let(:json_response) { JSON.parse(response.body) } let(:json_response) { JSON.parse(response.body) }
let(:xml_response) { Hash.from_xml(response.body)['hash'] } let(:xml_response) { Hash.from_xml(response.body)['hash'] }
let(:token) { current_application_settings.health_check_access_token }
let(:whitelisted_ip) { '127.0.0.1' }
let(:not_whitelisted_ip) { '127.0.0.2' }
before do before do
allow(Settings.monitoring).to receive(:ip_whitelist).and_return([whitelisted_ip])
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
end end
describe 'GET #index' do describe 'GET #index' do
context 'when services are up but NO access token' do context 'when services are up but accessed from outside whitelisted ips' do
before do
allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
end
it 'returns a not found page' do it 'returns a not found page' do
get :index get :index
expect(response).to be_not_found expect(response).to be_not_found
end end
end
context 'when services are up and an access token is provided' do context 'when services are accessed with token' do
it 'supports passing the token in the header' do it 'supports passing the token in the header' do
request.headers['TOKEN'] = token request.headers['TOKEN'] = token
get :index get :index
expect(response).to be_success expect(response).to be_success
expect(response.content_type).to eq 'text/plain' expect(response.content_type).to eq 'text/plain'
end end
it 'supports successful plaintest response' do it 'supports passing the token in query params' do
get :index, token: token get :index, token: token
expect(response).to be_success
expect(response.content_type).to eq 'text/plain'
end
end
end
context 'when services are up and accessed from whitelisted ips' do
before do
allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
end
it 'supports successful plaintext response' do
get :index
expect(response).to be_success expect(response).to be_success
expect(response.content_type).to eq 'text/plain' expect(response.content_type).to eq 'text/plain'
end end
it 'supports successful json response' do it 'supports successful json response' do
get :index, token: token, format: :json get :index, format: :json
expect(response).to be_success expect(response).to be_success
expect(response.content_type).to eq 'application/json' expect(response.content_type).to eq 'application/json'
expect(json_response['healthy']).to be true expect(json_response['healthy']).to be true
end end
it 'supports successful xml response' do it 'supports successful xml response' do
get :index, token: token, format: :xml get :index, format: :xml
expect(response).to be_success expect(response).to be_success
expect(response.content_type).to eq 'application/xml' expect(response.content_type).to eq 'application/xml'
expect(xml_response['healthy']).to be true expect(xml_response['healthy']).to be true
end end
it 'supports successful responses for specific checks' do it 'supports successful responses for specific checks' do
get :index, token: token, checks: 'email', format: :json get :index, checks: 'email', format: :json
expect(response).to be_success expect(response).to be_success
expect(response.content_type).to eq 'application/json' expect(response.content_type).to eq 'application/json'
expect(json_response['healthy']).to be true expect(json_response['healthy']).to be true
...@@ -58,33 +85,29 @@ describe HealthCheckController do ...@@ -58,33 +85,29 @@ describe HealthCheckController do
context 'when a service is down but NO access token' do context 'when a service is down but NO access token' do
it 'returns a not found page' do it 'returns a not found page' do
get :index get :index
expect(response).to be_not_found expect(response).to be_not_found
end end
end end
context 'when a service is down and an access token is provided' do context 'when a service is down and an endpoint is accessed from whitelisted ip' do
before do before do
allow(HealthCheck::Utils).to receive(:process_checks).with(['standard']).and_return('The server is on fire') allow(HealthCheck::Utils).to receive(:process_checks).with(['standard']).and_return('The server is on fire')
allow(HealthCheck::Utils).to receive(:process_checks).with(['email']).and_return('Email is on fire') allow(HealthCheck::Utils).to receive(:process_checks).with(['email']).and_return('Email is on fire')
allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
end end
it 'supports passing the token in the header' do it 'supports failure plaintext response' do
request.headers['TOKEN'] = token
get :index get :index
expect(response).to have_http_status(500)
expect(response.content_type).to eq 'text/plain'
expect(response.body).to include('The server is on fire')
end
it 'supports failure plaintest response' do
get :index, token: token
expect(response).to have_http_status(500) expect(response).to have_http_status(500)
expect(response.content_type).to eq 'text/plain' expect(response.content_type).to eq 'text/plain'
expect(response.body).to include('The server is on fire') expect(response.body).to include('The server is on fire')
end end
it 'supports failure json response' do it 'supports failure json response' do
get :index, token: token, format: :json get :index, format: :json
expect(response).to have_http_status(500) expect(response).to have_http_status(500)
expect(response.content_type).to eq 'application/json' expect(response.content_type).to eq 'application/json'
expect(json_response['healthy']).to be false expect(json_response['healthy']).to be false
...@@ -92,7 +115,8 @@ describe HealthCheckController do ...@@ -92,7 +115,8 @@ describe HealthCheckController do
end end
it 'supports failure xml response' do it 'supports failure xml response' do
get :index, token: token, format: :xml get :index, format: :xml
expect(response).to have_http_status(500) expect(response).to have_http_status(500)
expect(response.content_type).to eq 'application/xml' expect(response.content_type).to eq 'application/xml'
expect(xml_response['healthy']).to be false expect(xml_response['healthy']).to be false
...@@ -100,7 +124,8 @@ describe HealthCheckController do ...@@ -100,7 +124,8 @@ describe HealthCheckController do
end end
it 'supports failure responses for specific checks' do it 'supports failure responses for specific checks' do
get :index, token: token, checks: 'email', format: :json get :index, checks: 'email', format: :json
expect(response).to have_http_status(500) expect(response).to have_http_status(500)
expect(response.content_type).to eq 'application/json' expect(response.content_type).to eq 'application/json'
expect(json_response['healthy']).to be false expect(json_response['healthy']).to be false
......
...@@ -3,21 +3,25 @@ require 'spec_helper' ...@@ -3,21 +3,25 @@ require 'spec_helper'
describe HealthController do describe HealthController do
include StubENV include StubENV
let(:token) { current_application_settings.health_check_access_token }
let(:json_response) { JSON.parse(response.body) } let(:json_response) { JSON.parse(response.body) }
let(:token) { current_application_settings.health_check_access_token }
let(:whitelisted_ip) { '127.0.0.1' }
let(:not_whitelisted_ip) { '127.0.0.2' }
before do before do
allow(Settings.monitoring).to receive(:ip_whitelist).and_return([whitelisted_ip])
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
end end
describe '#readiness' do describe '#readiness' do
context 'authorization token provided' do shared_context 'endpoint responding with readiness data' do
before do let(:request_params) { {} }
request.headers['TOKEN'] = token
end subject { get :readiness, request_params }
it 'responds with readiness checks data' do
subject
it 'returns proper response' do
get :readiness
expect(json_response['db_check']['status']).to eq('ok') expect(json_response['db_check']['status']).to eq('ok')
expect(json_response['cache_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['queues_check']['status']).to eq('ok')
...@@ -27,22 +31,50 @@ describe HealthController do ...@@ -27,22 +31,50 @@ describe HealthController do
end end
end end
context 'without authorization token' do context 'accessed from whitelisted ip' do
it 'returns proper response' do before do
get :readiness allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
expect(response.status).to eq(404) end
it_behaves_like 'endpoint responding with readiness data'
end end
context 'accessed from not whitelisted ip' do
before do
allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
end end
it 'responds with resource not found' do
get :readiness
expect(response.status).to eq(404)
end end
describe '#liveness' do context 'accessed with valid token' do
context 'authorization token provided' do context 'token passed in request header' do
before do before do
request.headers['TOKEN'] = token request.headers['TOKEN'] = token
end end
it 'returns proper response' do it_behaves_like 'endpoint responding with readiness data'
get :liveness end
end
context 'token passed as URL param' do
it_behaves_like 'endpoint responding with readiness data' do
let(:request_params) { { token: token } }
end
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['db_check']['status']).to eq('ok')
expect(json_response['cache_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['queues_check']['status']).to eq('ok')
...@@ -51,11 +83,40 @@ describe HealthController do ...@@ -51,11 +83,40 @@ describe HealthController do
end end
end end
context 'without authorization token' do context 'accessed from whitelisted ip' do
it 'returns proper response' 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 get :liveness
expect(response.status).to eq(404) expect(response.status).to eq(404)
end 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 end
end end
...@@ -3,22 +3,22 @@ require 'spec_helper' ...@@ -3,22 +3,22 @@ require 'spec_helper'
describe MetricsController do describe MetricsController do
include StubENV include StubENV
let(:token) { current_application_settings.health_check_access_token }
let(:json_response) { JSON.parse(response.body) } let(:json_response) { JSON.parse(response.body) }
let(:metrics_multiproc_dir) { Dir.mktmpdir } let(:metrics_multiproc_dir) { Dir.mktmpdir }
let(:whitelisted_ip) { '127.0.0.1' }
let(:whitelisted_ip_range) { '10.0.0.0/24' }
let(:ip_in_whitelisted_range) { '10.0.0.1' }
let(:not_whitelisted_ip) { '10.0.1.1' }
before do before do
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
stub_env('prometheus_multiproc_dir', metrics_multiproc_dir) stub_env('prometheus_multiproc_dir', metrics_multiproc_dir)
allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(true) allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(true)
allow(Settings.monitoring).to receive(:ip_whitelist).and_return([whitelisted_ip, whitelisted_ip_range])
end end
describe '#index' do describe '#index' do
context 'authorization token provided' do shared_examples_for 'endpoint providing metrics' do
before do
request.headers['TOKEN'] = token
end
it 'returns DB ping metrics' do it 'returns DB ping metrics' do
get :index get :index
...@@ -83,7 +83,27 @@ describe MetricsController do ...@@ -83,7 +83,27 @@ describe MetricsController do
end end
end end
context 'without authorization token' do context 'accessed from whitelisted ip' do
before do
allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
end
it_behaves_like 'endpoint providing metrics'
end
context 'accessed from ip in whitelisted range' do
before do
allow(Gitlab::RequestContext).to receive(:client_ip).and_return(ip_in_whitelisted_range)
end
it_behaves_like 'endpoint providing metrics'
end
context 'accessed from not whitelisted ip' do
before do
allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
end
it 'returns proper response' do it 'returns proper response' do
get :index get :index
......
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