Skip to content

Commit 0725554

Browse files
committed
fix(controlplane): replace variadic publisher option with publish-only constructor
wire cannot resolve variadic providers: the CI generate check failed because NewAuditLogPublisher's new variadic options parameter made wire demand a []auditor.PublisherOption provider in the controlplane and testhelpers injectors. Restore the original NewAuditLogPublisher signature and expose the publish-only mode as a separate wire-friendly NewPublishOnlyAuditLogPublisher constructor used directly by the CAS. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: 0c60a332-e0f1-4c2b-94ae-533467e52f5c
1 parent 02fd7bb commit 0725554

4 files changed

Lines changed: 48 additions & 58 deletions

File tree

app/artifact-cas/cmd/wire.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
package main
2222

2323
import (
24-
"context"
25-
2624
"github.com/chainloop-dev/chainloop/app/artifact-cas/internal/conf"
2725
"github.com/chainloop-dev/chainloop/app/artifact-cas/internal/server"
2826
"github.com/chainloop-dev/chainloop/app/artifact-cas/internal/service"
@@ -46,7 +44,8 @@ func wireApp(*conf.Bootstrap, *conf.Server, *conf.Auth, credentials.Reader, log.
4644
newProtoValidator,
4745
newNatsConfig,
4846
natsconn.New,
49-
newAuditLogPublisher,
47+
// publish-only: the control plane owns the chainloop-audit stream configuration
48+
auditor.NewPublishOnlyAuditLogPublisher,
5049
service.NewAuditDispatcher,
5150
),
5251
)
@@ -77,9 +76,3 @@ func newNatsConfig(bc *conf.Bootstrap) *natsconn.Config {
7776

7877
return cfg
7978
}
80-
81-
// newAuditLogPublisher creates a publish-only audit log publisher: the control
82-
// plane owns the chainloop-audit stream configuration, the CAS only publishes to it
83-
func newAuditLogPublisher(rc *natsconn.ReloadableConnection, logger log.Logger) (*auditor.AuditLogPublisher, error) {
84-
return auditor.NewAuditLogPublisher(context.Background(), rc, logger, auditor.WithoutStreamManagement())
85-
}

app/artifact-cas/cmd/wire_gen.go

Lines changed: 1 addition & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/pkg/auditor/nats.go

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -41,48 +41,45 @@ type AuditLogPublisher struct {
4141
logger *log.Helper
4242
}
4343

