Skip to content

Commit

Permalink
add additional context and flavor
Browse files Browse the repository at this point in the history
  • Loading branch information
aajtodd committed Apr 2, 2024
1 parent ede5e86 commit e63895f
Showing 1 changed file with 64 additions and 1 deletion.
65 changes: 64 additions & 1 deletion design/src/rfcs/rfc0041_identity_cache_partitions.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,33 @@ a shared config instance will end up with their own identity cache which also re
again. Only if a user supplies an identity cache explicitly when creating shared config would it be re-used across
different clients.

### Design intent

Identity providers and identity caching are intentionally decoupled. This allows caching behavior to be more easily
customized and centrally configured while also removing the need for each identity provider to have to implement
caching. There is some fallout from sharing an identity cache though. This is fairly well documented on
`IdentityCachePartition` itself.

```rust,ignore
/// ...
///
/// Identities need cache partitioning because a single identity cache is used across
/// multiple identity providers across multiple auth schemes. In addition, a single auth scheme
/// may have many different identity providers due to operation-level config overrides.
///
/// ...
pub struct IdentityCachePartition(...)
```

Cache partitioning allows for different identity types to be stored in the same cache instance as long as they
are assigned to a different partition. Partitioning also solves the issue of overriding configuration on a per operation
basis where it would not be the correct or desired behavior to re-use or overwrite the cache if a different resolver
is used.

In other words cache partitioning is effectively tied to a particular instance of an identity resolver. Re-using the
same instance of a resolver _SHOULD_ be allowed to share a cache partition. The fact that this isn't the case
today is an oversight in how types are wrapped and threaded through the SDK.


The user experience if this RFC is implemented
----------------------------------------------
Expand Down Expand Up @@ -160,8 +187,11 @@ cache partition.
```rust,ignore
let scoped_creds = SharedCredentialsProvider::new(my_custom_provider());
let config_override = c1
.config()
.to_builder()
.set_credentials_provider(Some(scoped_creds));
...
```


Expand Down Expand Up @@ -220,11 +250,44 @@ impl ResolveIdentity for SharedCredentialsProvider {
Additionally a new behavior version must be introduced that conditionally creates a default `IdentityCache` on `SdkConfig`
if not explicitly configured (similar to how credentials provider works internally).

Alternatives Considered
-----------------------

`SdkConfig` [internally](https://github.com/smithy-lang/smithy-rs/blob/release-2024-03-25/aws/rust-runtime/aws-types/src/sdk_config.rs#L58-L59)
stores `SharedCredentialsProvider`/`SharedTokenProvider`. Neither of these types knows anything about cache partitioning.
One alternative would be to create and store a `SharedIdentityResolver` for each identity resolver type.

```rust,ignore
pub struct SdkConfig {
...
credentials_provider: Option<SharedCredentialsProvider>,
credentials_identity_provider: Option<SharedIdentityResolver>,
token_provider: Option<SharedTokenProvider>,
token_identity_provider: Option<SharedIdentityResolver>,
}
```

Setting one of the identity resolver types like `credentials_provider` would also create and set the equivalent
`SharedIdentityResolver` which would claim a cache partition. When generating the `From<SdkConfig>` implementations
the identity resolver type would be favored.

There are a few downsides to this approach:

1. `SdkConfig` would have to expose accessor methods for the equivalents
(e.g. `credentials_identity_provider(&self) -> Option<&SharedIdentityResolver>`). This creates additional noise
and confusion as well as the chance for using the type wrong.
2. Every new identity type added to `SdkConfig` would have to be sure to use `SharedIdentityResolver`.

The advantage of the proposed approach of letting `ResolveIdentity` implementations provide a cache partition means
`SdkConfig` does not need to change. It also gives customers more control over whether an identity resolver implementation
shares a cache partition or not.

Changes checklist
-----------------

- [ ] Add new `cache_partition()` method to `ResolveIdentity`
- [ ] Update `SharedIdentityResolver::new` to use the new `cache_partition()` method on the `resolver` to determine if a new cache partition should be created or not
- [ ] Claim a cache partition when `SharedCredentialsProvider` is created and override the new `ResolveIdentity` method
- [ ] Claim a cache partition when `SharedTokenProvider` is created and override the new `ResolveIdentity` method
- [ ] Introduce new behavior version
- [ ] Conditionally (gated on behavior version) create a new default `IdentityCache` on `SdkConfig` if not explicitly configured

0 comments on commit e63895f

Please sign in to comment.