fix: Avoid AssumeRoleWithWebIdentity for each reconcile

[chlunde]

Description of your changes

Use a shared client/credentials to avoid calling AssumeRoleWithWebIdentity for each reconcile.

aws.Config (v1/v2) is thread safe and checks token expiry/handles renewal. To handle

Fixes #828

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Ran for one week. Creating PR for discussion, how can we avoid a global variable?

As you can see below we get a significant reduction in the number of API calls to AWS:

[haarchri]

did we need to adjust also something for assumeRoleARN?

[chlunde]

@haarchri yeah, I think we can refactor it to share a bit more code, but first I wonder if there's a place to store this struct in a "less global" variable.

[chlunde]

@muvaf do you have any idea where we could store this state? Is a global variable like this OK if there are no other options? TAs you can see this should give a performance boost and cost reduction if all AWS security products are enabled (Detective/GuardDuty/CloudTrail).

[haarchri]

tested your PR today in our environment looks really great - if we adopt this after we clearify to save the state for assumeRoleARN would be perfect ;)

[muvaf]

I think this is a very good idea if we have the tools to get the caching right. A few questions:

• Does the token in the config ever expire? How do we know that so that we can request a new one without returning auth error when we shouldn't?
• Are we certain that there can ever be a single identity for a single pod, hence we don't need to use a map? Not really important today, but just checking.

Is a global variable like this OK if there are no other options?

I think that's OK.

[haarchri]

@chlunde any chance to finalize this PR ? i am happy to test this in our environment =)

[github-actions]

Crossplane does not currently have enough maintainers to address every issue and pull request. This pull request has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Adding a comment starting with /fresh will mark this PR as not stale.

[MisterMX]

/fresh

[kaessert]

I rebased changes in this pr and we're running this version now as an experiment. So far no issues and we're seing roughly a 65% drop in AssumeRole requests:

[tkaesserfm]

Version was running in one of our cluster for almost 10 days, looking great. Token is refreshed properly, everything works, no restarts.

@chlunde @haarchri @MisterMX What would be options to proceed here? The change would save a lot some bucks for everyone operating provider-aws on a bigger scale like we do.

I could make a fresh PR with rebased changes and even work on finalizing the PR but i don't want to steal @chlunde the show 😋

[chlunde]

We've been running with this code for 1.5 years without issues, but we only use one provider config per provider. If someone with more complex setups, assuming roles in different accounts, also could verify it would be nice.

[MisterMX]

I made a little improvement by using two separate sync.Mutex for V1 and V2 so they can be accessed in parallel.

I believe there is room for some more improvements by using sync.RWMutex and RLocks for accessing the default config fields. However, this would make the synchronization with writers much more complicated. So I left it out for now.

[MisterMX]

LGTM. Thank you very much @chlunde!