Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions internal/command/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log/slog"
"strings"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -38,20 +39,22 @@ func (d *deployCommand) run(ctx context.Context, ns *docker.Namespace, cmd *cobr
return fmt.Errorf("%w: %w", docker.ErrSetupFailed, err)
}

host := d.flags.host
if host == "" {
host = docker.NameFromImageRef(imageRef) + ".localhost"
hosts := d.flags.host
if len(hosts) == 0 {
hosts = []string{docker.NameFromImageRef(imageRef) + ".localhost"}
}

if ns.HostInUse(host) {
return docker.ErrHostnameInUse
}

settings, err := d.flags.buildSettings(imageRef, host)
settings, err := d.flags.buildSettings(imageRef, strings.Join(hosts, ","))
if err != nil {
return err
}

for _, host := range settings.Hosts() {
if ns.HostInUse(host) {
return docker.ErrHostnameInUse
}
}

baseName := docker.NameFromImageRef(imageRef)
name, err := ns.UniqueName(baseName)
if err != nil {
Expand All @@ -61,7 +64,7 @@ func (d *deployCommand) run(ctx context.Context, ns *docker.Namespace, cmd *cobr

app := docker.NewApplication(ns, settings)

return runWithProgress("Deploying "+host, func(progress docker.DeployProgressCallback) error {
return runWithProgress("Deploying "+settings.PrimaryHost(), func(progress docker.DeployProgressCallback) error {
if err := app.Deploy(ctx, progress); err != nil {
if cleanupErr := app.Destroy(context.Background(), true); cleanupErr != nil {
slog.Error("Failed to clean up after deploy failure", "app", name, "error", cleanupErr)
Expand Down
44 changes: 25 additions & 19 deletions internal/command/settings_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,25 @@ import (
)

type settingsFlags struct {
host string
disableTLS bool
env []string
smtpServer string
smtpPort string
smtpUsername string
smtpPassword string
smtpFrom string
cpus int
memory int
autoUpdate bool
backupPath string
autoBackup bool
host []string
canonicalHost string
disableTLS bool
env []string
smtpServer string
smtpPort string
smtpUsername string
smtpPassword string
smtpFrom string
cpus int
memory int
autoUpdate bool
backupPath string
autoBackup bool
}

func (f *settingsFlags) register(cmd *cobra.Command) {
cmd.Flags().StringVar(&f.host, "host", "", "hostname for the application")
cmd.Flags().StringArrayVar(&f.host, "host", nil, "hostname for the application (can be repeated to serve additional hostnames)")
cmd.Flags().StringVar(&f.canonicalHost, "canonical-host", "", "redirect all requests to this hostname (must be one of --host)")
cmd.Flags().BoolVar(&f.disableTLS, "disable-tls", false, "disable TLS for this application")
cmd.Flags().StringArrayVar(&f.env, "env", nil, "environment variable in KEY=VALUE format (can be repeated)")
cmd.Flags().StringVar(&f.smtpServer, "smtp-server", "", "SMTP server address")
Expand All @@ -53,10 +55,11 @@ func (f *settingsFlags) buildSettings(image, host string) (docker.ApplicationSet
}

s := docker.ApplicationSettings{
Image: image,
Host: host,
DisableTLS: f.disableTLS,
EnvVars: envVars,
Image: image,
Host: host,
CanonicalHost: f.canonicalHost,
DisableTLS: f.disableTLS,
EnvVars: envVars,
SMTP: docker.SMTPSettings{
Server: f.smtpServer,
Port: f.smtpPort,
Expand Down Expand Up @@ -88,7 +91,10 @@ func (f *settingsFlags) applyChanges(cmd *cobra.Command, existing docker.Applica
s.Image = image

if cmd.Flags().Changed("host") {
s.Host = f.host
s.Host = strings.Join(f.host, ",")
}
if cmd.Flags().Changed("canonical-host") {
s.CanonicalHost = f.canonicalHost
}
if cmd.Flags().Changed("disable-tls") {
s.DisableTLS = f.disableTLS
Expand Down
6 changes: 4 additions & 2 deletions internal/command/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ func (u *updateCommand) run(ctx context.Context, ns *docker.Namespace, cmd *cobr
}

if settings.Host != app.Settings.Host {
if ns.HostInUseByAnother(settings.Host, app.Settings.Name) {
return docker.ErrHostnameInUse
for _, host := range settings.Hosts() {
if ns.HostInUseByAnother(host, app.Settings.Name) {
return docker.ErrHostnameInUse
}
}
}

Expand Down
33 changes: 18 additions & 15 deletions internal/docker/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,18 @@ import (
)

var (
ErrApplicationExists = errors.New("application already exists")
ErrHostnameInUse = errors.New("hostname already in use")
ErrHostRequired = errors.New("host is required")
ErrInvalidBackup = errors.New("invalid backup archive")
ErrImageRequired = errors.New("image is required")
ErrApplicationNotRunning = errors.New("the application is not running")
ErrBackupPathRelative = errors.New("backup path must be absolute")
ErrAutoBackupWithoutPath = errors.New("auto-backup requires a backup path")
ErrSetupFailed = errors.New("setup failed")
ErrPullFailed = &describedError{
ErrApplicationExists = errors.New("application already exists")
ErrHostnameInUse = errors.New("hostname already in use")
ErrHostRequired = errors.New("host is required")
ErrInvalidBackup = errors.New("invalid backup archive")
ErrImageRequired = errors.New("image is required")
ErrApplicationNotRunning = errors.New("the application is not running")
ErrBackupPathRelative = errors.New("backup path must be absolute")
ErrAutoBackupWithoutPath = errors.New("auto-backup requires a backup path")
ErrMixedLocalhostHosts = errors.New("hosts must be all localhost or all public: TLS applies to every hostname an app serves")
ErrCanonicalHostNotServed = errors.New("canonical host must be one of the application's hostnames")
ErrSetupFailed = errors.New("setup failed")
ErrPullFailed = &describedError{
msg: "pull failed",
description: "Failed to download the application image. Check that the image name is correct and try again.",
}
Expand Down Expand Up @@ -108,7 +110,7 @@ func (a *Application) URL() string {
defaultPort = 443
}

base := scheme + "://" + a.Settings.Host
base := scheme + "://" + a.Settings.DisplayHost()

if a.namespace == nil {
return base
Expand Down Expand Up @@ -378,10 +380,11 @@ func (a *Application) deployWithVolume(ctx context.Context, vol *ApplicationVolu
shortContainerID := resp.ID[:12]

if err := a.namespace.Proxy().Deploy(ctx, DeployOptions{
AppName: a.Settings.Name,
Target: shortContainerID,
Host: a.Settings.Host,
TLS: a.Settings.TLSEnabled(),
AppName: a.Settings.Name,
Target: shortContainerID,
Hosts: a.Settings.Hosts(),
CanonicalHost: a.Settings.CanonicalHost,
TLS: a.Settings.TLSEnabled(),
}); err != nil {
a.namespace.client.ContainerRemove(ctx, resp.ID, container.RemoveOptions{Force: true})
if strings.Contains(err.Error(), "target not healthy") || strings.Contains(err.Error(), "deploy timed out") {
Expand Down
72 changes: 61 additions & 11 deletions internal/docker/application_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/hex"
"encoding/json"
"fmt"
"slices"
"strconv"
"strings"
)
Expand Down Expand Up @@ -86,16 +87,17 @@ type BackupSettings struct {
}

type ApplicationSettings struct {
Name string `json:"name"`
Image string `json:"image"`
Host string `json:"host"`
DisableTLS bool `json:"disableTLS"`
EnvVars map[string]string `json:"env"`
SMTP SMTPSettings `json:"smtp"`
Resources ContainerResources `json:"resources"`
AutoUpdate bool `json:"autoUpdate"`
Backup BackupSettings `json:"backup"`
Keys Keys `json:"keys"`
Name string `json:"name"`
Image string `json:"image"`
Host string `json:"host"`
CanonicalHost string `json:"canonicalHost,omitempty"`
DisableTLS bool `json:"disableTLS"`
EnvVars map[string]string `json:"env"`
SMTP SMTPSettings `json:"smtp"`
Resources ContainerResources `json:"resources"`
AutoUpdate bool `json:"autoUpdate"`
Backup BackupSettings `json:"backup"`
Keys Keys `json:"keys"`
}

func UnmarshalApplicationSettings(s string) (ApplicationSettings, error) {
Expand All @@ -109,24 +111,72 @@ func (s ApplicationSettings) Marshal() string {
return string(b)
}

// Host stores one or more hostnames as a comma-separated list, keeping the
// serialized settings backward compatible with single-host installs.
func (s ApplicationSettings) Hosts() []string {
if s.Host == "" {
return nil
}

var hosts []string
for _, h := range strings.Split(s.Host, ",") {
if h = strings.TrimSpace(h); h != "" {
hosts = append(hosts, h)
}
}
return hosts
}

func (s ApplicationSettings) PrimaryHost() string {
if hosts := s.Hosts(); len(hosts) > 0 {
return hosts[0]
}
return ""
}

// DisplayHost is the hostname visitors end up on: the canonical host when the
// proxy is redirecting to one, otherwise the app's first hostname.
func (s ApplicationSettings) DisplayHost() string {
if s.CanonicalHost != "" {
return s.CanonicalHost
}
return s.PrimaryHost()
}

func (s ApplicationSettings) Validate() error {
if s.Image == "" {
return ErrImageRequired
}
if s.Backup.AutoBackup && s.Backup.Path == "" {
return ErrAutoBackupWithoutPath
}
// TLS is a single proxy-wide switch covering every hostname the app
// serves, so localhost and public hostnames cannot be mixed.
hosts := s.Hosts()
for _, host := range hosts {
if IsLocalhost(host) != IsLocalhost(hosts[0]) {
return ErrMixedLocalhostHosts
}
}
// Redirecting to a hostname the app doesn't serve would send visitors to
// a route the proxy knows nothing about.
if s.CanonicalHost != "" && !slices.Contains(hosts, s.CanonicalHost) {
return ErrCanonicalHostNotServed
}
return nil
}

func (s ApplicationSettings) TLSEnabled() bool {
return s.Host != "" && !s.DisableTLS && !IsLocalhost(s.Host)
return s.Host != "" && !s.DisableTLS && !IsLocalhost(s.PrimaryHost())
Comment on lines 169 to +170

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed, the order-dependence was a real bug. Rather than trying to serve a mixed list correctly — kamal-proxy takes one --tls switch for the whole service, so there's no way to enable it per-hostname — 6e92319 rejects mixed localhost/public lists in Validate with a new ErrMixedLocalhostHosts. TLSEnabled keeps using the primary host, which is now unambiguous because validation guarantees every hostname agrees. The TUI settings form previously bypassed Validate entirely, so it now calls it on submit and surfaces the error inline.

}

func (s ApplicationSettings) Equal(other ApplicationSettings) bool {
if s.Name != other.Name || s.Image != other.Image || s.Host != other.Host || s.DisableTLS != other.DisableTLS {
return false
}
if s.CanonicalHost != other.CanonicalHost {
return false
}
if s.Resources != other.Resources {
return false
}
Expand Down
82 changes: 82 additions & 0 deletions internal/docker/application_settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,3 +338,85 @@ func TestAutoUpdateAndBackupMarshalRoundTrip(t *testing.T) {
assert.True(t, restored.Backup.AutoBackup)
assert.True(t, original.Equal(restored))
}

func TestHosts(t *testing.T) {
assert.Nil(t, ApplicationSettings{}.Hosts())
assert.Equal(t, []string{"app.example.com"}, ApplicationSettings{Host: "app.example.com"}.Hosts())
assert.Equal(t,
[]string{"app.example.com", "www.app.example.com"},
ApplicationSettings{Host: "app.example.com,www.app.example.com"}.Hosts())
assert.Equal(t,
[]string{"app.example.com", "www.app.example.com"},
ApplicationSettings{Host: " app.example.com , www.app.example.com ,"}.Hosts())
}

func TestPrimaryHost(t *testing.T) {
assert.Equal(t, "", ApplicationSettings{}.PrimaryHost())
assert.Equal(t, "app.example.com", ApplicationSettings{Host: "app.example.com"}.PrimaryHost())
assert.Equal(t, "app.example.com", ApplicationSettings{Host: "app.example.com,www.app.example.com"}.PrimaryHost())
}

func TestTLSEnabledWithMultipleHosts(t *testing.T) {
assert.True(t, ApplicationSettings{Host: "app.example.com,www.app.example.com"}.TLSEnabled())
assert.False(t, ApplicationSettings{Host: "app.localhost,www.app.example.com"}.TLSEnabled())
}

func TestValidateRejectsMixedLocalhostHosts(t *testing.T) {
valid := []string{
"app.example.com",
"app.example.com,www.app.example.com",
"app.localhost,alias.localhost",
}
for _, host := range valid {
assert.NoError(t, ApplicationSettings{Image: "img:latest", Host: host}.Validate(), host)
}

mixed := []string{
"app.example.com,app.localhost",
"app.localhost,app.example.com",
}
for _, host := range mixed {
assert.ErrorIs(t, ApplicationSettings{Image: "img:latest", Host: host}.Validate(), ErrMixedLocalhostHosts, host)
}
}

func TestValidateCanonicalHost(t *testing.T) {
base := ApplicationSettings{Image: "img:latest", Host: "app.example.com,www.app.example.com"}

for _, canonical := range []string{"", "app.example.com", "www.app.example.com"} {
s := base
s.CanonicalHost = canonical
assert.NoError(t, s.Validate(), canonical)
}

s := base
s.CanonicalHost = "other.example.com"
assert.ErrorIs(t, s.Validate(), ErrCanonicalHostNotServed)
}

func TestDisplayHost(t *testing.T) {
s := ApplicationSettings{Host: "app.example.com,www.app.example.com"}
assert.Equal(t, "app.example.com", s.DisplayHost())

s.CanonicalHost = "www.app.example.com"
assert.Equal(t, "www.app.example.com", s.DisplayHost())

assert.Equal(t, "", ApplicationSettings{}.DisplayHost())
}

func TestCanonicalHostMarshalRoundTrip(t *testing.T) {
original := ApplicationSettings{
Name: "app",
Image: "img:latest",
Host: "app.example.com,www.app.example.com",
CanonicalHost: "app.example.com",
}
restored, err := UnmarshalApplicationSettings(original.Marshal())
require.NoError(t, err)
assert.Equal(t, "app.example.com", restored.CanonicalHost)
assert.True(t, original.Equal(restored))

changed := original
changed.CanonicalHost = "www.app.example.com"
assert.False(t, original.Equal(changed))
}
Loading