Skip to content

Set data types for puppetdb parameters #411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 32 commits into from
Oct 28, 2025
Merged

Set data types for puppetdb parameters #411

merged 32 commits into from
Oct 28, 2025

Conversation

@chambersmp
Copy link
Contributor

@chambersmp chambersmp commented May 29, 2024 •
edited
Loading

Summary:
Enable data type validation for parameters in puppetdb classes and defined types. The goal is to refine user inputs to reduce risk of misconfiguration or failure to apply.

Each data type was configured based on default values supplied within the params.pp class, unit/acceptance tests and alternative values recommended in PuppetDB docs.

Changes:

  • Data types have been added to the parameters for both classes and defined types
  • Stdlib types provide validation for complex inputs (e.g. Stdlib::Host, Stdlib::Port::User)
  • Data values supplied by unit and acceptance tests were reviewed to ensure parameters supported expected input value types
  • Unit test param inputs refined to support expected data types.

Potential for Breaking Change:

  • Data type validation may render some user inputs invalid (i.e. Strings provided where Integer expected).
  • To guard against this some parameters use Variant to support historical String usage with Pattern matching

Related Issues:
resolves #105

Testing:

  • Mend tests are expected to fail on forked repositories due to missing secrets.
  • All CI tests (unit and acceptance) are passing.

@chambersmp chambersmp requested review from a team, bastelfreak, h0tw1r3 and smortex as code owners May 29, 2024 04:59
@chambersmp chambersmp marked this pull request as draft May 29, 2024 05:57
bastelfreak
bastelfreak reviewed May 29, 2024
View reviewed changes
@chambersmp chambersmp marked this pull request as ready for review June 7, 2024 02:11
smortex
smortex reviewed Jun 10, 2024
View reviewed changes
@h0tw1r3
Copy link
Contributor

h0tw1r3 commented Jun 11, 2024

FWIW, I would remove the maintenance label so the change does show up in the change log and release notes.

bastelfreak
bastelfreak reviewed Jun 28, 2024
View reviewed changes
@chambersmp chambersmp requested a review from bastelfreak July 1, 2024 03:46
@chambersmp
Copy link
Contributor Author

chambersmp commented Jul 1, 2024

As open_ssl_listen_port is no longer optional both params.pp and the unit test shared-examples have been updated to default to false instead of undef|nil.

@techsk8
Copy link

techsk8 commented Feb 27, 2025 •
edited
Loading

is there any blocker in getting this PR merged now that's approved?

@chambersmp
Copy link
Contributor Author

chambersmp commented Feb 28, 2025 •
edited
Loading

is there any blocker in getting this PR merged now that's approved?

Should be good to go, I haven't had a chance to test on live node so I haven't merged. Passing all unit tests.

@zaben903
Copy link

zaben903 commented Oct 16, 2025

Tested changes with a development environment Puppetserver.
Confirmed invalid data such as the example from #105 will return a compile time error.

class { 'puppetdb':
  listen_port => 'localhost',
 }
$ puppet agent -t
Info: Using environment 'production'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Loading facts
Notice: Requesting catalog from puppet:8140 (127.0.0.1)
Notice: Catalog compiled by puppet-testing
Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Class[Puppetdb]: parameter 'listen_port' expects a value of type Stdlib::Port::User = Integer[1024, 49151] or Pattern[/\A[0-9]+\Z/], got String (file: /etc/puppet/code/environments/production/manifests/site.pp, line: 32, column: 3) on node puppet-testing
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

When data is valid, this module works as expected compiling and implementing changes.
Confirmed working with Strings and Integers with pattern matching (where used).

class { 'puppetdb':
  ssl_listen_port => 8000, # or '8000'
}

class { 'puppetdb::master::config':
  puppetdb_port => 8000, # or '8000'
}
puppet agent -t
Info: Using environment 'production'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Loading facts
Notice: Requesting catalog from puppet:8140 (127.0.0.1)
Notice: Catalog compiled by puppet-testing
Info: Caching catalog for puppet-testing
Info: Applying configuration version '1760595330'
Notice: /Stage[main]/Puppetdb::Server::Jetty/Ini_setting[puppetdb_sslport]/value: value changed 8081 to 8000
Info: Class[Puppetdb::Server::Jetty]: Scheduling refresh of Service[puppetdb]
Notice: /Stage[main]/Puppetdb::Server/Service[puppetdb]: Triggered 'refresh' from 1 event
Notice: /Stage[main]/Puppetdb::Master::Puppetdb_conf/Ini_setting[puppetdbserver_urls]/value: value changed https://puppet-testing:8081/ to https://puppet-testing:8000/
Info: Class[Puppetdb::Master::Puppetdb_conf]: Scheduling refresh of Service[puppetserver]
Notice: /Stage[main]/Puppetdb::Master::Config/Service[puppetserver]: Triggered 'refresh' from 1 event
Notice: Applied catalog in 43.43 seconds

@chambersmp chambersmp merged commit a00ba4a into puppetlabs:main Oct 28, 2025
1 check passed
@chambersmp chambersmp deleted the feature_data_validation branch October 28, 2025 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smortex smortex smortex approved these changes

@bastelfreak bastelfreak Awaiting requested review from bastelfreak bastelfreak is a code owner

+1 more reviewer

@h0tw1r3 h0tw1r3 h0tw1r3 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Manifest should validate the passed parameters

6 participants

@chambersmp @h0tw1r3 @techsk8 @zaben903 @smortex @bastelfreak