Add option property to haproxy_listen #470
Conversation
Signed-off-by: Eric Heydrick <eheydrick@gmail.com>
ramereth
added
the
Release: Minor
| @@ -419,6 +419,13 @@ listen <%= key %> | |||
| <% if listen['default_backend'] -%> | |||
| default_backend <%= listen['default_backend'] %> | |||
| <% end %> | |||
| <% unless nil_or_empty?(listen['option']) %> | |||
| <% listen['option'].each do | option |%> | |||
| <% option.each do | option | %> | |||
There was a problem hiding this comment.
I can't say I am running haproxy in my envs, the example seems to be a flat array but it looks like we are iterating as if its an array of arrays. Can this be an array of array options? If so could we add an example to the documentation?
There was a problem hiding this comment.
Got to agree, can we confirm this is correct as it looks like a mistake?
There was a problem hiding this comment.
Yeah it does look funny. I based it on how it's done for frontend and backend. Looks like it's because :option takes an Array which then gets pushed into another array. And so the template has to iterate over both array dimensions.
We have the same situation with :acl except it gets flattened into a 1D array, hence no need for the extra iteration.
We could switch to the flatten approach for option or merge this as-is and do the cleanup later.
There was a problem hiding this comment.
Gotcha, I don't really have the time to dig into this to see why the code was written that way but it's certainly a way to do that. I lean towards using flatten as we have an established pattern and that is a little clearer as to the intent (without reading a lot of code) even if they are both equally correct/complete. That being said I could be talked into accepting it as is if other reviewers agree.
There was a problem hiding this comment.
I'm happy to switch to flatten if nobody objects.
| @@ -419,6 +419,13 @@ listen <%= key %> | |||
| <% if listen['default_backend'] -%> | |||
| default_backend <%= listen['default_backend'] %> | |||
| <% end %> | |||
| <% unless nil_or_empty?(listen['option']) %> | |||
| <% listen['option'].each do | option |%> | |||
| <% option.each do | option | %> | |||
There was a problem hiding this comment.
Got to agree, can we confirm this is correct as it looks like a mistake?
damacus
added
the
Waiting on Contributor
|
@bmhughes shall we take this piece on and make a new PR with it? Unless @eheydrick can fix the suggestions? Thanks! |
Yeh i'll take a look at this @damacus, got a feeling the nested array problem is a mistake from when I converted this over to a library only cookbook. |
|
Superceeded by #504 |
Signed-off-by: Eric Heydrick eheydrick@gmail.com
Description
Add
optionproperty tohaproxy_listenlike the frontend and backend resources already have.Issues Resolved
#469
Check List
## Unreleased