diff --git a/api/cluster/namespace.go b/api/cluster/namespace.go index 06c4c3f0..cd091294 100644 --- a/api/cluster/namespace.go +++ b/api/cluster/namespace.go @@ -84,7 +84,7 @@ func AddNamespace(c *gin.Context) { "upstream_test_type": "omitempty,oneof=" + model.UpstreamTestLocal + " " + model.UpstreamTestRemote + " " + model.UpstreamTestMirror, "deploy_mode": "omitempty,oneof=" + model.DeployModeLocal + " " + model.DeployModeRemote, "sync_strategy": "omitempty,oneof=" + model.SyncStrategyManual + " " + model.SyncStrategyAuto, - "sync_interval_minutes": "omitempty,min=1", + "sync_interval_minutes": "omitempty,min=0", }). Create() } @@ -98,7 +98,7 @@ func ModifyNamespace(c *gin.Context) { "upstream_test_type": "omitempty,oneof=" + model.UpstreamTestLocal + " " + model.UpstreamTestRemote + " " + model.UpstreamTestMirror, "deploy_mode": "omitempty,oneof=" + model.DeployModeLocal + " " + model.DeployModeRemote, "sync_strategy": "omitempty,oneof=" + model.SyncStrategyManual + " " + model.SyncStrategyAuto, - "sync_interval_minutes": "omitempty,min=1", + "sync_interval_minutes": "omitempty,min=0", }). Modify() } diff --git a/api/config/sync.go b/api/config/sync.go index a08b0ddb..f70f3c96 100644 --- a/api/config/sync.go +++ b/api/config/sync.go @@ -26,13 +26,20 @@ func SyncConfigBatch(c *gin.Context) { return } + // One rejected file must not discard the rest of the batch: a real + // configuration directory always holds something the validator dislikes, and + // dropping the whole sync over it would make the feature useless. written := 0 skipped := 0 + failures := make([]gin.H, 0) + for _, file := range json.Files { + relativePath := filepath.ToSlash(filepath.Join(file.BaseDir, file.Name)) + path, err := config.ResolveConfPath(helper.UnescapeURL(file.BaseDir), helper.UnescapeURL(file.Name)) if err != nil { - cosy.ErrHandler(c, err) - return + failures = append(failures, gin.H{"path": relativePath, "error": err.Error()}) + continue } if !json.Overwrite && helper.FileExists(path) { @@ -41,18 +48,18 @@ func SyncConfigBatch(c *gin.Context) { } if err = config.ValidateConfigFile(path, file.Content); err != nil { - cosy.ErrHandler(c, err) - return + failures = append(failures, gin.H{"path": relativePath, "error": err.Error()}) + continue } if err = os.MkdirAll(filepath.Dir(path), 0755); err != nil { - cosy.ErrHandler(c, err) - return + failures = append(failures, gin.H{"path": relativePath, "error": err.Error()}) + continue } if err = os.WriteFile(path, []byte(file.Content), 0644); err != nil { - cosy.ErrHandler(c, err) - return + failures = append(failures, gin.H{"path": relativePath, "error": err.Error()}) + continue } written++ @@ -65,10 +72,17 @@ func SyncConfigBatch(c *gin.Context) { } } - c.JSON(http.StatusOK, gin.H{ - "message": "ok", - "written": written, - "skipped": skipped, + // Only a batch where nothing could be applied is an error. + status := http.StatusOK + if written == 0 && skipped == 0 && len(failures) > 0 { + status = http.StatusInternalServerError + } + + c.JSON(status, gin.H{ + "message": "ok", + "written": written, + "skipped": skipped, + "failures": failures, }) } diff --git a/internal/clustersync/client.go b/internal/clustersync/client.go index 134c69cd..91115774 100644 --- a/internal/clustersync/client.go +++ b/internal/clustersync/client.go @@ -68,13 +68,20 @@ func (n nodeRef) post(ctx context.Context, path string, body any) error { // postWithStatus behaves like post but also reports the status code so callers // can detect endpoints that an older node does not implement yet. func (n nodeRef) postWithStatus(ctx context.Context, path string, body any) (int, error) { + _, status, err := n.postForBody(ctx, path, body) + return status, err +} + +// postForBody returns the response body so callers can inspect a partial +// success reported inside a 2xx answer. +func (n nodeRef) postForBody(ctx context.Context, path string, body any) ([]byte, int, error) { resp, err := n.client.R().SetContext(ctx).SetBody(body).Post(path) if err != nil { - return 0, err + return nil, 0, err } if resp.StatusCode() < http.StatusOK || resp.StatusCode() >= http.StatusMultipleChoices { - return resp.StatusCode(), fmt.Errorf("%s responded %d: %s", path, resp.StatusCode(), resp.String()) + return resp.Body(), resp.StatusCode(), fmt.Errorf("%s responded %d: %s", path, resp.StatusCode(), resp.String()) } - return resp.StatusCode(), nil + return resp.Body(), resp.StatusCode(), nil } diff --git a/internal/clustersync/content.go b/internal/clustersync/content.go index 67d4b43a..5722b0c3 100644 --- a/internal/clustersync/content.go +++ b/internal/clustersync/content.go @@ -7,6 +7,7 @@ import ( "strings" "unicode/utf8" + "github.com/0xJacky/Nginx-UI/internal/config" "github.com/0xJacky/Nginx-UI/internal/helper" "github.com/0xJacky/Nginx-UI/internal/nginx" "github.com/uozi-tech/cosy/logger" @@ -89,6 +90,15 @@ func CollectConfigFiles(root string) ([]ConfigFile, error) { return nil } + // A real configuration directory accumulates files the receiver will + // always reject, such as nginx.conf.bak.1738662518. Leaving them out of + // the batch keeps a whole-directory sync from being dragged down by one + // name that is not a valid configuration file anywhere. + if err := config.ValidateConfigFilename(path); err != nil { + logger.Debugf("cluster sync skips unsupported config name %s: %v", path, err) + return nil + } + relativeDir, err := filepath.Rel(confPath, filepath.Dir(path)) if err != nil { return nil diff --git a/internal/clustersync/content_test.go b/internal/clustersync/content_test.go index 8a952876..0a38de2f 100644 --- a/internal/clustersync/content_test.go +++ b/internal/clustersync/content_test.go @@ -163,3 +163,39 @@ func TestConfigFileRelativePathHandlesRootFiles(t *testing.T) { t.Fatalf("got %q, want %q", got, "conf.d/a.conf") } } + +func TestCollectConfigFilesSkipsNamesTheReceiverRejects(t *testing.T) { + // A real configuration directory accumulates backups like these. Including + // them used to make the receiver reject the whole batch. + confDir := withConfDir(t, map[string]string{ + "nginx.conf": "events {}\n", + "conf.d/a.conf": "# a\n", + "nginx.conf.bak.1738662518": "events {}\n", + "nginx_bak.conf": "events {}\n", + "conf.d/notes.txt": "not a config\n", + }) + + files, err := CollectConfigFiles(confDir) + if err != nil { + t.Fatalf("collect: %v", err) + } + + got := collectedPaths(files) + for _, path := range got { + if path == "nginx.conf.bak.1738662518" || path == "conf.d/notes.txt" { + t.Fatalf("unsupported config name must be skipped, got %v", got) + } + } + + // The .conf files are still collected. + want := map[string]bool{"conf.d/a.conf": true, "nginx_bak.conf": true} + for _, path := range got { + if !want[path] { + t.Fatalf("unexpected file %s in %v", path, got) + } + delete(want, path) + } + if len(want) != 0 { + t.Fatalf("missing files %v, got %v", want, got) + } +} diff --git a/internal/clustersync/push.go b/internal/clustersync/push.go index 8b5fa40d..7cb08452 100644 --- a/internal/clustersync/push.go +++ b/internal/clustersync/push.go @@ -2,6 +2,7 @@ package clustersync import ( "context" + "encoding/json" "errors" "fmt" "net/http" @@ -42,9 +43,9 @@ func configBatchItem(name string, files []ConfigFile, overwrite bool) item { name: name, push: func(ctx context.Context, node nodeRef) error { payload := configBatchPayload{Files: files, Overwrite: overwrite} - status, err := node.postWithStatus(ctx, "/api/config_sync_batch", payload) + body, status, err := node.postForBody(ctx, "/api/config_sync_batch", payload) if err == nil { - return nil + return batchFailure(body) } if status != http.StatusNotFound { return err @@ -56,6 +57,34 @@ func configBatchItem(name string, files []ConfigFile, overwrite bool) item { } } +// batchResponse is the answer of the batch receiver. Files it could not apply +// are reported individually instead of failing the whole request. +type batchResponse struct { + Written int `json:"written"` + Skipped int `json:"skipped"` + Failures []struct { + Path string `json:"path"` + Error string `json:"error"` + } `json:"failures"` +} + +// batchFailure turns a partially applied batch into an error so the summary +// does not claim a clean run. +func batchFailure(body []byte) error { + var response batchResponse + if err := json.Unmarshal(body, &response); err != nil || len(response.Failures) == 0 { + return nil + } + + failures := make([]error, 0, len(response.Failures)) + for _, failure := range response.Failures { + failures = append(failures, fmt.Errorf("%s: %s", failure.Path, failure.Error)) + } + + return fmt.Errorf("applied %d of %d files: %w", + response.Written, response.Written+len(response.Failures), errors.Join(failures...)) +} + // pushConfigFilesIndividually keeps mixed-version clusters working by using the // long-standing single file endpoint. func pushConfigFilesIndividually(ctx context.Context, node nodeRef, files []ConfigFile, overwrite bool) error { diff --git a/model/namespace.go b/model/namespace.go index 1d6ff40c..dc936933 100644 --- a/model/namespace.go +++ b/model/namespace.go @@ -63,8 +63,8 @@ func (n *Namespace) IsAutoSync() bool { return n != nil && n.SyncStrategy == SyncStrategyAuto } -// EffectiveSyncInterval returns the auto sync interval, falling back to the -// default when the stored value is unusable. +// EffectiveSyncInterval returns the auto sync interval. Zero means "use the +// default", which is what the API accepts when the field is left unset. func (n *Namespace) EffectiveSyncInterval() int { if n == nil || n.SyncIntervalMinutes <= 0 { return DefaultSyncIntervalMinutes