CredentialsProvider trait limits usage

[adamnfish]

awscala provides a CredentialsProvider trait that doesn't seem to offer any benefit. It simply extends the Java SDK's own AWSCredentialsProvider interface which provides identical methods.

One problem with this is that the client classes accept an awscala CredentialsProvider in the constructor, rather than Amazon's type. This means it isn't possible to pass e.g. a provider chain to these classes.

Consider the following from here (with imports inlined for clarity):

class STSClient(credentialsProvider: awscala.CredentialsProvider = awscala.CredentialsLoader.load())
  extends aws.AWSSecurityTokenServiceClient(credentialsProvider)
  with awscala.sts.STS

Were the above instead written as follow, it would be possible to provide any valid AWS credentials provider that exists now or in the future.

class STSClient(credentialsProvider: com.amazonaws.auth.AWSCredentialsProvider = awscala.CredentialsLoader.load())
  extends aws.AWSSecurityTokenServiceClient(credentialsProvider)
  with awscala.sts.STS

Note that the default argument remains a valid type here. I've adopted the latter example in my own code so I can pass provider chains.

I'm happy to raise a Pull Request that fixes this but I wanted to first check if there's a reason why this approach was taken. If I've missed something then let me know.

Thanks!

[seratch]

The purpose to have CredentialsProvider is to avoid any effects by AWS SDK's API changes. But I can understand your point. Initially, this library's goal is providing easy-to-use / handy way to deal with AWS SDK on the Scala REPL. If your pull requests meet it, any improvements are welcome.

[adamnfish]

This is still an issue and I'd add that the current approach relies on deprecated constructors in the SDK. The deprecation means that my workaround above no longer works if you want to enable "-Xfatal-warnings" in your build (which is a good best practice).

I can look at putting together a Pull Request that adds the ability to provide a com.amazonaws.auth.AWSCredentialsProvider. This would let people provide whatever valid credentials provider their application needs (including chains). However, I think the current approach of duplicating AWS' classes is a mistake and it might be better to just remove that altogether in a major release. The deprecation of this approach shows that this is not the way AWS expect people to use the client so at best it'll break at some point in the future.

Be good to get a steer from the maintainer(s) on this before I do too much work? Perhaps someone has a change in mind that preserves the existing behaviour while working around the deprecations?

[seratch]

@adamnfish Thank you for your suggestion. I would like to understand your intention correctly.

Do you mean that you're willing to improve CredentialsLoader to hold a singleton AWSCredentialsProvider instance to stop creating a DefaultCredentialsProvider every time calling AWSCredentialsProvider.load()? If so, I agree that it's reasonable.

Otherwise, I would like to know what the possible breaking changes in your mind are. No need to work on the code. Could you show me the pseudo code to use the new APIs in your mind?

[adamnfish]

My goal is to be able to use provider chains to authenticate AWScala's functionality. With that in mind I've created a branch that lets developers use AWS providers to authenticate AWScala clients. There's more detail below about why, but I'll try and outline my thoughts more clearly for you.

---

This library re-implements some of IAM's classes (e.g. Credentials, SessionCredentials, CredentialsProvider). There are a few problems with this approach.

These implementations only accept instances of the other re-implemented classes which means any functionality that hasn't been re-implemented isn't available.

This is the problem that is driving my efforts here. Credentials Provider Chains are a fundamental part of how AWS works and they should be easy to use. The DefaultAWSCredentialsProviderChain is already used in this library's core, which should help reinforce that they are important.

The current approach is deprecated in the 11 branch of the AWS SDK

AWScala already emits deprecation warnings because it depends on deprecated class constructors. AWS are forcing people towards the builder approach and it feels like they are deliberately trying to dissuade people from taking AWSScala's approach of re-implementing features found in the SDK. When the deprecation period ends it'll be impossible to create instances of AWS' classes outside of their own builders.

By creating client classes that extend AWS clients with fewer constructors, this library inherently limits the wealth of functionality that AWS' own clients provide. The deprecations and problems with authenticating calls correctly are areas where this approach isn't ideal but I'd be surprised if there are not others as well.

The re-implementations are not as good as the ones found in the SDK

