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

Fix a config error in the last sample in README #226

Merged
merged 1 commit into from
Jan 8, 2014

Conversation

huandu
Copy link
Contributor

@huandu huandu commented Jan 7, 2014

Passenger cgi param was wrong. See diff for details.

@3flex
Copy link
Contributor

3flex commented Jan 7, 2014

@huandu, I'm assuming this is related to http://docs.puppetlabs.com/guides/passenger.html#notes-on-ssl-verification. It would have been good to reference this for context in your issue description.

Having said that this looks correct. The current example in README would require a change to puppet.conf which isn't mentioned so the example should be corrected to work properly with the defaults.

@huandu
Copy link
Contributor Author

huandu commented Jan 8, 2014

@3flex The current example looks right but doesn't work on my server. Nginx always returns 403 errors for urls which restrict access by certname, e.g. /report/*. I investigated this issue for several hours yesterday and finally found this solution.

If puppet doc is correct, it could be a bug or regression in puppet v3.4.2, which is the version I'm using. However, it seems using HTTP_X_CLIENT_DN/HTTP_X_CLIENT_VERIFY is always safer. I'd recommend to use them instead of the value in current example.

@3flex
Copy link
Contributor

3flex commented Jan 8, 2014

@huandu Sorry, I wasn't clear! When I said the "current example" which would also need a puppet.conf update I was talking about what's in master, not your proposal. When I said "this looks correct" I was talking about your PR.

Your proposal looks good and looks like it aligns with Puppet's recommended SSL config for Passenger. 👍

@huandu
Copy link
Contributor Author

huandu commented Jan 8, 2014

@3flex Ah, well, I misunderstood your point. My bad and thanks for your explanation!

@jfryman
Copy link
Contributor

jfryman commented Jan 8, 2014

Thanks for the code! 🙇

jfryman pushed a commit that referenced this pull request Jan 8, 2014
Fix a config error in the last sample in README
@jfryman jfryman merged commit cd06f35 into voxpupuli:master Jan 8, 2014
@jfryman
Copy link
Contributor

jfryman commented Jan 8, 2014

Great discussion. I like that we're talking about better understanding of the module and how changes might have impact. Helps tremendously to consider the stability of this module. 🍻

cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Oct 23, 2017
Fix a config error in the last sample in README
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.

3 participants