Fixing by alvagante · Pull Request #6 · example42/puppet-pabawi

alvagante · 2026-02-23T21:10:59Z

No description provided.

- Update puppetlabs_spec_helper from ~> 6.0 to ~> 8.0 - Ensures compatibility with latest spec helper features and improvements - Maintains test infrastructure alignment with current Puppet ecosystem standards

Copilot

Pull request overview

This pull request makes structural and organizational changes to the Pabawi Puppet module, consolidating file paths under /opt/pabawi and modernizing code patterns. The changes affect module metadata, installation classes, integration classes, proxy configuration, and default data.

Changes:

Reorganizes application directory structure to use /opt/pabawi/app, /opt/pabawi/data, and /opt/pabawi/certs subdirectories
Updates OS support matrix, removing several OS versions and adding Debian 13
Modernizes code patterns (structured facts, namespaced functions, improved nginx configuration)

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
metadata.json	Version bump to 0.1.1 and OS support changes (removes Ubuntu 22.04, Debian 10-12, RHEL 7; adds Debian 13)
manifests/proxy/nginx.pp	Modernizes facts usage, adds OS-specific nginx configuration paths, improves file resource dependencies
manifests/integrations/puppetdb.pp	Changes SSL certificate paths from /etc/pabawi/ssl to /opt/pabawi/certs, updates default values, removes directory creation
manifests/integrations/hiera.pp	Changes control_repo_path from Optional to required with default '/opt/pabawi/control-repo', adds stdlib namespace to to_json
manifests/integrations/bolt.pp	Changes project_path from Optional to required with default '/opt/pabawi/control-repo', adds stdlib namespace to to_json
manifests/integrations/ansible.pp	Adds default values for inventory_path and playbook_path ('/opt/pabawi/ansible')
manifests/install/npm.pp	Changes install_dir to '/opt/pabawi/app', database_path to '/opt/pabawi/data/pabawi.db', simplifies directory creation
manifests/install/docker.pp	Changes JWT_SECRET default from empty string to 'not-set'
data/common.yaml	Updates default paths to match new directory structure, improves integration documentation example

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

Copilot · 2026-02-23T21:21:15Z

manifests/proxy/nginx.pp

+        default => [],
+      },
+    ],


The require array now includes an empty array when manage_package is false. While Puppet can handle empty arrays in require statements, this is inconsistent with the pattern used elsewhere in the codebase. Consider using the filter pattern seen in other files (e.g., manifests/install/npm.pp line 168) to exclude undefined values instead of using an empty array.

Suggested change

default => [],

},

],

default => undef,

},

].filter |$v| { $v != undef },

Copilot · 2026-02-23T21:21:16Z

manifests/integrations/puppetdb.pp

+#   pabawi::integrations::puppetdb::ssl_ca: '/opt/pabawi/certs/puppetdb/ca.pem'
+#   pabawi::integrations::puppetdb::ssl_cert: '/opt/pabawi/certs/puppetdb/cert.pem'
+#   pabawi::integrations::puppetdb::ssl_key: '/opt/pabawi/certs/puppetdb/key.pem'


The documentation example paths in the comments have not been updated to reflect the new default paths. The examples still reference '/etc/pabawi/ssl/puppetdb/' paths (lines 54-56), but the default parameters and code now use '/opt/pabawi/certs/'. This discrepancy could confuse users. The documentation should be updated to show examples using the new path structure.

Suggested change

# pabawi::integrations::puppetdb::ssl_ca: '/opt/pabawi/certs/puppetdb/ca.pem'

# pabawi::integrations::puppetdb::ssl_cert: '/opt/pabawi/certs/puppetdb/cert.pem'

# pabawi::integrations::puppetdb::ssl_key: '/opt/pabawi/certs/puppetdb/key.pem'

# pabawi::integrations::puppetdb::ssl_ca: '/opt/pabawi/certs/ca.pem'

# pabawi::integrations::puppetdb::ssl_cert: '/opt/pabawi/certs/pabawi.pem'

# pabawi::integrations::puppetdb::ssl_key: '/opt/pabawi/certs/pabawi_key.pem'

Copilot · 2026-02-23T21:21:16Z

manifests/integrations/ansible.pp

+  Optional[Stdlib::Absolutepath] $inventory_path = '/opt/pabawi/ansible',
  Optional[String[1]] $inventory_source = undef,
-  Optional[Stdlib::Absolutepath] $playbook_path = undef,
+  Optional[Stdlib::Absolutepath] $playbook_path = '/opt/pabawi/ansible',


The parameters inventory_path and playbook_path are declared as Optional[Stdlib::Absolutepath] but are given non-undef default values ('/opt/pabawi/ansible'). This is semantically inconsistent - if a parameter has a guaranteed default value, it should not be Optional. This differs from the approach taken in bolt.pp and hiera.pp where similar parameters had their Optional type removed when defaults were added. Consider either removing the Optional type wrapper to match the pattern in bolt.pp and hiera.pp, or using undef as the default if the path should truly be optional.

Copilot · 2026-02-23T21:21:16Z

manifests/integrations/puppetdb.pp

