add account id to supproted identity providers#3289
Conversation
| Aws::String m_secretKey; | ||
| Aws::String m_sessionToken; | ||
| Aws::Utils::DateTime m_expiration; | ||
| Aws::String m_accountId; |
There was a problem hiding this comment.
can we make this optional. so that its easy to check validity for the new field. Since this is not a required field
There was a problem hiding this comment.
expiration is optional and we treat it the same way, so i would prefer to treat this the same rather than introduce crt optional to this
| /** | ||
| * Initializes object with accessKeyId, secretKey, sessionToken, expiration date and account Id. | ||
| */ | ||
| AWSCredentials(const Aws::String& accessKeyId, |
There was a problem hiding this comment.
The same needs to be extended to AwsCredentialIdentityBase.h and all smithy creds classes for completion
There was a problem hiding this comment.
we return empty by default in smithy, i dont follow this comment, we only override when it is needed, and it is only needed on AwsCredentialIdentity
src/aws-cpp-sdk-core/include/smithy/identity/identity/AwsCredentialIdentity.h
Show resolved
Hide resolved
| return Aws::Crt::Optional<DateTime>(); | ||
| }; | ||
|
|
||
| virtual Aws::Crt::Optional<Aws::String> accountId() const { |
There was a problem hiding this comment.
why not return by const reference?
There was a problem hiding this comment.
we return expiration by value, why should we make it different for accountId?
There was a problem hiding this comment.
let's return by value,
yes, it is an additional copy, but it saves us from not being able to override this class with some different implementation that computes account id at runtime only and never stores it as a member variable.
sbiscigl
force-pushed
the
provider-account-id
branch
2 times, most recently
from
aa73452 to
e220a46
Compare
sbiscigl
force-pushed
the
provider-account-id
branch
from
e220a46 to
364f412
Compare
src/aws-cpp-sdk-core/include/smithy/identity/identity/AwsCredentialIdentity.h
Outdated
Show resolved
Hide resolved
sbiscigl
force-pushed
the
provider-account-id
branch
from
364f412 to
033416f
Compare
sbiscigl
force-pushed
the
provider-account-id
branch
from
033416f to
4416f82
Compare
3 participants
Description of changes:
Adds the ability for support identity providers to retrieve Account ID when used.
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.