Support IAM Task Roles

[gsaslis]

[NEW PR in favour of closed #13 due to deleted branch]

Moved test to same package as tested class

Added new Environment class

This is a simple wrapper for System.getenv calls, which we're only using for facilitating mocking in Unit Tests. This way, by mocking env var values, we can test the various scenarios of how credentials should be retrieved, depending on different configurations.

[WIP] Moved business logic of looking up creds from IAM Role in separate method

Also added a new method getKeysFromIamTaskRole(env) that is responsible for checking if it can retrieve the keys in case neither an access key nor an IAM role have been defined in the config. To do this, it looks up the special environment variable provided by the ECS agent, when running in an ECS node.

Changed JSON parsing from manual pattern matching to using com.hazelcast.com.eclipsesource.json.JsonObject

Since we have this utility class anyway, it makes little sense to me why we should keep here custom business logic about how we should parse the AWS responses.
What's more, the response for IAM task role does not contain new lines, or spaces, whereas the response for IAM role does. This meant we needed different pattern matching to support both and using the JsonObject seems like a nice simplification here.

The parseIamRole method should probably now be removed, but since it's a public method and I didn't want to make any breaking changes to the public API of the library, I just added a @Deprecated annotation there, since it's no longer being used by this class.

Added a further constructor to facilitate with unit tests.

Removed checks for what AWS config is valid

The logic of what configuration is valid (access + secret key vs. iam role vs. neither, which means IAM task role, if env var is set) is all implemented in the DescribeInstances class, so if we check these here too, it will mean duplication of the validation logic. Leave it up to DescribeInstances and its tests to verify that.

Changed test to correctly describe test case,

Now that we also check for env var (in case access key and iam role have not been defined), we might have a valid case when both are null.

Added test that checks the IAM role configuration case

Check that when iam role has been set and no access key has been set, we can correctly retrieve and parse the credentials.
In order to keep this as a Unit Test, we needed to mock out the HTTP calls, as well as their responses, so this test only covers the business logic on the library side. The UT does not cover testing that the EC2 instance actually does have the IAM role (nor should it, imho).

Added test that checks the IAM Task role

Check that when NO IAM role or Access Key have been set in the config, we can correctly retrieve and parse the credentials IF the appropriate env var has been set by the ECS agent.
In order to keep this as a Unit Test, we needed to mock out the System.getenv calls, which we did through the usage of the Environment wrapper class.

Removed tests for AWS config validation

This should go together with commit bd63e97.

Minor improvements to remove a few warnings

Added test case for when both access key and iam role have been defined

(it is an invalid configuration)

Changed constructor used for tests to behave more like real one

Improved documentation of retrieveRoleFromURI

Refactored tryGetDefaultIamRole

Now using helper retrieveRoleFromURI, so we can isolate network usage in our tests.
It's also simpler to read now. ;)

Changed policy on which we look up default IAM role.

We now attempt to look it up when either of the below holds true:

• <iam-role>DEFAULT</iam-role>
• <iam-role></iam-role>
• NO <iam-role> has been defined

Fixed checkstyle errors

[mesutcelik]

verify

[devOpsHazelcast]

Test PASSed.

[mesutcelik]

Thanks a lot for the contribution @gsaslis ! 👏 👏

[gsaslis]

And thank you for all the help!
Also a big thanks to @JChrist for his help.

On Thursday, 27 October 2016, Mesut Celik notifications@github.com wrote:

Thanks a lot for the contribution @gsaslis https://github.com/gsaslis !
👏 👏

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#14 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACTwktLhtVI3bowBOeslTMne5IvyYn4pks5q4HjxgaJpZM4KiJE4
.

Yorgos Saslis
Software Engineer