Skip to content
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

Revert the creation of Smithy.Identity.Abstractions project and move its interfaces to AWSSDK.Core #3584

Conversation

muhammad-othman
Copy link
Member

@muhammad-othman muhammad-othman commented Dec 19, 2024

Description

  • Removed Smithy.Identity.Abstractions project and update its references to use AWSSDK.Core instead.
  • Moved Smithy.Identity.Abstractions interfaces to AWSSDK.Core
  • Moved DefaultAWSCredentialsIdentityResolver, DefaultAWSCredentialsIdentityResolver and AnonymousIdentityResolver from internal namespace to Amazon.Runtime.
  • Included the generated changes for AutoScaling and S3 only to keep the PR small.

Motivation and Context

  • DOTNET-7844

Testing

  • DRY_RUN-6c97e06c-7f6b-480b-98b0-9cc2369be5c0

* permissions and limitations under the License.
*/

namespace Amazon.Runtime
Copy link
Member

Choose a reason for hiding this comment

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

For these new types have the namespace match the folder structure. So in this case the namespace is Amazon.Runtime.Auth. I know the types in the Credentials folder use the Amazon.Runtime namespace instead of Amazon.Runtime.Credentials but all of the other subnamespaces don't follow that pattern. I can remember why we are inconsistent with Credentials but we should not overload the Amazon.Runtime namespace`.

The other question are we comfortable with these interfaces being "public" to users or should we put them in Amazon.Runtime.Internal.Auth for now? I'm concern we still haven't defined the ISigner interface with it having a big TODO. We have more flexibility if we start with Amazon.Runtime.Internal.Auth.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to follow the same pattern that we used for the credentials, didn't know that it is the exception. Will update it in the next revision.

The reason I made these interfaces public is that some of them are needed for public configurations like ClientConfig.AWSTokenProvider & ClientConfig.IdentityResolverConfiguration or they types of these configurations properties. ISigner would be an exception to this since it is used by the IAuthScheme which shouldn't be created by the users.
What do you think about only moving ISigner&IAuthScheme to an internal namespace since these are the only ones that we don't expect the users to interact with?

Copy link
Member

Choose a reason for hiding this comment

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

If ISigner and IAuthScheme do need to be accessed by users I would move them to the internal namespace. Especially since currently are signers are in the internal namespace. We could potentially move the later when/if we decided to separate signers which is a customer ask.

@boblodgett boblodgett removed their request for review December 20, 2024 18:30
@muhammad-othman muhammad-othman merged commit c7b52b6 into sra-identity-auth Dec 23, 2024
2 checks passed
@muhammad-othman muhammad-othman deleted the muhamoth/DOTNET-7844-sra-remove-SmithyIdentityAbstractions-project branch December 23, 2024 09:36
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