Bring back healthcheck token access to monitoring resources, but mark this as deprecated

app/controllers/concerns/requires_whitelisted_monitoring_client.rb

@@ -7,11 +7,20 @@ module RequiresWhitelistedMonitoringClient
   private
 
   def validate_ip_whitelisted!
-    render_404 unless client_ip_whitelisted?
+    render_404 unless client_ip_whitelisted? || token_valid?
   end
 
   def client_ip_whitelisted?
-    Settings.monitoring.ip_whitelist.any? {|e| e.include?(Gitlab::RequestContext.client_ip) }
+    Settings.monitoring.ip_whitelist.any? { |e| e.include?(Gitlab::RequestContext.client_ip) }
+  end
+  
+  def token_valid?
+    token = params[:token].presence || request.headers['TOKEN']
+    token.present? &&
+      ActiveSupport::SecurityUtils.variable_size_secure_compare(
+        token,
+        current_application_settings.health_check_access_token
+      )
   end
 
   def render_404

config/gitlab.yml.example

@@ -552,7 +552,8 @@ production: &base
   # Built in monitoring settings
   monitoring:
     # IP whitelist to access monitoring endpoints
-    access_whitelist: 127.0.0.0/8
+    ip_whitelist:
+      - 127.0.0.0/8
 
   #
   # 5. Extra customization

doc/user/admin_area/monitoring/health_check.md

@@ -5,6 +5,7 @@
   - 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)
     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
 reachability to required services. These probes report on the status of the
@@ -12,7 +13,19 @@ database connection, Redis connection, and access to the filesystem. These
 endpoints [can be provided to schedulers like Kubernetes][kubernetes] to hold
 traffic until the system is ready or restart the container as needed.
 
-## Access Token
+## IP Whitelist
+
+To access monitoring resources client IP needs to be included in the whitelist.
+To add or remove hosts or ip ranges from the list you can edit `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
 accepted token can be found under the **Admin area ➔ Monitoring ➔ Health check**
@@ -47,10 +60,10 @@ which will then provide a report of system health in JSON format:
 
 ## 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
 
@@ -71,7 +84,7 @@ 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
 [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), 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`

spec/controllers/health_check_controller_spec.rb

@@ -5,6 +5,7 @@ describe HealthCheckController do
 
   let(:json_response) { JSON.parse(response.body) }
   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' }
 
@@ -23,6 +24,21 @@ describe HealthCheckController do
         get :index
         expect(response).to be_not_found
       end
+
+      context 'when services are accessed with token' do
+        it 'supports passing the token in the header' do
+          request.headers['TOKEN'] = token
+          get :index
+          expect(response).to be_success
+          expect(response.content_type).to eq 'text/plain'
+        end
+
+        it 'supports successful plaintest response' do
+          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