Conversation
Expose on-demand rotation of a connector's service-managed credentials, and surface the two connector fields the operation relies on. - `PaymentClient.rotate_payment_connector_credentials()` wraps the control plane operation, building the `credentialsToRotate` union and validating it against the model's constraints (non-empty, unique, known secret names) before the request goes out. - New `CoinbaseCdpSecret` constant (`API_KEY`, `WALLET_SECRET`) names the rotatable secrets; an enum rather than a bool so new kinds can be added without a breaking change. - Allowlist the operation for boto3 forwarding, so it is also reachable directly. - `get_payment_connector()` and `list_payment_connectors()` now return `provisionMode`, and `get_payment_connector()` returns `credentialsUpdatedAt` when the service supplies it. Both were previously dropped by the hand-built result dicts, so callers had no way to tell whether a connector's credentials are service-managed or how old they are. Rotation applies only to QUICK_CREATE connectors. MANUAL connectors are unaffected: the caller owns those credentials and rotates them with the payment provider directly. Tests - 16 new cases in tests/bedrock_agentcore/payments/test_client.py covering the union builder, enum/string inputs, de-duplication, each validation error, client token handling, ConflictException propagation, and the new response fields. - Full payments suite: 917 passed, 9 skipped; coverage 91%. Ruff lint and format clean.
|
|
||
| if normalized_type != PaymentConnectorType.COINBASE_CDP.value: | ||
| raise ValueError( | ||
| f"Credential rotation is not supported for connector type: '{normalized_type}'. " |
There was a problem hiding this comment.
Do we need to add this check here? Backend level check is good enough, no?
There was a problem hiding this comment.
Agreed, removed in f3e7621. The service is the single source of truth now — all three checks were duplicating constraints the model already enforces (CoinbaseCdpSecret enum, @length(min: 1), @uniqueItems), so a secret name the service adds would have been rejected client-side until the SDK cut a release. Invalid input now surfaces as a ValidationException from the service.
One thing beyond your comment: I removed the connector_type parameter too, not just the check. CredentialRotationConfig models exactly one member (coinbaseCDP), so the parameter was selecting from a set of one — dropping only the if would have silently built a coinbaseCDP payload for anyone passing StripePrivy. The README example never passed it, so nothing user-facing changes.
Left _build_provider_config_input's vendor check alone — that one picks between two genuinely different payload shapes, so an unknown vendor has no correct output.
Addresses review feedback: the SDK was re-implementing constraints the control plane model already enforces, so a secret name the service adds would be rejected client-side until the SDK shipped a new release. Drops from `_build_rotation_config_input`: - the `CoinbaseCdpSecret` allowlist check (service enforces the enum) - the empty-list check (`@length(min: 1)`) - de-duplication (`@uniqueItems`) The builder now only normalizes enum members to their string values and wraps them in the union member. Invalid input surfaces as a `ValidationException` from the service rather than a local `ValueError`. Also removes the `connector_type` parameter rather than just its check. `CredentialRotationConfig` models exactly one member (`coinbaseCDP`), so the parameter selected from a set of one; keeping it without the check would silently build a `coinbaseCDP` payload for a caller who passed `StripePrivy`. The README example never passed it, so this is not a user-facing change. `_build_provider_config_input` keeps its vendor check: that one picks between two genuinely different payload shapes, so an unknown vendor has no correct output. Tests - `TestBuildRotationConfigInput`: 7 cases down to 4, with the two new ones pinning the pass-through contract (duplicates and empty lists go out on the wire unchanged). - `test_rotate_validates_before_calling_service` becomes `test_rotate_defers_validation_to_service`, asserting the service's ClientError propagates and the unvalidated payload was forwarded. - Payments + init + utils suites: 921 passed, 9 skipped. Ruff lint and format clean.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #676 +/- ##
=======================================
Coverage ? 89.55%
=======================================
Files ? 123
Lines ? 10733
Branches ? 1682
=======================================
Hits ? 9612
Misses ? 733
Partials ? 388
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What
Expose on-demand rotation of a payment connector's service-managed credentials, and surface the two connector fields that operation relies on.
PaymentClient.rotate_payment_connector_credentials()wraps the control plane operation. It builds thecredentialsToRotateunion and validates it against the model's constraints — non-emptysecrets,@uniqueItemsde-duplication, known secret names — so bad input fails locally instead of round-tripping.CoinbaseCdpSecretconstant (API_KEY,WALLET_SECRET) names the rotatable secrets. An enum rather than a bool, so new secret kinds can be added without a breaking change._ALLOWED_PAYMENTS_CP_METHODS, so it is also reachable through direct boto3 forwarding.get_payment_connector()andlist_payment_connectors()now returnprovisionMode, andget_payment_connector()returnscredentialsUpdatedAtwhen the service supplies it. Both were previously dropped by the hand-built result dicts, leaving callers no way to tell whether a connector's credentials are service-managed or how old they are.Rotation applies only to connectors with a
provisionModeofQUICK_CREATE, whose credentials the service issued and stores.MANUALconnectors are unaffected — the caller owns those credentials and rotates them with the payment provider directly.StripePrivyis rejected with a clear error, since it has no rotatable service-managed secrets today.The rotation completes before the response returns, so the wrapper has no
wait_for_readyparameter and nothing to poll. Only one rotation runs at a time per connector, so a concurrent call surfacesConflictException.Tests
tests/bedrock_agentcore/payments/test_client.pyacross three classes: the union builder, the client method, and the new response fields. Covers enum and string inputs, de-duplication, each validation error, generated vs. caller-supplied client tokens, validation happening before any request is issued,ConflictExceptionpropagation, andcredentialsUpdatedAtbeing omitted forMANUALconnectors.constants.pyat 100%.ruff checkandruff format --checkclean.