From 02a4ba0491904cc258f69bfe8d8b0a4f47509114 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 16 Jun 2026 13:00:32 +0200 Subject: [PATCH] controller: immediate hub report push on geo change + always-report geo (B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Geo settings save and manual geo sync now fire an out-of-band, non-blocking hub report push (Router.reportPushNow seam, wired in main.go to BuildReport+Push in a goroutine) so the hub reflects the new geo state / clears a stale last_sync_error within seconds instead of after the next ~15-min cycle. Scope: geo handlers only. - builder.go always populates report.GeoRestriction (Enabled=false, empty countries when nil/disabled) via new buildGeoRestrictionReport helper, so the hub always renders the geo section ("Inaktív" when off) instead of hiding it via omitempty. - Tests: geo save success → push once; invalid country → no push (companion); buildGeoRestrictionReport(nil) → non-nil disabled (companion vs old nil-omit). Co-Authored-By: Claude Opus 4.8 (1M context) --- controller/cmd/controller/main.go | 12 ++++ controller/internal/api/geo.go | 8 +++ controller/internal/api/geo_test.go | 57 +++++++++++++++++++ controller/internal/report/builder.go | 44 ++++++++------ controller/internal/report/geo_report_test.go | 40 +++++++++++++ 5 files changed, 145 insertions(+), 16 deletions(-) create mode 100644 controller/internal/api/geo_test.go create mode 100644 controller/internal/report/geo_report_test.go diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 8cb301b..07ad62f 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -573,6 +573,18 @@ func main() { // --- Initialize API router --- apiRouter := api.NewRouter(cfg, *configPath, sett, stackMgr, syncer, cpuCollector, backupMgr, metricsStore, updater, notifier, logger) + if hubPusher != nil { + // Out-of-band, non-blocking hub report push (e.g. after a geo settings change) so + // the hub reflects the new state immediately instead of after the next ~15-min cycle. + apiRouter.SetReportPushTrigger(func() { + go func() { + rep := report.BuildReport(cfg, *configPath, stackMgr, backupMgr, cpuCollector, metricsStore, Version, sett.GetStoragePaths(), sett.GetGeoRestriction(), logger) + if err := hubPusher.Push(rep); err != nil { + logger.Printf("[WARN] [report] Out-of-band geo report push failed: %v", err) + } + }() + }) + } if assetsSyncer != nil { apiRouter.SetAssetsSyncer(assetsSyncer) } diff --git a/controller/internal/api/geo.go b/controller/internal/api/geo.go index 47ac3bf..e7f468c 100644 --- a/controller/internal/api/geo.go +++ b/controller/internal/api/geo.go @@ -82,6 +82,10 @@ func (r *Router) geoUpdateSettings(w http.ResponseWriter, req *http.Request) { }() } + // Push a fresh hub report out-of-band so the hub reflects the new geo state right + // away instead of after the next ~15-min cycle. + r.reportPushNow() + writeJSON(w, http.StatusOK, apiResponse{OK: true, Message: "Geo-korlátozás beállítva"}) } @@ -94,7 +98,11 @@ func (r *Router) geoTriggerSync(w http.ResponseWriter, _ *http.Request) { go func() { if err := r.geoSync.Sync(context.Background()); err != nil { r.logger.Printf("[ERROR] [api] Manual geo sync failed: %v", err) + return } + // On success the sync clears any stale last_sync_error — push a report so the hub + // reflects the cleared state immediately rather than after the next cycle. + r.reportPushNow() }() writeJSON(w, http.StatusOK, apiResponse{OK: true, Message: "Szinkronizálás elindítva"}) diff --git a/controller/internal/api/geo_test.go b/controller/internal/api/geo_test.go new file mode 100644 index 0000000..b8f1c35 --- /dev/null +++ b/controller/internal/api/geo_test.go @@ -0,0 +1,57 @@ +package api + +import ( + "bytes" + "io" + "log" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +func newGeoTestRouter(t *testing.T) (*Router, *int) { + t.Helper() + sett, err := settings.Load(filepath.Join(t.TempDir(), "settings.json"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatalf("settings.Load: %v", err) + } + calls := 0 + r := &Router{cfg: &config.Config{}, sett: sett, logger: log.New(io.Discard, "", 0)} + r.triggerReportPush = func() { calls++ } + return r, &calls +} + +func postGeoSettings(t *testing.T, r *Router, body string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/api/geo/settings", bytes.NewReader([]byte(body))) + rec := httptest.NewRecorder() + r.geoUpdateSettings(rec, req) + return rec +} + +func TestGeoUpdateSettings_Success_PushesReport(t *testing.T) { + r, calls := newGeoTestRouter(t) + rec := postGeoSettings(t, r, `{"enabled":true,"allowed_countries":["HU"]}`) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + if *calls != 1 { + t.Fatalf("report push called %d times, want 1 (successful save)", *calls) + } +} + +// COMPANION: a validation-failed save (invalid country code) must NOT push a report. +func TestGeoUpdateSettings_InvalidCountry_NoPush(t *testing.T) { + r, calls := newGeoTestRouter(t) + rec := postGeoSettings(t, r, `{"enabled":true,"allowed_countries":["XX"]}`) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (invalid country code)", rec.Code) + } + if *calls != 0 { + t.Fatalf("report push called %d times, want 0 (validation failed before save)", *calls) + } +} diff --git a/controller/internal/report/builder.go b/controller/internal/report/builder.go index 1b6fa38..e253437 100644 --- a/controller/internal/report/builder.go +++ b/controller/internal/report/builder.go @@ -155,22 +155,11 @@ func BuildReport( // App telemetry (metrics + log scan) r.AppTelemetry = buildAppTelemetrySection(stackMgr, metricsStore, logger) - // Geo-restriction status - if geoRestriction != nil { - gr := &GeoRestrictionReport{ - Enabled: geoRestriction.Enabled, - AllowedCountries: geoRestriction.AllowedCountries, - LastSync: geoRestriction.LastSync, - LastSyncError: geoRestriction.LastSyncError, - } - if len(geoRestriction.AppOverrides) > 0 { - gr.AppOverrides = make(map[string]GeoAppOverrideReport, len(geoRestriction.AppOverrides)) - for k, v := range geoRestriction.AppOverrides { - gr.AppOverrides[k] = GeoAppOverrideReport{AllowedCountries: v.AllowedCountries} - } - } - r.GeoRestriction = gr - } + // Geo-restriction status — ALWAYS present (even when never configured) so the hub + // always renders the section. A nil pointer (omitempty) made the hub hide the whole + // section for a never-configured controller; a present-but-disabled report renders + // "Inaktív" hub-side. + r.GeoRestriction = buildGeoRestrictionReport(geoRestriction) if debug && logger != nil { logger.Printf("[DEBUG] [report] BuildReport: complete — containers=%d, health=%s, deployed=%d, available=%d, app_telemetry=%d", @@ -180,6 +169,29 @@ func BuildReport( return r } +// buildGeoRestrictionReport always returns a non-nil report so the hub always renders the +// geo-restriction section. A nil or never-configured input yields Enabled=false with an +// empty (non-nil) country list — the hub renders that as "Inaktív". +func buildGeoRestrictionReport(geo *settings.GeoRestriction) *GeoRestrictionReport { + gr := &GeoRestrictionReport{Enabled: false, AllowedCountries: []string{}} + if geo == nil { + return gr + } + gr.Enabled = geo.Enabled + if geo.AllowedCountries != nil { + gr.AllowedCountries = geo.AllowedCountries + } + gr.LastSync = geo.LastSync + gr.LastSyncError = geo.LastSyncError + if len(geo.AppOverrides) > 0 { + gr.AppOverrides = make(map[string]GeoAppOverrideReport, len(geo.AppOverrides)) + for k, v := range geo.AppOverrides { + gr.AppOverrides[k] = GeoAppOverrideReport{AllowedCountries: v.AllowedCountries} + } + } + return gr +} + func buildContainerReport(stackMgr *stacks.Manager, metricsStore *metrics.MetricsStore) ContainerReport { cr := ContainerReport{} diff --git a/controller/internal/report/geo_report_test.go b/controller/internal/report/geo_report_test.go new file mode 100644 index 0000000..22ebb0c --- /dev/null +++ b/controller/internal/report/geo_report_test.go @@ -0,0 +1,40 @@ +package report + +import ( + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// buildGeoRestrictionReport must ALWAYS return a non-nil, present-but-disabled report for +// a nil input. The old inline code left Report.GeoRestriction nil (omitempty), which made +// the hub hide the whole geo section for a never-configured controller — this test fails +// against that old behaviour (companion). +func TestBuildGeoRestrictionReport_NilIsPresentAndDisabled(t *testing.T) { + gr := buildGeoRestrictionReport(nil) + if gr == nil { + t.Fatal("geo report is nil for nil input — the hub would omit the section") + } + if gr.Enabled { + t.Fatalf("Enabled = true, want false for nil input") + } + if gr.AllowedCountries == nil { + t.Fatalf("AllowedCountries is nil, want empty non-nil slice (renders as [], not null)") + } +} + +func TestBuildGeoRestrictionReport_PopulatedPassesThrough(t *testing.T) { + in := &settings.GeoRestriction{Enabled: true, AllowedCountries: []string{"HU", "DE"}} + gr := buildGeoRestrictionReport(in) + if gr == nil || !gr.Enabled || len(gr.AllowedCountries) != 2 { + t.Fatalf("passthrough failed: %+v", gr) + } +} + +// A disabled config with a nil country list must still yield an empty (non-nil) list. +func TestBuildGeoRestrictionReport_DisabledNilCountries(t *testing.T) { + gr := buildGeoRestrictionReport(&settings.GeoRestriction{Enabled: false}) + if gr == nil || gr.Enabled || gr.AllowedCountries == nil { + t.Fatalf("disabled report wrong: %+v", gr) + } +}