Skip to content

Refactors credential identity with separate implementing classes #4024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

cenedhryn
Copy link
Contributor

Motivation and Context

The new AwsCredentialsIdentity and AwsSessionCredentialsIdentity interfaces have inline classes that implement the interface. This PR extracts these inline classes and adds builder support for the identity interfaces in order to support future additional parameters.

@cenedhryn cenedhryn requested a review from a team as a code owner May 18, 2023 20:29
.add("accessKeyId", accessKeyId)
.build();
}
interface Builder {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extends CopyableBuilder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think it was necessarily needed, but we can add it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I played around with different versions of CopyableBuilder, but it doesn't work for the subtypes. If AwsCredentialsIdentity implements ToCopyableBuilder the returned builder will be of type AwsCredentialsIdentity.Builder for AwsSessionCredentialsIdentity too and you'll lose the specific information in the subtype.


@Override
public String toString() {
return ToString.builder("AwsCredentialsIdentity")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AwsSessionCredentialsIdentity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-) yep

Copy link
Contributor

@gosar gosar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG. Thanks for adding tests I didn't initially!

return hashCode;
}

static final class Builder implements AwsCredentialsIdentity.Builder {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like these could be private?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that'd be better

@cenedhryn cenedhryn merged commit 1f2ac6f into feature/master/sra-identity-auth May 19, 2023
@cenedhryn cenedhryn deleted the salande/add-default-credential-identity-classes branch May 19, 2023 17:02
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 4 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 109 Code Smells

65.1% 65.1% Coverage
2.5% 2.5% Duplication

cenedhryn added a commit that referenced this pull request May 23, 2023
…) (#4028)

* refactors credential identity to create separate classes
cenedhryn added a commit that referenced this pull request Apr 17, 2024
…) (#4028)

* refactors credential identity to create separate classes
cenedhryn added a commit that referenced this pull request Apr 22, 2024
…5119)

* Adds account ID as a built-in endpoint parameter (#4016)

* Refactors credential identity with separate implementing classes (#4024) (#4028)

* Adds accountId as a parameter to AWS credentials identity (#4029)

* Adds accountId to STS credentials providers (#4040)

* Adding accountId support to environment variable credential provider (#4327)

* Adds accountId support to process credentials provider (#4332)

* Adds account ID support for profile credentials provider sources (#4340)

* Adds accountId endpoint mode (#4984)

* Adding back existing endpoint files (#5120)

* changelog (#5121)
akidambisrinivasan pushed a commit to akidambisrinivasan/aws-sdk-java-v2 that referenced this pull request Jun 28, 2024
…ws#5119)

* Adds account ID as a built-in endpoint parameter (aws#4016)

* Refactors credential identity with separate implementing classes (aws#4024) (aws#4028)

* Adds accountId as a parameter to AWS credentials identity (aws#4029)

* Adds accountId to STS credentials providers (aws#4040)

* Adding accountId support to environment variable credential provider (aws#4327)

* Adds accountId support to process credentials provider (aws#4332)

* Adds account ID support for profile credentials provider sources (aws#4340)

* Adds accountId endpoint mode (aws#4984)

* Adding back existing endpoint files (aws#5120)

* changelog (aws#5121)
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.

3 participants