Fix/irsa

[jonathan-mothership]

I wasn't able to get this service to work on EKS with IRSA when the nodes have their own instance profiles (empty policy). The web identity credentials come last in the list and the instance profile was being used ahead of the token file and role passed into containers. The default credential chain for the v1 SDK is listed here: https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/credentials.html

This PR allows a config option of aws_use_irsa that will force the use of the WebIdentityTokenCredentialsProvider. The AWS SDK version was also bumped up.

[jonathan-mothership]

Related thread: aws/aws-sdk-java#2136

[brian-brazil]

Can we not do this without having to add a new config option? Having a config option for every single one of the ever increasing and conflicting number of auth options isn't sane.

[jonathan-mothership]

Yes, there's already a config var for auth, aws_role. FWIW I don't think that one is necessary since the default credential provider can look at the environment and pull AWS_ROLE_ARN from the environment, so not sure why there's an explicit configuration setting for that one.

The config option added by this PR is to opt-int to different behavior than what is provided by the default credential provider chain. In the case when there are two valid credentials available, instance profile and IRSA, the user needs to be able to specify when to use the one that comes later in the chain.

The AWS SDKs are not all the same in behavior. The other providers have swapped the last two in the chain, fixing the issue entirely (web identity comes before instance profile). Compare the last two in the order between the JS SDK: https://github.com/aws/aws-sdk-js/blob/5d8a5ead33431b1303cf63bf01db26c52c28ce69/lib/node_loader.js#L61-L69

and the Java SDK: https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/credentials.html#credentials-default

To address your concern, one possible solution would be to use a AWSCredentialsProviderChain that defines a custom order, favoring the WebIdentity credentials over the instance ones (as is done in the other SDKs). That seemed like too big of a change to step into so I opted for this compromise so that we can start using this project in our EKS clusters.

[jonathan-mothership]

V2 of the SDK has this fixed: aws/aws-sdk-java-v2@18af235

V1 is still stuck in 2016: https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-core/src/main/java/com/amazonaws/auth/DefaultAWSCredentialsProviderChain.java

[brian-brazil]

one possible solution would be to use a AWSCredentialsProviderChain that defines a custom order

Such things in the past have broken other users, I'm extremely hesitant to do anything other than what the SDK provides by default.

I'm not against updating to a newer version of the API.

[lachlancooper]

V2 of the SDK has this fixed: aws/aws-sdk-java-v2@18af235

V1 is still stuck in 2016: aws/aws-sdk-java:aws-java-sdk-core/src/main/java/com/amazonaws/auth/DefaultAWSCredentialsProviderChain.java@master

The credential provider chain in the V1 SDK was also fixed in 1.11.704: aws/aws-sdk-java@1322472. It now matches the priority order of other SDKs. (The docs need an update).

Updating to a newer version in #248 should have been enough to fix this, but I can confirm it's still not working as of 0.8.0. Is there some other change we need to make to fix the priority order here?

Edit: Upon further testing, IRSA does actually work with 0.8.0.

[jonathan-mothership]

The problem stems from this project using a custom clientBuilder flow rather than relying on the default client that uses the default ordering. I think using AmazonCloudWatchClientBuilder.defaultClient() instead of AmazonCloudWatchClientBuilder.standard() would do what you're asking but that would likely break previous configurations that relied on the existing behavior of this project.

This PR only adds very explicit support for IRSA (working in our environment) while leaving the rest of the authentication flow the same. A revamp of the whole auth flow was out of scope for this PR.

[brian-brazil]

I don't think that defaultClient would be appropriate, as per the docs it sounds like it wouldn't support passing in the region from the config file.

I would rather hope that standard and default both use the same chain.

[lachlancooper]

Upon further investigation I can see I was confused and the exporter (0.8.0) is actually working correctly with IRSA. I don't believe any further code change is required here.

[brian-brazil]

Okay, sounds like this can be closed then.