refactor(auditor): share audit dispatch logic between control plane and CAS (#3191)

Author: migmartri

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))
-	}
 }

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)

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))
+	}
+}