diff --git a/app/artifact-cas/internal/service/auditor.go b/app/artifact-cas/internal/service/auditor.go index 56e2f7405..bb4e75e17 100644 --- a/app/artifact-cas/internal/service/auditor.go +++ b/app/artifact-cas/internal/service/auditor.go @@ -16,44 +16,42 @@ package service import ( - "fmt" - "github.com/chainloop-dev/chainloop/app/controlplane/pkg/auditor" casJWT "github.com/chainloop-dev/chainloop/internal/robotaccount/cas" "github.com/chainloop-dev/chainloop/pkg/servicelogger" - "github.com/getsentry/sentry-go" "github.com/go-kratos/kratos/v2/log" "github.com/google/uuid" ) -// eventPublisher is the subset of auditor.AuditLogPublisher used by the dispatcher -type eventPublisher interface { - Publish(data *auditor.EventPayload) error -} - -// AuditDispatcher publishes CAS audit events. Unlike the control plane's -// biz.AuditorUseCase, the actor is always SYSTEM (CAS JWTs carry no user -// identity) and the org comes from the JWT claims instead of the request context. +// AuditDispatcher publishes CAS audit events. It delegates the shared +// generate -> publish -> error-reporting flow to the control plane's +// auditor.Dispatcher and only owns the CAS-specific actor/org policy: unlike +// the control plane's biz.AuditorUseCase, the actor is always SYSTEM (CAS JWTs +// carry no user identity) and the org comes from the JWT claims instead of the +// request context. type AuditDispatcher struct { - // nil when NATS is not configured, making the dispatcher a no-op - publisher eventPublisher - log *log.Helper + dispatcher *auditor.Dispatcher + log *log.Helper } func NewAuditDispatcher(publisher *auditor.AuditLogPublisher, logger log.Logger) *AuditDispatcher { - d := &AuditDispatcher{log: servicelogger.ScopedHelper(logger, "audit-dispatcher")} - // keep the interface nil when the publisher is disabled so shouldEmit short-circuits + // keep the Publisher interface nil when the publisher is disabled so the + // dispatcher short-circuits instead of holding a typed-nil interface + var p auditor.Publisher if publisher != nil { - d.publisher = publisher + p = publisher } - return d + return &AuditDispatcher{ + dispatcher: auditor.NewDispatcher(p, logger), + log: servicelogger.ScopedHelper(logger, "audit-dispatcher"), + } } // shouldEmit returns true when Dispatch would actually publish an event for the // given claims. Hooks use it to skip extra work (e.g. backend Describe round-trips). func (d *AuditDispatcher) shouldEmit(claims *casJWT.Claims) bool { - return d != nil && d.publisher != nil && claims != nil && !claims.SourceInternal + return d != nil && d.dispatcher.Enabled() && claims != nil && !claims.SourceInternal } // Dispatch generates and publishes an audit event with a SYSTEM actor and the @@ -70,18 +68,8 @@ func (d *AuditDispatcher) Dispatch(entry auditor.LogEntry, claims *casJWT.Claims return } - payload, err := auditor.GenerateAuditEvent(entry, + d.dispatcher.Dispatch(entry, auditor.WithActor(auditor.ActorTypeSystem, uuid.Nil, "", ""), auditor.WithOrgID(orgID), ) - if err != nil { - d.log.Errorw("msg", "failed to generate audit event", "error", err) - sentry.CaptureException(fmt.Errorf("failed to generate audit event: %w", err)) - return - } - - if err := d.publisher.Publish(payload); err != nil { - d.log.Errorw("msg", "failed to publish audit event", "error", err) - sentry.CaptureException(fmt.Errorf("failed to publish audit event: %w", err)) - } } diff --git a/app/artifact-cas/internal/service/auditor_test.go b/app/artifact-cas/internal/service/auditor_test.go index fd20aefda..267ac5e29 100644 --- a/app/artifact-cas/internal/service/auditor_test.go +++ b/app/artifact-cas/internal/service/auditor_test.go @@ -44,8 +44,18 @@ func (f *fakePublisher) Publish(data *auditor.EventPayload) error { return nil } -func newTestDispatcher(p eventPublisher) *AuditDispatcher { - return &AuditDispatcher{publisher: p, log: servicelogger.ScopedHelper(log.DefaultLogger, "test")} +// newTestDispatcher builds an AuditDispatcher backed by the given fake. A nil +// fake leaves the underlying publisher disabled, mirroring NewAuditDispatcher. +func newTestDispatcher(p *fakePublisher) *AuditDispatcher { + var pub auditor.Publisher + if p != nil { + pub = p + } + + return &AuditDispatcher{ + dispatcher: auditor.NewDispatcher(pub, log.DefaultLogger), + log: servicelogger.ScopedHelper(log.DefaultLogger, "test"), + } } func testUploadedEntry() auditor.LogEntry { @@ -63,51 +73,53 @@ const testOrgID = "1089bb36-e27b-428b-8009-d015c8737c54" func TestAuditDispatcherDispatch(t *testing.T) { tests := []struct { - name string - dispatcher *AuditDispatcher + name string + // publisher is the fake backing the dispatcher; nil means the dispatcher + // is disabled (no NATS). nilDispatcher exercises the nil-receiver path. + publisher *fakePublisher + nilDispatcher bool entry auditor.LogEntry claims *casJWT.Claims wantPublished int }{ { - name: "nil dispatcher is a no-op", - dispatcher: nil, - entry: testUploadedEntry(), - claims: &casJWT.Claims{OrgID: testOrgID}, + name: "nil dispatcher is a no-op", + nilDispatcher: true, + entry: testUploadedEntry(), + claims: &casJWT.Claims{OrgID: testOrgID}, }, { - name: "nil publisher is a no-op", - dispatcher: newTestDispatcher(nil), - entry: testUploadedEntry(), - claims: &casJWT.Claims{OrgID: testOrgID}, + name: "nil publisher is a no-op", + entry: testUploadedEntry(), + claims: &casJWT.Claims{OrgID: testOrgID}, }, { - name: "internal control plane traffic is skipped", - dispatcher: newTestDispatcher(&fakePublisher{}), - entry: testUploadedEntry(), - claims: &casJWT.Claims{OrgID: testOrgID, SourceInternal: true}, + name: "internal control plane traffic is skipped", + publisher: &fakePublisher{}, + entry: testUploadedEntry(), + claims: &casJWT.Claims{OrgID: testOrgID, SourceInternal: true}, }, { - name: "invalid org id is skipped", - dispatcher: newTestDispatcher(&fakePublisher{}), - entry: testUploadedEntry(), - claims: &casJWT.Claims{OrgID: "not-an-uuid"}, + name: "invalid org id is skipped", + publisher: &fakePublisher{}, + entry: testUploadedEntry(), + claims: &casJWT.Claims{OrgID: "not-an-uuid"}, }, { - name: "invalid entry is skipped", - dispatcher: newTestDispatcher(&fakePublisher{}), - entry: &events.CASArtifactUploaded{CASArtifactBase: &events.CASArtifactBase{}}, - claims: &casJWT.Claims{OrgID: testOrgID}, + name: "invalid entry is skipped", + publisher: &fakePublisher{}, + entry: &events.CASArtifactUploaded{CASArtifactBase: &events.CASArtifactBase{}}, + claims: &casJWT.Claims{OrgID: testOrgID}, }, { - name: "publish errors are swallowed", - dispatcher: newTestDispatcher(&fakePublisher{err: errors.New("nats is down")}), - entry: testUploadedEntry(), - claims: &casJWT.Claims{OrgID: testOrgID}, + name: "publish errors are swallowed", + publisher: &fakePublisher{err: errors.New("nats is down")}, + entry: testUploadedEntry(), + claims: &casJWT.Claims{OrgID: testOrgID}, }, { name: "client traffic is published", - dispatcher: newTestDispatcher(&fakePublisher{}), + publisher: &fakePublisher{}, entry: testUploadedEntry(), claims: &casJWT.Claims{OrgID: testOrgID}, wantPublished: 1, @@ -116,20 +128,24 @@ func TestAuditDispatcherDispatch(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + var d *AuditDispatcher + if !tc.nilDispatcher { + d = newTestDispatcher(tc.publisher) + } + // must never panic nor return an error - tc.dispatcher.Dispatch(tc.entry, tc.claims) + d.Dispatch(tc.entry, tc.claims) - if tc.dispatcher == nil || tc.dispatcher.publisher == nil { + if tc.publisher == nil { return } - fake := tc.dispatcher.publisher.(*fakePublisher) - require.Len(t, fake.published, tc.wantPublished) + require.Len(t, tc.publisher.published, tc.wantPublished) if tc.wantPublished == 0 { return } - got := fake.published[0] + got := tc.publisher.published[0] assert.Equal(t, auditor.AuditEventType, got.EventType) assert.Equal(t, events.CASArtifactUploadedActionType, got.Data.ActionType) assert.Equal(t, events.CASArtifactType, got.Data.TargetType) diff --git a/app/controlplane/pkg/auditor/dispatcher.go b/app/controlplane/pkg/auditor/dispatcher.go new file mode 100644 index 000000000..f85707511 --- /dev/null +++ b/app/controlplane/pkg/auditor/dispatcher.go @@ -0,0 +1,80 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package auditor + +import ( + "fmt" + + "github.com/chainloop-dev/chainloop/pkg/servicelogger" + "github.com/getsentry/sentry-go" + "github.com/go-kratos/kratos/v2/log" +) + +// Publisher publishes generated audit event payloads to the event bus. +// Implemented by *AuditLogPublisher; abstracted so it can be faked in tests and +// so a nil publisher can act as a no-op (NATS not configured). +type Publisher interface { + Publish(data *EventPayload) error +} + +// Dispatcher centralizes the generate -> publish -> error-reporting flow shared +// by every component that emits audit events (e.g. the control plane's +// biz.AuditorUseCase and the Artifact CAS). Callers resolve the actor and +// organization themselves and pass them as GeneratorOptions, so each component +// keeps its own actor/org policy (request context vs JWT claims) while sharing +// the common dispatch machinery. +type Dispatcher struct { + // nil when the publisher is not configured, making the dispatcher a no-op + publisher Publisher + log *log.Helper +} + +// NewDispatcher builds a Dispatcher. A nil publisher (e.g. NATS not configured) +// turns Dispatch into a no-op and makes Enabled report false. +func NewDispatcher(publisher Publisher, logger log.Logger) *Dispatcher { + return &Dispatcher{ + publisher: publisher, + log: servicelogger.ScopedHelper(logger, "auditor-dispatcher"), + } +} + +// Enabled reports whether Dispatch would actually publish an event. Callers can +// use it to skip extra work when the dispatcher is a no-op. +func (d *Dispatcher) Enabled() bool { + return d != nil && d.publisher != nil +} + +// Dispatch generates the audit event from entry and the given options and +// publishes it. Best-effort: failures are logged and reported to Sentry, never +// returned, so they can't fail or slow down the caller. A disabled dispatcher +// is a no-op. +func (d *Dispatcher) Dispatch(entry LogEntry, opts ...GeneratorOption) { + if !d.Enabled() { + return + } + + payload, err := GenerateAuditEvent(entry, opts...) + if err != nil { + d.log.Errorw("msg", "failed to generate audit event", "error", err) + sentry.CaptureException(fmt.Errorf("failed to generate audit event: %w", err)) + return + } + + if err := d.publisher.Publish(payload); err != nil { + d.log.Errorw("msg", "failed to publish audit event", "error", err) + sentry.CaptureException(fmt.Errorf("failed to publish audit event: %w", err)) + } +} diff --git a/app/controlplane/pkg/auditor/dispatcher_test.go b/app/controlplane/pkg/auditor/dispatcher_test.go new file mode 100644 index 000000000..fd677fa18 --- /dev/null +++ b/app/controlplane/pkg/auditor/dispatcher_test.go @@ -0,0 +1,131 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package auditor + +import ( + "encoding/json" + "errors" + "testing" + + "github.com/go-kratos/kratos/v2/log" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakePublisher records published payloads and optionally fails. +type fakePublisher struct { + published []*EventPayload + err error +} + +func (f *fakePublisher) Publish(data *EventPayload) error { + if f.err != nil { + return f.err + } + + f.published = append(f.published, data) + return nil +} + +// testLogEntry is a minimal LogEntry implementation for dispatcher tests. +type testLogEntry struct { + description string +} + +func (e *testLogEntry) ActionType() string { return "TEST_ACTION" } +func (e *testLogEntry) ActionInfo() (json.RawMessage, error) { return json.RawMessage(`{}`), nil } +func (e *testLogEntry) TargetType() TargetType { return "TEST_TARGET" } +func (e *testLogEntry) TargetID() *uuid.UUID { return nil } +func (e *testLogEntry) Description() string { return e.description } +func (e *testLogEntry) RequiresActor() bool { return false } + +func validEntry() LogEntry { + return &testLogEntry{description: "something happened"} +} + +func systemActor() GeneratorOption { + return WithActor(ActorTypeSystem, uuid.Nil, "", "") +} + +func TestDispatcherEnabled(t *testing.T) { + tests := []struct { + name string + dispatcher *Dispatcher + want bool + }{ + {name: "nil dispatcher", dispatcher: nil, want: false}, + {name: "nil publisher", dispatcher: NewDispatcher(nil, log.DefaultLogger), want: false}, + {name: "configured publisher", dispatcher: NewDispatcher(&fakePublisher{}, log.DefaultLogger), want: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, tc.dispatcher.Enabled()) + }) + } +} + +func TestDispatcherDispatch(t *testing.T) { + tests := []struct { + name string + publisher *fakePublisher + entry LogEntry + opts []GeneratorOption + wantPublished int + }{ + { + name: "publishes a valid event", + publisher: &fakePublisher{}, + entry: validEntry(), + opts: []GeneratorOption{systemActor()}, + wantPublished: 1, + }, + { + name: "generation failure is swallowed", + publisher: &fakePublisher{}, + // empty description makes GenerateAuditEvent fail + entry: &testLogEntry{description: ""}, + opts: []GeneratorOption{systemActor()}, + }, + { + name: "publish errors are swallowed", + publisher: &fakePublisher{err: errors.New("nats is down")}, + entry: validEntry(), + opts: []GeneratorOption{systemActor()}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + d := NewDispatcher(tc.publisher, log.DefaultLogger) + // must never panic nor return an error + d.Dispatch(tc.entry, tc.opts...) + + require.Len(t, tc.publisher.published, tc.wantPublished) + }) + } +} + +func TestDispatcherDispatchNoOpWhenDisabled(t *testing.T) { + // nil publisher and nil dispatcher must both be safe no-ops + assert.NotPanics(t, func() { + NewDispatcher(nil, log.DefaultLogger).Dispatch(validEntry(), systemActor()) + + var d *Dispatcher + d.Dispatch(validEntry(), systemActor()) + }) +} diff --git a/app/controlplane/pkg/biz/auditor.go b/app/controlplane/pkg/biz/auditor.go index da8ad912c..60f254ae8 100644 --- a/app/controlplane/pkg/biz/auditor.go +++ b/app/controlplane/pkg/biz/auditor.go @@ -23,7 +23,6 @@ import ( "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/auditor" "github.com/chainloop-dev/chainloop/pkg/otelx" - "github.com/getsentry/sentry-go" "github.com/go-kratos/kratos/v2/log" "github.com/google/uuid" ) @@ -31,18 +30,27 @@ import ( var auditorTracer = otelx.Tracer("chainloop-controlplane", "biz/auditor") type AuditorUseCase struct { - log *log.Helper - publisher *auditor.AuditLogPublisher + log *log.Helper + dispatcher *auditor.Dispatcher } func NewAuditorUseCase(p *auditor.AuditLogPublisher, logger log.Logger) *AuditorUseCase { + // keep the Publisher interface nil when the publisher is disabled so the + // dispatcher short-circuits instead of holding a typed-nil interface + var publisher auditor.Publisher + if p != nil { + publisher = p + } + return &AuditorUseCase{ - log: log.NewHelper(log.With(logger, "component", "biz/auditor")), - publisher: p, + log: log.NewHelper(log.With(logger, "component", "biz/auditor")), + dispatcher: auditor.NewDispatcher(publisher, logger), } } -// Dispatch logs an entry to the audit log asynchronously. +// Dispatch logs an entry to the audit log asynchronously. The actor is resolved +// from the request context (user, API token or system); the generation and +// publishing is delegated to the shared auditor.Dispatcher. func (uc *AuditorUseCase) Dispatch(ctx context.Context, entry auditor.LogEntry, orgID *uuid.UUID) { ctx, span := otelx.Start(ctx, auditorTracer, "AuditorUseCase.Dispatch") defer span.End() @@ -77,16 +85,5 @@ func (uc *AuditorUseCase) Dispatch(ctx context.Context, entry auditor.LogEntry, opts = append(opts, auditor.WithOrgID(*orgID)) } - payload, err := auditor.GenerateAuditEvent(entry, opts...) - if err != nil { - uc.log.Error("failed to generate audit event", "error", err) - sentry.CaptureException(fmt.Errorf("failed to generate audit event: %w", err)) - return - } - - // Send event to event bus - if err := uc.publisher.Publish(payload); err != nil { - uc.log.Error("failed to publish event", "error", err) - sentry.CaptureException(fmt.Errorf("failed to publish event: %w", err)) - } + uc.dispatcher.Dispatch(entry, opts...) }