Skip to content

Commit d3d3a0e

Browse files
aaron-zeisleraaron-zeisler
committed
refactor(credential): carry wire key identifiers through the accepted set
Replace the builder's WithExpiring{SDK,Mobile}Key methods with a param-struct API (SDKKeyParams/MobileKeyParams) so each accepted key can carry its optional wire "key" identifier alongside its value and expiry. AcceptedSet map values become acceptedKeyInfo (value, expiry, and the optional key identifier); the rotator's reconcile loops thread the identifier through, refreshing it on every reconcile and clearing it when a later payload carries none. Add util.PtrOrNil for the optional-field modelling. This is the storage layer for surfacing per-key identifiers on the status endpoint; no externally visible behaviour changes yet.
1 parent 7c45431 commit d3d3a0e

11 files changed

Lines changed: 231 additions & 181 deletions

‎internal/credential/accepted_set.go‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package credential
33
import (
44
"errors"
55
"fmt"
6-
"time"
76

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

102101
// NewEmptyCredentialError returns a MalformedCredentialSetError for a key-array entry whose
103-
// value field is empty. kind is "sdkKeys" or "mobileKeys"; identifier is the key's identifier
104-
// string (may be empty for old-format payloads that synthesize from the singular fields).
105-
func NewEmptyCredentialError(kind, identifier string) *MalformedCredentialSetError {
106-
if identifier == "" {
102+
// value field is empty. kind is "sdkKeys" or "mobileKeys"; key is the entry's wire "key" identifier
103+
// (may be empty for old-format payloads that synthesize from the singular fields).
104+
func NewEmptyCredentialError(kind, key string) *MalformedCredentialSetError {
105+
if key == "" {
107106
return &MalformedCredentialSetError{
108107
msg: fmt.Sprintf("malformed credential set: %s entry has an empty value", kind),
109108
}
110109
}
111110
return &MalformedCredentialSetError{
112-
msg: fmt.Sprintf("malformed credential set: %s entry %q has an empty value", kind, identifier),
111+
msg: fmt.Sprintf("malformed credential set: %s entry %q has an empty value", kind, key),
113112
}
114113
}

‎internal/credential/accepted_set_builder.go‎

Lines changed: 42 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -16,77 +16,69 @@ type AcceptedSetBuilder struct {
1616
func NewAcceptedSetBuilder() *AcceptedSetBuilder {
1717
return &AcceptedSetBuilder{
1818
set: AcceptedSet{
19-
sdkKeys: make(map[config.SDKKey]*time.Time),
20-
mobileKeys: make(map[config.MobileKey]*time.Time),
19+
sdkKeys: make(map[config.SDKKey]acceptedKeyInfo),
20+
mobileKeys: make(map[config.MobileKey]acceptedKeyInfo),
2121
},
2222
}
2323
}
2424

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

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

39-
// WithAnchor adds key (if not already present) and designates it as the anchor — the SDK key
40-
// that owns the environment's upstream connection. It is a no-op if the key is undefined.
41-
func (b *AcceptedSetBuilder) WithAnchor(key config.SDKKey) *AcceptedSetBuilder {
42-
if key.Defined() {
43-
b.addSDKKey(key, nil)
44-
b.set.anchor = key
40+
// WithSDKKey adds a server-side SDK key. It is a no-op if the value is undefined or already present
41+
// (the first metadata recorded for a value wins).
42+
func (b *AcceptedSetBuilder) WithSDKKey(p SDKKeyParams) *AcceptedSetBuilder {
43+
if !p.Value.Defined() || b.set.hasSDKKey(p.Value) {
44+
return b
4545
}
46+
b.set.sdkKeys[p.Value] = acceptedKeyInfo{key: p.Key, expiry: p.Expiry}
4647
return b
4748
}
4849

49-
// addSDKKey records the key with the given expiry (nil = permanent), skipping undefined keys and
50-
// keys already in the set (the first expiry recorded for a key wins).
51-
func (b *AcceptedSetBuilder) addSDKKey(key config.SDKKey, expiry *time.Time) {
52-
if !key.Defined() || b.set.hasSDKKey(key) {
53-
return
50+
// WithAnchor adds p.Value and designates it as the anchor — the SDK key that owns the environment's
51+
// upstream connection. The anchor is always permanent, so p.Expiry is ignored. It is a no-op if the
52+
// value is undefined. Unlike WithSDKKey it overwrites any existing entry for the value, since
53+
// designating the anchor takes precedence over an earlier non-anchor add.
54+
func (b *AcceptedSetBuilder) WithAnchor(p SDKKeyParams) *AcceptedSetBuilder {
55+
if !p.Value.Defined() {
56+
return b
5457
}
55-
b.set.sdkKeys[key] = expiry
56-
}
57-
58-
// WithMobileKey adds a permanent (non-expiring) mobile key. It is a no-op if the key is undefined or
59-
// already present.
60-
func (b *AcceptedSetBuilder) WithMobileKey(key config.MobileKey) *AcceptedSetBuilder {
61-
b.addMobileKey(key, nil)
58+
b.set.sdkKeys[p.Value] = acceptedKeyInfo{key: p.Key, expiry: nil}
59+
b.set.anchor = p.Value
6260
return b
6361
}
6462

65-
// WithExpiringMobileKey adds a mobile key that should be accepted until the given expiry. It is a
66-
// no-op if the key is undefined or already present.
67-
func (b *AcceptedSetBuilder) WithExpiringMobileKey(key config.MobileKey, expiry time.Time) *AcceptedSetBuilder {
68-
b.addMobileKey(key, &expiry)
69-
return b
70-
}
71-
72-
// WithPrimaryMobileKey adds key (if not already present) and designates it as the primary mobile
73-
// key — the singular default (the wire's mobKey) used where one mobile key is required, e.g. event
74-
// forwarding. It is a no-op if the key is undefined.
75-
func (b *AcceptedSetBuilder) WithPrimaryMobileKey(key config.MobileKey) *AcceptedSetBuilder {
76-
if key.Defined() {
77-
b.addMobileKey(key, nil)
78-
b.set.primaryMobileKey = key
63+
// WithMobileKey adds a mobile key. It is a no-op if the value is undefined or already present.
64+
func (b *AcceptedSetBuilder) WithMobileKey(p MobileKeyParams) *AcceptedSetBuilder {
65+
if !p.Value.Defined() || b.set.hasMobileKey(p.Value) {
66+
return b
7967
}
68+
b.set.mobileKeys[p.Value] = acceptedKeyInfo{key: p.Key, expiry: p.Expiry}
8069
return b
8170
}
8271

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

9284
// WithEnvironmentID sets the environment ID. It is a no-op if the ID is undefined.

‎internal/credential/accepted_set_builder_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,18 @@ import (
1212
func TestAcceptedSetBuilderValidation(t *testing.T) {
1313
// No SDK key at all is a caller error.
1414
_, err := NewAcceptedSetBuilder().
15-
WithMobileKey(config.MobileKey("mob")).
15+
WithMobileKey(MobileKeyParams{Value: "mob"}).
1616
WithEnvironmentID(config.EnvironmentID("env")).
1717
Build()
1818
require.ErrorIs(t, err, errAcceptedSetMissingSDKKey)
1919

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

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

4141
assert.Len(t, set.sdkKeys, 1)
4242
assert.Len(t, set.mobileKeys, 1)

‎internal/credential/rotator.go‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
// acceptedKeyInfo holds per-key metadata for the accepted-set maps.
1212
type acceptedKeyInfo struct {
1313
expiry *time.Time // nil = permanent
14+
key *string // wire "key" identifier — non-secret human-readable name; nil when absent
1415
}
1516

1617
type Rotator struct {
@@ -258,14 +259,21 @@ func reconcileAcceptedKeys[K reconcilableKey](
258259
// payload may carry for it. The caller must hold the write lock.
259260
func (r *Rotator) reconcileSDKKeys(set AcceptedSet, anchor config.SDKKey, now time.Time) {
260261
desired := make(map[config.SDKKey]*time.Time, len(set.sdkKeys))
261-
for key, expiry := range set.sdkKeys {
262-
if expiry != nil && !now.Before(*expiry) {
262+
for key, info := range set.sdkKeys {
263+
if info.expiry != nil && !now.Before(*info.expiry) {
263264
continue // already expired; treat as absent
264265
}
265-
desired[key] = expiry
266+
desired[key] = info.expiry
266267
}
267268
desired[anchor] = nil
268269
reconcileAcceptedKeys(desired, r.acceptedSDKKeys, &r.additions, &r.expirations, r.loggers, "SDK key")
270+
// Refresh the wire "key" identifier for every key now in the accepted set, including clearing it
271+
// when the new payload carries none — otherwise a stale identifier would linger in /status.
272+
for key, info := range set.sdkKeys {
273+
if accepted, ok := r.acceptedSDKKeys[key]; ok {
274+
accepted.key = info.key
275+
}
276+
}
269277
r.anchorKey = anchor
270278
}
271279

@@ -274,16 +282,23 @@ func (r *Rotator) reconcileSDKKeys(set AcceptedSet, anchor config.SDKKey, now ti
274282
// and permanent; an empty value means the set declared no mobile key. The caller must hold the lock.
275283
func (r *Rotator) reconcileMobileKeys(set AcceptedSet, now time.Time) {
276284
desired := make(map[config.MobileKey]*time.Time, len(set.mobileKeys))
277-
for key, expiry := range set.mobileKeys {
278-
if expiry != nil && !now.Before(*expiry) {
285+
for key, info := range set.mobileKeys {
286+
if info.expiry != nil && !now.Before(*info.expiry) {
279287
continue // already expired; treat as absent
280288
}
281-
desired[key] = expiry
289+
desired[key] = info.expiry
282290
}
283291
if set.primaryMobileKey.Defined() {
284292
desired[set.primaryMobileKey] = nil
285293
}
286294
reconcileAcceptedKeys(desired, r.acceptedMobileKeys, &r.additions, &r.expirations, r.loggers, "Mobile key")
295+
// Refresh the wire "key" identifier for every key now in the accepted set, including clearing it
296+
// when the new payload carries none — otherwise a stale identifier would linger in /status.
297+
for key, info := range set.mobileKeys {
298+
if accepted, ok := r.acceptedMobileKeys[key]; ok {
299+
accepted.key = info.key
300+
}
301+
}
287302
r.primaryMobileKey = set.primaryMobileKey
288303
}
289304

‎internal/credential/rotator_test.go‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/launchdarkly/go-sdk-common/v3/ldlogtest"
88
"github.com/launchdarkly/ld-relay/v8/config"
9+
"github.com/launchdarkly/ld-relay/v8/internal/util"
910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
1112
)
@@ -53,7 +54,7 @@ func TestReconcileAnchorOnly(t *testing.T) {
5354
anchor := config.SDKKey("anchor")
5455
now := time.Now()
5556

56-
r.Reconcile(mustBuild(t, NewAcceptedSetBuilder().WithAnchor(anchor)), now)
57+
r.Reconcile(mustBuild(t, NewAcceptedSetBuilder().WithAnchor(SDKKeyParams{Value: anchor})), now)
5758
additions, expirations := r.StepTime(now)
5859

5960
assert.ElementsMatch(t, []SDKCredential{anchor}, additions)
@@ -70,7 +71,7 @@ func TestReconcileMultipleSDKKeys(t *testing.T) {
7071
now := time.Now()
7172

7273
r.Reconcile(
73-
mustBuild(t, NewAcceptedSetBuilder().WithAnchor(anchor).WithSDKKey(other)), now)
74+
mustBuild(t, NewAcceptedSetBuilder().WithAnchor(SDKKeyParams{Value: anchor}).WithSDKKey(SDKKeyParams{Value: other})), now)
7475
additions, expirations := r.StepTime(now)
7576

7677
// Both server keys are accepted; only the anchor is primary.
@@ -89,7 +90,7 @@ func TestReconcileMultipleMobileKeys(t *testing.T) {
8990
now := time.Now()
9091

9192
r.Reconcile(
92-
mustBuild(t, NewAcceptedSetBuilder().WithAnchor(anchor).WithPrimaryMobileKey(mob1).WithMobileKey(mob2)), now)
93+
mustBuild(t, NewAcceptedSetBuilder().WithAnchor(SDKKeyParams{Value: anchor}).WithPrimaryMobileKey(MobileKeyParams{Value: mob1}).WithMobileKey(MobileKeyParams{Value: mob2})), now)
9394
additions, _ := r.StepTime(now)
9495

9596
// Every mobile key is accepted; the designated one is the primary.
@@ -106,11 +107,11 @@ func TestReconcileRevokesOmittedKeys(t *testing.T) {
106107
now := time.Now()
107108

108109
r.Reconcile(
109-
mustBuild(t, NewAcceptedSetBuilder().WithAnchor(anchor).WithSDKKey(other).WithPrimaryMobileKey(mob)), now)
110+
mustBuild(t, NewAcceptedSetBuilder().WithAnchor(SDKKeyParams{Value: anchor}).WithSDKKey(SDKKeyParams{Value: other}).WithPrimaryMobileKey(MobileKeyParams{Value: mob})), now)
110111
r.StepTime(now)
111112

112113
// Reconciling to just the anchor revokes the omitted server and mobile keys.
113-
r.Reconcile(mustBuild(t, NewAcceptedSetBuilder().WithAnchor(anchor)), now)
114+
r.Reconcile(mustBuild(t, NewAcceptedSetBuilder().WithAnchor(SDKKeyParams{Value: anchor})), now)
114115
additions, expirations := r.StepTime(now)
115116

116117
assert.Empty(t, additions)
@@ -132,10 +133,10 @@ func TestReconcileAcceptsExpiringKeysAsData(t *testing.T) {
132133

133134
r.Reconcile(
134135
mustBuild(t, NewAcceptedSetBuilder().
135-
WithAnchor(anchor).
136-
WithExpiringSDKKey(expiringSDK, now.Add(time.Hour)).
137-
WithPrimaryMobileKey(mob).
138-
WithExpiringMobileKey(expiringMobile, now.Add(time.Hour))),
136+
WithAnchor(SDKKeyParams{Value: anchor}).
137+
WithSDKKey(SDKKeyParams{Value: expiringSDK, Expiry: util.PtrOrNil(now.Add(time.Hour))}).
138+
WithPrimaryMobileKey(MobileKeyParams{Value: mob}).
139+
WithMobileKey(MobileKeyParams{Value: expiringMobile, Expiry: util.PtrOrNil(now.Add(time.Hour))})),
139140
now)
140141
additions, expirations := r.StepTime(now)
141142

@@ -158,9 +159,9 @@ func TestReconcilePrimaryMobileKeyIsAlwaysAccepted(t *testing.T) {
158159
now := time.Unix(1000, 0)
159160

160161
set := mustBuild(t, NewAcceptedSetBuilder().
161-
WithAnchor(anchor).
162-
WithExpiringMobileKey(mob, now.Add(-time.Hour)). // already expired in the payload...
163-
WithPrimaryMobileKey(mob)) // ...but designated as the primary
162+
WithAnchor(SDKKeyParams{Value: anchor}).
163+
WithMobileKey(MobileKeyParams{Value: mob, Expiry: util.PtrOrNil(now.Add(-time.Hour))}). // already expired in the payload...
164+
WithPrimaryMobileKey(MobileKeyParams{Value: mob})) // ...but designated as the primary
164165
r.Reconcile(set, now)
165166
r.StepTime(now)
166167

@@ -184,10 +185,10 @@ func TestReconcileExpiringKeysAreEvictedByStepTime(t *testing.T) {
184185

185186
r.Reconcile(
186187
mustBuild(t, NewAcceptedSetBuilder().
187-
WithAnchor(anchor).
188-
WithExpiringSDKKey(expiringSDK, expiry).
189-
WithPrimaryMobileKey(mob).
190-
WithExpiringMobileKey(expiringMobile, expiry)),
188+
WithAnchor(SDKKeyParams{Value: anchor}).
189+
WithSDKKey(SDKKeyParams{Value: expiringSDK, Expiry: util.PtrOrNil(expiry)}).
190+
WithPrimaryMobileKey(MobileKeyParams{Value: mob}).
191+
WithMobileKey(MobileKeyParams{Value: expiringMobile, Expiry: util.PtrOrNil(expiry)})),
191192
now)
192193
additions, expirations := r.StepTime(now)
193194
require.ElementsMatch(t, []SDKCredential{anchor, expiringSDK, mob, expiringMobile}, additions)
@@ -218,8 +219,8 @@ func TestReconcileAlreadyExpiredKeyIsIgnoredOnAdd(t *testing.T) {
218219

219220
r.Reconcile(
220221
mustBuild(t, NewAcceptedSetBuilder().
221-
WithAnchor(anchor).
222-
WithExpiringSDKKey(staleKey, alreadyExpired)),
222+
WithAnchor(SDKKeyParams{Value: anchor}).
223+
WithSDKKey(SDKKeyParams{Value: staleKey, Expiry: util.PtrOrNil(alreadyExpired)})),
223224
now)
224225
additions, expirations := r.StepTime(now)
225226

@@ -242,17 +243,17 @@ func TestReconcileDeExpiryRestoresKey(t *testing.T) {
242243
// First reconcile: key is accepted with a future expiry.
243244
r.Reconcile(
244245
mustBuild(t, NewAcceptedSetBuilder().
245-
WithAnchor(anchor).
246-
WithExpiringSDKKey(key, expiry)),
246+
WithAnchor(SDKKeyParams{Value: anchor}).
247+
WithSDKKey(SDKKeyParams{Value: key, Expiry: util.PtrOrNil(expiry)})),
247248
now)
248249
r.StepTime(now)
249250
require.ElementsMatch(t, []SDKCredential{key}, r.DeprecatedCredentials())
250251

251252
// Second reconcile: same key, no expiry (de-expiry).
252253
r.Reconcile(
253254
mustBuild(t, NewAcceptedSetBuilder().
254-
WithAnchor(anchor).
255-
WithSDKKey(key)),
255+
WithAnchor(SDKKeyParams{Value: anchor}).
256+
WithSDKKey(SDKKeyParams{Value: key})),
256257
now)
257258
r.StepTime(now)
258259

0 commit comments

Comments
 (0)