Skip to content

Use a stricter data type on apache::vhost::aliases #2253

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 1 commit into from
Jun 23, 2022
Merged

Use a stricter data type on apache::vhost::aliases #2253

merged 1 commit into from
Jun 23, 2022

Conversation

ekohl
Copy link
Collaborator

@ekohl ekohl commented Jun 23, 2022

This also implements a real test on the rendered template. It appears it was passing in something invalid ever since it was introduced in f1d64a0.

@ekohl ekohl requested a review from a team as a code owner June 23, 2022 15:31
@puppet-community-rangefinder
Copy link

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.

@ekohl ekohl mentioned this pull request Jun 23, 2022
Closed
ekohl
ekohl commented Jun 23, 2022
View reviewed changes
manifests/vhost.pp
@@ -1803,7 +1803,7 @@
Optional[Array[Hash]] $access_logs = undef,
Boolean $use_servername_for_filenames = false,
Boolean $use_port_for_filenames = false,
Optional[Variant[Array[Hash],Hash,String]] $aliases = undef,
Array[Hash[String[1], String[1]]] $aliases = [],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be even better if we use a Struct to describe the possible keys and enforce that path is present, but this is good enough for now.

@david22swan
Copy link
Member

david22swan commented Jun 23, 2022 •
edited
Loading

@ekohl Look's like the rubocop failure is still occurring, swapping the position of aliases and putting a another variable, or a few, between it and directories should fix it, i.e.:

              'log_level'                   => 'crit',
              'aliases'                     => [
                {
                  'alias' => '/image',
                  'path'  => '/rspec/image',
                },
              ],
              'access_log'                  => false,
              'access_log_file'             => 'httpd_access_log',
              'access_log_syslog'           => true,
              'access_log_format'           => '%h %l %u %t \"%r\" %>s %b',
              'access_log_env_var'          => '',
              'directories'                 => [
                {
                  'path'     => '/var/www/files',

@ekohl
Use a stricter data type on apache::vhost::aliases
a18271b 
This also implements a real test on the rendered template. It appears it
was passing in something invalid ever since it was introduced in
f1d64a0.

The test block is moved because Rubocop didn't quite understand it
otherwise.
@ekohl ekohl force-pushed the stricter-aliases-parameter branch from 67a35d3 to a18271b Compare June 23, 2022 16:43
@ekohl
Copy link
Collaborator Author

ekohl commented Jun 23, 2022

Looks like it passes that now. Let's see for the real unit tests.

david22swan
david22swan approved these changes Jun 23, 2022
View reviewed changes
Copy link
Member

@david22swan david22swan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@david22swan
Copy link
Member

Got one transient failure and one setup failure, but happy to merge :)
Looking forward to getting the next two in as well :)

@david22swan david22swan merged commit 3dd3b2a into puppetlabs:main Jun 23, 2022
@ekohl ekohl deleted the stricter-aliases-parameter branch June 23, 2022 17:38
@rrotter rrotter mentioned this pull request Sep 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@david22swan david22swan david22swan approved these changes

Assignees
No one assigned
Labels
backwards-incompatible community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ekohl @david22swan @ia-content