Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

<a name="SetOTELGRPCClientOptions"></a>
## func [SetOTELGRPCClientOptions](<https://github.com/go-coldbrew/core/blob/main/core.go#L600>)
## func [SetOTELGRPCClientOptions](<https://github.com/go-coldbrew/core/blob/main/core.go#L619>)

```go
func SetOTELGRPCClientOptions(opts ...otelgrpc.Option)
Expand All @@ -146,7 +146,7 @@ func SetOTELGRPCClientOptions(opts ...otelgrpc.Option)
Deprecated: Use SetOTELOptions instead. Only applies when OTEL\_USE\_LEGACY\_INSTRUMENTATION=true.

<a name="SetOTELGRPCServerOptions"></a>
## func [SetOTELGRPCServerOptions](<https://github.com/go-coldbrew/core/blob/main/core.go#L594>)
## func [SetOTELGRPCServerOptions](<https://github.com/go-coldbrew/core/blob/main/core.go#L613>)

```go
func SetOTELGRPCServerOptions(opts ...otelgrpc.Option)
Expand All @@ -155,7 +155,7 @@ func SetOTELGRPCServerOptions(opts ...otelgrpc.Option)
Deprecated: Use SetOTELOptions instead. Only applies when OTEL\_USE\_LEGACY\_INSTRUMENTATION=true.

<a name="SetOTELOptions"></a>
## func [SetOTELOptions](<https://github.com/go-coldbrew/core/blob/main/core.go#L607>)
## func [SetOTELOptions](<https://github.com/go-coldbrew/core/blob/main/core.go#L626>)

```go
func SetOTELOptions(opts grpcotel.Options)
Expand Down Expand Up @@ -314,7 +314,7 @@ type CB interface {
```

<a name="New"></a>
### func [New](<https://github.com/go-coldbrew/core/blob/main/core.go#L920>)
### func [New](<https://github.com/go-coldbrew/core/blob/main/core.go#L956>)

```go
func New(c config.Config) CB
Expand Down
9 changes: 7 additions & 2 deletions config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import "github.com/go-coldbrew/core/config"


<a name="Config"></a>
## type [Config](<https://github.com/go-coldbrew/core/blob/main/config/config.go#L12-L198>)
## type [Config](<https://github.com/go-coldbrew/core/blob/main/config/config.go#L12-L203>)

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

Expand All @@ -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
Expand Down Expand Up @@ -256,7 +261,7 @@ type Config struct {
```

<a name="Config.Validate"></a>
### func \(Config\) [Validate](<https://github.com/go-coldbrew/core/blob/main/config/config.go#L203>)
### func \(Config\) [Validate](<https://github.com/go-coldbrew/core/blob/main/config/config.go#L208>)

```go
func (c Config) Validate() []string
Expand Down
16 changes: 16 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand Down
68 changes: 68 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
36 changes: 36 additions & 0 deletions core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
143 changes: 143 additions & 0 deletions core_coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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 {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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())
}
}
Loading