feat(puppet-pabawi): refactor integrations and update dependencies by alvagante · Pull Request #7 · example42/puppet-pabawi

alvagante · 2026-03-06T21:21:56Z

Add SSH integration class (manifests/integrations/ssh.pp)
Refactor integration classes to use settings hash pattern (ansible, bolt, hiera, puppetdb, puppetserver)
Relocate command whitelist parameters from integration classes to install/docker and proxy/nginx classes
Update Gemfile to downgrade dependencies to Puppet 6.x compatible versions
Update README documentation to reflect command whitelist parameter changes
Add checkpoint task 10 validation results documenting all tests passing
Normalize BOLT_COMMAND_WHITELIST to generic COMMAND_WHITELIST parameter naming

- Add SSH integration class (manifests/integrations/ssh.pp) - Refactor integration classes to use settings hash pattern (ansible, bolt, hiera, puppetdb, puppetserver) - Relocate command whitelist parameters from integration classes to install/docker and proxy/nginx classes - Update Gemfile to downgrade dependencies to Puppet 6.x compatible versions - Update README documentation to reflect command whitelist parameter changes - Add checkpoint task 10 validation results documenting all tests passing - Normalize BOLT_COMMAND_WHITELIST to generic COMMAND_WHITELIST parameter naming

Copilot

Pull request overview

This PR refactors the puppet-pabawi Puppet module by consolidating per-integration parameters into a generic settings hash, adding a new SSH integration class, relocating command_whitelist parameters from integration classes to the install/proxy layer, and updating Gemfile dependencies to support Puppet 6.x.

Changes:

Refactored all integration classes (ansible, bolt, hiera, puppetdb, puppetserver) and added new ssh integration to use a Hash $settings = {} parameter pattern for .env configuration
Moved command_whitelist/command_whitelist_allow_all parameters from bolt.pp to install/docker.pp and proxy/nginx.pp; added nginx template rendering of whitelist (as comments only)
Downgraded test/development gem versions in Gemfile for Puppet 6.x compatibility

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
`manifests/integrations/ssh.pp`	New integration class using settings hash pattern
`manifests/integrations/puppetserver.pp`	Refactored to settings hash; SSL deploy logic expanded inline
`manifests/integrations/puppetdb.pp`	Refactored to settings hash; SSL deploy logic expanded inline
`manifests/integrations/hiera.pp`	Refactored to settings hash
`manifests/integrations/bolt.pp`	Refactored to settings hash; command_whitelist removed
`manifests/integrations/ansible.pp`	Refactored to settings hash
`manifests/install/docker.pp`	Added command_whitelist/command_whitelist_allow_all parameters and .env output
`manifests/proxy/nginx.pp`	Added command_whitelist parameters; passed to nginx_vhost.epp template
`templates/nginx_vhost.epp`	Added whitelist parameters rendered as comments only
`README.md`	Updated env examples for command whitelist rename
`Gemfile`	Downgraded gems to Puppet 6.x compatible versions
`metadata.json`	Version bump from 0.1.1 to 0.2.0
`.kiro/specs/puppet-pabawi-refactoring/checkpoint-task-10.md`	Added checkpoint test validation results

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T21:29:51Z

manifests/integrations/ssh.pp

+  $env_vars = $settings.reduce({}) |$memo, $pair| {
+    $key = $pair[0]
+    $value = $pair[1]
+
+    # Transform value based on type
+    $transformed = $value ? {
+      Array   => inline_template('[<%= @value.map { |v| "\"#{v}\"" }.join(",") %>]'),
+      Boolean => $value ? { true => 'true', false => 'false' },
+      Integer => String($value),
+      String  => $value,
+      Undef   => 'not-set',
+      default => pick($value, 'not-set'),
+    }
+
+    $env_key = upcase($key)
+    $memo + { "SSH_${env_key}" => $transformed }
+  }
+
+  # Build environment variable lines
+  $env_lines = $env_vars.map |$key, $value| {
+    "${key}=${value}"
+  }.join("\n")


