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

Added support for reverse proxy #147

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@karser
Copy link

commented May 1, 2019

The previous discussion https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/killbilling-users/JSmilPR1csk/D9RBlXfMCAAJ

Usage example:

    environment:
        - KILLBILL_TOMCAT_PROXY_NAME=my-killbill.domain
        - KILLBILL_TOMCAT_PROXY_PORT=443
        - KILLBILL_TOMCAT_SCHEME=https

Questions:

  • Do these variables have to be added to README.adoc despite they don't have defaults?
  • How can I build the image and test this PR?
@pierre

This comment has been minimized.

Copy link
Member

commented May 2, 2019

  • Do these variables have to be added to README.adoc despite they don't have defaults?

Yes, that way we keep track of all the environment variables available.

  • How can I build the image and test this PR?

To rebuild killbill/killbill:latest from scratch, run (under docker/):

make -e TARGET=base rebuild
make -e TARGET=killbill rebuild

I'm quite surprised you don't need any of the settings from https://github.com/killbill/killbill-docs/blob/v3/userguide/platform/userguide_deployment.adoc#x-forwarded-headers-support (e.g. org.killbill.jaxrs.location.full.url or org.killbill.jaxrs.location.host) which are need (among other things) to populate the Location header. I'd suggest testing with Kaui to verify it indeed works as expected.

@karser

This comment has been minimized.

Copy link
Author

commented May 2, 2019

Thanks, I'll build the image locally when I get to it.

I'm quite surprised you don't need any of the settings ... to populate the Location header

That's exactly what was failing when I switched to reverse proxy - the RedirectOnPostMiddleware was redirecting to http and tests didn't pass.
After my changes in Connector tests passed. KAUI works as well, at least I'm able to create new tenant and account. I can provide access to KAUI privately if you'd like to test it yourself).
image

@pierre

This comment has been minimized.

Copy link
Member

commented May 3, 2019

I was able to setup Traefik locally and try it out.

  1. With the default Kill Bill configuration, creating an account leads to the wrong Location header as expected (e.g. location: http://my-awesome-app.org:80/1.0/kb/accounts/566c9c98-9686-4aa4-b454-040f41d354e4)
  2. By adding the RemoteIpValve config from http://docs.killbill.io/0.20/userguide_deployment.html#_x_forwarded_headers_support, the Location header is correct (e.g. location: https://my-awesome-app.org:443/1.0/kb/accounts/a781f31e-34cc-4053-b8ee-4da34a0608a0)

Looking at the logs, it seems that Traefik does the right thing and adds the necessary headers out of the box:

killbill_1       | 1 > x-forwarded-for: 192.168.99.1
killbill_1       | 1 > x-forwarded-host: my-awesome-app.org
killbill_1       | 1 > x-forwarded-port: 443
killbill_1       | 1 > x-forwarded-proto: https
killbill_1       | 1 > x-forwarded-server: 2cc6d1dd40a3

So, it doesn't look like we need to do anything else?

Even better, it looks like we could simply add the RemoteIpValve config in the default image as it seems to be doing the right thing when the headers are absent. If you confirm this would make it work out of the box for your setup, I'll make the change.

cURL

curl -v \
     -k \
     -H Host:my-awesome-app.org \
     -X POST \
     -u admin:password \
     -H "X-Killbill-ApiKey: bob" \
     -H "X-Killbill-ApiSecret: lazar" \
     -H "Content-Type: application/json" \
     -H "X-Killbill-CreatedBy: demo" \
     -H "X-Killbill-Reason: demo" \
     -H "X-Killbill-Comment: demo" \
     -d "{}" \
     "https://192.168.99.100/1.0/kb/accounts" 

Configuration files

traefik.toml

debug = false

logLevel = "ERROR"
defaultEntryPoints = ["https"]

[entryPoints]
  [entryPoints.https]
  address = ":443"
  [entryPoints.https.tls]

[retry]

[docker]
endpoint = "unix:///var/run/docker.sock"
domain = "my-awesome-app.org"
watch = true
exposedByDefault = false

[acme]
email = "your-email-here@my-awesome-app.org"
storage = "acme.json"
entryPoint = "https"
onHostRule = true
[acme.httpChallenge]
entryPoint = "http"

docker-compose.yml

version: '3.2'
volumes:
  db:
networks:
  kb:
    external: true
services:
  killbill:
    image: killbill/killbill:0.20.10
    expose:
      - 8080
    environment:
      - KILLBILL_DAO_URL=jdbc:mysql://db:3306/killbill
      - KILLBILL_DAO_USER=root
      - KILLBILL_DAO_PASSWORD=killbill
    networks:
      - kb
      - default
    labels:
      - "traefik.docker.network=kb"
      - "traefik.enable=true"
      - "traefik.basic.frontend.rule=Host:my-awesome-app.org"
      - "traefik.basic.port=8080"
      - "traefik.basic.protocol=http"
  reverse-proxy:
    image: traefik
    command: --api --docker
    ports:
      - 443:443
      - 8080:8080
    networks:
      - kb
    volumes:
      - /var/run/docker.sock:/var/run/docker.sock
      - /traefik.toml:/traefik.toml
      - /acme.json:/acme.json
  db:
    image: killbill/mariadb:0.20
    volumes:
      - type: volume
        source: db
        target: /var/lib/mysql
    expose:
      - 3306
    environment:
      - MYSQL_ROOT_PASSWORD=killbill
    networks:
      - default

@karser karser force-pushed the karser:tomcat-reverse-proxy branch from ecefcc3 to 19bfc1f May 3, 2019

@karser

This comment has been minimized.

Copy link
Author

commented May 3, 2019

Indeed, valve header makes more sense, since no additional setup is required.
I can confirm that valve header works for me behind traefik.

pierre added a commit that referenced this pull request May 3, 2019

ansible: enable RemoteIpValve by default
See discussion at #147.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>

pierre added a commit to killbill/killbill-docs that referenced this pull request May 3, 2019

deployment: update doc for SSL termination
See killbill/killbill-cloud#147.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
@pierre

This comment has been minimized.

Copy link
Member

commented May 3, 2019

I can confirm that valve header works for me behind traefik.

Great, thanks for confirming! I've updated the images on Docker hub.

See killbill/killbill-docs#66 for the docs update.

@pierre pierre closed this May 3, 2019

@karser

This comment has been minimized.

Copy link
Author

commented May 3, 2019

Pulled the image - now the location is correct out of the box. Thank you @pierre

@karser karser deleted the karser:tomcat-reverse-proxy branch May 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.