From 97bdb8eae161930f5e8c6002aedfc8752f3617bb Mon Sep 17 00:00:00 2001 From: Pasha Sviderski Date: Wed, 10 Sep 2025 15:04:45 +1000 Subject: [PATCH] fix: skip validation for user-defined Caddy configs if caddy not running locally --- internal/machine/caddyconfig/caddyfile.go | 12 +- .../machine/caddyconfig/caddyfile_test.go | 149 +++++++++++++----- internal/machine/caddyconfig/client.go | 18 +++ internal/machine/caddyconfig/controller.go | 16 +- test/e2e/service_test.go | 7 +- 5 files changed, 153 insertions(+), 49 deletions(-) diff --git a/internal/machine/caddyconfig/caddyfile.go b/internal/machine/caddyconfig/caddyfile.go index a2e05ddf..3fbe7c55 100644 --- a/internal/machine/caddyconfig/caddyfile.go +++ b/internal/machine/caddyconfig/caddyfile.go @@ -95,7 +95,11 @@ func NewCaddyfileGenerator(machineID string, validator CaddyfileValidator, log * // [service-a x-caddy] // ... // [service-z x-caddy] -func (g *CaddyfileGenerator) Generate(ctx context.Context, records []store.ContainerRecord) (string, error) { +// +// If includeCustom is false, custom Caddy configs (x-caddy) are not included in the generated Caddyfile. +func (g *CaddyfileGenerator) Generate( + ctx context.Context, records []store.ContainerRecord, includeCustom bool, +) (string, error) { containers := make([]api.ServiceContainer, len(records)) for i, cr := range records { containers[i] = cr.Container @@ -113,6 +117,12 @@ func (g *CaddyfileGenerator) Generate(ctx context.Context, records []store.Conta return "", fmt.Errorf("generate base Caddyfile from service ports: %w", err) } + if !includeCustom { + return fmt.Sprintf("%s\n%s\n"+ + "# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine.\n", + caddyfileHeader, caddyfile), nil + } + upstreams := serviceUpstreams(containers) // Track validation errors for reporting. var configErrors []string diff --git a/internal/machine/caddyconfig/caddyfile_test.go b/internal/machine/caddyconfig/caddyfile_test.go index f027fd35..45f83bfd 100644 --- a/internal/machine/caddyconfig/caddyfile_test.go +++ b/internal/machine/caddyconfig/caddyfile_test.go @@ -18,8 +18,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestCaddyfileGenerator(t *testing.T) { - caddyfileHeader := `# This file is autogenerated by Uncloud based on the configuration of running services. +const testCaddyfileHeader = `# This file is autogenerated by Uncloud based on the configuration of running services. # Do not edit manually. Any manual changes will be overwritten on the next update. # Health check endpoint to verify Caddy reachability on this machine. @@ -38,6 +37,7 @@ http:// { } ` +func TestCaddyfileGenerator(t *testing.T) { tests := []struct { name string containers []store.ContainerRecord @@ -47,14 +47,14 @@ http:// { { name: "empty containers", containers: []store.ContainerRecord{}, - want: caddyfileHeader, + want: testCaddyfileHeader, }, { name: "HTTP container", containers: []store.ContainerRecord{ newContainerRecord(newContainer("10.210.0.2", "app.example.com:8080/http"), "mach1"), }, - want: caddyfileHeader + ` + want: testCaddyfileHeader + ` # Sites generated from service ports. http://app.example.com { @@ -71,7 +71,7 @@ http://app.example.com { newContainerRecord(newContainer("10.210.0.2", "app.example.com:8080/http"), "mach1"), newContainerRecord(newContainer("10.210.0.3", "app.example.com:8080/http"), "mach1"), }, - want: caddyfileHeader + ` + want: testCaddyfileHeader + ` # Sites generated from service ports. http://app.example.com { @@ -87,7 +87,7 @@ http://app.example.com { containers: []store.ContainerRecord{ newContainerRecord(newContainer("10.210.0.2", "secure.example.com:8000/https"), "mach1"), }, - want: caddyfileHeader + ` + want: testCaddyfileHeader + ` # Sites generated from service ports. https://secure.example.com { @@ -127,7 +127,7 @@ https://secure.example.com { "mach1", ), }, - want: caddyfileHeader + ` + want: testCaddyfileHeader + ` # Sites generated from service ports. http://app.example.com { @@ -157,14 +157,14 @@ https://secure.example.com { containers: []store.ContainerRecord{ newContainerRecord(newContainerWithoutNetwork("ignored.example.com:8080/http"), "mach1"), }, - want: caddyfileHeader, + want: testCaddyfileHeader, }, { name: "container with invalid port ignored", containers: []store.ContainerRecord{ newContainerRecord(newContainer("10.210.0.2", "invalid-port"), "mach1"), }, - want: caddyfileHeader, + want: testCaddyfileHeader, }, { name: "containers with unsupported protocols and host mode ignored", @@ -173,7 +173,7 @@ https://secure.example.com { newContainerRecord(newContainer("10.210.0.3", "5000/udp"), "mach1"), newContainerRecord(newContainer("10.210.0.4", "80:8080/tcp@host"), "mach1"), }, - want: caddyfileHeader, + want: testCaddyfileHeader, }, } @@ -183,7 +183,7 @@ https://secure.example.com { // Validator is not expected to be called in these tests. generator := NewCaddyfileGenerator("test-machine-id", nil, nil) - config, err := generator.Generate(ctx, tt.containers) + config, err := generator.Generate(ctx, tt.containers, true) if tt.wantErr { assert.Error(t, err) @@ -197,25 +197,6 @@ https://secure.example.com { } func TestCaddyfileGeneratorWithCustomConfigs(t *testing.T) { - caddyfileBase := `# This file is autogenerated by Uncloud based on the configuration of running services. -# Do not edit manually. Any manual changes will be overwritten on the next update. - -# Health check endpoint to verify Caddy reachability on this machine. -http:// { - handle /.uncloud-verify { - respond "test-machine-id" 200 - } - log -} - -(common_proxy) { - # Retry failed requests up to lb_retries times against other available upstreams. - lb_retries 3 - # Upstreams are marked unhealthy for fail_duration after a failed request (passive health checking). - fail_duration 30s -} -` - tests := []struct { name string containers []store.ContainerRecord @@ -275,7 +256,7 @@ web.example.com { time.Now(), ), }, - want: caddyfileBase + ` + want: testCaddyfileHeader + ` # User-defined config for service 'web'. # Custom config for web service web.example.com { @@ -297,7 +278,7 @@ bad.config.com { time.Now(), ), }, - want: caddyfileBase + ` + want: testCaddyfileHeader + ` # Skipped invalid user-defined configs: # - service 'bad-service': validation failed: invalid config detected `, @@ -316,7 +297,7 @@ bad.template.com { time.Now(), ), }, - want: caddyfileBase + ` + want: testCaddyfileHeader + ` # Skipped invalid user-defined configs: # - service 'bad-template': failed to render template: parse config as Go template: template: Caddyfile:3: unexpected "}" in operand `, @@ -335,7 +316,7 @@ localhost { time.Now(), ), }, - want: caddyfileBase + ` + want: testCaddyfileHeader + ` # Skipped invalid user-defined configs: # - service 'caddy': validation failed: invalid config detected `, @@ -354,7 +335,7 @@ localhost { time.Now(), ), }, - want: caddyfileBase, + want: testCaddyfileHeader, }, { name: "multiple services with mixed valid and invalid configs", @@ -388,7 +369,7 @@ bad.example.com { time.Now(), ), }, - want: caddyfileBase + ` + want: testCaddyfileHeader + ` # User-defined config for service 'api'. api.example.com { reverse_proxy api:8080 @@ -490,7 +471,7 @@ api.example.com { "test-machine-id", ), }, - want: caddyfileBase + ` + want: testCaddyfileHeader + ` # Sites generated from service ports. http://api.example.com { @@ -530,7 +511,7 @@ new.example.com { time.Now(), ), }, - want: caddyfileBase + ` + want: testCaddyfileHeader + ` # User-defined config for service 'web'. # New config new.example.com { @@ -815,7 +796,7 @@ invalid.example.com { time.Now(), ), }, - want: caddyfileBase + ` + want: testCaddyfileHeader + ` # User-defined config for service 'valid'. valid.example.com { respond "Valid config" @@ -843,7 +824,7 @@ valid.example.com { t.Run(tt.name, func(t *testing.T) { generator := NewCaddyfileGenerator("test-machine-id", validator, nil) - config, err := generator.Generate(ctx, tt.containers) + config, err := generator.Generate(ctx, tt.containers, true) if tt.wantErr { assert.Error(t, err) @@ -899,6 +880,94 @@ func newContainerRecordWithCaddyConfig(serviceName, ip, caddyConfig, machineID s } } +func TestCaddyfileGeneratorWithoutCustomConfigs(t *testing.T) { + // Test that when includeCustom is false (Caddy not available), x-caddy configs are skipped. + tests := []struct { + name string + containers []store.ContainerRecord + want string + }{ + { + name: "x-caddy configs are skipped", + containers: []store.ContainerRecord{ + newContainerRecordWithCaddyConfig( + "caddy", + "10.210.0.1", + `# Global config +{ + global directive +}`, + "test-machine-id", + time.Now(), + ), + newContainerRecordWithCaddyConfig( + "web", + "10.210.0.2", + `web.example.com { + reverse_proxy web:3000 +}`, + "test-machine-id", + time.Now(), + ), + newContainerRecordWithPorts( + "api", + "10.210.0.3", + []string{"api.example.com:8080/http"}, + "test-machine-id", + ), + }, + want: testCaddyfileHeader + ` +# Sites generated from service ports. + +http://api.example.com { + reverse_proxy 10.210.0.3:8080 { + import common_proxy + } + log +} + +# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine. +`, + }, + { + name: "no containers with x-caddy configs", + containers: []store.ContainerRecord{ + newContainerRecordWithPorts( + "api", + "10.210.0.3", + []string{"api.example.com:8080/http"}, + "test-machine-id", + ), + }, + want: testCaddyfileHeader + ` +# Sites generated from service ports. + +http://api.example.com { + reverse_proxy 10.210.0.3:8080 { + import common_proxy + } + log +} + +# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine. +`, + }, + } + + ctx := context.Background() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Validator is not expected to be called in these tests. + generator := NewCaddyfileGenerator("test-machine-id", nil, nil) + + config, err := generator.Generate(ctx, tt.containers, false) + require.NoError(t, err) + + assert.Equal(t, tt.want, config, "Generated Caddyfile doesn't match") + }) + } +} + func newContainerRecordWithPorts(serviceName, ip string, ports []string, machineID string) store.ContainerRecord { portsLabel := strings.Join(ports, ",") return store.ContainerRecord{ diff --git a/internal/machine/caddyconfig/client.go b/internal/machine/caddyconfig/client.go index 8a3a4c4b..c988e8f1 100644 --- a/internal/machine/caddyconfig/client.go +++ b/internal/machine/caddyconfig/client.go @@ -34,6 +34,24 @@ func NewCaddyAdminClient(socketPath string) *CaddyAdminClient { } } +// IsAvailable checks if the local Caddy instance is running and responding to admin API requests. +func (c *CaddyAdminClient) IsAvailable(ctx context.Context) bool { + // Caddy doesn't serve a /ping endpoint. It's a random endpoint we can use to check if Caddy is running. + req, err := http.NewRequestWithContext(ctx, "GET", "http://localhost/ping", nil) + if err != nil { + return false + } + + resp, err := c.client.Do(req) + if err != nil { + return false + } + defer resp.Body.Close() + + // Any HTTP response means Caddy is running and accessible. + return true +} + // Adapt converts a Caddyfile to JSON configuration without loading or running it. func (c *CaddyAdminClient) Adapt(ctx context.Context, caddyfile string) (string, error) { req, err := http.NewRequestWithContext(ctx, "POST", "http://localhost/adapt", strings.NewReader(caddyfile)) diff --git a/internal/machine/caddyconfig/controller.go b/internal/machine/caddyconfig/controller.go index 890f1ce5..687261a9 100644 --- a/internal/machine/caddyconfig/controller.go +++ b/internal/machine/caddyconfig/controller.go @@ -109,12 +109,20 @@ func filterHealthyContainers(containers []store.ContainerRecord) []store.Contain } func (c *Controller) generateAndLoadCaddyfile(ctx context.Context, containers []store.ContainerRecord) { - caddyfile, err := c.generateCaddyfile(ctx, containers) + // Check if Caddy is available before attempting to generate and load config. + caddyAvailable := c.client.IsAvailable(ctx) + + caddyfile, err := c.generateCaddyfile(ctx, containers, caddyAvailable) if err != nil { c.log.Error("Failed to generate Caddyfile configuration.", "err", err) return } + if !caddyAvailable { + c.log.Debug("Caddy is not running on this machine, skipping configuration load.", "path", c.caddyfilePath) + return + } + if err = c.client.Load(ctx, caddyfile); err != nil { c.log.Error("Failed to load new Caddy configuration into local Caddy instance.", "err", err, "path", c.caddyfilePath) @@ -123,8 +131,10 @@ func (c *Controller) generateAndLoadCaddyfile(ctx context.Context, containers [] } } -func (c *Controller) generateCaddyfile(ctx context.Context, containers []store.ContainerRecord) (string, error) { - caddyfile, err := c.generator.Generate(ctx, containers) +func (c *Controller) generateCaddyfile( + ctx context.Context, containers []store.ContainerRecord, caddyAvailable bool, +) (string, error) { + caddyfile, err := c.generator.Generate(ctx, containers, caddyAvailable) if err != nil { return "", fmt.Errorf("generate Caddyfile: %w", err) } diff --git a/test/e2e/service_test.go b/test/e2e/service_test.go index d97f8bbd..9e6bd448 100644 --- a/test/e2e/service_test.go +++ b/test/e2e/service_test.go @@ -379,19 +379,16 @@ func TestDeployment(t *testing.T) { require.NoError(t, err) assertServiceMatchesSpec(t, svc, spec) - // Check that the generated Caddyfile contains a comment with invalid user-defined configs. + // Check that the generated Caddyfile contains a comment that user-define configs were skipped. var config *pb.GetCaddyConfigResponse require.Eventually(t, func() bool { config, err = cli.Caddy.GetConfig(ctx, nil) if err != nil { return false } - return strings.Contains(config.Caddyfile, "invalid user-defined configs") + return strings.Contains(config.Caddyfile, "# NOTE: User-defined configs for services were skipped") }, 5*time.Second, 100*time.Millisecond) - assert.Regexp(t, "- service 'test-custom-caddy-config': validation failed:.*"+ - "/run/uncloud/caddy/admin.sock: connect:.*", config.Caddyfile, - "Expected comment about validation failure for service's user-defined Caddy config") assert.NotContains(t, config.Caddyfile, "test-custom-caddy-config.example.com {") // Now deploy caddy with custom config.