Preparations to update to PDK v3.0.0 #77
Conversation
Phil-Friderici
force-pushed
the
pre-pdk300
branch
from
72fcf0f to
4d18186
Compare
|
The unit tests should now work again :) I will create an additional PR to update PDK to v3.0.0. |
manifests/init.pp
Outdated
| purge => $purge_unmanaged_CAs, | ||
| recurse => $purge_unmanaged_CAs, | ||
| purge => $purge_unmanaged_cas, | ||
| recurse => $purge_unmanaged_cas, |
There was a problem hiding this comment.
I've been out of the game for a minute. Are puppet parameters case sensitive? I'm only asking so I can make sure that a release with this change follows semver correctly (i.e. is this a breaking change?)
There was a problem hiding this comment.
Variables are case sensitive, yes. https://www.puppet.com/docs/puppet/8/lang_variables#lang_var_naming-variable-names
There was a problem hiding this comment.
Only did it because puppet-lint does complain otherwise:
pdk (WARNING): puppet-lint: variable contains a capital letter (manifests/init.pp:46:11)
pdk (WARNING): puppet-lint: variable contains a capital letter (manifests/init.pp:73:16)
pdk (WARNING): puppet-lint: variable contains a capital letter (manifests/init.pp:74:16)
So the argument is simply: because of style guides ;)
Sure, the alternative could be to deactivate the checks instead.
There was a problem hiding this comment.
I kind of lean towards staying backwards compatible, but don't feel so strongly that I can't be convinced otherwise. If we were breaking other stuff that wasn't as simple as a disabled check I might feel differently I guess. Whatever you all think we should do works for me.
There was a problem hiding this comment.
Changed the commit so that checks for "puppet-lint: variable contains a capital letter" are ignored.
Maybe this could be changed in the next major release :)
Phil-Friderici
force-pushed
the
pre-pdk300
branch
from
e0e1622 to
6cd5767
Compare
|
Do you still utilize TravisCI these days ? I have switched to Github actions and I am satisfied with them. Never had any issues with them (nor billing). With Litmus you can even do acceptance testing, but to speak frankly, I am not so familiar with acceptance testing yet. You can see the Github actions in action in my fork Phil-Friderici#1. |
Checks for "puppet-lint: variable contains a capital letter" are currently only ignored. This is because it would break backward compatibility. Removes warnings/errors: pdk (WARNING): puppet-lint: class included by absolute name (::$class) (examples/ca.pp:1:9) pdk (WARNING): puppet-lint: class included by absolute name (::$class) (examples/init.pp:1:9) pdk (WARNING): puppet-lint: defined type not documented (manifests/ca.pp:38:1) pdk (WARNING): puppet-lint: class included by absolute name (::$class) (manifests/ca.pp:49:11) pdk (WARNING): puppet-lint: class included by absolute name (::$class) (manifests/ca.pp:50:11) pdk (WARNING): puppet-lint: class included by absolute name (::$class) (manifests/ca.pp:51:11) pdk (WARNING): puppet-lint: absolute class name reference (manifests/ca.pp:115:29) pdk (WARNING): puppet-lint: absolute class name reference (manifests/ca.pp:137:29) pdk (WARNING): puppet-lint: absolute class name reference (manifests/ca.pp:148:30) pdk (WARNING): puppet-lint: absolute class name reference (manifests/ca.pp:159:25) pdk (WARNING): puppet-lint: ensure set to present on file resource (manifests/ca.pp:109:23) pdk (WARNING): puppet-lint: ensure set to present on file resource (manifests/ca.pp:131:23) pdk (WARNING): puppet-lint: ensure set to present on file resource (manifests/ca.pp:142:24) pdk (ERROR): puppet-lint: there should be a single space or newline before a closing brace (manifests/ca.pp:165:4) pdk (ERROR): puppet-lint: there should be a single space or single newline after an opening brace (manifests/ca.pp:48:1) pdk (WARNING): puppet-lint: missing documentation for defined type parameter ca_cert::ca::ca_file_group (manifests/ca.pp:45:20) pdk (WARNING): puppet-lint: missing documentation for defined type parameter ca_cert::ca::ca_file_group (manifests/ca.pp:45:20) pdk (ERROR): puppet-lint: there should be a single space or newline before a closing brace (manifests/enable.pp:23:4) pdk (ERROR): puppet-lint: there should be a single space or single newline after an opening brace (manifests/enable.pp:3:1) pdk (WARNING): puppet-lint: class included by absolute name (::$class) (manifests/init.pp:54:11) pdk (WARNING): puppet-lint: class included by absolute name (::$class) (manifests/init.pp:55:11) pdk (ERROR): puppet-lint: there should be a single space or single newline after an opening brace (manifests/init.pp:53:1) pdk (WARNING): puppet-lint: variable contains a capital letter (manifests/init.pp:46:11) pdk (WARNING): puppet-lint: variable contains a capital letter (manifests/init.pp:73:16) pdk (WARNING): puppet-lint: variable contains a capital letter (manifests/init.pp:74:16) pdk (ERROR): puppet-lint: there should be a single space before an opening brace (manifests/params.pp:47:57) pdk (ERROR): puppet-lint: there should be a single space before an opening brace (manifests/params.pp:76:67) pdk (ERROR): puppet-lint: there should be a single space or newline before a closing brace (manifests/update.pp:12:4) pdk (ERROR): puppet-lint: there should be a single space or single newline after an opening brace (manifests/update.pp:3:1) pdk (WARNING): rubocop: Unnecessary disabling of `RSpec/VoidExpect`. (spec/defines/ca_spec.rb:2:1)
Removes warnings: pdk (WARNING): metadata-json-lint: Dependency puppet/archive has an open ended dependency version requirement >= 6.0.0 (metadata.json)
Removes errors: rspec ./spec/classes/ca_cert_spec.rb:192 # ca_cert on a Solaris based OS is expected to raise Puppet::Error with message matching /Unsupported osfamily/ rspec ./spec/classes/params_spec.rb:60 # ca_cert::params on an unsupported operating system is expected to raise Puppet::Error with message matching /Unsupported osfamily/
The code is only executed on osfamily RedHat with major version below 7. The unit tests did only skip the tests on RedHat 7 but not on 8. Removes errors: rspec './spec/classes/update_spec.rb[1:9:3:1]' # ca_cert::update on redhat-8-x86_64 with force_enable set to true is expected to contain Exec[enable_ca_trust] with command => "update-ca-trust force-enable" rspec './spec/classes/update_spec.rb[1:9:4:1]' # ca_cert::update on redhat-8-x86_64 with force_enable set to false is expected to contain Exec[enable_ca_trust] with command => "update-ca-trust enable"
When remote_file got replaced by archive [1] the unit tests were not updated accordingly. Removes errors. rspec ./spec/defines/ca_spec.rb:141 # ca_cert::ca os-dependent items On Debian based systems with a remote certificate is expected to contain Remote_file[/usr/local/share/ca-certificates/Globalsign_Org_Intermediate.crt] with ensure => "present" and source => "http://secure.globalsign.com/cacert/gsorganizationvalsha2g2r1.crt" rspec ./spec/defines/ca_spec.rb:202 # ca_cert::ca os-dependent items On RedHat based systems with a remote certificate is expected to contain Remote_file[/etc/pki/ca-trust/source/anchors/Globalsign_Org_Intermediate.crt] with ensure => "present" and source => "http://secure.globalsign.com/cacert/gsorganizationvalsha2g2r1.crt" rspec ./spec/defines/ca_spec.rb:246 # ca_cert::ca os-dependent items On RedHat based systems when explicitly distrusting a certificate is expected to contain Remote_file[/etc/pki/ca-trust/source/blacklist/Globalsign_Org_Intermediate.crt] with ensure => "present" and source => "http://secure.globalsign.com/cacert/gsorganizationvalsha2g2r1.crt" rspec ./spec/defines/ca_spec.rb:273 # ca_cert::ca os-dependent items On Suse 11 based systems with a remote certificate is expected to contain Remote_file[/etc/ssl/certs/Globalsign_Org_Intermediate.pem] with ensure => "present" and source => "http://secure.globalsign.com/cacert/gsorganizationvalsha2g2r1.pem" rspec ./spec/defines/ca_spec.rb:347 # ca_cert::ca os-dependent items On Suse 12 based systems with a remote certificate is expected to contain Remote_file[/etc/pki/trust/anchors/Globalsign_Org_Intermediate.crt] with ensure => "present" and source => "http://secure.globalsign.com/cacert/gsorganizationvalsha2g2r1.crt" rspec ./spec/defines/ca_spec.rb:393 # ca_cert::ca os-dependent items On Suse 12 based systems when explicitly distrusting a certificate is expected to contain Remote_file[/etc/pki/trust/blacklist/Globalsign_Org_Intermediate.crt] with ensure => "present" and source => "http://secure.globalsign.com/cacert/gsorganizationvalsha2g2r1.crt" [1] voxpupuli#69
Phil-Friderici
force-pushed
the
pre-pdk300
branch
from
6cd5767 to
20d6ed5
Compare
|
I've just never bothered to switch these to GitHub actions (I don't use Travis anymore). Internally we use GitLab for everything, including their CI tool. I'd love to switch if you have the time to do that too. |
It's all be done in commit 0ff967d [Add GitHub actions for automatic validation & testing], after merging the tests will automatically start running :) |
|
It looks like you closed the PR, so merging isn't an option. I'm hesitant to re-open just to merge since I'm not sure why it was closed to begin with. |
This PR collects changes to prepare for an update to PDK v3.0.0.