Drop puppet 3 support and use data types for parameters #167

treydock · 2017-10-31T13:06:27Z

This PR is incomplete, I have not updated the main pam class. I wanted to know, should things that currently accept string or boolean and use str2bool be continued or should the data type just be Boolean? These changes likely require a major version bump so figured maybe good time to force parameters to be the proper data type.

treydock · 2017-10-31T13:40:20Z

@ghoneycutt Just FYI, relates to PR discussion for MOTD module, the message for Optional[Array] changes in puppet >= 4.10.x in such a way that the only way to match the error is just to say /Array/.

Puppet 4.9.x: expects an Array value, got String
Puppet 4.10.x/5.x: expects a value of type Undef or Array, got String

ghoneycutt · 2017-10-31T14:41:04Z

Thanks for all the hard work!

I think it's time we drop string 'true', 'false' and just use Boolean.

ghoneycutt · 2017-10-31T14:42:50Z

.travis.yml

 ---
 language: ruby

-rvm:


Let's use the same Travis config as https://github.com/sensu/sensu-puppet/blob/master/.travis.yml

Only slightly different in testing for puppet-strings and some new minor releases of Puppet 5.

Updated to use the linked .travis.yml.

ghoneycutt · 2017-10-31T14:43:58Z

manifests/limits.pp

+  Optional[String] $config_file_source          = undef,
+  Pattern[/^[0-7]{4}$/] $config_file_mode       = '0640',
+  Stdlib::Absolutepath $limits_d_dir            = '/etc/security/limits.d',
+  Pattern[/^[0-7]{4}$/] $limits_d_dir_mode      = '0750',


You'd think stdlib would give us a mode pattern by now :)

ghoneycutt · 2017-10-31T14:44:36Z

metadata.json

    {"name":"ghoneycutt/common","version_requirement":">= 1.4.1 < 2.0.0"},
    {"name":"ghoneycutt/nsswitch","version_requirement":">= 1.3.0 < 2.0.0"},
-    {"name":"puppetlabs/stdlib","version_requirement":">= 4.6.0 < 5.0.0"}
+    {"name":"puppetlabs/stdlib","version_requirement":">= 4.13.1 < 5.0.0"}


Curious is there something special about 4.13.1 that has functionality we are using?

I think I pulled that from some official puppetlabs modules that got updated to only support Puppet 4/5. The 4.13.0 is needed for custom Stdlib data types, like the one for validating absolute path.

ghoneycutt · 2017-10-31T14:45:38Z

Happy to go to a new major version. Are there other things we should change about the module?

@Phil-Friderici what do you think?

treydock · 2017-10-31T15:32:00Z

@ghoneycutt Part of major version, maybe move various defaults to module level Hiera? The pam class code is extremely long and mostly just default values. This may be unrealistic given some defaults are based on $ensure_vas and $vas_major_version.

Maybe saving some lines by using pick instead of 5 line conditions for picking which value to use for Optional parameters.

ghoneycutt · 2017-10-31T15:47:06Z

I'd like to drop support for VAS in the next major version and instead just document what values should be used. We could setup a vas/ directory with the hiera structure in it so that you could copy those to your hiera and have VAS working. <-- @Phil-Friderici

Would definitely like to switch to data in modules, which we just did for sgnl05/sssd. This will also cut way down on lines of code.

https://github.com/sgnl05/sgnl05-sssd

treydock · 2017-10-31T15:56:09Z

@ghoneycutt I'd be happy to help with such changes via different pull requests. May be a good time to use something like https://skywinder.github.io/github-changelog-generator/ to generate CHANGELOG, something I've seen used in voxpupuli projects.

treydock · 2017-10-31T16:24:07Z

@ghoneycutt There are several deprecated parameters in init.pp. Okay to bundle a commit removing those into this pull request?

ghoneycutt · 2017-10-31T16:32:03Z

+1 to the changelog generator. I'm doing that on other projects.

ghoneycutt · 2017-10-31T16:34:27Z

I made a branch named puppet5. Could you point this against that branch? That way we can all work on the breaking changes and get them merged as they come in and time when we merge the whole thing into master.

I wont make any changes to master in the meantime, so we aren't stuck in rebase hell.

ghoneycutt · 2017-10-31T16:35:08Z

Go ahead and remove the deprecated params in their own commit.

treydock · 2017-10-31T16:43:46Z

@ghoneycutt Added commit for removing deprecated parameters, let me know if anything in this PR should be split into separate PR.

system_auth_ac_auth_lines system_auth_ac_account_lines system_auth_ac_password_lines system_auth_ac_session_lines

ghoneycutt · 2017-10-31T18:35:37Z

I dont think we actually need yard and redcarpet as puppet-strings will pull in its dependencies

Phil-Friderici · 2017-11-03T11:23:24Z

Happy to go to a new major version. Are there other things we should change about the module?
@Phil-Friderici what do you think?

Changing from 'true' and 'false' to booleans is great.
Chaning from 'unset' to undef would be great too. (used for $pam_d_login_oracle_options)

treydock force-pushed the puppet-data-types branch from 21f4fe8 to c89475d Compare October 31, 2017 13:53

ghoneycutt reviewed Oct 31, 2017

View reviewed changes

treydock force-pushed the puppet-data-types branch 3 times, most recently from fa5a82a to 6dea7a7 Compare October 31, 2017 15:26

treydock force-pushed the puppet-data-types branch from 6dea7a7 to 7816ad9 Compare October 31, 2017 15:40

treydock force-pushed the puppet-data-types branch from 7816ad9 to 3457c75 Compare October 31, 2017 15:52

treydock force-pushed the puppet-data-types branch from d5e6cae to d8b24e5 Compare October 31, 2017 16:36

treydock changed the base branch from master to puppet5 October 31, 2017 16:36

treydock added 3 commits October 31, 2017 14:00

Drop puppet 3 support and use data types for parameters

f4ffc2a

Fix conditions to not use == undef and != undef

b27debc

Remove deprecated parameters

9cd37d3

system_auth_ac_auth_lines system_auth_ac_account_lines system_auth_ac_password_lines system_auth_ac_session_lines

treydock force-pushed the puppet-data-types branch from 7b45477 to 9cd37d3 Compare October 31, 2017 18:00

ghoneycutt merged commit 927cb35 into ghoneycutt:puppet5 Oct 31, 2017

Drop puppet 3 support and use data types for parameters #167

Drop puppet 3 support and use data types for parameters #167

Uh oh!

Conversation

treydock commented Oct 31, 2017

Uh oh!

treydock commented Oct 31, 2017

Uh oh!

ghoneycutt commented Oct 31, 2017

Uh oh!

ghoneycutt Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

treydock Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

ghoneycutt Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

ghoneycutt Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

treydock Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

ghoneycutt commented Oct 31, 2017

Uh oh!

treydock commented Oct 31, 2017

Uh oh!

ghoneycutt commented Oct 31, 2017

Uh oh!

treydock commented Oct 31, 2017

Uh oh!

treydock commented Oct 31, 2017

Uh oh!

ghoneycutt commented Oct 31, 2017

Uh oh!

ghoneycutt commented Oct 31, 2017

Uh oh!

ghoneycutt commented Oct 31, 2017

Uh oh!

treydock commented Oct 31, 2017

Uh oh!

ghoneycutt commented Oct 31, 2017

Uh oh!

Phil-Friderici commented Nov 3, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants