Test by alvagante · Pull Request #1 · example42/puppet-pabawi

alvagante · 2026-02-22T15:37:19Z

No description provided.

…e documentation - Add Ansible, Hiera, and Puppet Server integration manifests - Remove custom and Terraform integration manifests (replaced by standard integrations) - Update Bolt and PuppetDB integration configurations - Expand README with complete setup, usage, and configuration documentation - Add detailed integration reference guide with examples - Update common.yaml with enhanced default configuration - Update docker SSL example with improved certificate handling - Refactor main module manifest with integration management logic - Update npm and docker installation methods with .env file generation - Establish concat-based .env file structure for multi-integration configuration

…iguration - Add GitHub Actions PR workflow for syntax validation and unit tests across Puppet 6, 7, and 8 - Refactor integration configuration from nested structure to flat parameter hierarchy for better clarity - Update common.yaml with simplified integration array and per-integration parameter documentation - Enhance integration manifests with improved parameter handling and documentation - Update full_integrations.pp example to reflect new configuration structure - Improve init.pp with refined integration inclusion logic and parameter management - Enables easier toggling of integrations via enabled parameter while maintaining pre-configuration capability

Copilot

Pull request overview

This PR refactors the module’s integration configuration to be .env-driven (via puppetlabs/concat), adds/expands multiple integrations (Bolt, PuppetDB, Puppet Server, Hiera, Ansible), and updates installation classes (npm/Docker) to generate the base .env configuration.

Changes:

Replace the old integration enablement flags/hash with an integrations array and have integrations emit .env fragments.
Add new integrations (Ansible, Hiera, Puppet Server) and rework existing Bolt/PuppetDB integration manifests to align with .env configuration and optional source-based content management.
Expand documentation/default Hiera data and add a GitHub Actions workflow for validations/unit tests.

Reviewed changes

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

Show a summary per file

File	Description
manifests/integrations/terraform.pp	Removes Terraform stub integration class.
manifests/integrations/custom.pp	Removes custom stub integration class.
manifests/integrations/puppetserver.pp	Adds Puppet Server integration that writes settings to `.env` and manages SSL assets.
manifests/integrations/puppetdb.pp	Refactors PuppetDB integration from YAML config output to `.env` fragments + SSL asset sourcing.
manifests/integrations/hiera.pp	Adds Hiera integration with optional package management and control-repo cloning + `.env`.
manifests/integrations/bolt.pp	Refactors Bolt integration to `.env`-based config and optional repo/package management.
manifests/integrations/ansible.pp	Adds Ansible integration with optional package management, repo cloning, and `.env` output.
manifests/install/npm.pp	Adds base `.env` creation via concat and new auth/log/db settings for npm install method.
manifests/install/docker.pp	Adds base `.env` creation via concat, mounts `.env` into the container, and adds auth/log/db settings.
manifests/init.pp	Switches to integrations array and includes integration classes accordingly.
examples/full_integrations.pp	Updates example to new integrations model and parameters (but currently redeclares classes).
examples/docker_custom_ssl.pp	Updates example Docker image reference.
data/common.yaml	Updates defaults to integrations array model and adds integration/install `.env`-related settings.
README.md	Major documentation expansion describing `.env` model, integrations, and examples.
.github/workflows/pr.yml	Adds CI workflow for PDK validation and unit tests (currently non-blocking).

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

Copilot · 2026-02-22T15:44:33Z

manifests/init.pp

+  # Process integrations - simply include each class
+  $integrations.each |String $name| {
+    $integration_class = "pabawi::integrations::${name}"

-      # Log that we're attempting to enable this integration
-      # If the class doesn't exist, Puppet will fail with a descriptive error
-      # This is intentional - missing integration classes should be caught early
-      notify { "pabawi_integration_${name}":
-        message  => "Enabling integration: ${integration_class}",
-        loglevel => 'notice',
-      }
-
-      # Include the integration class
-      # Puppet will fail compilation if the class doesn't exist
-      include $integration_class
+    notify { "pabawi_integration_${name}":
+      message  => "Enabling integration: ${integration_class}",
+      loglevel => 'notice',
    }
+
+    include $integration_class
  }


The integrations array is used to build class names and notify resource titles, but there’s no validation for invalid segment names or duplicates. If the array contains invalid characters, Puppet will error on an invalid class name; if it contains duplicates, notify { "pabawi_integration_${name}" } will be declared twice and fail compilation. Consider validating each $name against a strict pattern (e.g., integration name segment) and de-duplicating the list before iterating.

@copilot open a new pull request to apply changes based on this feedback allowed integrations are puppetdb, puppetserver, hiera, bolt, ansible

manifests/install/docker.pp

Copilot · 2026-02-22T15:44:34Z

manifests/integrations/ansible.pp

