Add expiration to public interface for Credentials (and ReadOnlyCredentials)

[benkehoe]

Re-opening #2391

I have a utility aws-export-credentials to allow AWS credentials to be retrieved in a variety of formats, like as environment variables. Adding this functionality to the AWS CLI is open as https://github.com/aws/aws-cli/issues/4668. The CLI caches credentials from, for example, assume role profiles. This would be nice for the "export credentials" functionality, but the Credentials class and ReadOnlyCredentials do not expose expiration as a public field.

I propose adding expiration as a public field to both Credentials and ReadOnlyCredentials, of type datetime.datetime, with None if there is no known expiration (the default implementation in the base Credentials class). RefreshableCredentials would return its existing _expiry_time.

Acknowledgements

• [X] I may be able to implement this feature request

SDK version used

N/A

Environment details (OS name and version, etc.)

N/A

[tim-finnigan]

Hi @benkehoe thanks for the feature request. I think further discussion is still needed to determine how your utility could be integrated into botocore. We actively mutate the expiry time in some of our providers so we’d need to think through how that could work with what you’re proposing here.

[benkehoe]

Huh, interesting. I don't see where that happens in the code, which provider(s) do that?

[tim-finnigan]

This occurs in utils.py with the IMDS credential provider. IMDS credentials currently recalculate expirations based on heuristics during outage events to improve availability. These are exceptional cases where the end user would be notified, but the system is in place. That behavior may introduce confusion/inconsistencies if we distribute the value in a public interface.

[benkehoe]

Fascinating! If I'm not mistaken, though, even though the fetcher does interesting stuff to calculate an expiration, the IMDS provider still ends up creating a RefreshableCredentials with a static expiration value. If RefreshableCredentials caches the credentials until that expiration value, shouldn't it not be a problem for something else to cache them for the same period?

[tim-finnigan]

We do cache the credentials but part of that involves doing refreshes 15 minutes prior to expiration, and that's when the mechanism referenced earlier kicks in. It calculates a value 20-45 minutes into the future that will be reevaluated in 5-30 minutes. So it's not necessarily valid the full duration after it's been extended.

Once we've entered this state, we're in kind of a weird guessing point. There is no functional system to tell us how long to extend for, so we're just making up numbers. That's why there is concern about exposing this publicly. Because someone may expect that cached value to be true, but we have no way to guarantee that.

[jamesls]

Just following up here, I'd also like to see this added, which would be useful in the PR linked above that I'm trying to drive to completion. The simplest way to address this is to pass this behavior/information back to the clients. We could update ReadOnlyCredentials to support both an expiry_time as well as some fuzzy_expiry boolean (or whatever want to call it). If the latter attribute is true it means we messed with the expiry time and it may or may not be accurate. The consumer then decides how they want to utilize that info.

However, after looking over the changes from #2664, I'm a little concerned we're messing with expiration times. Even though the IMDS fetcher in utils.py isn't really considered a public interface, I think it's in this weird gray area of maybe a breaking change because we're now breaking our (admittedly implicit) contract that credentials are guaranteed to be valid provided we're not at expiry.

In my ideal scenario, without having any idea how realistic it is to implement, the existing logic is moved up a layer into botocore.credentials.InstanceMetadataProvider. This means that the expiry time from IMDS is always exactly whatever the service gives us. The IMDS credential provider then has custom refresh logic that can ignore expiry times and do the jittered refresh, etc. that's in place.

This removes any confusion as well as the need to document these strange edge cases to customers. It's simply: "Credentials are valid provided we're not at creds.expiry_time." The inverse isn't necessarily true (expired credentials can still be valid, we just don't make any claims on validity once we're past expiry).

Do either of these approaches seem reasonable? If so I can start exploring implementation details.

[benkehoe]

I think moving the logic makes the most sense. Given that's a big change, I might propose having an private _fuzzy_expiry that means the expiry_time on the ReadOnlyCredentials will not be set. What would users do with an expiration if they knew it was fuzzy? Better just to leave it as None, I think. Then if/when the logic gets moved, that gets removed and the expiration gets set. I don't think it's a problem to leave the expiration off; there's no expiration for IAM User creds that will be rotated, for example.

[benkehoe]

Worth noting that the AWS CLI command aws configure export-credentials is using the private _expiry_time that may or may not exist, exactly as aws-export-credentials does. https://github.com/aws/aws-cli/blob/b578a39cec15b2c206d83e7575adc05068f0bebe/awscli/customizations/configure/exportcreds.py#L38