+  Optional[String[1]] $ssl_ca = '/opt/pabawi/certs/ca.pem',
+  Optional[String[1]] $ssl_cert = '/opt/pabawi/certs/pabawi.pem',
+  Optional[String[1]] $ssl_key = '/opt/pabawi/certs/pabawi_key.pem',


The SSL certificate path parameters (ssl_ca, ssl_cert, ssl_key) are declared as Optional[String[1]] but now have non-undef default values. This is semantically incorrect - Optional should only be used when a parameter can legitimately be undefined. With the current defaults, these parameters will never be undef, making the Optional wrapper misleading. Either remove the Optional wrapper or change the defaults back to undef if these paths should truly be optional.

Suggested change

Optional[String[1]] $ssl_ca = '/opt/pabawi/certs/ca.pem',

Optional[String[1]] $ssl_cert = '/opt/pabawi/certs/pabawi.pem',

Optional[String[1]] $ssl_key = '/opt/pabawi/certs/pabawi_key.pem',

String[1] $ssl_ca = '/opt/pabawi/certs/ca.pem',

String[1] $ssl_cert = '/opt/pabawi/certs/pabawi.pem',

String[1] $ssl_key = '/opt/pabawi/certs/pabawi_key.pem',

Copilot · 2026-02-23T21:21:16Z

metadata.json

 {
  "name": "example42-pabawi",
-  "version": "0.1.0",
+  "version": "0.1.1",


The version is being bumped from 0.1.0 to 0.1.1, indicating a patch release. However, the changes in this PR include breaking changes (removal of OS version support, changes to parameter types from Optional to required, changes to default paths). According to semantic versioning, breaking changes should be a major version bump (or at minimum a minor version bump for pre-1.0 releases). Consider bumping to 0.2.0 instead.

Suggested change

"version": "0.1.1",

"version": "0.2.0",

Copilot · 2026-02-23T21:21:17Z

manifests/install/npm.pp

  # Create .env file using concat
  $env_file_path = "${install_dir}/backend/.env"
  concat { 'pabawi_env_file':
    path    => $env_file_path,
    owner   => $user,
    group   => $group,
    mode    => '0600',
-    require => File["${install_dir}/backend"],
  }


The .env file path is hardcoded as ${install_dir}/backend/.env (line 118), but with the new install_dir default of '/opt/pabawi/app', this will create the .env file at '/opt/pabawi/app/backend/.env'. However, there is no code that creates the backend directory anymore since the removal of the directory creation logic in lines 103-110 of the original code. This will cause the concat resource to fail when trying to create the .env file in a non-existent directory.

Copilot · 2026-02-23T21:21:17Z

manifests/integrations/puppetdb.pp

@@ -104,7 +89,7 @@
    $target_path = $item[1]['path']

    if $source {
-      $dest_path = "/etc/pabawi/ssl/puppetdb/${name}.pem"
+      $dest_path = "/opt/pabawi/certs/${name}.pem"


The removed directory creation logic for /etc/pabawi/ssl and /etc/pabawi/ssl/puppetdb is not replaced with equivalent logic for the new /opt/pabawi/certs directory. When ssl_ca_source, ssl_cert_source, or ssl_key_source are provided, the code attempts to create files in /opt/pabawi/certs/ (line 92), but there is no code ensuring this directory exists. This will cause file resource failures when SSL sources are provided.

Copilot · 2026-02-23T21:21:17Z

metadata.json

    {
      "operatingsystem": "Ubuntu",
      "operatingsystemrelease": [
-        "22.04",
        "24.04"
      ]
    },
    {
      "operatingsystem": "Debian",
      "operatingsystemrelease": [
-        "10",
-        "11",
-        "12"
+        "13"


Removing support for Ubuntu 22.04, Debian 10, 11, and 12, as well as RedHat 7 could be a breaking change for existing users running these OS versions. Additionally, changing the Debian support to only version 13 is problematic because Debian 13 (Trixie) is not yet released as a stable version. As of the knowledge cutoff, Debian 12 (Bookworm) is the current stable release. This change will leave no stable Debian versions supported.

alvagante added 3 commits February 23, 2026 21:05

Some fixes

e9b133d

chore(puppet-pabawi): upgrade puppetlabs_spec_helper dependency

92f405b

- Update puppetlabs_spec_helper from ~> 6.0 to ~> 8.0 - Ensures compatibility with latest spec helper features and improvements - Maintains test infrastructure alignment with current Puppet ecosystem standards

Updated supported os

e1f6fb2

Copilot AI review requested due to automatic review settings February 23, 2026 21:10

Copilot started reviewing on behalf of alvagante February 23, 2026 21:11 View session

alvagante added 2 commits February 23, 2026 22:13

Updated Gemfile

275f1ba

Updated Gemfile

afd7ff9

Copilot AI reviewed Feb 23, 2026

View reviewed changes

alvagante added 4 commits February 23, 2026 22:26

Upated gemfile

0c0597a

Updated Gemfile

f91cfee

Updated Gemfile

591976c

Updated Gemfile

eddca1b

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing#6

Fixing#6
alvagante merged 9 commits intomainfrom
fixing

alvagante commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        default => [],
-      },
-    ],
+        default => undef,
+      },
+    ].filter |$v| { $v != undef },

Conversation

alvagante commented Feb 23, 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 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants