mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
## Overview Part of the **boundary correlation** feature. Fixes lazy creation of `boundary_sessions` rows so it works within the agent's RBAC constraints, and consumes the new `ConfinedProcessName` field reported by boundary. Pairs with coder/boundary#206, which adds `ConfinedProcessName` to `ReportBoundaryLogsRequest`. This branch bumps the `github.com/coder/boundary` module to pick up that work. ## Problem `ensureSession` did a pre-insert existence check via `GetBoundarySessionByID`. Agents are **not permitted to read boundary sessions**, so that read path is not viable when the session is created from an agent-reported log batch. ## Changes - **Remove the pre-insert read.** `ensureSession` now inserts directly and treats a primary-key unique violation as success, covering sessions already created by a prior batch, a reconnection, or another coderd replica — without requiring read access. - **Per-connection guard.** Add a mutex-protected `ensuredSessions` set so repeated log batches on the same connection skip the existence check and insert entirely, touching the database only for the logs. On a transient insert failure the session is left unmarked so the next batch retries. - **Consume `ConfinedProcessName`.** Pass `req.GetConfinedProcessName()` through to the session insert. - **Bump boundary module** from `v0.9.0` to `v0.9.1-0.20260706095856-35ba90f9e8b2`. - **Tests.** - Add `TestReportBoundaryLogsAgentRBAC` (`coderd/boundary_logs_test.go`), an integration test that connects as a real workspace agent, verifies the session and log are persisted under agent RBAC, and asserts the agent subject cannot read boundary sessions — guarding against reintroducing a pre-insert read. - Add `TestReportBoundaryLogsSessionGuard` (session inserted once across two batches, logs inserted per batch) and `TestReportBoundaryLogsSessionRetriedOnError` (insert retried after a transient error). - Regenerate `agent-firewall` CLI docs/golden files and adjust the clidocgen template to render the YAML path when a flag has no long name. > 🤖 This PR was opened by Coder Agents on behalf of @SasSwart.
253 lines
7.7 KiB
Go
253 lines
7.7 KiB
Go
package agentapi
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"sync"
|
|
"time"
|
|
|
|
"github.com/google/uuid"
|
|
"golang.org/x/xerrors"
|
|
|
|
"cdr.dev/slog/v3"
|
|
agentproto "github.com/coder/coder/v2/agent/proto"
|
|
"github.com/coder/coder/v2/coderd/boundaryusage"
|
|
"github.com/coder/coder/v2/coderd/database"
|
|
"github.com/coder/coder/v2/coderd/database/dbtime"
|
|
)
|
|
|
|
const maxBoundaryLogsPerBatch = 1000
|
|
|
|
// ErrBatchSizeExceeded matches any BatchSizeExceededError via errors.Is.
|
|
var ErrBatchSizeExceeded = xerrors.New("boundary logs batch size exceeded")
|
|
|
|
// BatchSizeExceededError is returned when a ReportBoundaryLogs request
|
|
// exceeds maxBoundaryLogsPerBatch. Match it with errors.As for the sizes,
|
|
// or errors.Is(err, ErrBatchSizeExceeded) for the category.
|
|
type BatchSizeExceededError struct {
|
|
BatchSize int
|
|
MaxSize int
|
|
}
|
|
|
|
func (e BatchSizeExceededError) Error() string {
|
|
return fmt.Sprintf("batch size %d exceeds maximum of %d", e.BatchSize, e.MaxSize)
|
|
}
|
|
|
|
func (BatchSizeExceededError) Is(target error) bool {
|
|
return target == ErrBatchSizeExceeded
|
|
}
|
|
|
|
type BoundaryLogsAPI struct {
|
|
Log slog.Logger
|
|
Database database.Store
|
|
AgentID uuid.UUID
|
|
WorkspaceID uuid.UUID
|
|
OwnerID uuid.UUID
|
|
TemplateID uuid.UUID
|
|
TemplateVersionID uuid.UUID
|
|
BoundaryUsageTracker *boundaryusage.Tracker
|
|
|
|
// mu guards ensuredSessions, which records session IDs already persisted
|
|
// by this connection so repeated batches skip the existence check and
|
|
// insert. The API is one instance per agent connection, so this lives for
|
|
// the session's lifetime.
|
|
mu sync.Mutex
|
|
ensuredSessions map[uuid.UUID]struct{}
|
|
}
|
|
|
|
func (a *BoundaryLogsAPI) ReportBoundaryLogs(ctx context.Context, req *agentproto.ReportBoundaryLogsRequest) (*agentproto.ReportBoundaryLogsResponse, error) {
|
|
var allowed, denied int64
|
|
|
|
if len(req.Logs) == 0 {
|
|
a.Log.Debug(ctx, "empty boundary logs request, skipping")
|
|
return &agentproto.ReportBoundaryLogsResponse{}, nil
|
|
}
|
|
|
|
if len(req.Logs) > maxBoundaryLogsPerBatch {
|
|
return nil, BatchSizeExceededError{BatchSize: len(req.Logs), MaxSize: maxBoundaryLogsPerBatch}
|
|
}
|
|
|
|
now := dbtime.Now()
|
|
|
|
// Parse session_id if present. Old boundary clients may not send it,
|
|
// so a missing or invalid session_id disables DB persistence but
|
|
// structured logging and usage tracking still run.
|
|
var sessionID uuid.UUID
|
|
persistEnabled := false
|
|
if raw := req.GetSessionId(); raw != "" {
|
|
parsed, parseErr := uuid.Parse(raw)
|
|
if parseErr != nil {
|
|
a.Log.Warn(ctx, "invalid session_id, persistence disabled for this batch",
|
|
slog.F("raw_session_id", raw),
|
|
slog.Error(parseErr))
|
|
} else {
|
|
sessionID = parsed
|
|
persistEnabled = true
|
|
}
|
|
}
|
|
|
|
if persistEnabled && !a.sessionEnsured(sessionID) {
|
|
// Lazy-create the boundary session on first log arrival.
|
|
// If this fails (transient DB error), we continue so that
|
|
// logs are still persisted. The session will be created on
|
|
// a subsequent batch since every request carries the session
|
|
// details. On success we record the session so later batches
|
|
// skip the existence check and insert entirely.
|
|
if sessionErr := a.ensureSession(ctx, sessionID, req.GetConfinedProcessName(), now); sessionErr != nil {
|
|
a.Log.Error(ctx, "failed to ensure boundary session",
|
|
slog.F("session_id", sessionID.String()),
|
|
slog.Error(sessionErr))
|
|
} else {
|
|
a.markSessionEnsured(sessionID)
|
|
}
|
|
}
|
|
|
|
// Collect batch insert params while iterating.
|
|
batch := database.InsertBoundaryLogsParams{
|
|
SessionID: sessionID,
|
|
OwnerID: a.OwnerID,
|
|
ID: nil,
|
|
SequenceNumber: nil,
|
|
CapturedAt: nil,
|
|
CreatedAt: nil,
|
|
Proto: nil,
|
|
Method: nil,
|
|
Detail: nil,
|
|
MatchedRule: nil,
|
|
}
|
|
|
|
for _, l := range req.Logs {
|
|
logTime := now
|
|
if l.Time != nil {
|
|
logTime = l.Time.AsTime()
|
|
}
|
|
|
|
switch r := l.Resource.(type) {
|
|
case *agentproto.BoundaryLog_HttpRequest_:
|
|
if r.HttpRequest == nil {
|
|
a.Log.Warn(ctx, "empty http request resource",
|
|
slog.F("workspace_id", a.WorkspaceID.String()))
|
|
continue
|
|
}
|
|
|
|
if l.Allowed {
|
|
allowed++
|
|
} else {
|
|
denied++
|
|
}
|
|
|
|
fields := []slog.Field{
|
|
slog.F("decision", allowBoolToString(l.Allowed)),
|
|
slog.F("session_id", req.SessionId),
|
|
slog.F("sequence_number", l.SequenceNumber),
|
|
slog.F("workspace_id", a.WorkspaceID.String()),
|
|
slog.F("template_id", a.TemplateID.String()),
|
|
slog.F("template_version_id", a.TemplateVersionID.String()),
|
|
slog.F("http_method", r.HttpRequest.Method),
|
|
slog.F("http_url", r.HttpRequest.Url),
|
|
slog.F("event_time", logTime.Format(time.RFC3339Nano)),
|
|
}
|
|
if l.Allowed {
|
|
fields = append(fields, slog.F("matched_rule", r.HttpRequest.MatchedRule))
|
|
}
|
|
|
|
a.Log.With(fields...).Info(ctx, "boundary_request")
|
|
|
|
var matchedRule string
|
|
if l.Allowed && r.HttpRequest.MatchedRule != "" {
|
|
matchedRule = r.HttpRequest.MatchedRule
|
|
}
|
|
batch.ID = append(batch.ID, uuid.New())
|
|
batch.SequenceNumber = append(batch.SequenceNumber, l.SequenceNumber)
|
|
batch.CapturedAt = append(batch.CapturedAt, now)
|
|
batch.CreatedAt = append(batch.CreatedAt, logTime)
|
|
batch.Proto = append(batch.Proto, "http")
|
|
batch.Method = append(batch.Method, r.HttpRequest.Method)
|
|
batch.Detail = append(batch.Detail, r.HttpRequest.Url)
|
|
batch.MatchedRule = append(batch.MatchedRule, matchedRule)
|
|
default:
|
|
a.Log.Warn(ctx, "unknown resource type",
|
|
slog.F("workspace_id", a.WorkspaceID.String()))
|
|
}
|
|
}
|
|
|
|
// Batch-insert all collected logs in a single query.
|
|
if persistEnabled && len(batch.ID) > 0 {
|
|
if insertErr := a.insertLogs(ctx, batch); insertErr != nil {
|
|
a.Log.Error(ctx, "failed to insert boundary logs",
|
|
slog.F("session_id", sessionID.String()),
|
|
slog.F("count", len(batch.ID)),
|
|
slog.Error(insertErr))
|
|
}
|
|
}
|
|
|
|
if a.BoundaryUsageTracker != nil && (allowed > 0 || denied > 0) {
|
|
a.BoundaryUsageTracker.Track(a.WorkspaceID, a.OwnerID, allowed, denied)
|
|
}
|
|
|
|
return &agentproto.ReportBoundaryLogsResponse{}, nil
|
|
}
|
|
|
|
// sessionEnsured reports whether this connection has already persisted the
|
|
// session, letting repeated batches skip the database round-trip.
|
|
func (a *BoundaryLogsAPI) sessionEnsured(sessionID uuid.UUID) bool {
|
|
a.mu.Lock()
|
|
defer a.mu.Unlock()
|
|
_, ok := a.ensuredSessions[sessionID]
|
|
return ok
|
|
}
|
|
|
|
// markSessionEnsured records that the session has been persisted.
|
|
func (a *BoundaryLogsAPI) markSessionEnsured(sessionID uuid.UUID) {
|
|
a.mu.Lock()
|
|
defer a.mu.Unlock()
|
|
if a.ensuredSessions == nil {
|
|
a.ensuredSessions = make(map[uuid.UUID]struct{})
|
|
}
|
|
a.ensuredSessions[sessionID] = struct{}{}
|
|
}
|
|
|
|
// ensureSession creates the boundary_sessions row if it does not
|
|
// already exist.
|
|
func (a *BoundaryLogsAPI) ensureSession(ctx context.Context, sessionID uuid.UUID, confinedProcess string, now time.Time) error {
|
|
if a.Database == nil {
|
|
return nil
|
|
}
|
|
|
|
_, err := a.Database.InsertBoundarySession(ctx, database.InsertBoundarySessionParams{
|
|
ID: sessionID,
|
|
WorkspaceAgentID: a.AgentID,
|
|
OwnerID: uuid.NullUUID{UUID: a.OwnerID, Valid: true},
|
|
ConfinedProcessName: confinedProcess,
|
|
StartedAt: now,
|
|
UpdatedAt: now,
|
|
})
|
|
if err != nil {
|
|
if database.IsUniqueViolation(err, database.UniqueBoundarySessionsPkey) {
|
|
a.Log.Debug(ctx, "boundary session already created",
|
|
slog.F("session_id", sessionID.String()))
|
|
return nil
|
|
}
|
|
return xerrors.Errorf("insert boundary session: %w", err)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
// insertLogs persists a batch of boundary log entries.
|
|
func (a *BoundaryLogsAPI) insertLogs(ctx context.Context, batch database.InsertBoundaryLogsParams) error {
|
|
if a.Database == nil {
|
|
return nil
|
|
}
|
|
_, err := a.Database.InsertBoundaryLogs(ctx, batch)
|
|
return err
|
|
}
|
|
|
|
//nolint:revive // This stringifies the boolean argument.
|
|
func allowBoolToString(b bool) string {
|
|
if b {
|
|
return "allow"
|
|
}
|
|
return "deny"
|
|
}
|