Skip to content

[chore][extension/basicauth] Extract shared basic auth logic into internal package#48414

Open
lazureykis wants to merge 4 commits into
open-telemetry:mainfrom
lazureykis:refactor/basicauth-extract-shared-internals
Open

[chore][extension/basicauth] Extract shared basic auth logic into internal package#48414
lazureykis wants to merge 4 commits into
open-telemetry:mainfrom
lazureykis:refactor/basicauth-extract-shared-internals

Conversation

@lazureykis
Copy link
Copy Markdown
Contributor

@lazureykis lazureykis commented May 15, 2026

Description

Extracts the core basic auth primitives from basicauthextension into a new extension/internal/basicauth package so they can be reused by a standalone AWS Secrets Manager auth extension (see #48277, #48406) without duplicating logic or pulling the AWS SDK into basicauthextension.

The new internal package provides:

  • Authenticate() — server-side header extraction, parsing, and credential validation
  • CredentialProvider interface — abstraction for username/password sourcing
  • NewRoundTripper() — HTTP transport with basic auth injection
  • NewPerRPCCredentials() — gRPC per-RPC credentials with basic auth

The existing basicauthextension is updated to delegate to the internal package. No user-facing behavior changes.

Link to tracking issue

Related to #48277

Testing

  • Comprehensive unit tests in extension/internal/basicauth/basicauth_test.go covering header extraction, parsing, authentication, round-tripper, and per-RPC credentials
  • Existing basicauthextension tests pass unchanged

Documentation

No user-facing changes — internal refactoring only.

…ackage

Move reusable basic auth primitives (header parsing, credential
validation, HTTP RoundTripper, gRPC PerRPCCredentials) into
extension/internal/basicauth so that other extensions needing
basic auth can share them without duplicating code.

Assisted-by: Claude Opus 4.6
Remove AuthDataUsername(), AuthDataPassword(), and EncodeGRPCMetadata()
which had no production callers. Keep the API surface minimal.

Assisted-by: Claude Opus 4.5
Copy link
Copy Markdown
Member

@frzifus frzifus left a comment

Choose a reason for hiding this comment

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

thanks @lazureykis ! Looks already quite good to me. Let me complete the review tomorrow morning!

Comment on lines +25 to +29
// CredentialProvider supplies username and password for client-side basic auth.
type CredentialProvider interface {
Username() string
Password() string
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would that be sufficient for all kinds of vaults?

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.

I think so. It's basic auth, no matter where data comes from, we only need username + password.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was thinking we could even align that and reuse some logic from the bearer token authentication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants