Fix race condition between service account availability and webhook invocation

[roehrijn]

In #174 a race condition is mentioned, where the webhook tries to mutate a Pod having a service account reference which is not yet populated to the ServiceAccountCache. This results in the pod being not mutated.

In our scenario this is a matter of a few milliseconds before the cache is properly populated.

As an alternative to #178, this PR tries to fix this by waiting for a certain amount of time for proper ServiceAccountCache population and being notified upon population. Additionally, to wait only for SAs which aren't in the cache at all and not just missing a RoleARN, I had to restructure the return values of the ServiceAccountCache methods from multiple return values to structured data in order to transport the required information without adding additional two return values.

_{Jan Roehrich [email protected], Mercedes-Benz Tech Innovation GmbH, legal info/Impressum}

[modulitos]

I have some minor questions/suggestions, but the overall approach looks good. I'm happy to work with you on getting this merged 😁

Thank you for the contribution!

[roehrijn]

I have some minor questions/suggestions, but the overall approach looks good. I'm happy to work with you on getting this merged 😁

Thank you for the contribution!

Hi @modulitos. This is good news. I appreciate seeing this eventually to be merged. In my eyes, every single comment of yours seems to be very valid and I'm going to resolve them step by step. I'm also going to add a test which tests exactly the fixed behavior. Currently I actually don't know how long it will take but I keep you posted.

[roehrijn]

@modulitos, I have also added a test which tests the asynchronous cache notification behavior. But now waiting for your thoughts about the open topics from above to not waste time working in the wrong direction.

[modulitos]

Just adding a note here, that an alternative solution for handling cache misses is to fetch and populate the service account from the kubernetes api server. Here's a quick list of tradeoffs from these two approaches:

1. Waiting for cache to populate via a lookup grace period (implemented in this PR):
   pros:

   • grace period gives customer control over the performance tradeoff

   cons:

   • adds complexity to the webhook's cache implementation

2. Fetch and populate cache misses from the api server
   pros:

   • simpler implementation

   cons:

   • Adding an rpc for a cache miss might have perf/reliability risks (are we going to make too many calls? Is GET blocked by some webhooks?)

I'm in favor of proceeding with the lookup grace period in this PR, but open to feedback.

[modulitos]

Great work! Thank you for the contribution. Fixes a bug impacting multiple users, without adding overhead to the API server 🎉