.rubocop.yml

@@ -464,11 +464,6 @@ Style/BlockComments:
   Enabled: false
 Style/BlockDelimiters:
   Enabled: false
-Style/BracesAroundHashParameters:
-  Description: |
-    Braces are actually required by Ruby 2.7 in order to disambiguate between
-    keword arguments and actual hash parameters. Removed in RuboCop v0.80.0.
-  Enabled: false
 Style/CaseEquality:
   Enabled: false
 Style/CharacterLiteral:

CHANGELOG.md

@@ -1,3 +1,12 @@
+# Minor Release 6.4.0
+
+## Improvements
+- Clean up temp files when metrics_tidy exits cleanly [#86](https://github.com/puppetlabs/puppetlabs-puppet_metrics_collector/pull/86)
+- Enable client ssl cert for metrics [#82](https://github.com/puppetlabs/puppetlabs-puppet_metrics_collector/pull/82)
+- Update to PDK 2.0, Updated Supported Puppet versions, and OS's [#83](https://github.com/puppetlabs/puppetlabs-puppet_metrics_collector/pull/83)
+## Changes:
+- Re-enable remote metric collection  [#85](https://github.com/puppetlabs/puppetlabs-puppet_metrics_collector/pull/85)
+
 # Minor Release 6.3.0
 
 ## Improvements

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/metrics_tidy

@@ -9,6 +9,10 @@ fail() {
   exit 1
 }
 
+cleanup() {
+  rm "$tmp"
+}
+
 # Clone, i.e. preserve, original stdout using fd 3.
 exec 3>&1
 # Send stderr and stdout to a temp file
@@ -18,6 +22,9 @@ exec &>"$tmp"
 # Run the fail() method on error
 trap fail ERR
 
+# Otherwise cleanup
+trap cleanup EXIT
+
 while [[ $1 ]]; do
   case "$1" in
     '-d'|'--directory')

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,20 +115,24 @@ 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)
 
+  metrics_array = []
   endpoint = "#{host_url}/metrics/v2/read"
   metrics_output = post_endpoint(endpoint, use_ssl, metrics.to_json)
+  return metrics_array if metrics_output.empty?
 
-  metrics_array = []
   metrics.each_index do |index|
     metric_name = metrics[index]['name']
     metric_data = metrics_output[index]

files/tk_metrics

@@ -1,7 +1,6 @@
 #!/opt/puppetlabs/puppet/bin/ruby
 
-# Metrics endpoints for our Trapper Keeper services do not require a client certificate.
-USE_CLIENTCERT = false
+USE_CLIENTCERT = true
 
 require_relative 'pe_metrics'
 

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" :

metadata.json

@@ -1,6 +1,6 @@
 {
   "name": "puppetlabs-puppet_metrics_collector",
-  "version": "6.3.0",
+  "version": "6.4.0",
   "author": "npwalker",
   "summary": "A Puppet module for gathering metrics from PE components",
   "license": "Apache-2.0",
@@ -10,36 +10,40 @@
   "dependencies": [
     {
       "name": "puppetlabs-stdlib",
-      "version_requirement": ">= 2.6.0 < 7.0.0"
+      "version_requirement": ">= 2.6.0 < 8.0.0"
     }
   ],
   "operatingsystem_support": [
     {
       "operatingsystem": "RedHat",
       "operatingsystemrelease": [
         "6",
-        "7"
+        "7",
+        "8"
       ]
     },
     {
       "operatingsystem": "CentOS",
       "operatingsystemrelease": [
         "6",
-        "7"
+        "7",
+        "8"
       ]
     },
     {
       "operatingsystem": "OracleLinux",
       "operatingsystemrelease": [
         "6",
-        "7"
+        "7",
+        "8"
       ]
     },
     {
       "operatingsystem": "Scientific",
       "operatingsystemrelease": [
         "6",
-        "7"
+        "7",
+        "8"
       ]
     },
     {
@@ -53,10 +57,10 @@
   "requirements": [
     {
       "name": "puppet",
-      "version_requirement": ">= 5.5.1"
+      "version_requirement": ">= 5.5.1 < 8.0.0"
     }
   ],
-  "pdk-version": "1.18.1",
+  "pdk-version": "2.0.0",
   "template-url": "https://github.com/puppetlabs/pdk-templates#master",
   "template-ref": "remotes/origin/master-0-ga58fd92"
 }

spec/classes/puppet_metrics_collector_system_spec.rb

@@ -20,13 +20,13 @@
     end
 
     context 'when vmware-toolbox-cmd is present on the PATH' do
-      let(:facts) { super().merge({puppet_metrics_collector: {have_vmware_tools: true}}) }
+      let(:facts) { super().merge(puppet_metrics_collector: {have_vmware_tools: true}) }
 
       it { is_expected.to contain_cron('vmware_metrics_collection').with_ensure('present') }
     end
 
     context 'when vmware-toolbox-cmd is not present on the PATH' do
-      let(:facts) { super().merge({puppet_metrics_collector: {have_vmware_tools: false}}) }
+      let(:facts) { super().merge(puppet_metrics_collector: {have_vmware_tools: false}) }
 
       it { is_expected.to contain_notify('vmware_tools_warning') }
       it { is_expected.to contain_cron('vmware_metrics_collection').with_ensure('absent') }

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