44-
type PublisherOption func(*publisherOptions)
44+
// NewAuditLogPublisher creates a publisher that owns the JetStream stream:
45+
// it creates or updates it on boot and after every NATS reconnection.
46+
func NewAuditLogPublisher(ctx context.Context, rc *natsconn.ReloadableConnection, logger log.Logger) (*AuditLogPublisher, error) {
47+
p := newPublisher(rc, logger)
48+
if p == nil {
49+
return nil, nil
50+
}
4551

46-
type publisherOptions struct {
47-
withoutStreamManagement bool
52+
if err := p.initJetStream(); err != nil {
53+
return nil, err
54+
}
55+
56+
go p.watchReconnect(rc.Subscribe(ctx))
57+
58+
return p, nil
4859
}
4960

50-
// WithoutStreamManagement makes the publisher publish-only: it never creates or
51-
// updates the JetStream stream. Meant for components (e.g. the Artifact CAS) that
52-
// publish to the stream owned and configured by the control plane, so they can't
53-
// accidentally override its configuration (e.g. downgrade the replica count).
54-
func WithoutStreamManagement() PublisherOption {
55-
return func(o *publisherOptions) {
56-
o.withoutStreamManagement = true
61+
// NewPublishOnlyAuditLogPublisher creates a publisher that never creates or
62+
// updates the JetStream stream. Meant for components (e.g. the Artifact CAS)
63+
// that publish to the stream owned and configured by the control plane, so they
64+
// can't accidentally override its configuration (e.g. downgrade the replica count).
65+
func NewPublishOnlyAuditLogPublisher(rc *natsconn.ReloadableConnection, logger log.Logger) (*AuditLogPublisher, error) {
66+
p := newPublisher(rc, logger)
67+
if p != nil {
68+
p.logger.Infow("msg", "stream management disabled, running in publish-only mode")
5769
}
70+
71+
return p, nil
5872
}
5973

60-
func NewAuditLogPublisher(ctx context.Context, rc *natsconn.ReloadableConnection, logger log.Logger, opts ...PublisherOption) (*AuditLogPublisher, error) {
74+
// newPublisher returns nil when NATS is not configured (the publisher is disabled)
75+
func newPublisher(rc *natsconn.ReloadableConnection, logger log.Logger) *AuditLogPublisher {
6176
l := log.NewHelper(log.With(logger, "component", "natsAuditLogPublisher"))
6277
if rc == nil {
6378
l.Infow("msg", "NATS connection not set, audit log publisher disabled")
64-
return nil, nil
65-
}
66-
67-
options := &publisherOptions{}
68-
for _, opt := range opts {
69-
opt(options)
70-
}
71-
72-
p := &AuditLogPublisher{rc: rc, logger: l}
73-
74-
if options.withoutStreamManagement {
75-
l.Infow("msg", "stream management disabled, running in publish-only mode")
76-
return p, nil
77-
}
78-
79-
if err := p.initJetStream(); err != nil {
80-
return nil, err
79+
return nil
8180
}
8281

83-
go p.watchReconnect(rc.Subscribe(ctx))
84-
85-
return p, nil
82+
return &AuditLogPublisher{rc: rc, logger: l}
8683
}
8784

8885
func (p *AuditLogPublisher) initJetStream() error {

app/controlplane/pkg/auditor/nats_test.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,35 +27,42 @@ import (
2727

2828
func TestNewAuditLogPublisher(t *testing.T) {
2929
tests := []struct {
30-
name string
31-
rc *natsconn.ReloadableConnection
32-
opts []PublisherOption
30+
name string
31+
rc *natsconn.ReloadableConnection
32+
constructor func(*natsconn.ReloadableConnection) (*AuditLogPublisher, error)
3333
// nil publisher means disabled (no NATS configured)
3434
wantNil bool
3535
}{
3636
{
37-
name: "nil connection disables the publisher",
38-
rc: nil,
37+
name: "nil connection disables the publisher",
38+
rc: nil,
39+
constructor: func(rc *natsconn.ReloadableConnection) (*AuditLogPublisher, error) {
40+
return NewAuditLogPublisher(context.Background(), rc, log.DefaultLogger)
41+
},
3942
wantNil: true,
4043
},
4144
{
42-
name: "nil connection with options still disables the publisher",
43-
rc: nil,
44-
opts: []PublisherOption{WithoutStreamManagement()},
45+
name: "nil connection disables the publish-only publisher",
46+
rc: nil,
47+
constructor: func(rc *natsconn.ReloadableConnection) (*AuditLogPublisher, error) {
48+
return NewPublishOnlyAuditLogPublisher(rc, log.DefaultLogger)
49+
},
4550
wantNil: true,
4651
},
4752
{
4853
// publish-only mode skips stream creation/updates so it doesn't
4954
// need a live JetStream context at construction time
5055
name: "publish-only mode skips stream management",
5156
rc: &natsconn.ReloadableConnection{},
52-
opts: []PublisherOption{WithoutStreamManagement()},
57+
constructor: func(rc *natsconn.ReloadableConnection) (*AuditLogPublisher, error) {
58+
return NewPublishOnlyAuditLogPublisher(rc, log.DefaultLogger)
59+
},
5360
},
5461
}
5562

5663
for _, tc := range tests {
5764
t.Run(tc.name, func(t *testing.T) {
58-
p, err := NewAuditLogPublisher(context.Background(), tc.rc, log.DefaultLogger, tc.opts...)
65+
p, err := tc.constructor(tc.rc)
5966
require.NoError(t, err)
6067
if tc.wantNil {
6168
assert.Nil(t, p)

0 commit comments

Comments
 (0)