endpoint: Always compute proxy policies after DNS proxy updates

[jrajahalme]

The planned logic was to only update proxy network policy if the added policy map entries included proxy redirections. This design did not consider the possibility a concurrent endpoint policy computation consuming the part of the policy map entries with redirection, which lead to the policy update to be missed after the remaining FQDN selectors were applied.

For example:

1. DNS proxy answer to query 'www.example.com' applies to two FQDN selectors: 'www.example.com' with an L3-only rule, and '*.example.com' with a rule on port 80 for which HTTP policy only allowing GET requests is applied.
2. New local security ID is allocated for the IP in the DNS answer
   3. Endpoint regenerations are spawned due to the new ID (which could also match an CIDR-based identity selector in addition to the FQDN selectors).
3. DNS proxy adds the new security ID to the '.example.com' selector associated with the HTTP rule. 5. Endpoint regeneration consumes the above ID update (from another goroutine). As this update is associated with an HTTP rule, proxy network plicy is recomputed. During this computation the new security ID is only associated with selector '.example.com'.
4. DNS proxy adds the same new security ID to the selector 'www.example.com', which has no HTTP rules.
5. DNS proxy waits for the FQDN selector updates to finish.
6. DNS proxy consumes the computed policy map keys, but only sees the ones for the selector 'www.example.com' as the ones for the selector `*.example.com' were already consumed. Since 'www.selector.com' has no HTTP rules, the proxy network policy update is skipped.
7. DNS proxy responds to the pod with the DNS answer.
8. pod sends a HTTP POST request to the resolved IP on port 80.
9. Traffic to port 80 is redirected to the proxy.
10. Proxy only has the ID associated with the destination IP in the rule that only allows PUT requests and drops the request.
11. Proxy policy is synced by a controller and the proxy policy is healed, so that it is eventyally consistent. This was too late for the traffic the pod was sending.

Fix this by unconditionally computing proxy policy at (8) above.

A more efficient implementation would make sure that all the map updates due to a specific DNS response get to each endpoint's policy map changes as one transaction, so that it is only possible to consume them as one unit. This would make sure that proxy network policy updates are done for all of the updated selectors if any of the them have proxy redirections.

DNS proxy now always updates the proxy policy to avoid intermittent policy drops.

[jrajahalme]

/test

[jrajahalme]

Removed dead code.

[jrajahalme]

/test

[christarazi]

My understand is that this is the core of the "fix". It seems like previously this logic was deliberate, so are we just losing an optimiztion or is there more to it? How can we understand the greater impact of this change?

[tommyp1ckles]

I'd be curious if extra contention on the proxy wait group might have some kind of impact.

[jrajahalme]

When policy update triggered due to the new identity managed to consume the MapChange with proxy redirection, but not the MapChange for the wildcard port rule (no proxy redirection), then this run here would get proxyChanges as false and not update the proxy NetworkPolicy, causing the traffic from the source pod to be dropped until the policy is updated again a git later.

There definitely is room for performance improvement. For example, we could delay the regeneration trigger due to new local identities due to DNS proxy results to be done after the FQDN selectors have already been updated on this code path (one or more callers up the call chain.

[jrajahalme]

Note that the wildcard port rule is the one needed to allow the traffic in the L7 proxy, so it was misguided to prune out the policy update in this case. If all the updates due to the new DNS identity were applied at the same policy update round, one of them would have a proxy port if a proxy network policy update is needed. When the race hit, the map change with the redirection was handled separately from the one without redirection, and we lost the context that the latter change without redirection was also significant for the proxy.

We could also fix this by making sure all the map updates due to a DNS result are applied atomically to the procy network policy, and then we could move back to using the optimization of not updating the proxy network policy on this hot path to DNS response being sent back to the source pod.

[jrajahalme]

/ci-e2e