internal/dag: remove Envoy header name quoting

[jpeach]

Remove the '%' quoting on Envoy header names so that operators can
configure Envoy header variables.

This fixes #2516.

Signed-off-by: James Peach jpeach@vmware.com

[jpeach]

@youngnick This is a draft because it's possible to have Envoy reject the config update (e.g. if you use a variable name that isn't known to Envoy). I'll un-draft if we have consensus that we are OK with that.

[jpeach]

xref #1176

[youngnick]

If the update is rejected, do we end up in the state where Envoy will not accept config until a restart?

[jpeach]

If the update is rejected, do we end up in the state where Envoy will not accept config until a restart?

Yes.

[youngnick]

I guess the full questions I have are:

• If someone configures this and gets it wrong, how do they know? Currently, it seems that the only way they will know is when their Envoys stop accepting updates.
• How could we build something to expose the errors to the user without needing to parse all the header values ourselves? I don't see how it's possible, frankly.

My position here is that, if Contour accepts the config, it's Contour's job to surface any problems with the config. With this PR, I don't see how that's possible. That's why we did not allow this level of configurability to begin with.

I appreciate that the functionality is useful, but I think that introducing a feature that may break all the Envoys for all users of Contour because one HTTPProxy has a typo in a header rewrite field is too risky.

[jpeach]

I guess the full questions I have are:

• If someone configures this and gets it wrong, how do they know? Currently, it seems that the only way they will know is when their Envoys stop accepting updates.

Right. There's no way to attribute the error to any specific proxy document.

• How could we build something to expose the errors to the user without needing to parse all the header values ourselves? I don't see how it's possible, frankly.

Right again. The canonical list of header values is in Envoy source code, and it's not readily extractible.

My position here is that, if Contour accepts the config, it's Contour's job to surface any problems with the config. With this PR, I don't see how that's possible. That's why we did not allow this level of configurability to begin with.

I appreciate that the functionality is useful, but I think that introducing a feature that may break all the Envoys for all users of Contour because one HTTPProxy has a typo in a header rewrite field is too risky.

Yep, it's unfortunate but I agree that it makes sense, particularly from a multi-team perspective.