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 ability to add nginx conditionals #267

Closed
wants to merge 4 commits into from

Conversation

bashtoni
Copy link

@bashtoni bashtoni commented Mar 5, 2014

This change allows conditionals to be added to nginx vhost templates.

Our specific use case for this is to set the HTTPS environment variable for SSL connections offloaded to the load balancer.

@carroarmato0
Copy link
Contributor

Really love to see this merged. Need this for some deployments of Gallery3 which needs it.

@jfryman
Copy link
Contributor

jfryman commented Jun 24, 2014

@bashtoni can you please rebase?

@@ -32,5 +32,10 @@
<% if @location_custom_cfg_append -%><% @location_custom_cfg_append.each do |value| -%>
<%= value %>
<% end -%><% end -%>

<% @location_conditions.each do |condition| -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

note: after you rebase, this should go in the templates/vhost/location_header.erb file.

@rabbitt
Copy link
Contributor

rabbitt commented Jun 25, 2014

after looking this over a bit more, I noticed it was missing a few things:

  • $location_conditionals wasn't passed to the default location (see manifests/resource/vhost.pp#L384)
  • nginx::resource::location was missing the conditionals parameter
  • no specs were updated to test the changes

I've created another PR (#364) that adds raw_prepend/raw_append to vhosts and locations (in addition to a bit of spec cleanup). It is meant as a very generic (hence raw) prepend/append directive and would likely solve the same problems this PR does.

@bashtoni
Copy link
Author

@jfryman closing this; IMO #364 fixes this problem in a better way.

@bashtoni bashtoni closed this Jun 27, 2014
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.

4 participants