Add option to pass configmap with mapping between SA and IAM role

[olemarkus]

Description of changes:

This PR adds an option for the webhook to watch a configmap for additional SA to Role mappings. This is useful where there are tooling that creates IAM Roles and already know what SA should use them. In particular kOps already has this mapping. Adding the annotation to the SAs is then just additional manual work.

A few notes:

• There is a number of ways to watch a configmap. I opted for using another informer that is namespaced rather than e.g mounting it on disk and watching the file.
• Previously, all SAs were added to cache regardless of if they had the annotation. This made an empty SA override what was populated from the ConfigMap if the SA was created/changed after the CM was read. I changed this to only add to cache if the annotation is present. I believe this doesn't change much as the webhook need to handle an SA that is not in cache anyway.
• The struct used in the configmap is the same as CacheResponse. This takes an orignally internal struct and makes it into a public interface. Mostly something to be aware of if one wants to change this struct in the future.

/cc @nckturner

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[nckturner]

Previously, all SAs were added to cache regardless of if they had the annotation. This made an empty SA override what was populated from the ConfigMap if the SA was created/changed after the CM was read. I changed this to only add to cache if the annotation is present. I believe this doesn't change much as the webhook need to handle an SA that is not in cache anyway.

What is the behavior if an annotation is removed and the configmap is not being used?

Also is it intended that someone can use both configmap and annotations (I assume so based on the PR). In that case, which takes precedence?

[olemarkus]

Previously, all SAs were added to cache regardless of if they had the annotation. This made an empty SA override what was populated from the ConfigMap if the SA was created/changed after the CM was read. I changed this to only add to cache if the annotation is present. I believe this doesn't change much as the webhook need to handle an SA that is not in cache anyway.

What is the behavior if an annotation is removed and the configmap is not being used?

That's a good catch. The addSA function doesn't consider the new and old SA version, so when the annotation is dropped, it will be dropped from cache as well.

Easiest way to change this is probably that addSAalso takes in the old object and if the old object has the annotation, but the new one doesn't, we pop the SA from cache. This is how I solved an SA being removed from the configmap as well.

Also is it intended that someone can use both configmap and annotations (I assume so based on the PR). In that case, which takes precedence?

Hopefully the two will match. Technically the last one takes precedence, but documentation-wise I would say that if the two differ, the behavior is undocumented. I have no idea what the user intent would be in such a situation either.

I do want to support both configmap and annotation in parallel both to preserve existing configurations, and because annotations offer more flexibility for those who really need to fine-tune the behavior.

[nckturner]

Easiest way to change this is probably that addSAalso takes in the old object and if the old object has the annotation, but the new one doesn't, we pop the SA from cache. This is how I solved an SA being removed from the configmap as well.

Hopefully the two will match. Technically the last one takes precedence, but documentation-wise I would say that if the two differ, the behavior is undocumented. I have no idea what the user intent would be in such a situation either.

I do want to support both configmap and annotation in parallel both to preserve existing configurations, and because annotations offer more flexibility for those who really need to fine-tune the behavior.

Understandable that you would want to support both, but I'm wondering—if we support hybrid environments, what happens if we have a case where an annotation or configmap entry is removed, but the alternative still exists? Even if we look at the old and new object of a certain type, it seems like we can get into the following states:

1. SA annotation removed, configmap still has entry, CacheResponse removed from cache, webhook no longer mutates pods for SA.
2. Configmap entry removed, SA still has annotation, CacheResponse removed from cache, webhook no longer mutates pods for SA.
3. SA not annotated, configmap entry added, CacheResponse added to cache, webhook mutates pods for SA.
4. Configmap does not have entry, SA annotation added, CacheResponse added to cache, webhook mutates pods for SA.

It seems to me that the only difference between 1 and 3 is the order in which prior events took place, and the same is true for 2 and 4. If my understanding is correct, and these would result in different webhook behavior, then I think we would need to solve this before allowing both modes to operate at the same time. I think we would adopt a strict precedence (not call it undefined) like service account always wins, and we might need to cache both types separately so we can see the state of both types when making decisions (service account annotation exists, but configmap doesn't => use mapping from service account annotation).

[olemarkus]

Thanks for the feedback. Had hoped to avoid having two caches, but seems like that is the best way to go about this. Added this now, plus a test that confirms the presedence order is as expected.

[wongma7]

2 suggestions, feel free to implement or ignore, i am fine with merging this as-is

[nckturner]

Thanks for the additional documentation -- one remaining question I have is what is the behavior if the configmap is improperly formatted? The unmarshal should fail, populateCacheFromCM should return an error, but it shouldn't affect the service account informer. Is that correct?