Extended authentication #663
Conversation
AWS ECR key exchange closes #637 Signed-off-by: Chas Honton <[email protected]>
Current coverage is 47.69% (diff: 80.70%)@@ master #663 diff @@
==========================================
Files 119 122 +3
Lines 6065 6267 +202
Methods 0 0
Messages 0 0
Branches 1045 1063 +18
==========================================
+ Hits 2824 2989 +165
- Misses 2992 3016 +24
- Partials 249 262 +13
|
|
Thanks ! |
|
Working on unit tests to extend coverage |
Signed-off-by: Chas Honton <[email protected]>
* Simplified some methods * Added a unit test * Minor tweaks * Only one entry point `createAuthConfig()`
There was a problem hiding this comment.
Looks good to me, with some minor change requests. I submitted a PR to your branch at https://github.com/chonton/docker-maven-plugin/pull/1 (which unfortunately contains also some updates happened on the main branch in the meantime. Also I was not able to update this PR from my branch).
I also added a unit test. It would be awesome if you could some unit tests for AwsSigner4 and EcrExtendedAuth as it seems to be easily testable, but contains some complex logic, too.
| } | ||
| } | ||
|
|
||
| private static void squeezeWhite(StringBuilder dst, String src) { |
There was a problem hiding this comment.
If I understand the method right, this could be replaced by src.replaceAll("\\s+"," ").trim() ?
| } | ||
| StringBuilder builder = unique.get(key); | ||
| if (builder != null) { | ||
| if (builder.length() > 0) { |
There was a problem hiding this comment.
can be simplified by using StringUtils.join()
There was a problem hiding this comment.
refactored this method. complicated by fact it was trying to accomplish two tasks in same loop.
| public AuthConfig createAuthConfig(boolean isPush, boolean skipExtendedAuth, Map authConfig, Settings settings, String user, String registry) | ||
| throws MojoExecutionException { | ||
|
|
||
| AuthConfig ret = createAuthConfig(isPush, authConfig, settings, user, registry); |
There was a problem hiding this comment.
I would rename the original createAuthConfig() to a private method with a distinguished name to clarify that this should be the single entry point.
Signed-off-by: Chas Honton <[email protected]>
Signed-off-by: Chas Honton <[email protected]>
|
Ping. Any additional changes needed? |
|
Just back from holidays, I will have a look today. |
|
[merge] |
AWS ECR key exchange
closes #637
replaces PR #646