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 option to disable SSL in Jetty #52

Merged

Conversation

christianberg
Copy link
Contributor

This patch introduces the optional parameter $disable_ssl, which
defaults to false. If set to true, the settings ssl-host and ssl-port
are completely removed from the Jetty section of the PuppetDB config
files.

This disables serving of HTTPS requests by PuppetDB, which can be useful
when SSL handling is offloaded to a reverse proxy server like Apache or
Nginx, as suggested in the PuppetDB documentation (see
http://docs.puppetlabs.com/puppetdb/1.2/connect_puppet_apply.html#option-a-set-up-an-ssl-proxy-for-puppetdb).

This patch introduces the optional parameter $disable_ssl, which
defaults to false. If set to true, the settings ssl-host and ssl-port
are completely removed from the Jetty section of the PuppetDB config
files.

This disables serving of HTTPS requests by PuppetDB, which can be useful
when SSL handling is offloaded to a reverse proxy server like Apache or
Nginx, as suggested in the PuppetDB documentation (see
http://docs.puppetlabs.com/puppetdb/1.2/connect_puppet_apply.html#option-a-set-up-an-ssl-proxy-for-puppetdb).
@kbarber
Copy link
Contributor

kbarber commented Apr 9, 2013

@christianberg thanks for contributing ...

This will need a rebase however.

@christianberg
Copy link
Contributor Author

Hi Ken,

thanks for your feedback! I meant to add tests, but held back because there
were none so far. Great that some have been added.

I'll add tests and update my pull request.

Christian

On Tue, Apr 9, 2013 at 2:37 PM, Ken Barber notifications@github.com wrote:

@christianberg https://github.com/christianberg thanks for contributing
...

This will need a rebase however, and some unit tests would be great - some
initial rspec-puppet tests were just merged in so you can use them as a
basis.


Reply to this email directly or view it on GitHubhttps://github.com//pull/52#issuecomment-16109850
.

@kbarber
Copy link
Contributor

kbarber commented Apr 10, 2013

@christianberg this needs a rebase now I've merged in the tests. Looking good though. I'm not sure if travis reinits the tests again on an open PR, try and do a force push and we'll see what happens.

@christianberg
Copy link
Contributor Author

It should merge cleanly with the other branch, but I'll push again to
trigger travis.

Am Mittwoch, 10. April 2013 schrieb Ken Barber :

@christianberg https://github.com/christianberg this needs a rebase now
I've merged in the tests. Looking good though. I'm not sure if travis
reinits the tests again on an open PR, try and do a force push and we'll
see what happens.


Reply to this email directly or view it on GitHubhttps://github.com//pull/52#issuecomment-16171307
.

@kbarber
Copy link
Contributor

kbarber commented Apr 10, 2013

@christianberg nah it won't merge without a rebase, look at the github error "This pull request cannot be automatically merged.". I think its because your PR had the other commits now already committed?

@christianberg
Copy link
Contributor Author

Sorry, I wasn't at my computer when I made that claim. :-) It merged cleanly when I tested it locally yesterday, but I guess that was before I made that trailing comma fix.

Tests are green now.

@kbarber
Copy link
Contributor

kbarber commented Apr 10, 2013

@christianberg there is 1 more thing, can you add the docs for this new param to the README.md as well?

@christianberg
Copy link
Contributor Author

Of course, done!

kbarber added a commit that referenced this pull request Apr 10, 2013
…sable_ssl

Add option to disable SSL in Jetty
@kbarber kbarber merged commit 340d890 into puppetlabs:master Apr 10, 2013
@kbarber kbarber mentioned this pull request May 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants