Skip to content

refactor(credential): carry wire key identifiers through the accepted set #729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aaron-zeisler merged 3 commits into from
Jun 30, 2026
Merged

refactor(credential): carry wire key identifiers through the accepted set #729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a094998
refactor(credential): carry wire key identifiers through the accepted…
aaron-zeisler Jun 30, 2026
09075d5
refactor(credential): reconcile key metadata in a single pass
aaron-zeisler Jun 30, 2026
5eceeb4
refactor(credential): store accepted-key metadata by value, trust the…
aaron-zeisler Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions internal/credential/accepted_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package credential
import (
"errors"
"fmt"
"time"

"github.com/launchdarkly/ld-relay/v8/config"
)
Expand All @@ -27,12 +26,12 @@ import (
//
// Construct an AcceptedSet with AcceptedSetBuilder (see accepted_set_builder.go).
type AcceptedSet struct {
// sdkKeys and mobileKeys store each accepted key once, keyed by value, so duplicates collapse
// without a containment scan. The map value is the key's expiry: a nil *time.Time means the key
// is permanent. A nil map is a valid empty set (reads return absent; only the builder writes).
sdkKeys map[config.SDKKey]*time.Time
// sdkKeys and mobileKeys store each accepted key once, keyed by value (the secret), so duplicates
// collapse without a containment scan. The map value carries the key's metadata (see
// acceptedKeyInfo). A nil map is a valid empty set (reads return absent; only the builder writes).
sdkKeys map[config.SDKKey]acceptedKeyInfo
anchor config.SDKKey
mobileKeys map[config.MobileKey]*time.Time
mobileKeys map[config.MobileKey]acceptedKeyInfo
primaryMobileKey config.MobileKey
envID config.EnvironmentID
}
Expand Down Expand Up @@ -100,15 +99,15 @@ func NewPrimaryMobileKeyNotInSetError() *MalformedCredentialSetError {
}

// NewEmptyCredentialError returns a MalformedCredentialSetError for a key-array entry whose
// value field is empty. kind is "sdkKeys" or "mobileKeys"; identifier is the key's identifier
// string (may be empty for old-format payloads that synthesize from the singular fields).
func NewEmptyCredentialError(kind, identifier string) *MalformedCredentialSetError {
if identifier == "" {
// value field is empty. kind is "sdkKeys" or "mobileKeys"; key is the entry's wire "key" identifier
// (may be empty for old-format payloads that synthesize from the singular fields).
func NewEmptyCredentialError(kind, key string) *MalformedCredentialSetError {
if key == "" {
return &MalformedCredentialSetError{
msg: fmt.Sprintf("malformed credential set: %s entry has an empty value", kind),
}
}
return &MalformedCredentialSetError{
msg: fmt.Sprintf("malformed credential set: %s entry %q has an empty value", kind, identifier),
msg: fmt.Sprintf("malformed credential set: %s entry %q has an empty value", kind, key),
}
}
92 changes: 42 additions & 50 deletions internal/credential/accepted_set_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,77 +16,69 @@ type AcceptedSetBuilder struct {
func NewAcceptedSetBuilder() *AcceptedSetBuilder {
return &AcceptedSetBuilder{
set: AcceptedSet{
sdkKeys: make(map[config.SDKKey]*time.Time),
mobileKeys: make(map[config.MobileKey]*time.Time),
sdkKeys: make(map[config.SDKKey]acceptedKeyInfo),
mobileKeys: make(map[config.MobileKey]acceptedKeyInfo),
},
}
}

// WithSDKKey adds a permanent (non-expiring) SDK key. It is a no-op if the key is undefined or
// already present.
func (b *AcceptedSetBuilder) WithSDKKey(key config.SDKKey) *AcceptedSetBuilder {
b.addSDKKey(key, nil)
return b
// SDKKeyParams describes one accepted server-side SDK key for the builder: the credential value plus
// the optional wire "key" identifier (nil when absent) and optional expiry (nil = permanent).
type SDKKeyParams struct {
Value config.SDKKey
Key *string
Expiry *time.Time
}

// WithExpiringSDKKey adds an SDK key that should be accepted until the given expiry. It is a no-op
// if the key is undefined or already present.
func (b *AcceptedSetBuilder) WithExpiringSDKKey(key config.SDKKey, expiry time.Time) *AcceptedSetBuilder {
b.addSDKKey(key, &expiry)
return b
// MobileKeyParams describes one accepted mobile key for the builder. See SDKKeyParams.
type MobileKeyParams struct {
Value config.MobileKey
Key *string
Expiry *time.Time
}

// WithAnchor adds key (if not already present) and designates it as the anchor — the SDK key
// that owns the environment's upstream connection. It is a no-op if the key is undefined.
func (b *AcceptedSetBuilder) WithAnchor(key config.SDKKey) *AcceptedSetBuilder {
if key.Defined() {
b.addSDKKey(key, nil)
b.set.anchor = key
// WithSDKKey adds a server-side SDK key. It is a no-op if the value is undefined or already present
// (the first metadata recorded for a value wins).
func (b *AcceptedSetBuilder) WithSDKKey(p SDKKeyParams) *AcceptedSetBuilder {
if !p.Value.Defined() || b.set.hasSDKKey(p.Value) {
Comment thread
aaron-zeisler marked this conversation as resolved.
return b
}
b.set.sdkKeys[p.Value] = acceptedKeyInfo{key: p.Key, expiry: p.Expiry}
return b
}

// addSDKKey records the key with the given expiry (nil = permanent), skipping undefined keys and
// keys already in the set (the first expiry recorded for a key wins).
func (b *AcceptedSetBuilder) addSDKKey(key config.SDKKey, expiry *time.Time) {
if !key.Defined() || b.set.hasSDKKey(key) {
return
// WithAnchor adds p.Value and designates it as the anchor — the SDK key that owns the environment's
// upstream connection. The anchor is always permanent, so p.Expiry is ignored. It is a no-op if the
// value is undefined. Unlike WithSDKKey it overwrites any existing entry for the value, since
// designating the anchor takes precedence over an earlier non-anchor add.
func (b *AcceptedSetBuilder) WithAnchor(p SDKKeyParams) *AcceptedSetBuilder {
if !p.Value.Defined() {
return b
}
b.set.sdkKeys[key] = expiry
}

// WithMobileKey adds a permanent (non-expiring) mobile key. It is a no-op if the key is undefined or
// already present.
func (b *AcceptedSetBuilder) WithMobileKey(key config.MobileKey) *AcceptedSetBuilder {
b.addMobileKey(key, nil)
b.set.sdkKeys[p.Value] = acceptedKeyInfo{key: p.Key, expiry: nil}
Comment thread
aaron-zeisler marked this conversation as resolved.
b.set.anchor = p.Value
return b
}

// WithExpiringMobileKey adds a mobile key that should be accepted until the given expiry. It is a
// no-op if the key is undefined or already present.
func (b *AcceptedSetBuilder) WithExpiringMobileKey(key config.MobileKey, expiry time.Time) *AcceptedSetBuilder {
b.addMobileKey(key, &expiry)
return b
}

// WithPrimaryMobileKey adds key (if not already present) and designates it as the primary mobile
// key — the singular default (the wire's mobKey) used where one mobile key is required, e.g. event
// forwarding. It is a no-op if the key is undefined.
func (b *AcceptedSetBuilder) WithPrimaryMobileKey(key config.MobileKey) *AcceptedSetBuilder {
if key.Defined() {
b.addMobileKey(key, nil)
b.set.primaryMobileKey = key
// WithMobileKey adds a mobile key. It is a no-op if the value is undefined or already present.
func (b *AcceptedSetBuilder) WithMobileKey(p MobileKeyParams) *AcceptedSetBuilder {
if !p.Value.Defined() || b.set.hasMobileKey(p.Value) {
Comment thread
aaron-zeisler marked this conversation as resolved.
return b
}
b.set.mobileKeys[p.Value] = acceptedKeyInfo{key: p.Key, expiry: p.Expiry}
return b
}

// addMobileKey records the key with the given expiry (nil = permanent), skipping undefined keys and
// keys already in the set (the first expiry recorded for a key wins).
func (b *AcceptedSetBuilder) addMobileKey(key config.MobileKey, expiry *time.Time) {
if !key.Defined() || b.set.hasMobileKey(key) {
return
// WithPrimaryMobileKey adds p.Value and designates it as the primary mobile key — the singular
// default (the wire's mobKey) used where one mobile key is required, e.g. event forwarding. The
// primary is always permanent, so p.Expiry is ignored. It is a no-op if the value is undefined.
func (b *AcceptedSetBuilder) WithPrimaryMobileKey(p MobileKeyParams) *AcceptedSetBuilder {
if !p.Value.Defined() {
return b
}
b.set.mobileKeys[key] = expiry
b.set.mobileKeys[p.Value] = acceptedKeyInfo{key: p.Key, expiry: nil}
b.set.primaryMobileKey = p.Value
return b
}

// WithEnvironmentID sets the environment ID. It is a no-op if the ID is undefined.
Expand Down
16 changes: 8 additions & 8 deletions internal/credential/accepted_set_builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ import (
func TestAcceptedSetBuilderValidation(t *testing.T) {
// No SDK key at all is a caller error.
_, err := NewAcceptedSetBuilder().
WithMobileKey(config.MobileKey("mob")).
WithMobileKey(MobileKeyParams{Value: "mob"}).
WithEnvironmentID(config.EnvironmentID("env")).
Build()
require.ErrorIs(t, err, errAcceptedSetMissingSDKKey)

// An SDK key with no designated anchor is malformed.
var malformed *MalformedCredentialSetError
_, err = NewAcceptedSetBuilder().WithSDKKey(config.SDKKey("sdk")).Build()
_, err = NewAcceptedSetBuilder().WithSDKKey(SDKKeyParams{Value: "sdk"}).Build()
require.ErrorAs(t, err, &malformed)

// WithAnchor adds the key and designates it as the anchor, so Build succeeds.
set, err := NewAcceptedSetBuilder().WithAnchor(config.SDKKey("sdk")).Build()
set, err := NewAcceptedSetBuilder().WithAnchor(SDKKeyParams{Value: "sdk"}).Build()
require.NoError(t, err)
assert.True(t, set.hasSDKKey(config.SDKKey("sdk")))
assert.Equal(t, config.SDKKey("sdk"), set.anchor)
Expand All @@ -32,11 +32,11 @@ func TestAcceptedSetBuilderValidation(t *testing.T) {
func TestAcceptedSetBuilderDeduplicates(t *testing.T) {
// Adding the same key more than once (including via WithPrimary*) keeps a single entry.
set := mustBuild(t, NewAcceptedSetBuilder().
WithSDKKey(config.SDKKey("sdk")).
WithAnchor(config.SDKKey("sdk")).
WithSDKKey(config.SDKKey("sdk")).
WithMobileKey(config.MobileKey("mob")).
WithPrimaryMobileKey(config.MobileKey("mob")))
WithSDKKey(SDKKeyParams{Value: "sdk"}).
WithAnchor(SDKKeyParams{Value: "sdk"}).
WithSDKKey(SDKKeyParams{Value: "sdk"}).
WithMobileKey(MobileKeyParams{Value: "mob"}).
WithPrimaryMobileKey(MobileKeyParams{Value: "mob"}))

assert.Len(t, set.sdkKeys, 1)
assert.Len(t, set.mobileKeys, 1)
Expand Down
63 changes: 31 additions & 32 deletions internal/credential/rotator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
// acceptedKeyInfo holds per-key metadata for the accepted-set maps.
type acceptedKeyInfo struct {
expiry *time.Time // nil = permanent
key *string // wire "key" identifier — non-secret human-readable name; nil when absent
}

type Rotator struct {
Expand All @@ -28,11 +29,11 @@ type Rotator struct {

// acceptedSDKKeys is the full set of accepted SDK keys with optional per-key expiry.
// A nil expiry means the key is permanent. The anchor is always present with a nil expiry.
acceptedSDKKeys map[config.SDKKey]*acceptedKeyInfo
acceptedSDKKeys map[config.SDKKey]acceptedKeyInfo

// acceptedMobileKeys is the full set of accepted mobile keys with optional per-key expiry.
// A nil expiry means the key is permanent.
acceptedMobileKeys map[config.MobileKey]*acceptedKeyInfo
acceptedMobileKeys map[config.MobileKey]acceptedKeyInfo

@aaron-zeisler aaron-zeisler Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for reviewers: Here I decided to go with values instead of pointers. This caused a handful of random-looking changes throughout the pull request.


expirations []SDKCredential
additions []SDKCredential
Expand All @@ -51,8 +52,8 @@ type InitialCredentials struct {
func NewRotator(loggers ldlog.Loggers) *Rotator {
r := &Rotator{
loggers: loggers,
acceptedSDKKeys: make(map[config.SDKKey]*acceptedKeyInfo),
acceptedMobileKeys: make(map[config.MobileKey]*acceptedKeyInfo),
acceptedSDKKeys: make(map[config.SDKKey]acceptedKeyInfo),
acceptedMobileKeys: make(map[config.MobileKey]acceptedKeyInfo),
}
return r
}
Expand All @@ -70,10 +71,10 @@ func (r *Rotator) Initialize(credentials []SDKCredential) {
switch cred := cred.(type) {
case config.SDKKey:
r.anchorKey = cred
r.acceptedSDKKeys[cred] = &acceptedKeyInfo{}
r.acceptedSDKKeys[cred] = acceptedKeyInfo{}
case config.MobileKey:
r.primaryMobileKey = cred
r.acceptedMobileKeys[cred] = &acceptedKeyInfo{}
r.acceptedMobileKeys[cred] = acceptedKeyInfo{}
case config.EnvironmentID:
r.primaryEnvironmentID = cred
}
Expand Down Expand Up @@ -222,23 +223,23 @@ type reconcilableKey interface {
// key no longer desired is dropped and queued as an expiration. Per-key expiry is stored as data on
// the accepted entry; the cleanup ticker is what later acts on it. The caller must hold the write lock.
func reconcileAcceptedKeys[K reconcilableKey](
desired map[K]*time.Time,
accepted map[K]*acceptedKeyInfo,
desired map[K]acceptedKeyInfo,
accepted map[K]acceptedKeyInfo,
additions *[]SDKCredential,
expirations *[]SDKCredential,
loggers ldlog.Loggers,
kind string,
) {
// First pass: walk every key the set wants us to accept. If we already accept it, just refresh
// its expiry; if it's new, start accepting it and queue it as an addition.
for key, expiry := range desired {
if info, ok := accepted[key]; ok {
info.expiry = expiry
} else {
accepted[key] = &acceptedKeyInfo{expiry: expiry}
// First pass: walk every key the set wants us to accept. Writing the desired entry over the
// accepted one refreshes its metadata — both the expiry and the wire "key" identifier, clearing
// the identifier when the new payload carries none so a stale name never lingers in /status. A key
// we don't yet accept is also queued as an addition.
for key, want := range desired {
if _, ok := accepted[key]; !ok {
*additions = append(*additions, key)
loggers.Infof("%s %s is now accepted", kind, key.Masked())
}
accepted[key] = want
}
// Second pass: walk every key we currently accept and drop the ones the set no longer wants.
// Keys still desired were handled above, so skip them; the rest are revoked outright (removed
Expand All @@ -254,34 +255,32 @@ func reconcileAcceptedKeys[K reconcilableKey](
}

// reconcileSDKKeys diffs the desired SDK keys against the accepted set and applies the result via
// reconcileAcceptedKeys. The anchor is always accepted and permanent, regardless of any expiry the
// payload may carry for it. The caller must hold the write lock.
// reconcileAcceptedKeys. The set is trusted as well-formed: BuildAcceptedSet / the builder guarantee
// the anchor is present and permanent (WithAnchor forces a nil expiry), so no special handling is
// needed here. The caller must hold the write lock.
func (r *Rotator) reconcileSDKKeys(set AcceptedSet, anchor config.SDKKey, now time.Time) {
desired := make(map[config.SDKKey]*time.Time, len(set.sdkKeys))
for key, expiry := range set.sdkKeys {
if expiry != nil && !now.Before(*expiry) {
desired := make(map[config.SDKKey]acceptedKeyInfo, len(set.sdkKeys))
for key, info := range set.sdkKeys {
Comment thread
aaron-zeisler marked this conversation as resolved.
if info.expiry != nil && !now.Before(*info.expiry) {
continue // already expired; treat as absent
}
desired[key] = expiry
desired[key] = info
}
desired[anchor] = nil
reconcileAcceptedKeys(desired, r.acceptedSDKKeys, &r.additions, &r.expirations, r.loggers, "SDK key")
r.anchorKey = anchor
}

// reconcileMobileKeys mirrors reconcileSDKKeys for mobile keys. The primary mobile key — the wire's
// singular mobKey, used where one mobile key is required (e.g. event forwarding) — is always accepted
// and permanent; an empty value means the set declared no mobile key. The caller must hold the lock.
// reconcileMobileKeys mirrors reconcileSDKKeys for mobile keys. The set is trusted as well-formed:
// when a primary mobile key is designated, the builder guarantees it is present and permanent
// (WithPrimaryMobileKey forces a nil expiry). An empty primary means the set declared no mobile key.
// The caller must hold the lock.
func (r *Rotator) reconcileMobileKeys(set AcceptedSet, now time.Time) {
desired := make(map[config.MobileKey]*time.Time, len(set.mobileKeys))
for key, expiry := range set.mobileKeys {
if expiry != nil && !now.Before(*expiry) {
desired := make(map[config.MobileKey]acceptedKeyInfo, len(set.mobileKeys))
for key, info := range set.mobileKeys {
Comment thread
aaron-zeisler marked this conversation as resolved.
if info.expiry != nil && !now.Before(*info.expiry) {
continue // already expired; treat as absent
}
desired[key] = expiry
}
if set.primaryMobileKey.Defined() {
desired[set.primaryMobileKey] = nil
desired[key] = info
}
reconcileAcceptedKeys(desired, r.acceptedMobileKeys, &r.additions, &r.expirations, r.loggers, "Mobile key")
r.primaryMobileKey = set.primaryMobileKey
Expand Down
Loading
Loading