+  # Clone playbook repository if source is provided
+  if $playbook_source and $playbook_path {
+    # Ensure parent directory exists
+    $playbook_parent_dir = dirname($playbook_path)
+    exec { "create_ansible_playbook_parent_dir_${playbook_path}":
+      command => "mkdir -p ${playbook_parent_dir}",
+      path    => ['/usr/bin', '/bin'],


When playbook_source is provided but playbook_path is undef, the module silently skips cloning and writes an empty ANSIBLE_PLAYBOOK_PATH to .env. This is difficult to debug and likely unintended. Add a validation that fails if playbook_source is set without playbook_path (or make playbook_path required when playbook_source is used).

@copilot open a new pull request to apply changes based on this feedback if enabled is true the playbook path and inventory path are mandatory

Copilot · 2026-02-22T15:44:34Z

manifests/integrations/puppetserver.pp

+      elsif $source =~ /^https:\/\/.+$/ {
+        exec { "download_puppetserver_${name}":
+          command => "curl -sL -o ${dest_path} ${source}",
+          path    => ['/usr/bin', '/bin'],
+          creates => $dest_path,
+        }
+        -> file { $dest_path:
+          ensure => file,
+          mode   => $mode,
+          owner  => 'root',
+          group  => 'root',
+        }


The https certificate download relies on curl via an exec but doesn’t ensure curl is installed and doesn’t verify the downloaded content (checksum/signature). This is fragile on minimal hosts and has security implications. Prefer a managed/verified download approach, or at least manage the curl package and add appropriate safeguards.

add optional curl package installation (default true)

Copilot · 2026-02-22T15:44:34Z

data/common.yaml

+# PuppetDB integration configuration
+# pabawi::integrations::puppetdb::enabled: true  # Set to false to disable but keep configured
 pabawi::integrations::puppetdb::server_url: 'https://puppetdb.example.com:8081'
+# pabawi::integrations::puppetdb::port: 8081
+# pabawi::integrations::puppetdb::ssl_enabled: true
+# pabawi::integrations::puppetdb::ssl_cert: 'file:///etc/puppetlabs/puppet/ssl/certs/agent.pem'
+# pabawi::integrations::puppetdb::ssl_key: 'file:///etc/puppetlabs/puppet/ssl/private_keys/agent.pem'
+# pabawi::integrations::puppetdb::ssl_ca: 'file:///etc/puppetlabs/puppet/ssl/certs/ca.pem'
+# pabawi::integrations::puppetdb::ssl_reject_unauthorized: true


The commented PuppetDB SSL parameters (ssl_cert, ssl_key, ssl_ca) are shown as file://... URIs, but in the manifests these parameters represent file paths that are written into .env; the *_source parameters are the ones that accept file:///https:// sources. These examples will mislead users into putting URIs where paths are expected.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-22T15:44:35Z

manifests/init.pp

 class pabawi (
  Boolean $proxy_manage = true,
  String[1] $proxy_class = 'pabawi::proxy::nginx',
  Boolean $install_manage = true,
  String[1] $install_class = 'pabawi::install::npm',
-  Boolean $bolt_enable = false,
-  Boolean $puppetdb_enable = false,
-  Hash[String[1], Boolean] $integrations = {},
+  Array[String[1]] $integrations = [],
 ) {


This change removes bolt_enable, puppetdb_enable, and the integrations hash, but the existing unit spec still references those parameters (e.g., bolt_enable: true, integrations hash values). That spec will fail to compile against the updated class interface; it should be updated to pass integrations => [...] and assert the included integration classes accordingly.

@copilot open a new pull request to apply changes based on this feedback

manifests/integrations/puppetdb.pp

Copilot · 2026-02-22T15:44:35Z

manifests/integrations/puppetdb.pp

+      elsif $source =~ /^https:\/\/.+$/ {
+        exec { "download_puppetdb_${name}":
+          command => "curl -sL -o ${dest_path} ${source}",
+          path    => ['/usr/bin', '/bin'],
+          creates => $dest_path,
+        }
+        -> file { $dest_path:
+          ensure => file,
+          mode   => $mode,
+          owner  => 'root',
+          group  => 'root',
+        }


The https certificate download uses exec with curl, but the module doesn’t ensure curl is installed and doesn’t perform any integrity verification (checksum/signature). This can break runs on minimal systems and has security implications. Prefer a managed download mechanism with verification (or at least manage the curl package and add failure handling).

Copilot · 2026-02-22T15:44:36Z

manifests/integrations/puppetserver.pp

+#
+class pabawi::integrations::puppetserver (
+  Boolean $enabled = true,
+  Optional[String[1]] $server_url = undef,


server_url is documented as a URL, but it’s only typed as Optional[String[1]]. Since this is written directly into .env, invalid values won’t be caught until runtime. Consider constraining this to a URL pattern (and align expectations with the separate port parameter).

Suggested change

Optional[String[1]] $server_url = undef,

Optional[Stdlib::HTTPUrl] $server_url = undef,

Copilot · 2026-02-22T15:44:36Z

examples/full_integrations.pp

 class { 'pabawi':
-  bolt_enable     => true,
-  puppetdb_enable => true,
-  integrations    => {
-    'terraform' => true,
-    'custom'    => true,
-  },
+  integrations => ['bolt', 'puppetdb', 'hiera'],
 }

+# Configure Bolt integration via Hiera or class parameters
 class { 'pabawi::integrations::bolt':
-  project_path  => '/opt/bolt-project',
-  bolt_settings => {
-    'timeout'     => 300,
-    'concurrency' => 10,
-  },
+  project_path         => '/opt/bolt-project',
+  command_whitelist    => ['plan run', 'task run'],
+  execution_timeout    => 300000,
+  manage_package       => false,
 }


This example declares class { 'pabawi': integrations => [...] }, which will include each integration class, and then later declares those same integration classes again with parameters. In Puppet this results in a class redeclaration/parameter conflict. To keep the example runnable, either configure integrations via Hiera (and don’t redeclare the classes here) or remove them from the integrations array and declare the integration classes explicitly once.

Copilot · 2026-02-23T07:37:18Z

@alvagante I've opened a new pull request, #2, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-02-23T07:40:13Z

@alvagante I've opened a new pull request, #3, to work on those changes. Once the pull request is ready, I'll request review from you.

…abled is true Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

Copilot · 2026-02-23T07:42:20Z

@alvagante I've opened a new pull request, #4, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-23T07:42:34Z

@alvagante I've opened a new pull request, #5, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…y/ssl_ca examples Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

Make inventory_path and playbook_path mandatory when enabled is true

Fix ssl_cert/ssl_key/ssl_ca examples to use file paths instead of file:// URIs

Update unit specs to match refactored integrations interface

Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

Restrict integrations to explicit allowlist and deduplicate entries

alvagante added 2 commits February 22, 2026 16:15

Copilot AI review requested due to automatic review settings February 22, 2026 15:37

Copilot started reviewing on behalf of alvagante February 22, 2026 15:37 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

Initial plan

161fc4e

Copilot AI mentioned this pull request Feb 23, 2026

Restrict integrations to explicit allowlist and deduplicate entries #2

Merged

alvagante and others added 2 commits February 23, 2026 08:37

Update manifests/install/docker.pp

4036679

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Initial plan

03ad480

Copilot AI mentioned this pull request Feb 23, 2026

Make inventory_path and playbook_path mandatory when enabled is true #3

Merged

Copilot AI and others added 2 commits February 23, 2026 07:41

Add mandatory validation for playbook_path and inventory_path when en…

603137f

…abled is true Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

Initial plan

03260d3

Copilot AI mentioned this pull request Feb 23, 2026

Fix ssl_cert/ssl_key/ssl_ca examples to use file paths instead of file:// URIs #4

Merged

Initial plan

d6429e8

Copilot AI mentioned this pull request Feb 23, 2026

Update unit specs to match refactored integrations interface #5

Merged

alvagante and others added 10 commits February 23, 2026 08:42

Update manifests/integrations/puppetdb.pp

62c296b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: use plain file paths instead of file:// URIs for ssl_cert/ssl_ke…

871fedf

…y/ssl_ca examples Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

Update spec tests to match new integrations array interface

074e6ef

Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

feat: restrict integrations to allowed values and deduplicate entries

9d1cde5

Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

Merge pull request #3 from example42/copilot/sub-pr-1-again

5b8b1e7

Make inventory_path and playbook_path mandatory when enabled is true

Merge pull request #4 from example42/copilot/sub-pr-1-another-one

2353370

Fix ssl_cert/ssl_key/ssl_ca examples to use file paths instead of file:// URIs

Merge pull request #5 from example42/copilot/sub-pr-1-yet-again

68b1ec5

Update unit specs to match refactored integrations interface

Merge branch 'test' into copilot/sub-pr-1

4767918

fix: restore Stdlib::Absolutepath types removed unnecessarily

88c6344

Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>

Merge pull request #2 from example42/copilot/sub-pr-1

42c893b

Restrict integrations to explicit allowlist and deduplicate entries

alvagante merged commit b749e9b into main Feb 23, 2026
2 of 3 checks passed

alvagante deleted the test branch February 23, 2026 10:00

	Optional[String[1]] $server_url = undef,
	Optional[Stdlib::HTTPUrl] $server_url = undef,

Conversation

alvagante commented Feb 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

alvagante Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

alvagante Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

alvagante Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

alvagante Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

alvagante Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants