Better data types on apache::vhost parameters #2252

ekohl · 2022-06-21T08:39:19Z

This tightens data types where needed and does some additional minor cleanups around module inclusion. See individual commits for details.

Replaces #2251 which had some CI problems. No actual difference in code.

They now both only accept arrays of non-empty strings. The default is set to an empty array, which behaves as if it's not set.

This was previously filtered out but it's best if the information simply never is entered in the first place. The include is also simplified and only applied if the vhost is present.

Ideally a Struct would be used instead of Hash for $rewrites, but this is a closer description of the real data type.

puppet-community-rangefinder · 2022-06-21T08:39:25Z

apache::vhost is a type

Breaking changes to this file WILL impact these 128 modules (exact match):

Breaking changes to this file MAY impact these 35 modules (near match):

This module is declared in 174 of 579 indexed public `Puppetfiles`.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

david22swan · 2022-06-21T13:20:39Z

Look's like the spec tests are still having the issue.
Not sure what could be causing it, can you remember what the last thing that you changed before this started happening was?

ekohl · 2022-06-21T13:33:19Z

I now pushed now without the last commit, see if that helps. I'll just keep removing commits until I find the one that breaks. If I could run it locally I'd just use git bisect but still blocked on https://tickets.puppetlabs.com/browse/MODULES-11161.

david22swan · 2022-06-21T15:15:34Z

Have found the error, it look's like my auto spec command doesn't run the define test's, gonna have to fix that, so i missed it the last time.

  1) apache::vhost os-independent items on oraclelinux-6-x86_64  set everything! is expected to compile into a catalogue without dependency cycles
     Failure/Error: it { is_expected.to compile }
       error during compilation: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Unknown variable: '_access_logs'. (file: /Users/david.swan/Github/puppetlabs-apache/spec/fixtures/modules/apache/manifests/vhost.pp, line: 2467, column: 13) (line: 3) on node david.swan-c02l57z6fft1
     # ./spec/defines/vhost_spec.rb:548:in `block (6 levels) in <top (required)>'
     # ./.bundle/ruby/2.6.0/bin/rspec:23:in `load'
     # ./.bundle/ruby/2.6.0/bin/rspec:23:in `<top (required)>'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/cli/exec.rb:74:in `load'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/cli/exec.rb:74:in `kernel_load'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/cli/exec.rb:28:in `run'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/cli.rb:463:in `exec'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/cli.rb:27:in `dispatch'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/cli.rb:18:in `start'
     # /Users/david.swan/.rbenv/versions/2.6.3/lib/ruby/2.6.0/bundler/friendly_errors.rb:124:in `with_friendly_errors'
     # /Users/david.swan/.rbenv/versions/2.6.3/bin/bundle:23:in `load'
     # /Users/david.swan/.rbenv/versions/2.6.3/bin/bundle:23:in `<main>'

ekohl · 2022-06-21T15:18:14Z

Oh, that may explain it. I've seen that if a test fails it's slow to handle it and this would affect a lot of tests so there's a lot of errors. Thanks, will fix!

This always required an array of hashes and closer matches the actual behavior. It also fixes the check on the template use by checking the correct variable. bb96180 broke this. Fixes: bb96180

This also pulls in mod_jk if needed.

ekohl · 2022-06-21T15:28:44Z

I see EL acceptance tests fail across multiple repositories so there is some deeper problem. Are you aware of that?

david22swan

LGTM
And yes we are aware, the failures on Puppet 6 are unrelated and come from further up the pipeline.
So gonna go ahead and merge :)
Thanks for the work :)

ekohl added 3 commits June 15, 2022 18:39

Make headers and request_headers arrays

2f6f6f3

They now both only accept arrays of non-empty strings. The default is set to an empty array, which behaves as if it's not set.

Stricter data type on apache::vhost::filters

d00aabb

This was previously filtered out but it's best if the information simply never is entered in the first place. The include is also simplified and only applied if the vhost is present.

Stricter data types on apache::vhost::rewrite*

7b22dae

Ideally a Struct would be used instead of Hash for $rewrites, but this is a closer description of the real data type.

ekohl requested a review from a team as a code owner June 21, 2022 08:39

ekohl mentioned this pull request Jun 21, 2022

Better data types on apache::vhost parameters #2251

Closed

ia-content added the community label Jun 21, 2022

ekohl force-pushed the better-vhost-data-types branch from f73fdcc to 88d7b26 Compare June 21, 2022 13:31

ekohl added 3 commits June 21, 2022 17:23

Stricter data type on apache::vhost::access_logs

56c9e1e

This always required an array of hashes and closer matches the actual behavior. It also fixes the check on the template use by checking the correct variable. bb96180 broke this. Fixes: bb96180

Stricter data type on apache::vhost::modsec_disable_ips

bcd35a7

Stricter data type on apache::vhost::jk_mounts

8c51550

This also pulls in mod_jk if needed.

ekohl force-pushed the better-vhost-data-types branch from 88d7b26 to 8c51550 Compare June 21, 2022 15:24

david22swan approved these changes Jun 22, 2022

View reviewed changes

david22swan merged commit 77d3e6c into puppetlabs:main Jun 22, 2022

ekohl deleted the better-vhost-data-types branch June 22, 2022 09:20

ekohl linked an issue Jun 22, 2022 that may be closed by this pull request

Parameter type for vhost.pp $headers is incorrect #2247

Closed

ekohl mentioned this pull request Jun 22, 2022

Parameter type for vhost.pp $headers is incorrect #2247

Closed

david22swan added the bugfix label Aug 5, 2022

sircubbi mentioned this pull request Aug 10, 2022

fix duplicate definition of auth_basic-mod #2287

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better data types on apache::vhost parameters #2252

Better data types on apache::vhost parameters #2252

ekohl commented Jun 21, 2022

puppet-community-rangefinder bot commented Jun 21, 2022

david22swan commented Jun 21, 2022

ekohl commented Jun 21, 2022

david22swan commented Jun 21, 2022

ekohl commented Jun 21, 2022

ekohl commented Jun 21, 2022

david22swan left a comment •

edited

Loading

Better data types on apache::vhost parameters #2252

Better data types on apache::vhost parameters #2252

Conversation

ekohl commented Jun 21, 2022

puppet-community-rangefinder bot commented Jun 21, 2022

apache::vhost is a type

This module is declared in 174 of 579 indexed public Puppetfiles.

david22swan commented Jun 21, 2022

ekohl commented Jun 21, 2022

david22swan commented Jun 21, 2022

ekohl commented Jun 21, 2022

ekohl commented Jun 21, 2022

david22swan left a comment • edited Loading

Choose a reason for hiding this comment

This module is declared in 174 of 579 indexed public `Puppetfiles`.

david22swan left a comment •

edited

Loading