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 the docker options override configuration for traefik #100

Merged
merged 10 commits into from
Mar 23, 2023

Conversation

stepbeekio
Copy link
Contributor

See #98 for more detailed proposal. This change adds the additional_ports configuration for traefik. Includes documentation to give entrypoint examples for this escape hatch. The ruby code impacted is relatively small - we essentially just allow a user to add other published ports to the traefik container.

The obvious downside to this is that by using this configuration in isolation a user could bring down their service. Traefik wouldn't know which port to bind as the default entrypoint and would fail to serve traffic.

Added a unit test and tested locally against a running deploy.

@kjellberg
Copy link
Contributor

This can also be used to enable port 443 for https. Just a thought, why not rename additional_ports to just ports, which defaults to [80]. People messing with ports should be smart enough to include port 80 if they configure it.

traefik:
  ports:
    - 80
    - 443
    - 9000

@stepbeekio
Copy link
Contributor Author

@kjellberg That's a good point. I think it would fit better with moving away from traefik defaults too. I'll get time later today to edit.

@kjellberg
Copy link
Contributor

Actually I need this right now. Was trying to enable https for a website but I had to patch mrsk to add port 443, just 5 seconds before you created this PR. Cloudflare messes up with session cookies, using flexible ssl, so would be nice to support additional ports.

If this get merged, I'd be happy to create a patch that enables ssl with as simple as:

servers:
  web:
    hostname: example.com
    hosts:
      - 206.189.112.50
traefik:
  ports:
    - 80
    - 443
  letsencrypt:
     email: [email protected]

Another thought:

What if two different projects deploys to the same server. How do we keep the traefik config consistent? Should mrsk support this?

Project A
traefik:
  ports:
    - 80
    - 9000
    - 9001
Project B
traefik:
  ports:
    - 80
    - 8080

Deploying Project B will override ports config from Project A. I guess we already have the same problem with args.

@stepbeekio
Copy link
Contributor Author

@kjellberg Made suggested change to ports and updated README accordingly. Deployed a config using this locally and I like it.

@stepbeekio stepbeekio changed the title Added the additional_ports configuration for traefik Added the ports override configuration for traefik Mar 10, 2023
@stepbeekio stepbeekio changed the title Added the ports override configuration for traefik Added the docker options override configuration for traefik Mar 14, 2023
@stepbeekio stepbeekio force-pushed the feature/multiple-traefik-entrypoints branch from 2cac6d0 to 53046ef Compare March 14, 2023 20:11
@stepbeekio stepbeekio mentioned this pull request Mar 14, 2023
else
["--volume /var/run/docker.sock:/var/run/docker.sock"]
end
end
Copy link
Member

Choose a reason for hiding this comment

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

I was looking to have all this be generic. That there's nothing specific about publish or volumes. That it should all just be options that turn into this. Like we do for app.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad - I misunderstood. Updated to do just this.

@stepbeekio
Copy link
Contributor Author

@dhh I've edited the optionize method to allow for nested lists inside arguments that get expanded. I figured that making optionize more flexible in what it accepted would be better than introducing a one-off for traefik options. I'm not a ruby dev so have no idea if there's a smarter way to do this.

@nerdinand
Copy link

I ran into problems with this feature with the ActiveSupport main-branch. There, docker_option_args returns [], because config.raw_config.dig(:traefik, "options") returns nil for some reason.

@dhh dhh merged commit 01d6847 into basecamp:main Mar 23, 2023
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