fix: write generated Caddyfile to disk only if succesfully loaded into local Caddy (means valid)

internal/machine/caddyconfig/caddyfile.go

@@ -54,6 +54,9 @@ https://{{$hostname}} {
 	}
 	log
 }{{end}}
+`
+	caddyfileUnavailabeFooter = `# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine
+#       or the latest generated config is invalid. Please check the Caddy logs if it's running.
 `
 )
 
@@ -118,9 +121,7 @@ func (g *CaddyfileGenerator) Generate(
 	}
 
 	if !includeCustom {
-		return fmt.Sprintf("%s\n%s\n"+
+		return fmt.Sprintf("%s\n%s\n%s", caddyfileHeader, caddyfile, caddyfileUnavailabeFooter), nil
-			"# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine.\n",
-			caddyfileHeader, caddyfile), nil
 	}
 
 	upstreams := serviceUpstreams(containers)

internal/machine/caddyconfig/caddyfile_test.go

@@ -926,7 +926,8 @@ http://api.example.com {
 	log
 }
 
-# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine.
+# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine
+#       or the latest generated config is invalid. Please check the Caddy logs if it's running.
 `,
 		},
 		{
@@ -949,7 +950,8 @@ http://api.example.com {
 	log
 }
 
-# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine.
+# NOTE: User-defined configs for services were skipped because Caddy is not running on this machine
+#       or the latest generated config is invalid. Please check the Caddy logs if it's running.
 `,
 		},
 	}

internal/machine/caddyconfig/controller.go

@@ -111,42 +111,51 @@ func filterHealthyContainers(containers []store.ContainerRecord) []store.Contain
 func (c *Controller) generateAndLoadCaddyfile(ctx context.Context, containers []store.ContainerRecord) {
 	// Check if Caddy is available before attempting to generate and load config.
 	caddyAvailable := c.client.IsAvailable(ctx)
-
+	caddyfile, err := c.generator.Generate(ctx, containers, caddyAvailable)
-	caddyfile, err := c.generateCaddyfile(ctx, containers, caddyAvailable)
 	if err != nil {
 		c.log.Error("Failed to generate Caddyfile configuration.", "err", err)
 		return
 	}
 
 	if !caddyAvailable {
+		// Caddy is not running so the generated Caddyfile should not include user-defined configs thus must be valid.
+		// It's safe to write the config to disk so that when Caddy is deployed on this machine, it can pick it up.
+		if err = c.writeCaddyfile(caddyfile); err != nil {
+			c.log.Error("Failed to write Caddyfile to disk.", "err", err)
+			return
+		}
 		c.log.Debug("Caddy is not running on this machine, skipping configuration load.", "path", c.caddyfilePath)
 		return
 	}
 
+	// Caddy is available, try to load the config which may fail if the config is invalid. Generally, a config can
+	// pass the adaptation/validation step but still fail to load, for example, if it references resources that are
+	// not available.
 	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)
-	} else {
+		// Don't write invalid config to disk.
+		return
+	}
+
+	// Config loaded successfully, now write it to disk.
+	if err = c.writeCaddyfile(caddyfile); err != nil {
+		c.log.Error("Failed to write Caddyfile to disk after successful load.", "err", err)
+		// Config is already loaded in Caddy, so this is not critical.
+	}
+
 	c.log.Info("New Caddy configuration loaded into local Caddy instance.", "path", c.caddyfilePath)
 }
-}
 
-func (c *Controller) generateCaddyfile(
+// writeCaddyfile writes the Caddyfile content to disk with proper permissions.
-	ctx context.Context, containers []store.ContainerRecord, caddyAvailable bool,
+func (c *Controller) writeCaddyfile(caddyfile string) error {
-) (string, error) {
+	if err := os.WriteFile(c.caddyfilePath, []byte(caddyfile), 0o640); err != nil {
-	caddyfile, err := c.generator.Generate(ctx, containers, caddyAvailable)
+		return fmt.Errorf("write Caddyfile to file '%s': %w", c.caddyfilePath, err)
-	if err != nil {
-		return "", fmt.Errorf("generate Caddyfile: %w", err)
 	}
-
+	if err := fs.Chown(c.caddyfilePath, "", CaddyGroup); err != nil {
-	if err = os.WriteFile(c.caddyfilePath, []byte(caddyfile), 0o640); err != nil {
+		return fmt.Errorf("change owner of Caddyfile '%s': %w", c.caddyfilePath, err)
-		return "", fmt.Errorf("write Caddyfile to file '%s': %w", c.caddyfilePath, err)
 	}
-	if err = fs.Chown(c.caddyfilePath, "", CaddyGroup); err != nil {
+	return nil
-		return "", fmt.Errorf("change owner of Caddyfile '%s': %w", c.caddyfilePath, err)
-	}
-
-	return caddyfile, nil
 }
 
 func (c *Controller) generateJSONConfig(containers []store.ContainerRecord) error {

test/e2e/service_test.go

@@ -438,6 +438,48 @@ myapp.example.com {
 
 		assert.NotContains(t, config.Caddyfile, "invalid user-defined configs",
 			"Should not have validation failure comments after caddy is deployed")
+
+		// Store the current valid config for later comparison.
+		validConfig := config.Caddyfile
+
+		// Now deploy a service with invalid Caddyfile that references missing cert files and check it isn't included.
+		invalidServiceName := "test-invalid-caddy-config"
+		t.Cleanup(func() {
+			err := cli.RemoveService(ctx, invalidServiceName)
+			if !errors.Is(err, api.ErrNotFound) {
+				require.NoError(t, err)
+			}
+		})
+
+		invalidCaddyfile := `test-invalid.example.com {
+	tls cert.pem key.pem
+}`
+		invalidSpec := api.ServiceSpec{
+			Name: invalidServiceName,
+			Container: api.ContainerSpec{
+				Image: "portainer/pause:latest",
+			},
+			Caddy: &api.CaddySpec{
+				Config: invalidCaddyfile,
+			},
+		}
+
+		invalidDeployment := cli.NewDeployment(invalidSpec, nil)
+		_, err = invalidDeployment.Run(ctx)
+		require.NoError(t, err)
+
+		invalidSvc, err := cli.InspectService(ctx, invalidServiceName)
+		require.NoError(t, err)
+		assertServiceMatchesSpec(t, invalidSvc, invalidSpec)
+
+		// Wait a bit for any config updates to potentially happen.
+		time.Sleep(2 * time.Second)
+
+		// Check that the Caddy config hasn't changed.
+		newConfig, err := cli.Caddy.GetConfig(ctx, nil)
+		require.NoError(t, err)
+		assert.Equal(t, validConfig, newConfig.Caddyfile,
+			"Caddy config should not change when an invalid user-defined Caddy config is deployed")
 	})
 
 	t.Run("replicated", func(t *testing.T) {