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
4 changes: 4 additions & 0 deletions cmd/asb-api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"fmt"
"math"
"os"
"strconv"
"time"
Expand Down Expand Up @@ -140,6 +141,9 @@ func parsePositiveFloatEnv(key string, fallback float64) (float64, error) {
if err != nil {
return 0, fmt.Errorf("parse %s: %w", key, err)
}
if math.IsNaN(value) || math.IsInf(value, 0) {
return 0, fmt.Errorf("%s must be finite", key)
}
if value <= 0 {
return 0, fmt.Errorf("%s must be greater than zero", key)
}
Expand Down
12 changes: 12 additions & 0 deletions cmd/asb-api/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,15 @@ func TestLoadServerConfigRejectsInvalidValues(t *testing.T) {
t.Fatal("loadServerConfig() error = nil, want non-nil")
}
}

func TestLoadServerConfigRejectsNonFiniteRateLimit(t *testing.T) {
for _, raw := range []string{"NaN", "Inf", "-Inf"} {
t.Run(raw, func(t *testing.T) {
t.Setenv("ASB_HTTP_RATE_LIMIT_RPS", raw)

if _, err := loadServerConfig(); err == nil {
t.Fatalf("loadServerConfig() error = nil for %q, want non-nil", raw)
}
})
}
}
11 changes: 8 additions & 3 deletions internal/api/httpapi/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Service interface {

type Server struct {
service Service
handler http.Handler
maxBody int64
timeouts requestTimeouts
rateLimiter *ratelimit.Limiter
Expand Down Expand Up @@ -64,6 +65,10 @@ func NewServer(service Service, options ...Option) *Server {
for _, option := range options {
option(server)
}
server.handler = http.HandlerFunc(server.serveHTTP)
if server.rateLimiter != nil {
server.handler = server.rateLimiter.Middleware(server.handler)
}
return server
}

Expand Down Expand Up @@ -96,11 +101,11 @@ func WithRateLimiter(limiter *ratelimit.Limiter) Option {
}

func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if s.rateLimiter != nil {
s.rateLimiter.Middleware(http.HandlerFunc(s.serveHTTP)).ServeHTTP(w, r)
if s.handler == nil {
s.serveHTTP(w, r)
return
}
s.serveHTTP(w, r)
s.handler.ServeHTTP(w, r)
}

func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) {
Expand Down
8 changes: 4 additions & 4 deletions internal/app/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ func NewMetrics(serviceName string, opts MetricsOptions) (*Metrics, error) {
prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: prefix + "_budget_exhaustion_total",
Help: "Count of ASB proxy budget exhaustion events by handle.",
Help: "Count of ASB proxy budget exhaustion events by connector kind.",
},
[]string{"handle"},
[]string{"connector_kind"},
),
)
if err != nil {
Expand Down Expand Up @@ -299,11 +299,11 @@ func (metrics *Metrics) recordPolicyEvaluation(capability string, allowed bool)
metrics.policyEval.WithLabelValues(labelOrUnknown(capability), outcome).Inc()
}

func (metrics *Metrics) recordBudgetExhaustion(handle string) {
func (metrics *Metrics) recordBudgetExhaustion(connectorKind string) {
if metrics == nil {
return
}
metrics.budgetExhaust.WithLabelValues(labelOrUnknown(handle)).Inc()
metrics.budgetExhaust.WithLabelValues(labelOrUnknown(connectorKind)).Inc()
}

func (metrics *Metrics) recordArtifactCreated(connectorKind string) {
Expand Down
2 changes: 1 addition & 1 deletion internal/app/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ func TestServiceMetrics_BudgetExhaustion(t *testing.T) {
}

families := mustGatherMetrics(t, registry)
if got := metricValueWithLabels(families, "asb_budget_exhaustion_total", map[string]string{"handle": "ph_budget_metrics"}); got != 1 {
if got := metricValueWithLabels(families, "asb_budget_exhaustion_total", map[string]string{"connector_kind": "github"}); got != 1 {
t.Fatalf("budget exhaustion count = %v, want 1", got)
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/app/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ func (s *Service) ExecuteGitHubProxy(ctx context.Context, req *core.ExecuteGitHu
if s.runtime != nil {
if err := s.runtime.AcquireProxyRequest(ctx, req.ProxyHandle); err != nil {
if errors.Is(err, core.ErrResourceBudgetExceeded) {
s.metrics.recordBudgetExhaustion(req.ProxyHandle)
s.metrics.recordBudgetExhaustion(artifact.ConnectorKind)
}
return nil, fmt.Errorf("execute github proxy %q: acquire proxy request budget: %w", req.ProxyHandle, err)
}
Expand Down Expand Up @@ -545,7 +545,7 @@ func (s *Service) ExecuteGitHubProxy(ctx context.Context, req *core.ExecuteGitHu
if acquired && s.runtime != nil {
if err := s.runtime.CompleteProxyRequest(cleanupCtx, req.ProxyHandle, responseBytes); err != nil {
if errors.Is(err, core.ErrResourceBudgetExceeded) {
s.metrics.recordBudgetExhaustion(req.ProxyHandle)
s.metrics.recordBudgetExhaustion(artifact.ConnectorKind)
}
return nil, fmt.Errorf("execute github proxy %q operation %q: release proxy request budget: %w", req.ProxyHandle, req.Operation, err)
}
Expand Down
7 changes: 5 additions & 2 deletions internal/bootstrap/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,16 @@ func newRuntimeStore(ctx context.Context) (core.RuntimeStore, func(), readinessP
Password: os.Getenv("ASB_REDIS_PASSWORD"),
DB: 0,
})
closeClient := func() { _ = client.Close() }
if err := instrumentDefaultRedisClient(client); err != nil {
closeClient()
return nil, nil, nil, nil, err
}
if err := client.Ping(ctx).Err(); err != nil {
closeClient()
return nil, nil, nil, nil, err
}
return redisstore.NewRuntimeStore(client), func() { _ = client.Close() }, func(ctx context.Context) error {
return redisstore.NewRuntimeStore(client), closeClient, func(ctx context.Context) error {
return client.Ping(ctx).Err()
}, redisPoolStats(client), nil
}
Expand All @@ -507,7 +510,7 @@ func pgxPoolDBStats(pool *pgxpool.Pool) func() sql.DBStats {
}
}

func redisPoolStats(client goredis.UniversalClient) func() *goredis.PoolStats {
func redisPoolStats(client *goredis.Client) func() *goredis.PoolStats {
if client == nil {
return nil
}
Expand Down
Loading