Skip to content

Persistent DCRCredentialStore: wire into EmbeddedAuthServer #5185

@tgrunnagle

Description

@tgrunnagle

Description

Sub-task 3 of 3 for Phase 3 of the DCR story (#4976, parent #4979). Wire the persistent DCRCredentialStore (in-memory from sub-issue 1, Redis from sub-issue 2) into pkg/authserver/runner/embeddedauthserver.go in place of the Phase 2 in-memory stub, and consolidate the runner-side abstraction so there is exactly one DCRCredentialStore implementation per backend. This is the last missing piece for the Datadog-style upstream demo to survive authserver restarts and scale across replicas: restarting (or scaling out to) an authserver backed by Redis reuses already-registered DCR credentials instead of re-registering at the upstream.

Context

Sub-issue 1 lands the storage-level DCRCredentialStore interface and the in-memory implementation. Sub-issue 2 lands the Redis implementation. This sub-issue swaps the runner's wiring so a single storage_type: redis config toggles DCR persistence alongside the rest of authserver state, and removes the Phase 2 standalone in-memory stub now that MemoryStorage itself implements the interface.

Wiring site: createStorage(ctx, cfg.Storage) at pkg/authserver/runner/embeddedauthserver.go:455 selects memory vs. Redis for the main Storage backend; the DCRCredentialStore reuses that same branch via a type assertion so no second selection branch is needed.

This sub-issue introduces no new protocol behavior. It is a wiring change.

Dependencies: sub-issue 1 (types, interface, in-memory implementation), sub-issue 2 (Redis implementation)
Blocks: (none — terminal task in the DAG)

Acceptance Criteria

Wiring — pkg/authserver/runner/embeddedauthserver.go

  • The EmbeddedAuthServer struct field that holds the DCR store (introduced in Authserver DCR integration (Phase 2, Steps 2a-2g) #4978) is now typed against the storage-level DCRCredentialStore interface.
  • NewEmbeddedAuthServer derives the DCR store from the same Storage value returned by createStorage(ctx, cfg.Storage) (see embeddedauthserver.go:97 and :455). Since both MemoryStorage and RedisStorage now implement the storage-level DCRCredentialStore interface, a plain stor.(DCRCredentialStore) type assertion is sufficient. Do NOT add a second storage-selection branch.
  • The Phase 2 standalone in-memory stub from pkg/authserver/runner/dcr_store.go is removed (or reduced to a re-export/alias) so there is exactly one DCRCredentialStore implementation per backend. Document this in the commit message.
  • buildPureOAuth2Config remains pure (unchanged signature, no ctx, no I/O) — the architectural gate established in Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 is preserved.

Cross-cutting

  • task build, task test, task lint-fix, and task license-check all pass.
  • No secrets appear in any log record (grep assertion from Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 applies unchanged: client_secret, registration_access_token, initial_access_token, refresh tokens are never arguments to slog.* calls).
  • Code reviewed and approved.

Technical Approach

Recommended Implementation

The wiring site at embeddedauthserver.go:455 (createStorage) already returns storage.Storage. Since both backends now implement DCRCredentialStore, a single type assertion is all the wiring change needs:

dcrStore, ok := stor.(storage.DCRCredentialStore)
if !ok {
    return nil, fmt.Errorf("storage backend %T does not implement DCRCredentialStore", stor)
}

Pass dcrStore into the EmbeddedAuthServer struct field introduced by #4978; delete the Phase 2 NewInMemoryDCRCredentialStore() fallback construction (or demote it to the no-op default for unit tests only).

buildPureOAuth2Config MUST remain pure — no ctx, no I/O. The DCR store is consumed by the resolver added in #4978; the wiring change only swaps the implementation passed in, not the call shape.

Verify with task build, task test, task lint-fix, task license-check. Run the secret-grep assertion from #4978. Build the whole repo to confirm the runner and storage packages compile together.

Patterns & Frameworks

  • Plain testing + github.com/stretchr/testify per .claude/rules/testing.md — NOT Ginkgo. require.NoError over t.Fatal.
  • Authserver restart integration — extend pkg/authserver/integration_test.go (do NOT create a new integration-test file) to cover the durable-restart case: boot an EmbeddedAuthServer with a mock AS, register, tear down, rebuild against the same MemoryStorage instance, and assert the second boot makes zero AS requests. This is the direct analog of the Phase 2 cache-hit integration assertion from Authserver DCR integration (Phase 2, Steps 2a-2g) #4978, just crossing an EmbeddedAuthServer lifecycle boundary.
  • Always drive builds/tests/lint through task (task build, task test, task lint-fix, task license-check, task gen). Never go test ./... or golangci-lint run directly.
  • Commit-message style: imperative mood, capitalized subject, no trailing period, 50-char limit, no Conventional Commits prefixes (CLAUDE.md).

Code Pointers

  • pkg/authserver/runner/embeddedauthserver.go:97 — call site for createStorage; the storage value returned here is the single source of truth for the DCR backend after this PR.
  • pkg/authserver/runner/embeddedauthserver.go:455-467createStorage backend-selection switch; NO change required here. The DCR store is derived via type assertion on the returned storage.Storage.
  • pkg/authserver/runner/dcr_store.go — Phase 2 standalone stub. Remove or demote to a thin alias.
  • pkg/authserver/integration_test.go — extend with the authserver-restart case; do NOT create a new integration-test file.

Component Interfaces

// pkg/authserver/runner/embeddedauthserver.go (wiring change; sketch)

stor, err := createStorage(ctx, cfg.Storage)
if err != nil {
    return nil, fmt.Errorf("failed to create storage: %w", err)
}

dcrStore, ok := stor.(storage.DCRCredentialStore)
if !ok {
    return nil, fmt.Errorf("storage backend %T does not implement DCRCredentialStore", stor)
}

// existing authserver.New call, now passing dcrStore into the DCR
// resolver as well (field name per #4978, e.g. e.dcrStore = dcrStore).

Testing Strategy

Integration Tests

  • pkg/authserver/integration_test.go (extend, do NOT create a new file): boot an EmbeddedAuthServer with a mock AS serving metadata + /register; trigger DCR during NewEmbeddedAuthServer; close the server; rebuild a second EmbeddedAuthServer against the same MemoryStorage instance (the Redis variant is covered by the Redis integration test in sub-issue 2); assert the second boot makes ZERO HTTP requests to the mock AS — the persisted credential short-circuits the resolver. This is the direct analog of the Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 cache-hit integration case, crossing an EmbeddedAuthServer lifecycle boundary.

Edge Cases

  • Type assertion failure surfaces as a clear NewEmbeddedAuthServer error rather than a nil-pointer panic at first DCR resolve. (If sub-issue 1 chose to keep DCRCredentialStore as a method on every Storage impl, the assertion is unconditional and this case becomes a compile-time guarantee — preferred.)

Out of Scope

  • Types, in-memory backend, and DCRCredentialStore interface — sub-issue 1.
  • Redis backend — sub-issue 2.
  • All other items listed in the parent issue's "Out of Scope" section (auto re-register, RFC 7592 rotation, cross-replica session/token delivery, OTEL metrics, CLI persistence, provider-specific quirks, delete API).

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    authenhancementNew feature or requestgoPull requests that update go code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions