Page MenuHomePhabricator

Service mesh configuration and network policies for evaluator-orchestrator callbacks
Open, In Progress, MediumPublic

Description

Because the evaluator runs untrusted user code, it currently isn't allowed to open network connections to anywhere. In order to let user code make calls to other wikifunctions, we're now loosening that policy in a limited way. In particular, only a limited set of functions are allowed (none of which in turn are implemented in user code). That restriction is enforced at a few levels:

  1. The orchestrator grows a new HTTP endpoint /v1/callback with the same semantics as the normal evaluation entry point, except that it enforces this function allowlist, along with other limitations on the input format.
  1. The orchestrator's Envoy sidecar grows a second inbound TLS port, which routes the URL /v1/callback to the same local upstream (i.e. the same orchestrator instance) and emits a 403 for everything else.
  1. The evaluator's Envoy sidecar grows a route to the limited port on the orchestrator. (The URL limitation needs to be on the orchestrator side, though, because a compromised evaluator could just skip the local Envoy.)
  1. The evaluator's egress network policy (previously "nothing to nobody") grows a rule allowing traffic to the limited port on the orchestrator.

#1 is another part of the parent task; the rest are in scope here. #2 is a straightforward Envoy listener definition, and #3 and #4 are more interesting.

The evaluator-to-orchestrator callback traffic can't go through ingress; we'd lose port isolation. It also doesn't really need to go through LVS, since it might as well stay intra-DC, and standing up a whole new LVS service to reach the same backend would incur a lot more overhead complexity than we need. My current thinking is for the evaluator Envoy's upstream to be the orchestrator's Kubernetes svc.cluster.local address, which is nonstandard but buys us into just the right level of abstraction here.

That in turn means doing a little config finagling, because right now our whole service mesh setup assumes that Envoy listeners are 1:1 with LVS services. We could extend that at the mesh.configuration level, to add a values switch for intra-cluster traffic, but I don't think that's actually the right call; my instinct is that the reusability of this is pretty low. Instead, we'll inject a minimal services_proxy dict into the evaluator's values, overriding the cluster-wide one from helmfile-defaults, containing the cluster.local address and dedicated port number, but otherwise duplicating the config generated in Puppet. (That introduces some risk of drift from what's in Puppet. It doesn't change often, at least. Someday, this can be a custom codepath in Sophroid keyed on the Node info, which would make it easier to keep things in sync.)

Finally for the network policy, we can add an extraRules the same as the orchestrator's, but pointed back the other way, which should clear us of any DNAT-related issues.

I have a stack of patches in progress for this, will upload today or tomorrow.

Event Timeline

RLazarus changed the task status from Open to In Progress.
RLazarus triaged this task as Medium priority.
RLazarus moved this task from Inbox to In Progress on the ServiceOps new board.

I've reserved port 4974 for this, the next available service port after 4970 (evaluator) and 4971 (orchestrator main port).

Change #1296065 had a related patch set uploaded (by RLazarus; author: RLazarus):

[operations/deployment-charts@master] mesh.networkpolicy: Handle a services_proxy entry with no upstream.ips

https://gerrit.wikimedia.org/r/1296065

Change #1296067 had a related patch set uploaded (by RLazarus; author: RLazarus):

[operations/deployment-charts@master] mesh.configuration: Add restricted_listeners

https://gerrit.wikimedia.org/r/1296067

Change #1296068 had a related patch set uploaded (by RLazarus; author: RLazarus):

[operations/deployment-charts@master] mesh.service: Add TLS service ports for restricted_listeners

https://gerrit.wikimedia.org/r/1296068

Change #1296069 had a related patch set uploaded (by RLazarus; author: RLazarus):

[operations/deployment-charts@master] function-{evaluator,orchestrator}: sextant update mesh modules

https://gerrit.wikimedia.org/r/1296069

Change #1296070 had a related patch set uploaded (by RLazarus; author: RLazarus):

[operations/deployment-charts@master] orchestrator: Add restricted_listeners ports to network egress policy

https://gerrit.wikimedia.org/r/1296070

Change #1296071 had a related patch set uploaded (by RLazarus; author: RLazarus):

[operations/deployment-charts@master] wikifunctions: Add mesh.restricted_listeners port to orchestrator

https://gerrit.wikimedia.org/r/1296071

Change #1296072 had a related patch set uploaded (by RLazarus; author: RLazarus):

[operations/deployment-charts@master] function-evaluator: Add outgoing Envoy config and egress policy for callbacks

https://gerrit.wikimedia.org/r/1296072

Change #1296073 had a related patch set uploaded (by RLazarus; author: RLazarus):

[operations/puppet@production] services_proxy: "Reserve" local port 6520 for wikifunctions orchestrator

https://gerrit.wikimedia.org/r/1296073

Change #1296073 merged by RLazarus:

[operations/puppet@production] services_proxy: "Reserve" local port 6520 for wikifunctions orchestrator

https://gerrit.wikimedia.org/r/1296073

Change #1296070 abandoned by RLazarus:

[operations/deployment-charts@master] orchestrator: Add restricted_listeners ports to network egress policy

Reason:

See discussion on Ibf9f6ca4.

https://gerrit.wikimedia.org/r/1296070

Change #1296067 merged by jenkins-bot:

[operations/deployment-charts@master] mesh.configuration: Add restricted_listeners

https://gerrit.wikimedia.org/r/1296067

Change #1296065 merged by jenkins-bot:

[operations/deployment-charts@master] mesh.networkpolicy: Add ingress ports for restricted_listeners

https://gerrit.wikimedia.org/r/1296065

Change #1296068 merged by jenkins-bot:

[operations/deployment-charts@master] mesh.service: Add TLS service ports for restricted_listeners

https://gerrit.wikimedia.org/r/1296068

Change #1296069 merged by jenkins-bot:

[operations/deployment-charts@master] function-{evaluator,orchestrator}: sextant update mesh modules

https://gerrit.wikimedia.org/r/1296069

Moving to Pending until the app-side work is done on the orchestrator and evaluator, then we can wrap this up (and feel free to assign back to me for the config merge and deploy, if you like!). In order:

  1. Orchestrator starts accepting requests to /v1/callback (Envoy sends everything to the same port from the orchestrator's POV; it's just the new URL endpoint)
  2. Merge https://gerrit.wikimedia.org/r/1296071 to expose the orchestrator callback-only port via the service mesh
  3. Merge https://gerrit.wikimedia.org/r/1296072 to make it available to the evaluator as localhost:6520
  4. Evaluator starts talking to it
Jdforrester-WMF subscribed.

This is waiting for us to be ready to meaningfully test it as part of deploying.