diff --git a/cmd/campaign-service/server.go b/cmd/campaign-service/server.go index 20a08bfe..6922ae50 100644 --- a/cmd/campaign-service/server.go +++ b/cmd/campaign-service/server.go @@ -9,12 +9,15 @@ package main import ( "context" + "fmt" "log/slog" "net/http" "os" "sync" + connsvcsvr "github.com/linuxfoundation/lfx-v2-campaign-service/gen/http/lfx_v2_campaign_service_connections/server" svcsvr "github.com/linuxfoundation/lfx-v2-campaign-service/gen/http/lfx_v2_campaign_service_svc/server" + connsvc "github.com/linuxfoundation/lfx-v2-campaign-service/gen/lfx_v2_campaign_service_connections" svc "github.com/linuxfoundation/lfx-v2-campaign-service/gen/lfx_v2_campaign_service_svc" "github.com/linuxfoundation/lfx-v2-campaign-service/internal/container" "github.com/linuxfoundation/lfx-v2-campaign-service/internal/infrastructure/config" @@ -34,15 +37,34 @@ func StartServer(ctx context.Context, cfg *config.Config) error { return err } + // NOTE: debug.LogPayloads() is intentionally NOT applied. Every authenticated + // payload carries a BearerToken (a valid JWT), and the connection service's + // create/set-credential payloads carry plaintext provider credentials, so + // enabling it would leak those secrets into logs. debug.HTTP() IS still + // applied below, but in clue v1.2.1 it does not log headers or statuses — it + // only propagates the runtime /debug toggle into each request's context (so + // debug-level logs elsewhere activate); it decodes no payload. endpoints := svc.NewEndpoints(cont.Service) - if cfg.Debug { - endpoints.Use(debug.LogPayloads()) + + // The container always initializes Connections (NewContainer wires it in both + // the DB and no-DB paths), so construct the endpoints unconditionally. Fail + // loudly if it's unexpectedly nil rather than silently skipping the mount — + // a mis-wired container should crash at startup, not serve 404s on the + // connection routes. + if cont.Connections == nil { + return fmt.Errorf("container misconfigured: Connections service is nil") } + connEndpoints := connsvc.NewEndpoints(cont.Connections) - return handleHTTPServer(ctx, cfg, endpoints, cont) + return handleHTTPServer(ctx, cfg, endpoints, connEndpoints, cont) } -func handleHTTPServer(ctx context.Context, cfg *config.Config, endpoints *svc.Endpoints, cont *container.Container) error { +// buildMux constructs the Goa muxer and mounts BOTH the campaign service and the +// connection service onto it. It is a seam so tests can assert that the +// connection routes are actually reachable (the bug this fixes — routes that +// compile but are never mounted return 404) without standing up a full server. +// It returns an error only for a programmer-level mis-wiring (nil connEndpoints). +func buildMux(ctx context.Context, cfg *config.Config, endpoints *svc.Endpoints, connEndpoints *connsvc.Endpoints) (goahttp.Muxer, error) { mux := goahttp.NewMuxer() if cfg.Debug { debug.MountPprofHandlers(debug.Adapt(mux)) @@ -70,6 +92,27 @@ func handleHTTPServer(ctx context.Context, cfg *config.Config, endpoints *svc.En ) svcsvr.Mount(mux, server) + // Mount the connection routes unconditionally. connEndpoints is always + // non-nil (StartServer constructs it and fails loudly if the service is + // mis-wired), and the connection service is wired even without a database + // (with a nil repo) so its routes return the typed 503 rather than a bare + // 404. A nil here would be a programmer error, so fail loudly rather than + // silently skipping the mount and serving 404s. + if connEndpoints == nil { + return nil, fmt.Errorf("buildMux: connEndpoints is nil (connection routes would be unmounted)") + } + connServer := connsvcsvr.New(connEndpoints, mux, goahttp.RequestDecoder, goahttp.ResponseEncoder, eh, nil) + connsvcsvr.Mount(mux, connServer) + + return mux, nil +} + +func handleHTTPServer(ctx context.Context, cfg *config.Config, endpoints *svc.Endpoints, connEndpoints *connsvc.Endpoints, cont *container.Container) error { + mux, err := buildMux(ctx, cfg, endpoints, connEndpoints) + if err != nil { + return err + } + var handler http.Handler = mux handler = middleware.RequestIDMiddleware()(handler) if cfg.Debug { diff --git a/cmd/campaign-service/server_test.go b/cmd/campaign-service/server_test.go new file mode 100644 index 00000000..8eec48f8 --- /dev/null +++ b/cmd/campaign-service/server_test.go @@ -0,0 +1,67 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package main + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + connsvc "github.com/linuxfoundation/lfx-v2-campaign-service/gen/lfx_v2_campaign_service_connections" + svc "github.com/linuxfoundation/lfx-v2-campaign-service/gen/lfx_v2_campaign_service_svc" + "github.com/linuxfoundation/lfx-v2-campaign-service/internal/infrastructure/config" + "github.com/linuxfoundation/lfx-v2-campaign-service/internal/service" +) + +// TestConnectionRoutesAreMounted locks in the invariant this PR establishes: the +// connection routes are actually reachable on the mux. The bug being fixed — +// generated routes that compile but are never mounted — is invisible to the +// service-layer tests (which call handlers directly), so without this test a +// future deletion of the connsvcsvr.Mount call would silently reintroduce the +// 404 regression. We assert a known connection route resolves to a real handler +// (any non-404 status, e.g. 401/503, proves it is mounted). +func TestConnectionRoutesAreMounted(t *testing.T) { + endpoints := svc.NewEndpoints(service.NewCampaignService(nil)) + // The no-DB connection service is a real Service whose routes return a typed + // 503 rather than 404 — perfect for proving the route is mounted. + connEndpoints := connsvc.NewEndpoints(service.NewConnectionService(nil, nil)) + + mux, err := buildMux(context.Background(), &config.Config{}, endpoints, connEndpoints) + if err != nil { + t.Fatalf("buildMux: %v", err) + } + + // One route per mounted server is enough to lock in the mount. + cases := []struct { + name string + method string + path string + }{ + {"connection google-ads create", http.MethodPost, "/projects/proj-123/connection-google-ads"}, + {"campaign health livez", http.MethodGet, "/livez"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(tc.method, tc.path, strings.NewReader("{}")) + req.Header.Set("Content-Type", "application/json") + mux.ServeHTTP(rec, req) + if rec.Code == http.StatusNotFound { + t.Errorf("%s %s returned 404 — route is not mounted", tc.method, tc.path) + } + }) + } +} + +// TestBuildMuxNilConnEndpointsFailsLoud verifies the fail-loud guard: a nil +// connEndpoints (a programmer-level mis-wiring) returns an error rather than +// silently building a mux with the connection routes unmounted. +func TestBuildMuxNilConnEndpointsFailsLoud(t *testing.T) { + endpoints := svc.NewEndpoints(service.NewCampaignService(nil)) + if _, err := buildMux(context.Background(), &config.Config{}, endpoints, nil); err == nil { + t.Fatal("expected buildMux to fail loudly when connEndpoints is nil, got nil error") + } +} diff --git a/docs/knowledge/code/cmd-campaign-service.md b/docs/knowledge/code/cmd-campaign-service.md index 953b41f2..8cf7b1d8 100644 --- a/docs/knowledge/code/cmd-campaign-service.md +++ b/docs/knowledge/code/cmd-campaign-service.md @@ -7,6 +7,16 @@ resource: "cmd/campaign-service" # cmd/campaign-service -The LFX V2 Campaign Service. +The LFX V2 Campaign Service. `server.go` builds the HTTP server and mounts each +wired Goa service's handlers — currently the health and connection servers. +Every service the container wires must also be mounted here: a service +constructed in the container but not mounted is unreachable (its routes 404) +even though the code compiles, which is the bug this change fixes for the +connection routes. `debug.LogPayloads()` is intentionally not applied to any +service: payloads carry bearer tokens and (for connections) plaintext provider +credentials, so DEBUG payload logging would leak secrets. `debug.HTTP()` is +still applied, but in clue v1.2.1 it does not log headers or statuses — it only +propagates the runtime `/debug` toggle into the request context (activating +debug-level logs elsewhere); it decodes no payload. See [cmd/campaign-service](../../../cmd/campaign-service). diff --git a/docs/knowledge/log.md b/docs/knowledge/log.md index 9dc95362..557da167 100644 --- a/docs/knowledge/log.md +++ b/docs/knowledge/log.md @@ -10,6 +10,9 @@ strictly parsed to reject impossible calendar values; name lookups propagate errors instead of masking them as not-found. Added the `internal/platform/twitter` code concept and index entry. +**Update** — Mount connection routes in the HTTP server (LFXV2-2556): the +`cmd/campaign-service` concept now notes that every container-wired service +must also be mounted in `server.go`, or its routes 404 despite compiling. **Update** — Dropped the Goa CLI path allowlist; twitter-api-secret FP is fingerprint-only in `.gitleaksignore`. Clarified `.grype.yaml` rationale