diff --git a/README.md b/README.md index 71dfaea..286bcef 100755 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ func OTELMeterProvider() otelmetric.MeterProvider OTELMeterProvider returns the global OTel MeterProvider. This is a convenience accessor for code that needs the interface type. -## func [SetOTELGRPCClientOptions]() +## func [SetOTELGRPCClientOptions]() ```go func SetOTELGRPCClientOptions(opts ...otelgrpc.Option) @@ -146,7 +146,7 @@ func SetOTELGRPCClientOptions(opts ...otelgrpc.Option) Deprecated: Use SetOTELOptions instead. Only applies when OTEL\_USE\_LEGACY\_INSTRUMENTATION=true. -## func [SetOTELGRPCServerOptions]() +## func [SetOTELGRPCServerOptions]() ```go func SetOTELGRPCServerOptions(opts ...otelgrpc.Option) @@ -155,7 +155,7 @@ func SetOTELGRPCServerOptions(opts ...otelgrpc.Option) Deprecated: Use SetOTELOptions instead. Only applies when OTEL\_USE\_LEGACY\_INSTRUMENTATION=true. -## func [SetOTELOptions]() +## func [SetOTELOptions]() ```go func SetOTELOptions(opts grpcotel.Options) @@ -314,7 +314,7 @@ type CB interface { ``` -### func [New]() +### func [New]() ```go func New(c config.Config) CB diff --git a/config/README.md b/config/README.md index e7bbf18..5234f19 100755 --- a/config/README.md +++ b/config/README.md @@ -66,7 +66,7 @@ import "github.com/go-coldbrew/core/config" -## type [Config]() +## type [Config]() Config is the configuration for the Coldbrew server It is populated from environment variables and has sensible defaults for all fields so that you can just use it as is without any configuration The following environment variables are supported and can be used to override the defaults for the fields @@ -78,6 +78,11 @@ type Config struct { GRPCPort int `envconfig:"GRPC_PORT" default:"9090"` // HTTP Port, defaults to 9091 HTTPPort int `envconfig:"HTTP_PORT" default:"9091"` + // AdminPort is an optional dedicated port for admin endpoints (pprof, metrics, swagger). + // When set to a non-zero value, admin endpoints are served on this port instead of HTTPPort. + // This allows network-level isolation (e.g., Kubernetes NetworkPolicy) to restrict access + // to profiling and metrics data. Default 0 (no dedicated admin server; admin endpoints served on HTTPPort). + AdminPort int `envconfig:"ADMIN_PORT" default:"0"` // Name of the Application AppName string `envconfig:"APP_NAME" default:""` // Environment e.g. Production / Integration / Development @@ -256,7 +261,7 @@ type Config struct { ``` -### func \(Config\) [Validate]() +### func \(Config\) [Validate]() ```go func (c Config) Validate() []string diff --git a/config/config.go b/config/config.go index 0cb2810..e7303ad 100644 --- a/config/config.go +++ b/config/config.go @@ -16,6 +16,11 @@ type Config struct { GRPCPort int `envconfig:"GRPC_PORT" default:"9090"` // HTTP Port, defaults to 9091 HTTPPort int `envconfig:"HTTP_PORT" default:"9091"` + // AdminPort is an optional dedicated port for admin endpoints (pprof, metrics, swagger). + // When set to a non-zero value, admin endpoints are served on this port instead of HTTPPort. + // This allows network-level isolation (e.g., Kubernetes NetworkPolicy) to restrict access + // to profiling and metrics data. Default 0 (no dedicated admin server; admin endpoints served on HTTPPort). + AdminPort int `envconfig:"ADMIN_PORT" default:"0"` // Name of the Application AppName string `envconfig:"APP_NAME" default:""` // Environment e.g. Production / Integration / Development @@ -212,6 +217,17 @@ func (c Config) Validate() []string { if c.GRPCPort == c.HTTPPort && c.GRPCPort != 0 { warnings = append(warnings, "GRPCPort and HTTPPort are the same, this will cause a port conflict") } + if c.AdminPort < 0 || c.AdminPort > 65535 { + warnings = append(warnings, "AdminPort is out of valid range (0-65535)") + } + if c.AdminPort > 0 { + if c.AdminPort == c.GRPCPort { + warnings = append(warnings, "AdminPort and GRPCPort are the same, this will cause a port conflict") + } + if c.AdminPort == c.HTTPPort { + warnings = append(warnings, "AdminPort equals HTTPPort; admin endpoints will be served on HTTPPort (combined mode)") + } + } if c.NewRelicOpentelemetrySample < 0 || c.NewRelicOpentelemetrySample > 1.0 { warnings = append(warnings, "NewRelicOpentelemetrySample should be between 0.0 and 1.0") } diff --git a/config/config_test.go b/config/config_test.go index 409bdf1..4ef2364 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -186,6 +186,74 @@ func TestValidateLogLevel(t *testing.T) { } } +func TestValidateAdminPortRange(t *testing.T) { + c := Config{ + GRPCPort: 9090, + HTTPPort: 9091, + AdminPort: 70000, + } + warnings := c.Validate() + found := false + for _, w := range warnings { + if strings.Contains(w, "AdminPort") && strings.Contains(w, "range") { + found = true + } + } + if !found { + t.Error("out-of-range AdminPort should produce a warning") + } +} + +func TestValidateAdminPortConflictGRPC(t *testing.T) { + c := Config{ + GRPCPort: 9090, + HTTPPort: 9091, + AdminPort: 9090, + } + warnings := c.Validate() + found := false + for _, w := range warnings { + if strings.Contains(w, "AdminPort") && strings.Contains(w, "GRPCPort") { + found = true + } + } + if !found { + t.Error("AdminPort == GRPCPort should produce a warning") + } +} + +func TestValidateAdminPortConflictHTTP(t *testing.T) { + c := Config{ + GRPCPort: 9090, + HTTPPort: 9091, + AdminPort: 9091, + } + warnings := c.Validate() + found := false + for _, w := range warnings { + if strings.Contains(w, "AdminPort") && strings.Contains(w, "HTTPPort") { + found = true + } + } + if !found { + t.Error("AdminPort == HTTPPort should produce a warning") + } +} + +func TestValidateAdminPortZeroNoWarning(t *testing.T) { + c := Config{ + GRPCPort: 9090, + HTTPPort: 9091, + AdminPort: 0, + } + warnings := c.Validate() + for _, w := range warnings { + if strings.Contains(w, "AdminPort") { + t.Errorf("AdminPort=0 should not produce AdminPort warnings, got: %s", w) + } + } +} + func TestValidateTimeoutExceedsShutdown(t *testing.T) { c := Config{ GRPCPort: 9090, diff --git a/core.go b/core.go index 88cd4b1..a85700d 100644 --- a/core.go +++ b/core.go @@ -59,6 +59,7 @@ type cb struct { closers []io.Closer grpcServer *grpc.Server httpServer *http.Server + adminServer *http.Server cancelFunc context.CancelFunc gracefulWait sync.WaitGroup creds credentials.TransportCredentials @@ -550,6 +551,24 @@ func (c *cb) initHTTP(ctx context.Context) (*http.Server, error) { } adminMux.Handle(swaggerPattern, http.StripPrefix(swaggerPattern, c.openAPIHandler)) } + if c.config.AdminPort > 0 && c.config.AdminPort != c.config.HTTPPort { + // Separate servers: admin endpoints on AdminPort, gateway on HTTPPort. + adminAddr := fmt.Sprintf("%s:%d", c.config.ListenHost, c.config.AdminPort) + c.adminServer = &http.Server{ + Addr: adminAddr, + Handler: adminMux, + } + log.Info(ctx, "msg", "Starting admin server", "address", adminAddr) + + gwServer := &http.Server{ + Addr: gatewayAddr, + Handler: gzipHandler, + } + log.Info(ctx, "msg", "Starting HTTP gateway server", "address", gatewayAddr) + return gwServer, nil + } + + // Combined server: admin + gateway on HTTPPort (default behavior). adminMux.Handle("/", gzipHandler) gwServer := &http.Server{ Addr: gatewayAddr, @@ -817,6 +836,15 @@ func (c *cb) Run() error { } return err }) + if c.adminServer != nil { + g.Go(func() error { + err := c.runHTTP(gctx, c.adminServer) + if errors.Is(err, http.ErrServerClosed) { + return nil + } + return err + }) + } // When one server exits with an unexpected error (or parent context is // cancelled by signal handler), stop the peer so g.Wait() completes. g.Go(func() error { @@ -827,6 +855,9 @@ func (c *cb) Run() error { if c.httpServer != nil { c.httpServer.Close() } + if c.adminServer != nil { + c.adminServer.Close() + } return nil }) err = g.Wait() @@ -878,6 +909,11 @@ func (c *cb) Stop(dur time.Duration) error { log.Info(context.Background(), "msg", "graceful shutdown timer finished", "duration", d) } log.Info(context.Background(), "msg", "Server shut down started, bye bye") + if c.adminServer != nil { + if err := c.adminServer.Shutdown(ctx); err != nil { + log.Error(context.Background(), "msg", "admin server shutdown error", "err", err) + } + } if c.httpServer != nil { if err := c.httpServer.Shutdown(ctx); err != nil { log.Error(context.Background(), "msg", "http server shutdown error", "err", err) diff --git a/core_coverage_test.go b/core_coverage_test.go index 43ff707..b4f1436 100644 --- a/core_coverage_test.go +++ b/core_coverage_test.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "testing" "time" @@ -1449,3 +1450,145 @@ func TestProcessConfig_NRAutoDisable(t *testing.T) { }) } } + +func TestInitHTTP_AdminPortSeparation(t *testing.T) { + // removed t.Parallel() — core tests mutate package-level globals + c := &cb{ + config: config.Config{ + GRPCPort: 19090, + HTTPPort: 19091, + AdminPort: 19092, + ListenHost: "127.0.0.1", + }, + svc: []CBService{&testService{}}, + } + svr, err := c.initHTTP(context.Background()) + if err != nil { + t.Fatalf("initHTTP failed: %v", err) + } + + // Gateway server should NOT serve admin endpoints. + for _, path := range []string{"/debug/pprof/", "/metrics"} { + req := httptest.NewRequest("GET", path, nil) + w := httptest.NewRecorder() + svr.Handler.ServeHTTP(w, req) + // Gateway has no routes for admin paths — must not return pprof/prometheus content. + body := w.Body.String() + if strings.Contains(body, "pprof") || strings.Contains(body, "go_goroutines") { + t.Errorf("admin endpoint %s should NOT be on gateway server when AdminPort is set (got body containing admin content)", path) + } + } + + // Admin server should serve admin endpoints. + if c.adminServer == nil { + t.Fatal("expected adminServer to be set when AdminPort > 0") + } + for _, path := range []string{"/debug/pprof/", "/metrics"} { + req := httptest.NewRequest("GET", path, nil) + w := httptest.NewRecorder() + c.adminServer.Handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("expected 200 for admin %s, got %d", path, w.Code) + } + } +} + +func TestInitHTTP_AdminPortZero_CombinedBehavior(t *testing.T) { + // removed t.Parallel() — core tests mutate package-level globals + c := &cb{ + config: config.Config{ + GRPCPort: 19090, + HTTPPort: 19091, + AdminPort: 0, + ListenHost: "127.0.0.1", + }, + svc: []CBService{&testService{}}, + } + svr, err := c.initHTTP(context.Background()) + if err != nil { + t.Fatalf("initHTTP failed: %v", err) + } + + // Admin server should NOT be created when AdminPort is 0. + if c.adminServer != nil { + t.Fatal("expected adminServer to be nil when AdminPort is 0") + } + + // Combined server should serve admin endpoints on HTTPPort. + for _, path := range []string{"/debug/pprof/", "/metrics"} { + req := httptest.NewRequest("GET", path, nil) + w := httptest.NewRecorder() + svr.Handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("expected 200 for %s on combined server, got %d", path, w.Code) + } + } +} + +func TestInitHTTP_AdminPortEqualsHTTPPort_CombinedMode(t *testing.T) { + // removed t.Parallel() — core tests mutate package-level globals + c := &cb{ + config: config.Config{ + GRPCPort: 19090, + HTTPPort: 19091, + AdminPort: 19091, // same as HTTPPort — should use combined mode + ListenHost: "127.0.0.1", + }, + svc: []CBService{&testService{}}, + } + svr, err := c.initHTTP(context.Background()) + if err != nil { + t.Fatalf("initHTTP failed: %v", err) + } + + // Should NOT create a separate admin server. + if c.adminServer != nil { + t.Fatal("expected adminServer to be nil when AdminPort == HTTPPort") + } + + // Combined server should serve admin endpoints. + for _, path := range []string{"/debug/pprof/", "/metrics"} { + req := httptest.NewRequest("GET", path, nil) + w := httptest.NewRecorder() + svr.Handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("expected 200 for %s on combined server, got %d", path, w.Code) + } + } +} + +func TestInitHTTP_AdminPortSwagger(t *testing.T) { + // removed t.Parallel() — core tests mutate package-level globals + handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte("swagger-admin")) + }) + c := &cb{ + config: config.Config{ + GRPCPort: 19090, + HTTPPort: 19091, + AdminPort: 19092, + ListenHost: "127.0.0.1", + SwaggerURL: "/swagger/", + }, + svc: []CBService{&testService{}}, + openAPIHandler: handler, + } + _, err := c.initHTTP(context.Background()) + if err != nil { + t.Fatalf("initHTTP failed: %v", err) + } + + // Swagger should be on admin server. + if c.adminServer == nil { + t.Fatal("expected adminServer to be set when AdminPort > 0") + } + req := httptest.NewRequest("GET", "/swagger/index.html", nil) + w := httptest.NewRecorder() + c.adminServer.Handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("expected 200 for swagger on admin server, got %d", w.Code) + } + if w.Body.String() != "swagger-admin" { + t.Fatalf("expected 'swagger-admin' body, got %q", w.Body.String()) + } +}