You mentioned that each call of load creates a new DefaultCredentialsProvider and resolves credentials out of it. Provider Chains are quite complicated and include (for example) internal state for refreshing session tokens as well configuration for whether to cache a successful chain item or re-check the chain every time. The approach of duplicating the existing classes means committing to doing work that doesn't advance the goals of this library as IAM evolves.

Solutions

There are a couple of ways to move forwards.

Allow AWS' own classes to be used directly

I've created a PR that fixes this the easiest way by allowing instances of AWS' IAM classes to be used directly by AWScala. This fixes my problem by letting me provide AWS Provider Chains as the means of authenticating AWScala's wrapped API calls.

Pros:

• backwards-compatible
• very simple change
• fixes the problem I'm trying to solve

Cons:

• still relies on deprecated constructors
• does not address other problems (e.g. reloading the chain on every call)

Implement provider chains in AWScala

This would fix my problem in the short term but doesn't feel super valuable to me.

Pros:

• fixes the problem I'm trying to solve
• backwards compatible

Cons:

• more to maintain in the future
• doesn't address any other problems

Remove the provider stuff from AWScala

This is my preferred approach but it's a lot more work. I'm also sure there must be good reasons why it originally built this way that are only clear to those more familiar with AWScala's source code.

All AWS SDK client classes accept IAM's authentication in any form. It's just as easy to pass hand-made credentials as it is to pass a complex chain of authentication that works across automated environments in highly complex systems. AWScala's clients should work the same way.

The actual details about how this should then be done are what we should discuss if this sounds sensible. This could be implemented as a small change on top of the branch I proposed above. We first allow AWSCredentialsProvider instances to be used in AWScala's classes as is and then gradually remove references to things like BasicCredentialsProvider and DefaultCredentialsProvider in favour of the equivalent features in the AWS SDK. Alternatively, this is an opportunity to look more broadly at how my problem highlights a shortcoming in the design of AWScala.

Would it be best to implicitly extend a properly configured AWS client instead of the current approach of extending clients? Would AWScala's features be more usefully provided as functions that take a client (implicitly or explicitly)? In both approaches, this lets AWScala focus on adding value instead of having to keep up with the AWS clients it extends.

These are just a couple of suggestions off the top of my head, but both would fix all the problems being discussed here. The deprecation issue is resolved as well as (looking ahead) the problems of AWScala not working when the deprecation period ends. It lets developers configure AWS clients the way they want to. They'd then be able to take advantage of existing important functionality like AWS Provider Chains and other features of AWS' clients that aren't exposed by AWScala's constructors. It would also fix performance problems like .load() because AWScala is no longer taking charge of the authentication it can let AWS deal with that the way it prefers.

Pros:

• fixes the problem I'm trying to solve
• fixes related problems others might be having when using more advanced IAM features
• Would be a good opportunity to improve AWScala
• future-proof the library (deprecation)

Cons:

• significant rewrite
• not currently required (my simpler fix above is enough for now, although the deprecation is still a concern)
• I'm probably missing something :-)

AWScala is a really useful tool! I'm coming from a position of "I really want to keep using it but as it stands it doesn't meet requirements" and would like to help solve that in a way that makes it better for everyone.

[seratch]

Thank you for the detailed and clear explanation on this issue. I believe that I understood your points for sure.

First of all, your pull request #181 looks reasonable and I'd love to merge the changes for 0.8.x series. I will cut off 0.7.x branch for 0.7.x series maintenance. Then, I will merge #181 , plus ship a new version including the changes as 0.8.0. Thank you for working on it!

Regarding the long-term proposal, to be honest, I myself don't have enough time to work on the rewriting. But I am fine with the replacement of the extended Scala Credential classes. The intention to have the child classes in Scala world was that I just hesitated to expose the Java SDK classes to AWScala users. I don't have any strong objections towards changing the parts.

[davecromberge]

@seratch do you have any timeframe on when you plan to release 0.8.0? I am about to create a workaround using profile credentials, but will not bother if you plan to release soon. Thanks for providing this library.

[seratch]

@davecromberge I've already published 0.8.0 😄 If you have some idea to add or find any problems with 0.8.0, please let me know. I will work on 0.8.1 release at any time.

[davecromberge]

Thanks @seratch! I will be sure to report any problems should I encounter them 👍