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

Add additional config to the locations resource and fix set_header in vhost resource #289

Merged
merged 1 commit into from
Apr 9, 2014

Conversation

b4ldr
Copy link
Member

@b4ldr b4ldr commented Apr 2, 2014

Following parameters where added to the location resource. These
already existed in the vhost resource so the same logic and defaults
where used

  • proxy_set_header
  • proxy_connect_timeout
  • proxy_redirect

The set_header variable expectes a hash of headers to set. Puppet is
does not allow hash keys to have a hyphen in the name or begin with an
capital leter. As (i belive) all headers start with a
capital letter this limitation means you can not set any header.
further more the majority of headers also contain an underscore.

I have included a patch which allows a key to be passed all in lower case and
use underscores in place of hyphens. The template will then make the
conversion. e.g. to set the header Accept-Charset: utf-8. one would
pass

set_header => { accept_charset => 'utf-8' }

@jfryman
Copy link
Contributor

jfryman commented Apr 6, 2014

If you hard quote the keys, doesn't this become unnecessary?

set_header => { 'Accept-Charset' => 'utf-8' }

I'd be concerned that this may cause unintended side-effects with keys that shouldn't be modified.

Following parameters where added to the location resourse.  These
already existsed in the vhost resource so the same logic and defaults
where used
  * proxy_set_header
  * proxy_connect_timeout
  * proxy_redirect
@b4ldr
Copy link
Member Author

b4ldr commented Apr 9, 2014

Hi jfryman,

I didn't know about that syntax, thats works fine and i agree is a better way to go. I have removed the set_header stuff from the pull request.

@jfryman
Copy link
Contributor

jfryman commented Apr 9, 2014

Good deal! Thanks for the additions!

jfryman pushed a commit that referenced this pull request Apr 9, 2014
Add additional config to the locations resource and fix set_header in vhost resource
@jfryman jfryman merged commit 20a7ea0 into voxpupuli:master Apr 9, 2014
@ZhukV
Copy link

ZhukV commented Jun 21, 2015

👍

cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Oct 23, 2017
Add additional config to the locations resource and fix set_header in vhost resource
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.

None yet

3 participants