The settings hash transform logic is duplicated verbatim across all six integration classes (ansible.pp, bolt.pp, hiera.pp, puppetdb.pp, puppetserver.pp, ssh.pp). Each copy contains identical code to convert values based on type (Array to JSON, Boolean to lowercase string, Integer to String, etc.). Any future bug fix or enhancement to this transformation must be applied to all six files separately. Consider extracting this into a custom Puppet function (e.g., pabawi::env_value_format) to provide a single implementation that can be tested and maintained in one place.

Copilot · 2026-03-06T21:29:52Z

Gemfile

+  gem 'puppetlabs_spec_helper', '~> 2.0'
+  gem 'rspec-puppet', '~> 2.0'
+  gem 'rspec-puppet-facts', '~> 2.0'


The Gemfile downgrade of puppetlabs_spec_helper from ~> 8.0 to ~> 2.0, rspec-puppet from ~> 5.0 to ~> 2.0, and rspec-puppet-facts from ~> 5.0 to ~> 2.0 is a significant downgrade to very old versions that have had many bug fixes and feature improvements since then. These major version downgrades may introduce incompatibilities with other gems or test helpers in the suite. For Puppet 6.x compatibility, puppetlabs_spec_helper ~> 3.0 or ~> 4.0, and rspec-puppet ~> 2.11, would be more appropriate choices that still support Puppet 6.x without regressing to 2.x versions from many years ago.

Suggested change

gem 'puppetlabs_spec_helper', '~> 2.0'

gem 'rspec-puppet', '~> 2.0'

gem 'rspec-puppet-facts', '~> 2.0'

gem 'puppetlabs_spec_helper', '~> 4.0'

gem 'rspec-puppet', '~> 2.11'

gem 'rspec-puppet-facts', '~> 3.0'

Copilot · 2026-03-06T21:29:52Z

README.md


 # Bolt Integration (order: 20)
 BOLT_PROJECT_PATH=/opt/bolt-project
-BOLT_COMMAND_WHITELIST=["ls","pwd"]
+COMMAND_WHITELIST=["ls","pwd"]
 ...


The COMMAND_WHITELIST entry in the example .env output (line 222) is shown under the "Bolt Integration (order: 20)" section, immediately after BOLT_PROJECT_PATH. However, with the refactoring, the command whitelist is now written by pabawi::install::docker (order: 10) as part of the base configuration, not by the Bolt integration. The README example is misleading because it implies COMMAND_WHITELIST is part of the Bolt integration's env fragment, while it actually belongs to the base configuration fragment written at order 10.

Copilot · 2026-03-06T21:29:52Z

manifests/integrations/puppetserver.pp

+  # Deploy SSL certificates if sources are provided
+  if $ssl_ca_source {
+    $ssl_ca_path = $settings['ssl_ca']
+    $ssl_ca_mode = '0644'

-  file { '/etc/pabawi/ssl/puppetserver':
-    ensure => directory,
-    mode   => '0755',
-    owner  => 'root',
-    group  => 'root',
+    # Handle file:// URLs
+    if $ssl_ca_source =~ /^file:\/\/(.+)$/ {
+      $source_path = $1
+      file { $ssl_ca_path:
+        ensure => file,
+        source => $source_path,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
+    # Handle https:// URLs
+    elsif $ssl_ca_source =~ /^https:\/\/.+$/ {
+      exec { 'download_puppetserver_ssl_ca':
+        command => "curl -sL -o ${ssl_ca_path} ${ssl_ca_source}",
+        path    => ['/usr/bin', '/bin'],
+        creates => $ssl_ca_path,
+      }
+      -> file { $ssl_ca_path:
+        ensure => file,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
+    # Handle direct file paths
+    else {
+      file { $ssl_ca_path:
+        ensure => file,
+        source => $ssl_ca_source,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
  }


Same missing parent directory creation issue as in puppetdb.pp: SSL certificate files are created at caller-specified paths (e.g., /opt/pabawi/certs/puppetserver/ca.pem), but the parent directory is never ensured to exist. The file resource will fail if the directory does not already exist on the target system.

Copilot · 2026-03-06T21:29:52Z

manifests/integrations/puppetserver.pp

+    if $ssl_ca_source =~ /^file:\/\/(.+)$/ {
+      $source_path = $1
+      file { $ssl_ca_path:
+        ensure => file,
+        source => $source_path,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
+    # Handle https:// URLs
+    elsif $ssl_ca_source =~ /^https:\/\/.+$/ {
+      exec { 'download_puppetserver_ssl_ca':
+        command => "curl -sL -o ${ssl_ca_path} ${ssl_ca_source}",
+        path    => ['/usr/bin', '/bin'],
+        creates => $ssl_ca_path,
+      }
+      -> file { $ssl_ca_path:
+        ensure => file,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
+    # Handle direct file paths
+    else {
+      file { $ssl_ca_path:
+        ensure => file,
+        source => $ssl_ca_source,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
  }

-  # Helper function to handle SSL file sources
-  $ssl_files = {
-    'ca'   => { 'source' => $ssl_ca_source, 'path' => $ssl_ca },
-    'cert' => { 'source' => $ssl_cert_source, 'path' => $ssl_cert },
-    'key'  => { 'source' => $ssl_key_source, 'path' => $ssl_key },
+  if $ssl_cert_source {
+    $ssl_cert_path = $settings['ssl_cert']
+    $ssl_cert_mode = '0644'
+
+    # Handle file:// URLs
+    if $ssl_cert_source =~ /^file:\/\/(.+)$/ {
+      $source_path = $1


Same variable reassignment bug as in puppetdb.pp: the variable $source_path is assigned inside three separate if blocks (at lines 91-92, 132-133, and 173-174). In Puppet, variables are class-scoped, so if two or more of the SSL sources use file:// URLs, Puppet will fail to compile with "Cannot reassign variable '$source_path'". Each capture variable should use a distinct name (e.g., $ssl_ca_source_path, $ssl_cert_source_path, $ssl_key_source_path).

Copilot · 2026-03-06T21:29:54Z

manifests/integrations/puppetdb.pp

+  # Deploy SSL certificates if sources are provided
+  if $ssl_ca_source {
+    $ssl_ca_path = $settings['ssl_ca']
+    $ssl_ca_mode = '0644'
+
+    # Handle file:// URLs
+    if $ssl_ca_source =~ /^file:\/\/(.+)$/ {
+      $source_path = $1
+      file { $ssl_ca_path:
+        ensure => file,
+        source => $source_path,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
+    # Handle https:// URLs
+    elsif $ssl_ca_source =~ /^https:\/\/.+$/ {
+      exec { 'download_puppetdb_ssl_ca':
+        command => "curl -sL -o ${ssl_ca_path} ${ssl_ca_source}",
+        path    => ['/usr/bin', '/bin'],
+        creates => $ssl_ca_path,
+      }
+      -> file { $ssl_ca_path:
+        ensure => file,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
+    # Handle direct file paths
+    else {
+      file { $ssl_ca_path:
+        ensure => file,
+        source => $ssl_ca_source,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }
+    }
  }


When SSL certificates are deployed to a path like /opt/pabawi/certs/puppetdb/ca.pem, the parent directory (/opt/pabawi/certs/puppetdb/) is never created by this class. The file resource managing the certificate file will fail if its parent directory does not exist. Unlike the git repository cloning blocks where a mkdir -p exec is used to ensure parent directories, no equivalent is present for the SSL certificate file deployments. A file resource ensuring the directory exists (or an exec with mkdir -p) should be added as a prerequisite for each certificate file resource.

Copilot · 2026-03-06T21:29:54Z

README.md

 ```
 BOLT_ENABLED=true
 BOLT_PROJECT_PATH=/opt/bolt-project
-BOLT_COMMAND_WHITELIST=["plan run","task run","command run"]
-BOLT_COMMAND_WHITELIST_ALLOW_ALL=false
+COMMAND_WHITELIST=["plan run","task run","command run"]
+COMMAND_WHITELIST_ALLOW_ALL=false
 BOLT_EXECUTION_TIMEOUT=300000
 ```



The .env file output example in the README still shows BOLT_COMMAND_WHITELIST header comment context (line 221 area), and both updated lines (222, 347-348) correctly show COMMAND_WHITELIST and COMMAND_WHITELIST_ALLOW_ALL. However, the README sections at around lines 190–202 (not changed by this PR but now outdated) still reference old individual parameters like pabawi::integrations::bolt::project_path and pabawi::integrations::bolt::command_whitelist as valid Hiera keys, which no longer work after the settings hash refactoring. These sections of the README should also be updated in this PR to match the new API.

Copilot · 2026-03-06T21:29:54Z

manifests/integrations/puppetserver.pp

-    'group'  => 'root',
-  })
+  # Deploy SSL certificates if sources are provided
+  if $ssl_ca_source {


Same validation/guard mismatch as in puppetdb.pp: the SSL source-path consistency validation (lines 65–83) is wrapped in if $enabled { ... }, but the SSL file deployment (lines 85 onwards) is unconditional. If $enabled => false and an SSL source is provided without the corresponding settings path, no helpful error is raised but the file resource will be called with a undef title, causing an obscure compilation error.

Suggested change

if $ssl_ca_source {

if $enabled and $ssl_ca_source {

Copilot · 2026-03-06T21:29:55Z

manifests/integrations/ssh.pp

+class pabawi::integrations::ssh (
+  Boolean $enabled = true,
+  Hash $settings = {},
+) {


The ssh integration is not listed in the Enum constraint for the $integrations parameter in manifests/init.pp (line 53). It currently reads:

Array[Enum['puppetdb', 'puppetserver', 'hiera', 'bolt', 'ansible']] $integrations = []

Since the ssh integration class is being added by this PR, users will get a type validation failure if they try to include it via the main pabawi class (e.g., integrations => ['ssh']). The string 'ssh' should be added to the Enum type list in init.pp.

Copilot · 2026-03-06T21:29:55Z

manifests/integrations/puppetdb.pp

+      exec { 'download_puppetdb_ssl_ca':
+        command => "curl -sL -o ${ssl_ca_path} ${ssl_ca_source}",
+        path    => ['/usr/bin', '/bin'],
+        creates => $ssl_ca_path,
+      }
+      -> file { $ssl_ca_path:
+        ensure => file,
+        mode   => $ssl_ca_mode,
+        owner  => 'root',
+        group  => 'root',
+      }


The curl commands used to download SSL certificates (e.g., curl -sL -o ${ssl_ca_path} ${ssl_ca_source}) use -L which follows redirects, but crucially they do not use --fail (to treat HTTP errors as failure) or --cacert/--ssl-verify flags to enforce TLS certificate verification. This means a certificate could be silently downloaded from an HTTP redirect or a server with an invalid certificate and stored as a trusted CA or client certificate, which is a serious security risk. The --fail flag should be added at minimum, and TLS verification should be explicitly enforced.

Copilot AI review requested due to automatic review settings March 6, 2026 21:21

Copilot started reviewing on behalf of alvagante March 6, 2026 21:22 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

docs: updates

eeaa3bd

alvagante merged commit 52fbff9 into main Mar 7, 2026
2 of 3 checks passed

alvagante deleted the 020 branch March 7, 2026 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(puppet-pabawi): refactor integrations and update dependencies#7

feat(puppet-pabawi): refactor integrations and update dependencies#7
alvagante merged 2 commits intomainfrom
020

alvagante commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

alvagante commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants