Skip to content
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.

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

Allow values to be hashes at prepend,append,custom cfg for locations #266

Merged
merged 17 commits into from
Mar 19, 2014

Conversation

ese
Copy link
Contributor

@ese ese commented Mar 5, 2014

Some times you need to add the same parameter many times ( for example add_header or proxy_set_header) at location scope.

This workaround allow use hashes for this purpose

For example:

nginx::nginx_locations:
  'location_name':
    vhost: example.com
    location: /
    proxy: "http://unicorn"
    location_cfg_append:
      proxy_set_header:
        Host: '"$host"'
        X-Forwarded-For: '"$proxy_add_x_forwarded_for"'

result:

  location / {
    proxy_pass          http://unicorn;
    proxy_read_timeout  90;
    proxy_set_header Host "$host";
    proxy_set_header X-Forwarded-For "$proxy_add_x_forwarded_for";
  }

@jfryman
Copy link
Contributor

jfryman commented Mar 5, 2014

Wow! This is pretty awesome. I'd love to see some tests wrapped here.

@ese
Copy link
Contributor Author

ese commented Mar 6, 2014

I have been looking for but i dont understand why the tests fail.

Allow arrays to be specified as values in location_cfg_{prepend,append} and
vhost_cfg_{ssl_,}{prepend,append} parameters.
@grooverdan
Copy link
Contributor

warning this probably needs merging with #201

watch the white space and differences in tests. Its probably the cause

Resolved Conflicts:
	spec/defines/resource_location_spec.rb
	templates/vhost/vhost_location_alias.erb
	templates/vhost/vhost_location_directory.erb
	templates/vhost/vhost_location_empty.erb
	templates/vhost/vhost_location_fastcgi.erb
	templates/vhost/vhost_location_proxy.erb
	templates/vhost/vhost_location_stub_status.erb
@grooverdan
Copy link
Contributor

@ese
Copy link
Contributor Author

ese commented Mar 13, 2014

all tests works now, after a lot of pain.

I dont consider errors in ruby 1.8.7 (It is only a little different behavior in sort) becouse it has no support and no maintenance since june 2013

@jfryman
Copy link
Contributor

jfryman commented Mar 13, 2014

I'm hesitant to merge this even with the failing tests. Do you know the root cause?

@ese
Copy link
Contributor Author

ese commented Mar 13, 2014

I dont know but its possible stop support ruby 1.8.7 in tests editing .travis.yml

I can do the change and push, i think its better than merge with broken tests

@grooverdan
Copy link
Contributor

Wow. Thanks @ese. Well done and I really appreciate the effort here to fix these kinds of tests.

Looks great to me. Ready to merge @jfryman ? (and sorry for the noise in the mean time)

@jfryman
Copy link
Contributor

jfryman commented Mar 17, 2014

This is looking really good. Stellar job on both of your parts, @ese and @grooverdan! 👏

I dont consider errors in ruby 1.8.7 (It is only a little different behavior in sort) becouse it has no support and no maintenance since june 2013

I thought about this point over the weekend. I'm 👍 on removing the 1.8.7 tests. However, this should be documented in the readme. Would one of you add this?

Also, does this PR make #273 necessary?

Finally, this functionality should be more visible. Can you provide some commentary on when to use this, along with some examples?

Once this lands, I'll also cut a new release for the forge.

grooverdan added a commit to grooverdan/puppetlabs-nginx that referenced this pull request Mar 17, 2014
Set minimum version as per voxpupuli#266
jfryman pushed a commit that referenced this pull request Mar 19, 2014
Allow values to be hashes at prepend,append,custom cfg for locations
@jfryman jfryman merged commit 8173432 into voxpupuli:master Mar 19, 2014
@mschuett
Copy link

mschuett commented Apr 1, 2014

Will there be any way to insert custom text?

I really liked the previous solution #259 because it allowed me to add a custom if condition into templates.
With the current version this is no longer possible, as the added ';' breaks nginx syntax.

@grooverdan
Copy link
Contributor

vhost_cfg_prepend => { '' => 'if ($slow) { limit_rate 10k; break; } set $dummy value' },

Not pretty I agree. Would a condition if the text ends in '}' then no ';' is added? Is this even possible?

@mschuett
Copy link

mschuett commented Apr 2, 2014

Actually I am not sure why everything needs to fit into a hash... The fact that it is a custom attribute already means it does not fit into the common structure.

Right now we are happily running commit 7cf6dac. -- If necessary I will just fork a branch with a custom_custom_cfg_prepend attribute.

<% if @location_custom_cfg_prepend -%><% @location_custom_cfg_prepend.each do |key,value| -%>
<% if value.is_a?(Hash) -%><% value.each do |subkey,subvalue| -%>
<% Array(subvalue).each do |asubvalue| -%>
<%= key %> <%= subkey %> <%= asubvalue %>;
Copy link
Contributor

Choose a reason for hiding this comment

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

FUCK!!!
Why hash? Why ';' ?
It's options for logical structures such as if, whithout ';'

[location_custom_cfg_prepend] - Expects a array with extra directives
to put before anything else inside location (used with all other types
except custom_cfg). Used for logical structures such as if.
[location_custom_cfg_append] - Expects a array with extra directives
to put before anything else inside location (used with all other types
except custom_cfg). Used for logical structures such as if.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove char ';' in PR #295

cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Oct 23, 2017
Set minimum version as per voxpupuli#266
cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Oct 23, 2017
Allow values to be hashes at prepend,append,custom cfg for locations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants