DiUS / pact_broker-docker

'Dockerised' pact broker
http://pact.io
MIT License
76 stars 102 forks source link

Large pacts over 1mb are rejected by nginx config #89

Closed YOU54F closed 5 years ago

YOU54F commented 5 years ago

Hey,

We have a pact test where the provider service is expecting a base64 encoded PDF to be received in their request. My completed pact contract is 2.6mb and is rejected by nginx config, when attempting to push up to the pact-broker.

    Could not publish pact:
    Failed to publish xxxxx-service/xxxxx-service pact due to error: PactBroker::Client::Error - <html>
    <head><title>413 Request Entity Too Large</title></head>
    <body bgcolor="white">
    <center><h1>413 Request Entity Too Large</h1></center>
    <hr><center>nginx/1.14.0 (Ubuntu)</center>
    </body>
    </html>

Looking at the nginx docs, the client_max_body_size is 1mb

I have been able to get this to work by changing client_max_body_size to 10M

pact_broker-docker/container/etc/nginx/sites-enabled/webapp.conf

From

server {
    listen 80;
    # server_name pact-broker;
    root /home/app/pact_broker/public;
    passenger_enabled on;
    passenger_user app;

    # If this is a Ruby app, specify a Ruby version:
    passenger_ruby /usr/bin/ruby;
}

To

server {
    listen 80;
    # server_name pact-broker;
    root /home/app/pact_broker/public;
    client_max_body_size 10M;
    passenger_enabled on;
    passenger_user app;

    # If this is a Ruby app, specify a Ruby version:
    passenger_ruby /usr/bin/ruby;
}

I have tested this locally by spinning up the repo, with the above change, and I can publish my pact and view it on the broker.

Would you be happy adding it into the main image? If not, not problem, I will just set up something up to subscribe to updates to the pushed docker image, and build our own with the updated nginx.conf.

Many thanks in advance!

YOU54F commented 5 years ago

The image I have created with the change in, is published here https://hub.docker.com/r/you54f/pact-broker-nginx. 👍

bethesque commented 5 years ago

Hey, sorry. I only just saw this. Just wondering what, if any, negative implications there might be for increasing it to 10MB?

YOU54F commented 5 years ago

No worries duck,

I'll do some reading up on any potential side-effects, I think setting it too high could open a web-service up to DOS attacks, but with it only being 10mb, I don't think it makes much of an issue.

We have slimmed down the pacts now by using a much smaller encoded base64 string, so our pacts are no longer over 1mb

I've recently been trialling pb with puma (https://github.com/YOU54F/pact-standalone-docker-slim/tree/master/pact_broker_puma) and currently messing about with trying to get pact broker working in AWS lambda.

Have any of your other customers ever had issues with large pacts? Looking at the issues logs, I seem to the only one who has raised this?

bethesque commented 5 years ago

I'm thinking this may be redundant now the new puma version is out? Once it's stable, I'll be deprecating this image.

bethesque commented 5 years ago

Oh, and yes, you're the first to raise this!

YOU54F commented 5 years ago

Thanks @bethesque, I’ll trial the new puma based image at work next week and pass back any feedback 👌