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

Consolidated proxy-related configuration options and constants under Kamal::Configuration::Proxy #1013

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kpumuk
Copy link
Contributor

@kpumuk kpumuk commented Sep 30, 2024

Currently, some configuration options for kamal-proxy are stored in the Kamal::Configuration::Proxy, but some are in the Kamal::Configuration itself, and prefixed with proxy_. This inconsistency leads to a harder to understand codebase, but also limits refactoring options.

For example, making proxy image configurable would allow to test Kamal with a custom kamal-proxy image.

@kpumuk kpumuk marked this pull request as draft September 30, 2024 21:48
@kpumuk kpumuk marked this pull request as ready for review September 30, 2024 21:58
@djmb
Copy link
Collaborator

djmb commented Oct 1, 2024

Thanks for this PR, @kpumuk!

There's two different types of configuration here - app level deploy time configuration for the proxy (i.e. everything under the proxy key in deploy.yml) and then static boot time configuration for the proxy.

Maybe the stuff that you are moving from Kamal::Configuration could go in a new Kamal::Configuration::KamalProxy class instead?

@kpumuk
Copy link
Contributor Author

kpumuk commented Oct 1, 2024

Thanks for the response. It is a very interesting problem to deal with the nature of Kamal proxy, that is at the same time global (as in shared between multiple processes) and app-specific (as in routing configuration).

I wanted to be able to configure custom container image, but now that you pointed out the difference, it might be a footgun to allow to do so. E.g. if somebody has two apps deployed, and one has a config like:

proxy:
  image: doodle/kamal-proxy:v15.0.0
  ssl: true

... apps will fight against each other on kamal proxy reboot. On the other hand, currently one needs to edit source code of Kamal to test against a custom build of kamal-proxy.

@djmb
Copy link
Collaborator

djmb commented Oct 1, 2024

There's a kamal proxy boot_config command for the global config. It stores it on each server we are deploying to, since as you say it may be shared across multiple apps.

We could maybe allow the image to be set there,

We also have a check for the kamal-proxy version to ensure it is new enough. I guess we could allow a different image but keep the same version check. If you were developing a custom version you'd want to ensure that it was up to date with the version kamal requires and you should match version numbers with basecamp/kamal-proxy.

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.

2 participants