Skip to content

feat: add retry credentials to all modules using binding - #332

Open
NicoleMGomes wants to merge 9 commits into
mainfrom
feat/retry-all-modules
Open

NicoleMGomes wants to merge 9 commits into
mainfrom
feat/retry-all-modules

Conversation

@NicoleMGomes

@NicoleMGomes NicoleMGomes commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Adds binding rotation support to all SDK modules that read credentials from BTP service bindings (volume mounts or environment variables). When a BTP binding rotates at runtime, long-lived clients previously held stale credentials and would fail until the application was restarted. This change makes every module resilient to secret rotation without requiring a restart or client recreation.

Two resilience layers are applied consistently across all affected modules:

  • Proactive: Before every credential use, ConfigFactory.has_changed() checks whether the secret directory mtime has changed. If so, credentials are re-read and any cached tokens or sessions are discarded.
  • Reactive (object store only): On an InvalidAccessKeyId or SignatureDoesNotMatch S3 error, credentials are refreshed and the failed operation is retried once.

Modules updated: objectstore, print, adms, dms, auditlog.

All changes are backward-compatible. The config= explicit parameter path continues to work on all create_client functions; callers using explicit config simply do not get rotation tracking.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring
  • Dependency update

How to Test

  1. Run the unit test suites for all affected modules:
    uv run pytest tests/objectstore/unit/ tests/print/unit/ tests/adms/unit/ tests/dms/unit/ tests/core/unit/auditlog/ -v
  2. Confirm all rotation-specific test classes pass:
    • tests/objectstore/unit/test_s3_client.py::TestRotation
    • tests/print/unit/test_http.py::TestTokenProviderRotation
    • tests/adms/unit/test_ias_fetcher.py::TestIasTokenFetcherRotation
    • tests/dms/unit/test_auth.py::TestAuthRotation
    • tests/core/unit/auditlog/unit/test_http_transport.py::TestHttpTransportRotation
  3. Run pre-commit checks: uvx pre-commit run --all-files
  4. For end-to-end verification: deploy an app on BTP using one of the affected modules, trigger a binding rotation on the service instance, and confirm requests continue to succeed without a restart.

Checklist

  • I have read the Contributing Guidelines
  • I have verified that my changes solve the issue
  • I have added/updated automated tests to cover my changes
  • All tests pass locally
  • I have verified that my code follows the Code Guidelines
  • I have updated documentation (if applicable)
  • I have added type hints for all public APIs
  • My code does not contain sensitive information (credentials, tokens, etc.)
  • I have followed Conventional Commits for commit messages

Additional Notes

The rotation pattern is now documented in docs/GUIDELINES.md under "Credential Binding Rotation" and is enforced by the code review skill. Any future module that reads from service bindings should follow the same pattern: accept a ConfigFactory on the auto-detection path, implement _refresh_if_rotated(), and include the four standard rotation test cases (proactive, reactive, no-rebuild, static config).

Internal note: ObjectStoreClient.__init__ now accepts a ConfigFactory[ObjectStoreBindingData] instead of a plain ObjectStoreBindingData. This is not a public API break since ObjectStoreClient is intended to be constructed via create_client(), but any code directly instantiating ObjectStoreClient will need to wrap the config in a factory callable.

@NicoleMGomes NicoleMGomes changed the title feat: add retry credentials to object store feat: add retry credentials to all modules using binding Sep 14, 2026
@NicoleMGomes
NicoleMGomes marked this pull request as ready for review September 14, 2026 13:34
@NicoleMGomes
NicoleMGomes requested a review from a team as a code owner September 14, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant