Add nginx_http_extra_params variable in default vars

[sysarcher]

Hello Everyone, I would like to propose an additional variable in the defaults/main.yml file called nginx_http_extra_params.

Purpose

If someone wants to retain the defaults in jdauphant.nginx/defaults/main.yml and bring in additional variables to http_params, he should be able to use the nginx_http_extra_params variable. This would append to http_params retaining the defaults.

This would also help us in maintaining a common set of group_vars for nginx config and have additional params when needed for our deploys.

Example

# retain defaults and add additional `client_max_body_size` param
- role: jdauphant.nginx
      nginx_http_extra_params:
       - client_max_body_size 200M

[jdauphant]

Hello @shrmrf,
Thanks for the proposition.
What do you think by somethings like that:

• We create nginx_http_params_defaults in vars/main.yml and we put all the default we already have
• In defaults/main.yml we change to nginx_http_params: {{ nginx_http_params_defaults }}

By doing that people can use the power of ansible to combine or not parameters by doing something like that: nginx_http_params: {{ nginx_http_params_defaults + my_extra_params }}

[sysarcher]

Yeah, makes sense. This is what I proposed too but instead of concatenating the two lists to create the parameters, I just added another one. Both are good IMO. (though nginx_http_extra_params is a better name than my_extra_params hehe)

I can add it to the pull request if you like but I was scared that you will break code of people already abusing the nginx_http_params somehow.

It makes more sense to have the operation nginx_http_params: {{ nginx_http_params_defaults + my_extra_params }} done in /vars instead of /defaults if we take the addition approach.

Let me know what you think and I can update my pull request.

[jdauphant]

There is little misunderstand I think.
When I was talking about nginx_http_params: {{ nginx_http_params_defaults + my_extra_params }}, it's a possible usage by the user and "my_extra_params" will be an user variable not a variable from this role.

By implementing just:

• nginx_http_params_defaults in vars/main.yml and we put all the defaults params we already have
• In defaults/main.yml we change nginx_http_params to nginx_http_params: {{ nginx_http_params_defaults }}

We normally have retrocompatibility, we limit the complexity of the project and we have possibility to manage extra params in the playbook easely (for people that need advance usage).

[sysarcher]

hmmm.... Okay. I think this is better. So a usage will look like:

vars:
  - my_extra_params:
      - client_max_body_size 200M
# retain defaults and add additional `client_max_body_size` param
- role: jdauphant.nginx
      nginx_http_params: {{ nginx_http_params_defaults + my_extra_params }}

This is a good solution too. Agreed. Should I update my pull request?

[jdauphant]

yes, it's exactly that !
If you can update the PR, it will be perfect :)

Thanks for your help

[sysarcher]

Hello @jdauphant updated the pull request to the best of my understanding.

[jdauphant]

Thanks for the change, I made a few comments

[jdauphant]

We have two number "4)"

[sysarcher]

updated wth the requests!

[jdauphant]

Kudos for the documentation !

[jdauphant]

I don't think this syntax is good, it break on CI : https://travis-ci.org/jdauphant/ansible-role-nginx/builds/189216001#L1713

It will be more : nginx_http_params: "{{ nginx_http_default_params }}"

[jdauphant]

We can put this variable in the file "vars/main.yml" instead, it should not be changeable by the user

[sysarcher]

woops... sorry. I'll fix these asap . Did a quick/dirty job that fixed my issue and pull requested it naively.

[sysarcher]

I updated the pull request. Also pulled in your changes from yesterday.

[jdauphant]

@shrmrf Thanks for your help