make directory mode configurable for client_body_temp_path and proxy_temp_path

[UiP9AV6Y]

Pull Request (PR) description

nginx manages the directory permissions on its own,
so the default value is undef to avoid conflicts.

This Pull Request (PR) fixes the following issues

Related to #1443

[puppet-community-rangefinder]

nginx::config is a class

that may have no external impact to Forge modules.

nginx is a class

that may have no external impact to Forge modules.

This module is declared in 9 of 578 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.

[root-expert]

I think that's a breaking change. In #1443 mode was set to 0700 to make the module idempotent. Probably this change will break this?

[UiP9AV6Y]

i could change the module parameter default value to 0700. the reason why i chose undef as default value, is, because it is the only way to ensure puppet does not manage the filemode unless explicitly told to do so. changing the default value to NotUndef would no longer allow this.

# this is the same as not configuring it at all
nginx::client_body_temp_mode: ~

i do not know in which version of nginx this was implemented, but current versions manage the filemode on its own.

this leaves us with 3 options:

leave the default value to undef

this would leave the mode management to either nginx (in newer versions)
or the system umask (in older versions)

change the default value to 0700

this would not change the behaviour of the puppet
module but would require the module user to
set this value to whatever nginx uses (in newer
versions) otherwise the puppet runs will report changes every time

implement explicit hands off logic

$real_client_body_temp_mode = $nginx::manage_client_body_temp_mode ? {
  true    => $nginx::client_body_temp_mode,
  default => undef,
}

[root-expert]

Setting this to undef will probably cause some issues as well, if the directory and the file don't exists, puppet will create them with 0644 permissions.

If the mode is omitted (or explicitly set to undef), Puppet does not enforce permissions on existing files and creates new files with permissions of 0644.
The documentation doesn't state what will happen on next Puppet run though

The weird thing is that before #1443 PR even though the parameter was omitted, Puppet still changed the mode 0755
Or maybe I'm missing something?

[UiP9AV6Y]

puppet creates directories with 0755 permissions when no mode parameter is specified

root@01b032aec128:/src# cat poc.pp 
file { '/opt/poc':
  ensure => directory,
  owner  => '0',
  group  => '0',
}

root@01b032aec128:/src# puppet apply ./poc.pp 
Notice: Compiled catalog for 01b032aec128.example.com in environment production in 0.03 seconds
Notice: /Stage[main]/Main/File[/opt/poc]/ensure: created
Notice: Applied catalog in 0.15 seconds

root@01b032aec128:/src# stat /opt/poc
  File: /opt/poc
  Size: 4096      	Blocks: 8          IO Block: 4096   directory
Device: 2ch/44d	Inode: 3020743     Links: 2
Access: (0755/drwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2021-11-27 07:05:40.301078681 +0000
Modify: 2021-11-27 07:05:40.301078681 +0000
Change: 2021-11-27 07:05:40.301078681 +0000
 Birth: -

root@01b032aec128:/src# puppet --version
7.12.1

[smortex]

+1 for setting the default value to the current default value of 0700 and not break backward compatibility. If some systems need different values, they are already affected, and their users are welcome to send e.g. hiera data for these edge cases.

[ekohl]

Please add puppet-strings @param documentation to new parameters.

[UiP9AV6Y]

is there anything else that needs to be done for this PR to be merged?

the failing CI / Puppet / 7 - Archlinux rolling (pull_request)
check seems to be unrelated to this change as far as i can tell

//cc @ekohl

[ekohl]

Please update your puppet-strings version and the regenerate REFERENCE.md. It's outputting the old format which generates a HUGE diff.

[UiP9AV6Y]

i was still using ruby 2.5; puppet-strings 3.x requires ruby2.7+, which is why my local dev env never got a newer gem version.

i have rebuilt the reference using ruby-2.7/puppet-strings-3.0.1

My concern has been addressed. I didn't look at the specific changes.