-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: Add possibility to customize domain for MQTT broker #359
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally looks good, I just have the niggle with re-using domain
to mean different things.
helm/flowforge/README.md
Outdated
@@ -85,6 +85,7 @@ To use STMP to send email | |||
- `forge.broker.enabled` (default `false`) | |||
- `forge.broker.url` URL to access the broker from inside the cluster (default `mqtt://flowforge-broker.[namespace]:1883`) | |||
- `forge.broker.public_url` URL to access the broker from outside the cluster (default `ws://mqtt.[forge.domain]`, uses `wss://` if `forge.https` is `true`) | |||
- `forge.broker.domain` the domain the broker will be hosted on (default `mgtt.[forge.domain]`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only thought here is that we use forge.domain
to mean a domain we will append hostnames to, where as here we are using forge.broker.domain
to mean a fully qualified hostname.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the point. How about renaming to forge.broker.subdomain
as a subdomain of forge.domain
? It will be less confusing and will probably prevent questions about self-hosting MQTT broker separately from the FlowFuse platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm tempted to say forge.broker.hostname
and label it as needing to be a FQDN (and possibility improve the forge.entrypoint
doc as that is similar and very vague)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a single char typo in the docs (I will merge the fix)
Description
This PR adds the
forge.broker.domain
configuration option to allow running the MQTT broker on a custom domain. It also includes updates to the chart's schema and the documentation update.The example of the FlowFuse Platform running with the broker on custom domain is available here.
Related Issue(s)
Closes #295
Checklist
flowforge.yml
?FlowFuse/helm
to update ConfigMap TemplateFlowFuse/CloudProject
to update values for Staging/ProductionLabels
backport
labelarea:migration
label