puppet-nginx icon indicating copy to clipboard operation
puppet-nginx copied to clipboard

`nginx::resource::location` says `location_custom_cfg_prepend` takes an array, but the code doesn't work

Open ElvenSpellmaker opened this issue 6 years ago • 0 comments

Hi there,

The parameter location_custom_cfg_prepend says that it takes an array of values, and is the recommended way to prepend an if statement. Shown here: https://github.com/voxpupuli/puppet-nginx/blob/v0.12.0/manifests/resource/location.pp#L69

However, when I supply the parameter to my location:

    location_custom_cfg_prepend => [
      'if ($request_method != POST) { return 405; }',
    ],

I get no output at all in my config file.

The problem code seems to be here: https://github.com/voxpupuli/puppet-nginx/blob/v0.12.0/templates/server/location_header.erb#L37-L51

On line 46 (https://github.com/voxpupuli/puppet-nginx/blob/v0.12.0/templates/server/location_header.erb#L46) there is a loop over the array, using the value parameter.

This is incorrect as the value parameter isn't set for arrays (each on arrays sets the first parameter only, in this case the key), and so an array supplied to this is ignored because the value contains nothing. If I change this to a key however by using Array(key).each do |asubvalue|, I get the output twice obviously. 🤔

I assume this affects the other cfg functions as I've spotted the code below, and elsewhere for the append.

The only way to get this to work is to do the following and convert this to a hash with an array in it, which is pretty ugly:

    @location_custom_cfg_prepend = {
      'if' => ['($request_method != POST) { return 405; }'],
    }

This has been raised before in a very concise way about a different parameter: https://github.com/voxpupuli/puppet-nginx/issues/1173, but seeing as the class doc for this parameter clearly states it takes an array, and the code looks like it should be able to, but doesn't quite handle it correctly, it seems like a bug to me.

ElvenSpellmaker avatar May 21 '18 17:05 ElvenSpellmaker