Merge pull request #85 from Sharpie/PE-31705-reenable-remote-metric-collection

(PE-31705) Re-enable remote metric collection

Author: Sharpie

README.md

@@ -21,9 +21,8 @@ Table of Contents
 This module collects metrics provided by the status endpoints of Puppet Enterprise services.
 The metrics can be used to identify performance issues that may be addressed by performance tuning.
 
-> In PE 2018.1.13 and newer and PE 2019.4 and newer, the `/metrics/v1` endpoints are disabled by default and access to the `/metrics/v2` endpoints are restricted to localhost ... in response to CVE-2020-7943. 
-This module requires access those endpoints to collect additional metrics from PuppetDB, and those metrics will not be collected from remote PuppetDB hosts until these restricted are resolved.
-Refer to [Configuration for Distributed Metrics Collection](#Configuration-for-distributed-metrics-collection) for a workaround. 
+
+> For PE versions older than 2019.8.5, access to the `/metrics/v2` API endpoint is restricted to `localhost` as a mitigation for [CVE-2020-7943](https://puppet.com/security/cve/CVE-2020-7943/). This module requires access the `/metrics/v2` API to collect a complete set of performance metrics from PuppetDB. Refer to [Configuration for Distributed Metrics Collection](#Configuration-for-distributed-metrics-collection) for a workaround.
 
 
 ## Setup

files/pe_metrics.rb

@@ -42,6 +42,7 @@ def coalesce(higher_precedence, lower_precedence, default = nil)
 USE_SSL            = coalesce(options[:ssl], config['ssl'], true)
 EXCLUDES           = config['excludes']
 ADDITIONAL_METRICS = config['additional_metrics']
+REMOTE_METRICS_ENABLED = config['remote_metrics_enabled']
 
 # Metrics endpoints for our Puma services require a client certificate with SSL.
 # Metrics endpoints for our Trapper Keeper services do not require a client certificate.
@@ -114,12 +115,15 @@ def post_endpoint(url, use_ssl, post_data)
   endpoint_data = {}
 end
 
-# PE-28451 Disables Metrics API v1 (/metrics/v1/beans/) and restricts v2 (/metrics/v2/read/) to localhost by default.
-
 def retrieve_additional_metrics(host, port, use_ssl, metrics_type, metrics)
   if metrics_type == 'puppetdb'
     host = '127.0.0.1' if host == CERTNAME
-    return [] unless ['127.0.0.1', 'localhost'].include?(host)
+    unless REMOTE_METRICS_ENABLED || ['127.0.0.1', 'localhost'].include?(host)
+      # Puppet services released between May, 2020 and Feb 2021 had
+      # the /metrics API disabled due to:
+      #   https://puppet.com/security/cve/CVE-2020-7943/
+      return []
+    end
   end
 
   host_url = generate_host_url(host, port, use_ssl)

manifests/pe_metric.pp

@@ -10,6 +10,7 @@
   Boolean                   $ssl                      = true,
   Array[String]             $excludes                 = puppet_metrics_collector::version_based_excludes($title),
   Array[Hash]               $additional_metrics       = [],
+  Optional[Boolean]         $remote_metrics_enabled   = lookup('puppet_metrics_collector::pe_metric::remote_metrics_enabled', {'default_value' => undef}), # lint:ignore:140chars
   Optional[String]          $override_metrics_command = undef,
   Optional[Enum['influxdb','graphite','splunk_hec']] $metrics_server_type = undef,
   Optional[String]          $metrics_server_hostname  = undef,
@@ -30,15 +31,28 @@
     force  => true,
   }
 
+  $_remote_metrics_enabled = if $remote_metrics_enabled =~ Boolean {
+    $remote_metrics_enabled
+  } elsif fact('pe_server_version') =~ NotUndef {
+    if versioncmp(fact('pe_server_version'), '2019.8.5') >= 0 {
+      true
+    } else {
+      false
+    }
+  } else {
+    false
+  }
+
   $config_hash = {
-    'metrics_type'       => $metrics_type,
-    'pe_version'         => $facts['pe_server_version'],
-    'clientcert'         => $::clientcert,
-    'hosts'              => $hosts.sort(),
-    'metrics_port'       => $metrics_port,
-    'ssl'                => $ssl,
-    'excludes'           => $excludes,
-    'additional_metrics' => $additional_metrics,
+    'metrics_type'           => $metrics_type,
+    'pe_version'             => $facts['pe_server_version'],
+    'clientcert'             => $::clientcert,
+    'hosts'                  => $hosts.sort(),
+    'metrics_port'           => $metrics_port,
+    'ssl'                    => $ssl,
+    'excludes'               => $excludes,
+    'additional_metrics'     => $additional_metrics,
+    'remote_metrics_enabled' => $_remote_metrics_enabled,
   }
 
   file { "${puppet_metrics_collector::config_dir}/${metrics_type}.yaml" :

spec/defines/pe_metric_spec.rb

@@ -0,0 +1,41 @@
+require 'spec_helper'
+
+describe 'puppet_metrics_collector::pe_metric' do
+  let(:title) { 'test-service' }
+  let(:params) {
+    {metrics_port: 9000}
+  }
+  # This define has an undeclared dependency on the main
+  # puppet_metrics_collector class.
+  let(:pre_condition) { 'include puppet_metrics_collector' }
+
+  it 'compiles with minimal parameters set' do
+    expect(subject).to compile
+  end
+
+  describe 'remote metric collection' do
+    it 'is disabled by default due to CVE-2020-7943' do
+      expect(subject).to contain_file('/opt/puppetlabs/puppet-metrics-collector/config/test-service.yaml').with_content(/remote_metrics_enabled: false/)
+    end
+
+    context 'when the PE version is 2019.8.5 or newer' do
+      let(:facts) {
+        {pe_server_version: '2019.8.5'}
+      }
+
+      it 'is enabled by default' do
+        expect(subject).to contain_file('/opt/puppetlabs/puppet-metrics-collector/config/test-service.yaml').with_content(/remote_metrics_enabled: true/)
+      end
+    end
+
+    context 'when the PE version is 2019.8.4 or older' do
+      let(:facts) {
+        {pe_server_version: '2019.8.4'}
+      }
+
+      it 'is disabled by default' do
+        expect(subject).to contain_file('/opt/puppetlabs/puppet-metrics-collector/config/test-service.yaml').with_content(/remote_metrics_enabled: false/)
+      end
+    end
+  end
+end