From 40c80390db4be95208f27c0e2fce9ba1ed899c9f Mon Sep 17 00:00:00 2001 From: Aaron Zeisler Date: Thu, 6 Aug 2026 13:01:21 -0700 Subject: [PATCH 1/3] chore(concurrent-keys): carry the hasViews marker through the credential wire format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds hasViews to each sdkKeys[]/mobileKeys[] entry and carries it onto the accepted-key params. Nothing reads it yet — the filter that consumes it follows in the next commit. A plain bool rather than a pointer: an absent field means "not view-scoped" and there is no third state, unlike expiry where nil genuinely means permanent. Old-format payloads synthesize their accepted keys from the singular sdkKey/mobKey and so never set it, which is correct — the backend forbids views on a default key. --- internal/envfactory/env_params.go | 16 +++++--- internal/envfactory/env_rep.go | 17 ++++---- internal/envfactory/env_rep_test.go | 61 +++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 13 deletions(-) diff --git a/internal/envfactory/env_params.go b/internal/envfactory/env_params.go index 35acffcd..20d275b2 100644 --- a/internal/envfactory/env_params.go +++ b/internal/envfactory/env_params.go @@ -46,18 +46,22 @@ type EnvironmentParams struct { // AcceptedSDKKey is one entry in the accepted SDK key set for an environment. // Expiry is zero if the key is permanent. +// HasViews is true if the SDK key is associated with a view. type AcceptedSDKKey struct { - Key string - Value config.SDKKey - Expiry time.Time + Key string + Value config.SDKKey + Expiry time.Time + HasViews bool } // AcceptedMobileKey is one entry in the accepted mobile key set for an environment. // Expiry is zero if the key is permanent. +// HasViews is true if the mobile key is associated with a view. type AcceptedMobileKey struct { - Key string - Value config.MobileKey - Expiry time.Time + Key string + Value config.MobileKey + Expiry time.Time + HasViews bool } func (e EnvironmentParams) WithFilter(key config.FilterKey) EnvironmentParams { diff --git a/internal/envfactory/env_rep.go b/internal/envfactory/env_rep.go index d83f7ec0..0b150f72 100644 --- a/internal/envfactory/env_rep.go +++ b/internal/envfactory/env_rep.go @@ -105,9 +105,10 @@ type ExpiringKeyRep struct { // Key is the human-readable identifier (non-secret, e.g. "default-sdk"); Value is // the credential secret (e.g. "sdk-xxxx-..."). See the EnvironmentRep TERMINOLOGY comment. type ConcurrentKeyRep struct { - Key string `json:"key"` - Value string `json:"value"` - Expiry *int64 `json:"expiry,omitempty"` // Unix-ms; nil = permanent + Key string `json:"key"` + Value string `json:"value"` + Expiry *int64 `json:"expiry,omitempty"` // Unix-ms; nil = permanent + HasViews bool `json:"hasViews"` } func ToTime(millisecondTime ldtime.UnixMillisecondTime) time.Time { @@ -135,8 +136,9 @@ func (r EnvironmentRep) ToParams() EnvironmentParams { params.AcceptedSDKKeys = make([]AcceptedSDKKey, 0, len(r.SDKKeys)) for _, k := range r.SDKKeys { entry := AcceptedSDKKey{ - Key: k.Key, - Value: config.SDKKey(k.Value), + Key: k.Key, + Value: config.SDKKey(k.Value), + HasViews: k.HasViews, } if k.Expiry != nil { entry.Expiry = time.UnixMilli(*k.Expiry) @@ -162,8 +164,9 @@ func (r EnvironmentRep) ToParams() EnvironmentParams { params.AcceptedMobileKeys = make([]AcceptedMobileKey, 0, len(r.MobileKeys)) for _, k := range r.MobileKeys { entry := AcceptedMobileKey{ - Key: k.Key, - Value: config.MobileKey(k.Value), + Key: k.Key, + Value: config.MobileKey(k.Value), + HasViews: k.HasViews, } if k.Expiry != nil { entry.Expiry = time.UnixMilli(*k.Expiry) diff --git a/internal/envfactory/env_rep_test.go b/internal/envfactory/env_rep_test.go index 2499b045..6baaea75 100644 --- a/internal/envfactory/env_rep_test.go +++ b/internal/envfactory/env_rep_test.go @@ -159,6 +159,67 @@ func TestEnvironmentRepNewFormatWithArrays(t *testing.T) { assert.Equal(t, AcceptedMobileKey{Key: "mob-key-1", Value: config.MobileKey("mob-f41c")}, params.AcceptedMobileKeys[0]) } +// TestEnvironmentRepViewScopedKeys pins the hasViews wire contract on both arrays: it decodes onto +// ConcurrentKeyRep and is carried through ToParams onto the accepted entries, where BuildAcceptedSet +// consumes it. +// +// The absent-field case is the important one. hasViews is a plain bool, so an entry that omits it — +// every entry a backend that predates the field emits — decodes to false and is treated as not +// view-scoped. An explicit false is indistinguishable from absent, which is the intent: there is no +// third state. +func TestEnvironmentRepViewScopedKeys(t *testing.T) { + jsonStr := `{ + "envID": "68e5179e8307e4099c277e2a", + "envKey": "production", + "envName": "Production", + "mobKey": "mob-primary", + "projKey": "my-project", + "projName": "My Project", + "sdkKey": { "value": "sdk-anchor" }, + "sdkKeys": [ + { "key": "default-sdk", "value": "sdk-anchor" }, + { "key": "service-a", "value": "sdk-service-a", "hasViews": false }, + { "key": "view-scoped", "value": "sdk-viewy", "hasViews": true } + ], + "mobileKeys": [ + { "key": "default-mob", "value": "mob-primary" }, + { "key": "view-scoped-mob", "value": "mob-viewy", "hasViews": true } + ], + "version": 26 + }` + + var rep EnvironmentRep + require.NoError(t, json.Unmarshal([]byte(jsonStr), &rep)) + + require.Len(t, rep.SDKKeys, 3) + assert.False(t, rep.SDKKeys[0].HasViews, "an absent hasViews must decode to false") + assert.False(t, rep.SDKKeys[1].HasViews) + assert.True(t, rep.SDKKeys[2].HasViews) + + require.Len(t, rep.MobileKeys, 2) + assert.False(t, rep.MobileKeys[0].HasViews) + assert.True(t, rep.MobileKeys[1].HasViews) + + params := rep.ToParams() + + require.Len(t, params.AcceptedSDKKeys, 3) + assert.Equal(t, AcceptedSDKKey{Key: "default-sdk", Value: config.SDKKey("sdk-anchor")}, params.AcceptedSDKKeys[0]) + assert.Equal(t, AcceptedSDKKey{Key: "service-a", Value: config.SDKKey("sdk-service-a")}, params.AcceptedSDKKeys[1]) + assert.Equal(t, AcceptedSDKKey{ + Key: "view-scoped", + Value: config.SDKKey("sdk-viewy"), + HasViews: true, + }, params.AcceptedSDKKeys[2]) + + require.Len(t, params.AcceptedMobileKeys, 2) + assert.Equal(t, AcceptedMobileKey{Key: "default-mob", Value: config.MobileKey("mob-primary")}, params.AcceptedMobileKeys[0]) + assert.Equal(t, AcceptedMobileKey{ + Key: "view-scoped-mob", + Value: config.MobileKey("mob-viewy"), + HasViews: true, + }, params.AcceptedMobileKeys[1]) +} + // TestEnvironmentRepOldFormatNoArrays verifies that an old-format payload (singular sdkKey/mobKey // only, no sdkKeys/mobileKeys arrays) is normalized by ToParams() into a consistent accepted set. // The wire rep's SDKKeys/MobileKeys remain nil, but params.AcceptedSDKKeys/AcceptedMobileKeys are From 9eb9ee6e17d4924d134ba72af4f65025ead0a583 Mon Sep 17 00:00:00 2001 From: Aaron Zeisler Date: Thu, 6 Aug 2026 13:02:07 -0700 Subject: [PATCH 2/3] fix(concurrent-keys): reject SDK keys and mobile keys scoped to a view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sdkKeys[]/mobileKeys[] arrays hand relay every non-default credential for an environment, including keys scoped to a view. A view-scoped key is only entitled to a subset of the environment's flags, but relay serves the whole environment payload and has no view support — so accepting one would silently over-deliver every flag to an SDK that must see only a subset. Filter them in BuildAcceptedSet, the single funnel for both the auto-config stream and the offline archive. They never enter the accepted set, so an SDK presenting one is rejected because the credential is absent from the lookup map, and no state for it reaches /status, event forwarding, or the expiry ticker. BuildAcceptedSet returns the identifiers it dropped so the handlers that apply a set can WARN; it takes no logger because the stream parse boundary validates every payload and would double-log each one. Rejection is keyed by credential value rather than evaluated per entry. The accepted-set builder is first-wins, so if two entries carry the same value and only one is marked, per-entry filtering would admit the credential via the unmarked entry while still reporting it rejected. Tainting the value makes one marked entry sufficient whichever position it holds. Excluding the designated key from the taint set is also what keeps the anchor guard structural: the marker is disregarded on the anchor and the primary mobile key, because dropping a designated key would take the whole environment down and the backend forbids views on a default key in the first place. Acquiring a view mid-session needed no new code: reconcileAcceptedKeys already revokes any key absent from the desired set immediately rather than on an expiry timestamp, and RemoveConnectionMapping unmaps before the streams are torn down so a reconnect is rejected. --- internal/autoconfig/stream_manager.go | 13 +- internal/envfactory/reconcile_helper.go | 66 +++++-- internal/envfactory/reconcile_helper_test.go | 191 +++++++++++++++++-- relay/autoconfig_actions.go | 36 +++- relay/filedata_actions.go | 6 +- 5 files changed, 271 insertions(+), 41 deletions(-) diff --git a/internal/autoconfig/stream_manager.go b/internal/autoconfig/stream_manager.go index 1c77fe87..ddb5a742 100644 --- a/internal/autoconfig/stream_manager.go +++ b/internal/autoconfig/stream_manager.go @@ -498,16 +498,13 @@ func (s *StreamManager) dispatchEnvAction(id config.EnvironmentID, rep envfactor } // validateCredentialPayload checks that an environment rep carries a structurally valid credential -// set. It is run at the stream parse boundary — before the rep's version is recorded via Upsert — -// mirroring how an unparseable event is handled by gotMalformedEvent. +// set. It runs at the stream parse boundary, before the rep's version is recorded via Upsert: a +// malformed payload must not advance the version, or the backend's fresh put — which carries the same +// version — would be deduplicated away by the MessageReceiver. // -// A malformed credential payload must preserve the previous accepted set and force a -// stream reconnect (RAC is one-way push with no NAK channel, so the reconnect is what makes the -// backend resend a fresh put). Validating here rather than after Upsert is essential: the version is -// not advanced, so the fresh put — which carries the same version — is not deduplicated away by the -// MessageReceiver. Any error from BuildAcceptedSet is a *MalformedCredentialSetError. +// Not every error is a *MalformedCredentialSetError, so callers treat them all alike. func (s *StreamManager) validateCredentialPayload(rep envfactory.EnvironmentRep) error { - _, err := envfactory.BuildAcceptedSet(rep.ToParams()) + _, _, err := envfactory.BuildAcceptedSet(rep.ToParams()) return err } diff --git a/internal/envfactory/reconcile_helper.go b/internal/envfactory/reconcile_helper.go index 4a5f418e..c9531024 100644 --- a/internal/envfactory/reconcile_helper.go +++ b/internal/envfactory/reconcile_helper.go @@ -1,10 +1,31 @@ package envfactory import ( + "github.com/launchdarkly/ld-relay/v8/config" "github.com/launchdarkly/ld-relay/v8/internal/credential" "github.com/launchdarkly/ld-relay/v8/internal/util" ) +// collectViewScopedValues returns the set of credential values that any entry marks as view-scoped, +// excluding the designated key (the anchor, or the primary mobile key) which is never filtered. +// +// Two entries can carry the same value with only one of them marked. Keying on the value means one +// marked entry rejects it, whichever position it holds. +func collectViewScopedValues[E any, V comparable](entries []E, designated V, get func(E) (V, bool)) map[V]bool { + var viewScoped map[V]bool + for _, e := range entries { + value, hasViews := get(e) + if !hasViews || value == designated { + continue + } + if viewScoped == nil { + viewScoped = make(map[V]bool) + } + viewScoped[value] = true + } + return viewScoped // nil is a valid empty set to read from +} + // BuildAcceptedSet converts an EnvironmentParams into the AcceptedSet needed by // EnvContext.ReconcileCredentials. // @@ -27,9 +48,15 @@ import ( // primary (params.MobileKey undefined), or an array entry with an empty value. The caller must // preserve the previous accepted state and, for RAC handlers, reconnect the stream with jitter to // force a fresh put. This is the single home for the anchor invariant. -func BuildAcceptedSet(params EnvironmentParams) (credential.AcceptedSet, error) { +// +// Keys scoped to a view are filtered out here rather than at authentication time, making this the +// single funnel for both RAC and the offline archive. The second return value names the keys that were +// dropped, so callers can log what they lost. An SDK presenting one of them gets a 401: the key is +// simply absent from the lookup map. +func BuildAcceptedSet(params EnvironmentParams) (credential.AcceptedSet, []string, error) { anchor := params.SDKKey b := credential.NewAcceptedSetBuilder().WithEnvironmentID(params.EnvID) + var rejected []string // Add every accepted SDK key, designating the anchor as we encounter it. WithAnchor both adds and // designates, and forces the anchor permanent — so a payload that (wrongly) carries an expiry on @@ -38,15 +65,23 @@ func BuildAcceptedSet(params EnvironmentParams) (credential.AcceptedSet, error) // // Entries with an empty value are structurally malformed: relay would silently accept them but // they can never authenticate any SDK. Reject loudly rather than produce a credential-short env. + viewScopedSDKValues := collectViewScopedValues(params.AcceptedSDKKeys, anchor, + func(k AcceptedSDKKey) (config.SDKKey, bool) { return k.Value, k.HasViews }) + anchorInArray := false for _, k := range params.AcceptedSDKKeys { if !k.Value.Defined() { - return credential.AcceptedSet{}, credential.NewEmptyCredentialError("sdkKeys", k.Key) + return credential.AcceptedSet{}, nil, credential.NewEmptyCredentialError("sdkKeys", k.Key) } - if k.Value == anchor { + switch { + // A marker on the anchor's own entry is disregarded: dropping the designated key would take the + // whole environment down, and the backend forbids views on a default key in the first place. + case k.Value == anchor: anchorInArray = true b.WithAnchor(credential.SDKKeyParams{Value: k.Value, Key: util.PtrOrNil(k.Key)}) - } else { + case viewScopedSDKValues[k.Value]: + rejected = append(rejected, k.Key) + default: b.WithSDKKey(credential.SDKKeyParams{Value: k.Value, Key: util.PtrOrNil(k.Key), Expiry: util.PtrOrNil(k.Expiry)}) } } @@ -55,21 +90,28 @@ func BuildAcceptedSet(params EnvironmentParams) (credential.AcceptedSet, error) // synthesizes it into the array for old-format payloads). A defined anchor absent from the array is // a structurally malformed payload — reject it. if anchor.Defined() && !anchorInArray { - return credential.AcceptedSet{}, credential.NewAnchorNotInSetError() + return credential.AcceptedSet{}, nil, credential.NewAnchorNotInSetError() } // Add every accepted mobile key, designating the primary as we encounter it. Like the anchor, // WithPrimaryMobileKey forces the primary permanent, so an expiry the payload may carry on the // primary's own entry cannot demote it. + viewScopedMobileValues := collectViewScopedValues(params.AcceptedMobileKeys, params.MobileKey, + func(k AcceptedMobileKey) (config.MobileKey, bool) { return k.Value, k.HasViews }) + primaryMobileInArray := false for _, k := range params.AcceptedMobileKeys { if !k.Value.Defined() { - return credential.AcceptedSet{}, credential.NewEmptyCredentialError("mobileKeys", k.Key) + return credential.AcceptedSet{}, nil, credential.NewEmptyCredentialError("mobileKeys", k.Key) } - if k.Value == params.MobileKey { + switch { + // Like the anchor, a marker on the primary's own entry is disregarded rather than honored. + case k.Value == params.MobileKey: primaryMobileInArray = true b.WithPrimaryMobileKey(credential.MobileKeyParams{Value: k.Value, Key: util.PtrOrNil(k.Key)}) - } else { + case viewScopedMobileValues[k.Value]: + rejected = append(rejected, k.Key) + default: b.WithMobileKey(credential.MobileKeyParams{Value: k.Value, Key: util.PtrOrNil(k.Key), Expiry: util.PtrOrNil(k.Expiry)}) } } @@ -79,7 +121,7 @@ func BuildAcceptedSet(params EnvironmentParams) (credential.AcceptedSet, error) // without this guard the primary would be silently left undesignated, clearing it on reconcile and // breaking event forwarding. (An undefined mobKey is valid — a server-side-only environment.) if params.MobileKey.Defined() && !primaryMobileInArray { - return credential.AcceptedSet{}, credential.NewPrimaryMobileKeyNotInSetError() + return credential.AcceptedSet{}, nil, credential.NewPrimaryMobileKeyNotInSetError() } // A non-empty mobileKeys[] with no designated primary (undefined mobKey) is malformed: the reconcile @@ -89,12 +131,12 @@ func BuildAcceptedSet(params EnvironmentParams) (credential.AcceptedSet, error) // server-side-only environment. Old-format payloads synthesize the array from mobKey only, so an // undefined mobKey yields an empty array and is unaffected.) if len(params.AcceptedMobileKeys) > 0 && !params.MobileKey.Defined() { - return credential.AcceptedSet{}, credential.NewPrimaryMobileKeyMissingError() + return credential.AcceptedSet{}, nil, credential.NewPrimaryMobileKeyMissingError() } set, err := b.Build() if err != nil { - return credential.AcceptedSet{}, err + return credential.AcceptedSet{}, nil, err } - return set, nil + return set, rejected, nil } diff --git a/internal/envfactory/reconcile_helper_test.go b/internal/envfactory/reconcile_helper_test.go index df87d178..aed4d8fa 100644 --- a/internal/envfactory/reconcile_helper_test.go +++ b/internal/envfactory/reconcile_helper_test.go @@ -50,7 +50,7 @@ func TestBuildAcceptedSet_HappyPath(t *testing.T) { []AcceptedSDKKey{{Key: "default", Value: "sdk-anchor"}}, "mob-primary", ) - set, err := BuildAcceptedSet(params) + set, _, err := BuildAcceptedSet(params) require.NoError(t, err) expected := mustBuild(t, credential.NewAcceptedSetBuilder(). @@ -72,7 +72,7 @@ func TestBuildAcceptedSet_MultipleKeys(t *testing.T) { }, "mob-primary", ) - set, err := BuildAcceptedSet(params) + set, _, err := BuildAcceptedSet(params) require.NoError(t, err) expected := mustBuild(t, credential.NewAcceptedSetBuilder(). @@ -99,8 +99,8 @@ func TestBuildAcceptedSet_Rename(t *testing.T) { "mob-primary", ) - setOld, errOld := BuildAcceptedSet(paramsOldName) - setNew, errNew := BuildAcceptedSet(paramsNewName) + setOld, _, errOld := BuildAcceptedSet(paramsOldName) + setNew, _, errNew := BuildAcceptedSet(paramsNewName) require.NoError(t, errOld) require.NoError(t, errNew) @@ -143,8 +143,8 @@ func TestBuildAcceptedSet_Deexpiry(t *testing.T) { "mob-primary", ) - setWithExpiry, errWithExpiry := BuildAcceptedSet(paramsWithExpiry) - setNoExpiry, errNoExpiry := BuildAcceptedSet(paramsNoExpiry) + setWithExpiry, _, errWithExpiry := BuildAcceptedSet(paramsWithExpiry) + setNoExpiry, _, errNoExpiry := BuildAcceptedSet(paramsNoExpiry) require.NoError(t, errWithExpiry) require.NoError(t, errNoExpiry) @@ -173,7 +173,7 @@ func TestBuildAcceptedSet_AnchorNotInArray(t *testing.T) { }, "mob-primary", ) - _, err := BuildAcceptedSet(params) + _, _, err := BuildAcceptedSet(params) require.Error(t, err) var malformed *credential.MalformedCredentialSetError @@ -194,7 +194,7 @@ func TestBuildAcceptedSet_PrimaryMobileNotInArray(t *testing.T) { {Key: "other", Value: "mob-other"}, // ...but NOT in the array }, } - _, err := BuildAcceptedSet(params) + _, _, err := BuildAcceptedSet(params) require.Error(t, err) var malformed *credential.MalformedCredentialSetError @@ -211,7 +211,7 @@ func TestBuildAcceptedSet_NoMobileKey(t *testing.T) { SDKKey: SDKKeyRep{Value: config.SDKKey("sdk-anchor")}, // no MobKey, no MobileKeys } - set, err := BuildAcceptedSet(rep.ToParams()) + set, _, err := BuildAcceptedSet(rep.ToParams()) require.NoError(t, err) expected := mustBuild(t, credential.NewAcceptedSetBuilder(). @@ -235,7 +235,7 @@ func TestBuildAcceptedSet_MobileKeysWithoutPrimary(t *testing.T) { {Key: "mob-1", Value: "mob-primary"}, // ...but the array is non-empty }, } - _, err := BuildAcceptedSet(params) + _, _, err := BuildAcceptedSet(params) require.Error(t, err) var malformed *credential.MalformedCredentialSetError @@ -254,7 +254,7 @@ func TestBuildAcceptedSet_EmptyMobileArrayValid(t *testing.T) { AcceptedSDKKeys: []AcceptedSDKKey{{Key: "default", Value: "sdk-anchor"}}, AcceptedMobileKeys: []AcceptedMobileKey{}, // empty } - set, err := BuildAcceptedSet(params) + set, _, err := BuildAcceptedSet(params) require.NoError(t, err) expected := mustBuild(t, credential.NewAcceptedSetBuilder(). @@ -273,7 +273,7 @@ func TestBuildAcceptedSet_AnchorUndefined(t *testing.T) { }, "mob-primary", ) - _, err := BuildAcceptedSet(params) + _, _, err := BuildAcceptedSet(params) require.Error(t, err) var malformed *credential.MalformedCredentialSetError @@ -289,7 +289,7 @@ func TestBuildAcceptedSet_NoSDKKeys(t *testing.T) { AcceptedSDKKeys: []AcceptedSDKKey{}, AcceptedMobileKeys: []AcceptedMobileKey{}, } - _, err := BuildAcceptedSet(params) + _, _, err := BuildAcceptedSet(params) require.Error(t, err, "a set with no SDK key at all must be rejected") } @@ -312,7 +312,7 @@ func TestBuildAcceptedSet_MixedUpdate(t *testing.T) { }, "mob-primary", ) - set, err := BuildAcceptedSet(params) + set, _, err := BuildAcceptedSet(params) require.NoError(t, err) expected := mustBuild(t, credential.NewAcceptedSetBuilder(). @@ -336,7 +336,7 @@ func TestBuildAcceptedSet_AnchorNeverExpiring(t *testing.T) { }, "mob-primary", ) - set, err := BuildAcceptedSet(params) + set, _, err := BuildAcceptedSet(params) require.NoError(t, err) // Anchor is permanent (WithAnchor), not expiring — identical to a payload with no anchor expiry. @@ -362,7 +362,7 @@ func TestBuildAcceptedSet_MultipleMobileKeys(t *testing.T) { {Key: "mob-2", Value: "mob-secondary"}, }, } - set, err := BuildAcceptedSet(params) + set, _, err := BuildAcceptedSet(params) require.NoError(t, err) expected := mustBuild(t, credential.NewAcceptedSetBuilder(). @@ -388,7 +388,7 @@ func TestBuildAcceptedSet_ExpiringMobileKey(t *testing.T) { {Key: "mob-old", Value: "mob-old", Expiry: expiry1}, // expiring }, } - set, err := BuildAcceptedSet(params) + set, _, err := BuildAcceptedSet(params) require.NoError(t, err) expected := mustBuild(t, credential.NewAcceptedSetBuilder(). @@ -398,3 +398,160 @@ func TestBuildAcceptedSet_ExpiringMobileKey(t *testing.T) { WithPrimaryMobileKey(credential.MobileKeyParams{Value: "mob-primary", Key: util.PtrOrNil("mob-1")})) assert.Equal(t, expected, set, "expiring mobile key must land as an expiring key in the set") } + +// TestBuildAcceptedSet_ViewScopedKeys covers the ingestion filter across both arrays. A key scoped to a +// view may only see a subset of the environment's flags; relay serves the whole environment payload, so +// admitting one would silently over-deliver. Such a key is therefore never added to the set, and an SDK +// presenting it is rejected because the credential is simply absent from the lookup map. The dropped +// keys are returned to the caller so it can WARN. +func TestBuildAcceptedSet_ViewScopedKeys(t *testing.T) { + const ( + anchor = config.SDKKey("sdk-anchor") + primary = config.MobileKey("mob-primary") + ) + + // The four entries every case starts from; individual cases add view-scoped entries alongside them. + anchorEntry := AcceptedSDKKey{Key: "default-sdk", Value: anchor} + extraSDK := AcceptedSDKKey{Key: "service-a", Value: "sdk-service-a"} + primaryEntry := AcceptedMobileKey{Key: "default-mob", Value: primary} + extraMob := AcceptedMobileKey{Key: "mob-extra", Value: "mob-extra"} + + // base is the set with only the two designated keys; cases add whatever survived the filter. + base := func() *credential.AcceptedSetBuilder { + return credential.NewAcceptedSetBuilder(). + WithEnvironmentID("env-abc"). + WithAnchor(credential.SDKKeyParams{Value: anchor, Key: util.PtrOrNil("default-sdk")}). + WithPrimaryMobileKey(credential.MobileKeyParams{Value: primary, Key: util.PtrOrNil("default-mob")}) + } + acceptedExtraSDK := credential.SDKKeyParams{Value: extraSDK.Value, Key: util.PtrOrNil(extraSDK.Key)} + acceptedExtraMob := credential.MobileKeyParams{Value: extraMob.Value, Key: util.PtrOrNil(extraMob.Key)} + + // baseWithExtras is base plus both non-designated keys — the expectation for every case where the + // filter drops nothing that was going to be accepted anyway. + baseWithExtras := func() *credential.AcceptedSetBuilder { + return base().WithSDKKey(acceptedExtraSDK).WithMobileKey(acceptedExtraMob) + } + + tests := []struct { + name string + sdkKeys []AcceptedSDKKey + mobileKeys []AcceptedMobileKey + wantSet func() *credential.AcceptedSetBuilder + wantRejected []string + }{ + { + // Baseline: nothing view-scoped behaves exactly as it did before the field existed. + name: "no view-scoped keys", + sdkKeys: []AcceptedSDKKey{anchorEntry, extraSDK}, + mobileKeys: []AcceptedMobileKey{primaryEntry, extraMob}, + wantSet: baseWithExtras, + }, + { + name: "view-scoped non-anchor SDK key is excluded", + sdkKeys: []AcceptedSDKKey{anchorEntry, extraSDK, {Key: "view-sdk", Value: "sdk-viewy", HasViews: true}}, + mobileKeys: []AcceptedMobileKey{primaryEntry, extraMob}, + wantSet: baseWithExtras, + wantRejected: []string{"view-sdk"}, + }, + { + name: "view-scoped non-primary mobile key is excluded", + sdkKeys: []AcceptedSDKKey{anchorEntry, extraSDK}, + mobileKeys: []AcceptedMobileKey{primaryEntry, extraMob, {Key: "view-mob", Value: "mob-viewy", HasViews: true}}, + wantSet: baseWithExtras, + wantRejected: []string{"view-mob"}, + }, + { + // Both arrays filter in one pass; SDK keys are walked first, hence the order. + name: "view-scoped keys in both arrays are excluded", + sdkKeys: []AcceptedSDKKey{anchorEntry, {Key: "view-sdk", Value: "sdk-viewy", HasViews: true}}, + mobileKeys: []AcceptedMobileKey{primaryEntry, {Key: "view-mob", Value: "mob-viewy", HasViews: true}}, + wantSet: base, + // Every non-designated key is view-scoped, so only the anchor and primary survive. + wantRejected: []string{"view-sdk", "view-mob"}, + }, + { + // A view-scoped key is dropped outright rather than being admitted as an expiring key — + // the marker takes precedence over expiry handling. + name: "view-scoped key carrying an expiry is still excluded", + sdkKeys: []AcceptedSDKKey{anchorEntry, {Key: "view-sdk", Value: "sdk-viewy", Expiry: expiry1, HasViews: true}}, + mobileKeys: []AcceptedMobileKey{primaryEntry}, + wantSet: base, + wantRejected: []string{"view-sdk"}, + }, + { + // Two entries, one value, only the later one marked. The accepted-set builder is + // first-wins, so filtering per-entry would admit the credential via the unmarked entry + // while still reporting it rejected. One marked entry must reject the value outright. + name: "value marked by any duplicate entry is excluded", + sdkKeys: []AcceptedSDKKey{ + anchorEntry, + {Key: "clean-alias", Value: "sdk-dup"}, + {Key: "view-alias", Value: "sdk-dup", HasViews: true}, + }, + mobileKeys: []AcceptedMobileKey{primaryEntry}, + wantSet: base, + wantRejected: []string{"clean-alias", "view-alias"}, + }, + { + // The same, with the marked entry first — the outcome must not depend on array order. + name: "value marked by any duplicate entry is excluded regardless of order", + sdkKeys: []AcceptedSDKKey{ + anchorEntry, + {Key: "view-alias", Value: "sdk-dup", HasViews: true}, + {Key: "clean-alias", Value: "sdk-dup"}, + }, + mobileKeys: []AcceptedMobileKey{primaryEntry}, + wantSet: base, + wantRejected: []string{"view-alias", "clean-alias"}, + }, + { + // The mobile analogue, so both loops are pinned against per-entry filtering. + name: "mobile value marked by any duplicate entry is excluded", + sdkKeys: []AcceptedSDKKey{anchorEntry}, + mobileKeys: []AcceptedMobileKey{ + primaryEntry, + {Key: "clean-mob-alias", Value: "mob-dup"}, + {Key: "view-mob-alias", Value: "mob-dup", HasViews: true}, + }, + wantSet: base, + wantRejected: []string{"clean-mob-alias", "view-mob-alias"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := EnvironmentParams{ + EnvID: "env-abc", + SDKKey: anchor, + MobileKey: primary, + AcceptedSDKKeys: tt.sdkKeys, + AcceptedMobileKeys: tt.mobileKeys, + } + + set, rejected, err := BuildAcceptedSet(params) + + // A view-scoped key is filtered, never fatal — the environment always keeps operating. + require.NoError(t, err) + assert.Equal(t, mustBuild(t, tt.wantSet()), set) + assert.Equal(t, tt.wantRejected, rejected) + }) + } +} + +// TestBuildAcceptedSet_ViewScopedKeysEmptyOnError verifies that the rejected list is empty whenever an +// error is returned. The caller discards the whole payload and preserves its previous credentials in +// that case, so reporting keys it did not act on would produce a misleading WARN. +func TestBuildAcceptedSet_ViewScopedKeysEmptyOnError(t *testing.T) { + // The anchor is absent from sdkKeys[] — malformed — and a view-scoped entry is present alongside. + params := EnvironmentParams{ + EnvID: "env-abc", + SDKKey: "sdk-anchor", + AcceptedSDKKeys: []AcceptedSDKKey{{Key: "view-sdk", Value: "sdk-viewy", HasViews: true}}, + AcceptedMobileKeys: []AcceptedMobileKey{}, + } + + _, rejected, err := BuildAcceptedSet(params) + + require.Error(t, err) + assert.Empty(t, rejected) +} diff --git a/relay/autoconfig_actions.go b/relay/autoconfig_actions.go index c59c4a1a..2f4b1697 100644 --- a/relay/autoconfig_actions.go +++ b/relay/autoconfig_actions.go @@ -1,6 +1,10 @@ package relay import ( + "strings" + + "github.com/launchdarkly/go-sdk-common/v3/ldlog" + "github.com/launchdarkly/ld-relay/v8/config" "github.com/launchdarkly/ld-relay/v8/internal/envfactory" "github.com/launchdarkly/ld-relay/v8/internal/sdkauth" @@ -11,8 +15,34 @@ const ( logMsgAutoConfUpdateUnknownEnv = "Got auto-configuration update for environment %q but did not have previous configuration - will add" logMsgAutoConfDeleteUnknownEnv = "Got auto-configuration delete message for environment %s but did not have previous configuration - ignoring" logMsgAutoConfReceivedAllEnvironments = "Finished processing auto-configuration data" + + logMsgViewScopedKeysRejected = "Environment %q: rejecting credentials scoped to a view: %s." + + " The Relay Proxy serves the entire environment payload and cannot filter it to a view," + + " so SDKs presenting these credentials will be denied." ) +// logViewScopedKeys reports the view-scoped credentials that BuildAcceptedSet filtered out of a +// payload. This logs once per payload that actually reaches a handler. +func logViewScopedKeys(loggers ldlog.Loggers, envName string, rejected []string) { + if len(rejected) > 0 { + loggers.Warnf(logMsgViewScopedKeysRejected, envName, joinKeyIdentifiers(rejected)) + } +} + +// joinKeyIdentifiers renders wire key identifiers for a log message. The backend requires a non-empty +// identifier on every array entry, but the offline archive is an operator-supplied file, so substitute +// a placeholder rather than emitting a message that names nothing. +func joinKeyIdentifiers(identifiers []string) string { + named := make([]string, 0, len(identifiers)) + for _, id := range identifiers { + if id == "" { + id = "" + } + named = append(named, id) + } + return strings.Join(named, ", ") +} + // relayAutoConfigActions is an implementation of the autoconfig.MessageHandler interface. The low-level // autoconfig.StreamManager component, which manages the configuration stream protocol, will call the // interface methods on this object to let us know when environments have been added or changed. @@ -32,11 +62,12 @@ func (a *relayAutoConfigActions) AddEnvironment(params envfactory.EnvironmentPar return } - set, buildErr := envfactory.BuildAcceptedSet(params) + set, rejected, buildErr := envfactory.BuildAcceptedSet(params) if buildErr != nil { a.r.loggers.Errorf(logMsgAutoConfEnvInitError, params.Identifiers.GetDisplayName(), buildErr) return } + logViewScopedKeys(a.r.loggers, params.Identifiers.GetDisplayName(), rejected) env.ReconcileCredentials(set) } @@ -51,7 +82,7 @@ func (a *relayAutoConfigActions) UpdateEnvironment(params envfactory.Environment env.SetTTL(params.TTL) env.SetSecureMode(params.SecureMode) - set, buildErr := envfactory.BuildAcceptedSet(params) + set, rejected, buildErr := envfactory.BuildAcceptedSet(params) if buildErr != nil { // Credential payloads are validated at the stream parse boundary (see StreamManager) before // being dispatched here, so a malformed set should not reach this point. Log defensively and @@ -59,6 +90,7 @@ func (a *relayAutoConfigActions) UpdateEnvironment(params envfactory.Environment a.r.loggers.Errorf(logMsgAutoConfEnvInitError, params.Identifiers.GetDisplayName(), buildErr) return } + logViewScopedKeys(a.r.loggers, params.Identifiers.GetDisplayName(), rejected) env.ReconcileCredentials(set) } diff --git a/relay/filedata_actions.go b/relay/filedata_actions.go index 0ce77763..80194915 100644 --- a/relay/filedata_actions.go +++ b/relay/filedata_actions.go @@ -58,7 +58,7 @@ func (a *relayFileDataActions) AddEnvironment(ae filedata.ArchiveEnvironment) { return } - set, buildErr := envfactory.BuildAcceptedSet(ae.Params) + set, rejected, buildErr := envfactory.BuildAcceptedSet(ae.Params) if buildErr != nil { var malformed *credential.MalformedCredentialSetError if errors.As(buildErr, &malformed) { @@ -69,6 +69,7 @@ func (a *relayFileDataActions) AddEnvironment(ae filedata.ArchiveEnvironment) { // No reconnect for offline mode: preserve previous state (env was just created with // the singular sdkKey from envConfig) and wait for the next archive reload. } else { + logViewScopedKeys(a.r.loggers, ae.Params.Identifiers.GetDisplayName(), rejected) env.ReconcileCredentials(set) } @@ -101,7 +102,7 @@ func (a *relayFileDataActions) UpdateEnvironment(ae filedata.ArchiveEnvironment) env.SetTTL(ae.Params.TTL) env.SetSecureMode(ae.Params.SecureMode) - set, buildErr := envfactory.BuildAcceptedSet(ae.Params) + set, rejected, buildErr := envfactory.BuildAcceptedSet(ae.Params) if buildErr != nil { var malformed *credential.MalformedCredentialSetError if errors.As(buildErr, &malformed) { @@ -112,6 +113,7 @@ func (a *relayFileDataActions) UpdateEnvironment(ae filedata.ArchiveEnvironment) } // Preserve previous credentials; no reconnect (offline path has no live stream). } else { + logViewScopedKeys(a.r.loggers, ae.Params.Identifiers.GetDisplayName(), rejected) env.ReconcileCredentials(set) } From ffc86d5160ee5aab3b38a34e72f2c4416247c926 Mon Sep 17 00:00:00 2001 From: Aaron Zeisler Date: Thu, 6 Aug 2026 10:51:58 -0700 Subject: [PATCH 3/3] refactor(concurrent-keys): make every credential-set rejection a malformed-payload error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AcceptedSetBuilder.Build returned a bare sentinel when no SDK key survived, while every other rejection was a *MalformedCredentialSetError. That framed an empty set as a caller mistake, which it no longer is: the payload reaches it by combining an undefined anchor with an sdkKeys[] array that is either empty or made up entirely of keys relay excludes — the view-scoped case being new. Return the same type, with a message describing the payload rather than the builder, since that is what an operator reading the log can act on. With the type now uniform, the offline handlers no longer branch on it. Both branches already preserved previous credentials and differed only in message text, and the fallback text was the wrong one: it claimed the environment was auto-configured, and in UpdateEnvironment that the environment failed to initialize when it had already been running. Log the accurate message unconditionally instead. Stop documenting the error type as a contract. Nothing in production inspects it now, and BuildAcceptedSet cannot honor such a promise on its own — the guarantee would depend on a function in another package whose author has no reason to know about it. The taxonomy stays pinned by tests, which is where a claim like this can actually be enforced; TestBuildAcceptedSet_NoSDKKeys now covers both shapes that reach the empty-set path, as it is the only test that would catch this classification regressing. --- internal/autoconfig/stream_manager.go | 2 -- internal/credential/accepted_set.go | 16 ++++++---- internal/credential/accepted_set_builder.go | 6 ++-- .../credential/accepted_set_builder_test.go | 6 ++-- internal/envfactory/reconcile_helper.go | 16 +++++----- internal/envfactory/reconcile_helper_test.go | 32 ++++++++++++++----- relay/filedata_actions.go | 20 +++--------- 7 files changed, 52 insertions(+), 46 deletions(-) diff --git a/internal/autoconfig/stream_manager.go b/internal/autoconfig/stream_manager.go index ddb5a742..5c15b845 100644 --- a/internal/autoconfig/stream_manager.go +++ b/internal/autoconfig/stream_manager.go @@ -501,8 +501,6 @@ func (s *StreamManager) dispatchEnvAction(id config.EnvironmentID, rep envfactor // set. It runs at the stream parse boundary, before the rep's version is recorded via Upsert: a // malformed payload must not advance the version, or the backend's fresh put — which carries the same // version — would be deduplicated away by the MessageReceiver. -// -// Not every error is a *MalformedCredentialSetError, so callers treat them all alike. func (s *StreamManager) validateCredentialPayload(rep envfactory.EnvironmentRep) error { _, _, err := envfactory.BuildAcceptedSet(rep.ToParams()) return err diff --git a/internal/credential/accepted_set.go b/internal/credential/accepted_set.go index 24dee4f3..c3011929 100644 --- a/internal/credential/accepted_set.go +++ b/internal/credential/accepted_set.go @@ -1,7 +1,6 @@ package credential import ( - "errors" "fmt" "github.com/launchdarkly/ld-relay/v8/config" @@ -48,11 +47,6 @@ func (s AcceptedSet) hasMobileKey(key config.MobileKey) bool { return ok } -// errAcceptedSetMissingSDKKey is returned by AcceptedSetBuilder.Build when no SDK key was added. An -// environment must always have at least one SDK key (its anchor), so an empty set indicates a caller -// mistake rather than a benign edge case — surfacing it avoids a silent misconfiguration. -var errAcceptedSetMissingSDKKey = errors.New("accepted credential set must contain at least one SDK key") - // MalformedCredentialSetError is returned when a credential payload cannot produce a valid // AcceptedSet. This covers: // @@ -65,6 +59,9 @@ var errAcceptedSetMissingSDKKey = errors.New("accepted credential set must conta // would keep using the previous (possibly revoked) primary. (No mobile keys at all is valid.) // 4. An entry in sdkKeys[] or mobileKeys[] has an empty value — a credential that would be // accepted by relay but can never authenticate any SDK. +// 5. No SDK key survived at all, so the environment would have nothing to authenticate with. A +// payload reaches this by combining an undefined anchor with an sdkKeys[] array that is either +// empty or entirely made up of keys relay excludes, such as keys scoped to a view. // // Validation happens before Reconcile is called; Rotator.Reconcile trusts the set it is handed. // Because the error is raised before any state mutation, the environment's previous accepted set is @@ -86,6 +83,13 @@ func newMissingAnchorError() *MalformedCredentialSetError { return &MalformedCredentialSetError{msg: "malformed credential set: anchor SDK key is missing"} } +// newNoSDKKeysError returns a MalformedCredentialSetError for a set that ended up with no SDK key at +// all. The message describes the payload rather than the builder, because that is what an operator +// reading the log can act on. +func newNoSDKKeysError() *MalformedCredentialSetError { + return &MalformedCredentialSetError{msg: "malformed credential set: no usable SDK key in sdkKeys[]"} +} + // NewAnchorNotInSetError returns a MalformedCredentialSetError for a payload whose designated anchor // (sdkKey.value) is defined but not present in the sdkKeys[] array — a structural inconsistency. The // anchor value is a secret, so it is deliberately not included in the message. diff --git a/internal/credential/accepted_set_builder.go b/internal/credential/accepted_set_builder.go index 47136a8b..d6332749 100644 --- a/internal/credential/accepted_set_builder.go +++ b/internal/credential/accepted_set_builder.go @@ -89,13 +89,13 @@ func (b *AcceptedSetBuilder) WithEnvironmentID(id config.EnvironmentID) *Accepte return b } -// Build validates and returns the accumulated AcceptedSet. It returns errAcceptedSetMissingSDKKey if -// no SDK key was added, or a *MalformedCredentialSetError if no anchor was designated (via +// Build validates and returns the accumulated AcceptedSet. It returns a +// *MalformedCredentialSetError if no SDK key was added, or if no anchor was designated (via // WithAnchor). Because WithAnchor also adds the key, a designated anchor is always among the // accepted SDK keys. func (b *AcceptedSetBuilder) Build() (AcceptedSet, error) { if len(b.set.sdkKeys) == 0 { - return AcceptedSet{}, errAcceptedSetMissingSDKKey + return AcceptedSet{}, newNoSDKKeysError() } if !b.set.anchor.Defined() { return AcceptedSet{}, newMissingAnchorError() diff --git a/internal/credential/accepted_set_builder_test.go b/internal/credential/accepted_set_builder_test.go index 3f89f9c9..3b7b6be4 100644 --- a/internal/credential/accepted_set_builder_test.go +++ b/internal/credential/accepted_set_builder_test.go @@ -10,15 +10,15 @@ import ( ) func TestAcceptedSetBuilderValidation(t *testing.T) { - // No SDK key at all is a caller error. + // No SDK key at all is malformed: the environment would have nothing to authenticate with. + var malformed *MalformedCredentialSetError _, err := NewAcceptedSetBuilder(). WithMobileKey(MobileKeyParams{Value: "mob"}). WithEnvironmentID(config.EnvironmentID("env")). Build() - require.ErrorIs(t, err, errAcceptedSetMissingSDKKey) + require.ErrorAs(t, err, &malformed) // An SDK key with no designated anchor is malformed. - var malformed *MalformedCredentialSetError _, err = NewAcceptedSetBuilder().WithSDKKey(SDKKeyParams{Value: "sdk"}).Build() require.ErrorAs(t, err, &malformed) diff --git a/internal/envfactory/reconcile_helper.go b/internal/envfactory/reconcile_helper.go index c9531024..e71eebf2 100644 --- a/internal/envfactory/reconcile_helper.go +++ b/internal/envfactory/reconcile_helper.go @@ -41,13 +41,13 @@ func collectViewScopedValues[E any, V comparable](entries []E, designated V, get // The builder de-duplicates by value, so an anchor or primary mobile key that also appears in its // array is added only once. // -// A *credential.MalformedCredentialSetError is returned (with an empty AcceptedSet) for a -// structurally malformed payload: an undefined anchor (params.SDKKey not set), a defined anchor that -// is absent from params.AcceptedSDKKeys, a defined primary mobile key (params.MobileKey) that is -// absent from params.AcceptedMobileKeys, a non-empty params.AcceptedMobileKeys with no designated -// primary (params.MobileKey undefined), or an array entry with an empty value. The caller must -// preserve the previous accepted state and, for RAC handlers, reconnect the stream with jitter to -// force a fresh put. This is the single home for the anchor invariant. +// An error is returned (with an empty AcceptedSet) for a structurally malformed payload: an undefined +// anchor (params.SDKKey not set), a defined anchor that is absent from params.AcceptedSDKKeys, a +// defined primary mobile key (params.MobileKey) that is absent from params.AcceptedMobileKeys, a +// non-empty params.AcceptedMobileKeys with no designated primary (params.MobileKey undefined), an +// array entry with an empty value, or no usable SDK key at all. The caller must preserve the previous +// accepted state and, for RAC handlers, reconnect the stream with jitter to force a fresh put. This is +// the single home for the anchor invariant. // // Keys scoped to a view are filtered out here rather than at authentication time, making this the // single funnel for both RAC and the offline archive. The second return value names the keys that were @@ -61,7 +61,7 @@ func BuildAcceptedSet(params EnvironmentParams) (credential.AcceptedSet, []strin // Add every accepted SDK key, designating the anchor as we encounter it. WithAnchor both adds and // designates, and forces the anchor permanent — so a payload that (wrongly) carries an expiry on // the anchor's own entry cannot demote it. An undefined anchor never matches a (defined) array - // value, so it is never designated and Build returns a *MalformedCredentialSetError. + // value, so it is never designated and Build rejects the payload. // // Entries with an empty value are structurally malformed: relay would silently accept them but // they can never authenticate any SDK. Reject loudly rather than produce a credential-short env. diff --git a/internal/envfactory/reconcile_helper_test.go b/internal/envfactory/reconcile_helper_test.go index aed4d8fa..240cbaa9 100644 --- a/internal/envfactory/reconcile_helper_test.go +++ b/internal/envfactory/reconcile_helper_test.go @@ -281,16 +281,32 @@ func TestBuildAcceptedSet_AnchorUndefined(t *testing.T) { assert.Contains(t, malformed.Error(), "anchor SDK key is missing") } -// TestBuildAcceptedSet_NoSDKKeys verifies that when neither an anchor nor any array SDK keys are -// present, Build returns an error (the set has no SDK key at all). +// TestBuildAcceptedSet_NoSDKKeys verifies that when no SDK key survives, the payload is rejected as +// malformed. Two shapes reach this, both requiring an undefined anchor: an empty array, and an array +// whose every entry is filtered out for being scoped to a view. The second is why the case is worth +// pinning by error type — a filtered-to-empty array is a payload problem, not a caller mistake. func TestBuildAcceptedSet_NoSDKKeys(t *testing.T) { - params := EnvironmentParams{ - SDKKey: "", // undefined anchor - AcceptedSDKKeys: []AcceptedSDKKey{}, - AcceptedMobileKeys: []AcceptedMobileKey{}, + tests := []struct { + name string + sdkKeys []AcceptedSDKKey + }{ + {"empty array", []AcceptedSDKKey{}}, + {"every entry view-scoped", []AcceptedSDKKey{{Key: "view-sdk", Value: "sdk-viewy", HasViews: true}}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := EnvironmentParams{ + SDKKey: "", // undefined anchor + AcceptedSDKKeys: tt.sdkKeys, + AcceptedMobileKeys: []AcceptedMobileKey{}, + } + _, _, err := BuildAcceptedSet(params) + + require.Error(t, err, "a set with no SDK key at all must be rejected") + var malformed *credential.MalformedCredentialSetError + require.ErrorAs(t, err, &malformed, "every rejection from BuildAcceptedSet is a malformed-payload error") + }) } - _, _, err := BuildAcceptedSet(params) - require.Error(t, err, "a set with no SDK key at all must be rejected") } // TestBuildAcceptedSet_MixedUpdate verifies add + re-anchor + remove in a single params update diff --git a/relay/filedata_actions.go b/relay/filedata_actions.go index 80194915..27008d73 100644 --- a/relay/filedata_actions.go +++ b/relay/filedata_actions.go @@ -1,11 +1,8 @@ package relay import ( - "errors" "time" - "github.com/launchdarkly/ld-relay/v8/internal/credential" - "github.com/launchdarkly/ld-relay/v8/internal/sdkauth" "github.com/launchdarkly/ld-relay/v8/internal/envfactory" @@ -23,6 +20,8 @@ const ( logMsgOfflineEnvTimeoutError = "Unable to initialize offline environment %q: timed out waiting for client creation" logMsgInternalErrorUpdatedEnvNotFound = "Unexpected error in file data processing: environment ID %s not found when updating" logMsgInternalErrorNoUpdatesForEnv = "Unexpected error in file data processing: environment ID %s not found in envUpdates" + + logMsgOfflineMalformedPayload = "Malformed credential payload for offline environment %q — preserving previous credentials: %s" ) // relayFileDataActions is an implementation of the filedata.UpdateHandler interface. The low-level @@ -60,12 +59,7 @@ func (a *relayFileDataActions) AddEnvironment(ae filedata.ArchiveEnvironment) { set, rejected, buildErr := envfactory.BuildAcceptedSet(ae.Params) if buildErr != nil { - var malformed *credential.MalformedCredentialSetError - if errors.As(buildErr, &malformed) { - a.r.loggers.Errorf("Malformed credential payload for offline environment %q — preserving previous credentials: %s", ae.Params.Identifiers.GetDisplayName(), buildErr) - } else { - a.r.loggers.Errorf(logMsgAutoConfEnvInitError, ae.Params.Identifiers.GetDisplayName(), buildErr) - } + a.r.loggers.Errorf(logMsgOfflineMalformedPayload, ae.Params.Identifiers.GetDisplayName(), buildErr) // No reconnect for offline mode: preserve previous state (env was just created with // the singular sdkKey from envConfig) and wait for the next archive reload. } else { @@ -104,13 +98,7 @@ func (a *relayFileDataActions) UpdateEnvironment(ae filedata.ArchiveEnvironment) set, rejected, buildErr := envfactory.BuildAcceptedSet(ae.Params) if buildErr != nil { - var malformed *credential.MalformedCredentialSetError - if errors.As(buildErr, &malformed) { - a.r.loggers.Errorf("Malformed credential payload for offline environment %q — preserving previous credentials: %s", ae.Params.Identifiers.GetDisplayName(), buildErr) - } else { - // The environment was found above; this is a credential-build failure, not a missing env. - a.r.loggers.Errorf(logMsgAutoConfEnvInitError, ae.Params.Identifiers.GetDisplayName(), buildErr) - } + a.r.loggers.Errorf(logMsgOfflineMalformedPayload, ae.Params.Identifiers.GetDisplayName(), buildErr) // Preserve previous credentials; no reconnect (offline path has no live stream). } else { logViewScopedKeys(a.r.loggers, ae.Params.Identifiers.GetDisplayName(), rejected)