Issue #322: Move AwsKeyManager to its own module

[ALRubinger]

Notes to Reviewers

• Note the ⚠️ below and impact on versioning of web5-kt this carries via an API change.
• Please do not squash merge this PR; rebase merge is fine. We should preserve 1eb180c258d790822e77742b59d3f209dc7d6b3f so that it may be cleanly reverted as part of #328

In this PR

Reviewers, please assess impact of the following changes introduced by this PR:

• Create new module with artifactId: web5-keymanager-aws
• Add this new module to the distribution
• Move AwsKeyManager into it, along with AwsKeyManagerTest
• ⚠️ Refactor package name of AwsKeyManager from web5.sdk.crypto to web5.sdk.keymanager.aws. This represents an API change and in strict terms would trigger a major version bump to a 2.x.y series of web5-kt on the next release.
• Remove the crypto module dependency on com.amazonaws:aws-java-sdk-kms, the motivation behind this change and this PR.
• Add explicit dependencyManagement for commons-codec:commons-codec, needed in test scope by the crypto module which is no longer getting this transitively via com.amazonaws:aws-java-sdk-kms
• Add explicit dependencyManagament for org.apache.httpcomponents:httpcore and explicit declaration on it in dids module, which is no longer receiving this transitively since removing the com.amazonaws:aws-java-sdk-kms via crypto.

[mistermoe]

@ALRubinger lmk what you think about this rationale:

i think we should go with:

Remove the kover-maven-plugin enforcement on this module alone so that the build passes even without coverage (recommend against this)

reason being,

• we're no worse off than we were before
• we benefit from a dependency tree with less depth sooner
• we should definitely create an issue to set up proper tests for AwsKeyManager with a test harness. We can set up localstack for this, but it will definitely take non-negligible time to do.

[ALRubinger]

@mistermoe OKee, short term salve, new issue #328 for medium term proper fix (tests).

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 76.17%. Comparing base (281aeb7) to head (ca0a40a).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #327 +/- ## ========================================== + Coverage 73.60% 76.17% +2.57% ========================================== Files 45 44 -1 Lines 2163 2090 -73 Branches 402 399 -3 ========================================== Hits 1592 1592 + Misses 376 303 -73 Partials 195 195 ``` | [Components](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/327/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | Coverage Δ | | |---|---|---| | [credentials](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/327/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `81.31% <ø> (ø)` | | | [crypto](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/327/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `61.67% <ø> (+12.50%)` | :arrow_up: | | [dids](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/327/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `80.50% <ø> (ø)` | | | [common](https://app.codecov.io/gh/TBD54566975/web5-kt/pull/327/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | `65.21% <ø> (ø)` | |

[michaelneale]

I like this. And I don't think Aws key manager was used in anger just yet? so risk of api change for it is low. With the distribution, does this make pulling in aws stuff optional now?

[ALRubinger]

I like this. And I don't think Aws key manager was used in anger just yet? so risk of api change for it is low. With the distribution, does this make pulling in aws stuff optional now?

1. So I did add this to the distribution pom, so it's included by default when bringing in everything. If we want to treat this as an extension, it doesn't need to be in the distro. What this enables so far is for folks to bring in things piecemeal ie. dids, crypto, etc.

2. Technically it is a published API change which would trigger a major version update, but we could make statements about web5-kt thus far that treat the entire 1.x.y series as open to breaking API changes like this while the project reaches API stability over time. Project leads can make that call, but yeah - as a dev if upgraded within a major version and it broke the API I'd be confused as to why.

[mistermoe]

I think we should exclude it from the default distro @ALRubinger, and treat it as an extension. Using AwsKeyManager is quite the commitment. Removing it from the web5 distro should, I think, make web5-kt default rollup distro usable on android? You and @michaelneale would know much better than I

[michaelneale]

I would be ok with removing it I think as part of distro - if that is ok? and we can document how to bring it in if you need it.

[ALRubinger]

Great; I've got no opinions on it - really is about what the expectations are for the distro. An Android distro would exclude it anyway and add in new Android things.

I just pushed a commit to exclude it for now and we'll have to document alongside DevRel how to add this in when wanting AwsKeyManager support as an extension of sorts. cc: @blackgirlbytes @acekyd @angiejones

[michaelneale]

yeah I think when we are ready to have flavoured distros like android/cloud can look at it then? but for now I think clear docs and examples are great. Anyone doing AWS dev AND using java would have no issues copy/paste into their pom/gradle I think

[tomdaffurn]

Yep, super happy to have AWS as an optional extension. It's far from the common usage, and adds a lot of weight