refactor(beholder): extract newRotatingAuthFromConfig helper and add NewChipIngressHeaderProvider by pkcll · Pull Request #2096 · smartcontractkit/chainlink-common

pkcll · 2026-05-27T17:45:15Z

Summary

Introduces pkg/chipingress.NewHeaderProvider, a self-contained constructor for
the ChIP Ingress auth header provider. Lets non-beholder callers (e.g. the
DurableEmitter wiring in smartcontractkit/chainlink#22648) construct auth without
importing pkg/beholder.

Motivation

pkg/chipingress is a separate Go module and cannot depend on pkg/beholder.
Today, core/services/chainlink/application.go (in smartcontractkit/chainlink#22648)
reaches into beholder.NewStaticAuth solely to obtain a
chipingress.HeaderProvider, and only the static-auth case is wired — rotating
auth is silently unsupported on the DurableEmitter path.

Changes

New: pkg/chipingress/header_provider.go
- Signer interface (duck-typed; satisfied by beholder.Signer, lazySigner,
  keystore.CSASigner).
- HeaderProviderConfig with fields: AuthHeaders, AuthHeadersTTL,
  AuthPublicKeyHex, AuthKeySigner, InsecureConnection.
- NewHeaderProvider — uniformly returns nil / static / rotating provider
  based on config, mirroring pkg/beholder/client.go:200-212 selection logic.
pkg/beholder/auth.go: rename internal helper
newRotatingAuthFromConfig -> buildRotatingAuth (single call site updated).
Removed: pkg/beholder/chipingress_auth.go (+ its test) — replaced by the
new package-local implementation.

Parity Contract

The new chipingress provider preserves wire-protocol parity with beholder:

Header name: X-Beholder-Node-Auth-Token
V2 header format unchanged
Identical validation error strings, e.g.
"auth: public key hex required for rotating auth (TTL > 0)",
"auth: headers TTL must be at least 10 minutes",
"auth: failed to decode public key hex"

Documented as a doc comment on NewHeaderProvider. A cross-package parity test
is deferred to a follow-up PR — the chipingress module version must be bumped in
the chainlink-common root go.mod first so pkg/beholder can import the new
symbols.

Public Surface

Exactly three exports from the new file: Signer, HeaderProviderConfig,
NewHeaderProvider. Static/rotating provider types are unexported.

Testing

pkg/chipingress — 18 test cases covering static, rotating, nil, transport
security, and validation error paths.
pkg/beholder/... — existing suite passes; helper rename is internal.

Consumer

smartcontractkit/chainlink#22648 (DurableEmitter Core Integration) will adopt
this in core/services/chainlink/application.go, collapsing the inline auth
branch into a single chipingress.NewHeaderProvider(...) call.

Follow-ups (separate PRs)

Bump pkg/chipingress version in chainlink-common root go.mod.
Add cross-package parity test:
pkg/beholder/chipingress_auth_parity_test.go (table-driven, 8 cases)
plus pkg/beholder/export_test.go exposing BuildRotatingAuth.
Integrate in DurableEmitter: Refactor Core Integration chainlink#22648.

…NewChipIngressHeaderProvider - Extract rotating auth setup logic from NewGRPCClient into newRotatingAuthFromConfig helper in auth.go - Add NewChipIngressHeaderProvider in chipingress_auth.go for external consumers needing a chipingress.HeaderProvider from beholder config - Add exhaustive test coverage for all auth paths (static, rotating, error cases, boundary conditions)

github-actions · 2026-05-27T17:46:41Z

📊 API Diff Results

No changes detected for module github.com/smartcontractkit/chainlink-common

View full report

patrickhuie19 · 2026-05-27T17:50:42Z

+
+// newRotatingAuthFromConfig validates config and returns a rotating Auth and its lazySigner.
+// Returns (nil, nil, nil) if AuthHeadersTTL == 0 (no rotating auth configured).
+func newRotatingAuthFromConfig(cfg Config) (Auth, *lazySigner, error) {


is there a better name that RotatingAuth? if not, can we have a comment about what type of downstream consumers need this?

patrickhuie19 · 2026-05-27T17:51:53Z

+// NewChipIngressHeaderProvider creates a new chipingress.HeaderProvider
+// using the same auth logic as NewGRPCClient.
+// Returns nil provider (no error) if no auth is configured.
+func NewChipIngressHeaderProvider(cfg Config) (chipingress.HeaderProvider, error) {


can this be extracted out of pkg/beholder into a chip specific pkg?

Yes, I was thinking that’d be the next refactoring step, but we could probably just do it in one shot.

…ngress Move the header-provider construction logic out of pkg/beholder and into pkg/chipingress so the chipingress module owns its auth surface. - Add chipingress.NewHeaderProvider plus HeaderProviderConfig, Signer, and TransportSecurityRequirer, along with internal static/rotating providers and the V2 auth-header helper. The X-Beholder-Node-Auth-Token header name and wire format are preserved verbatim. - Port the 18 existing test cases to pkg/chipingress, using a local fake signer and TransportSecurityRequirer type assertions. - Delete pkg/beholder/chipingress_auth.go and its test (the function was new in this PR; no external callers exist). - Rename beholder's internal newRotatingAuthFromConfig to buildRotatingAuth and update its single call site in client.go.

github-actions · 2026-05-28T17:40:57Z

✅ API Diff Results - `github.com/smartcontractkit/chainlink-common/pkg/chipingress`

✅ Compatible Changes (3)

`./` (3)

HeaderProviderConfig — ➕ Added
NewHeaderProvider — ➕ Added
Signer — ➕ Added

📄 View full apidiff report

Trim the chipingress header-provider API to the minimum needed by callers that want to build an auth provider from the same config beholder uses. - Unexport newStaticHeaderProvider and newRotatingHeaderProvider; the config-driven NewHeaderProvider is the sole entry point. - Delete the TransportSecurityRequirer interface; tests use a local inline interface for the RequireTransportSecurity() type assertion. - Document the parity contract on NewHeaderProvider, mapping each beholder.Config field to its HeaderProviderConfig counterpart so the two code paths stay in lockstep. Note: ChipIngressInsecureConnection on beholder.Config maps to InsecureConnection here (the chipingress package has no need for a redundant prefix on its own type names). Public surface is now exactly: Signer, HeaderProviderConfig, NewHeaderProvider. A defensive parity test against beholder's internal wiring will land in a follow-up PR after the chipingress module version is bumped in chainlink-common's root go.mod.

…s-auth

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

patrickhuie19 · 2026-05-28T19:36:30Z

+	return s.headers, nil
+}
+
+func (s *staticHeaderProvider) RequireTransportSecurity() bool {


/nit for methods that return a bool, prefer to prefix with Is..., i.e. IsTLSRequired

The intention of this PR is to extract auth, keep the surface area minimal, and keep it as close as possible to pkg/beholder.

patrickhuie19 · 2026-05-28T19:40:28Z

+// preserved verbatim from the original beholder-side implementation to keep
+// the wire protocol unchanged.
+const (
+	authHeaderKey = "X-Beholder-Node-Auth-Token"


/nit should this be named to something with beholder in it? the default for grpc authorization is authorization, so perhaps the difference in default behavior protects the naming as is.

grpc authorization is authorization

Yes, the Authorization header is the standard header for auth tokens in HTTP/gRPC requests.
We have tickets in place to support this. Most of the work has been completed, and only a few items remain to fully wire it up.

should this be named to something with beholder in it
we were going to replace it with authorization header at some point

patrickhuie19 · 2026-05-28T19:43:26Z

+
+		// Double-check after acquiring the lock in case another goroutine
+		// already refreshed.
+		lastUpdated = time.Unix(0, r.lastUpdatedNanos.Load())


/nit can leave for a follow up, but technically we could just acquire the lock and only make the top level if time.Since(lastUpdated) > r.ttl { check as its duplicated on line 178 to check under the lock

The intention is to keep 1:1 parity with existing implementation in pkg/beholder

pkcll mentioned this pull request May 27, 2026

DurableEmitter: Refactor Core Integration smartcontractkit/chainlink#22648

Draft

DylanTinianov previously approved these changes May 27, 2026

View reviewed changes

pkcll requested review from hendoxc and jmank88 May 27, 2026 17:49

DylanTinianov self-requested a review May 27, 2026 17:50

patrickhuie19 reviewed May 27, 2026

View reviewed changes

DylanTinianov reviewed May 27, 2026

View reviewed changes

Comment thread pkg/beholder/chipingress_auth.go Outdated

patrickhuie19 reviewed May 27, 2026

View reviewed changes

DylanTinianov self-requested a review May 27, 2026 17:53

pkcll dismissed DylanTinianov’s stale review via 11f202a May 28, 2026 17:39

DylanTinianov approved these changes May 28, 2026

View reviewed changes

DylanTinianov previously approved these changes May 28, 2026

View reviewed changes

DylanTinianov self-requested a review May 28, 2026 18:32

pkcll dismissed DylanTinianov’s stale review via 3ad470c May 28, 2026 18:43

test(chipingress): fix govet shadow in header provider test

2d74430

DylanTinianov approved these changes May 28, 2026

View reviewed changes

pkcll marked this pull request as ready for review May 28, 2026 19:25

pkcll requested a review from a team as a code owner May 28, 2026 19:25

Copilot AI review requested due to automatic review settings May 28, 2026 19:25

Merge branch 'main' into CRE-4422-refactor-durable-emitter-chipingres…

7b4a4a5

…s-auth

Copilot started reviewing on behalf of pkcll May 28, 2026 19:25 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

DylanTinianov requested a review from patrickhuie19 May 28, 2026 19:27

jmank88 approved these changes May 28, 2026

View reviewed changes

patrickhuie19 reviewed May 28, 2026

View reviewed changes

patrickhuie19 approved these changes May 28, 2026

View reviewed changes

hendoxc approved these changes May 28, 2026

View reviewed changes

DylanTinianov added this pull request to the merge queue May 28, 2026

Merged via the queue into main with commit 58c7145 May 28, 2026
34 of 36 checks passed

DylanTinianov deleted the CRE-4422-refactor-durable-emitter-chipingress-auth branch May 28, 2026 20:58

Conversation

pkcll commented May 27, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

Parity Contract

Public Surface

Testing

Consumer

Follow-ups (separate PRs)

Uh oh!

github-actions Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 API Diff Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ API Diff Results - github.com/smartcontractkit/chainlink-common/pkg/chipingress

✅ Compatible Changes (3)

./ (3)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pkcll commented May 27, 2026 •

edited by atlassian Bot

Loading

github-actions Bot commented May 27, 2026 •

edited

Loading

github-actions Bot commented May 28, 2026 •

edited

Loading

✅ API Diff Results - `github.com/smartcontractkit/chainlink-common/pkg/chipingress`

`./` (3)