Skip to content

docs: document UPSTREAM_METADATA header variable#335

Merged
mattklein123 merged 3 commits intoenvoyproxy:masterfrom
turbinelabs:doc-2148-dynamic-host-headers
Dec 11, 2017
Merged

docs: document UPSTREAM_METADATA header variable#335
mattklein123 merged 3 commits intoenvoyproxy:masterfrom
turbinelabs:doc-2148-dynamic-host-headers

Conversation

@zuercher
Copy link
Copy Markdown
Member

@zuercher zuercher commented Dec 9, 2017

Provides documentation for the changes related to envoyproxy/envoy#2148.

Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
The original protocol which is already added by Envoy as a
:ref:`x-forwarded-proto <config_http_conn_man_headers_x-forwarded-proto>` request header.

%UPSTREAM_METADATA(namespace, key[, key...])%
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of worry about parsing here being a big ugly when it comes to escaping spaces, quotes, dots etc. Do you have any thoughts on how we can specify this to allow cheap parsing? E.g. if the key list is a JSON list, we could reuse the JSON parser, etc.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like:

%UPSTREAM_METADATA("namespace", "foo", "bar")%

and then document that the strings are escaped as in JSON. And in envoy stuff some brackets around it and use the JSON parser?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about %UPSTREAM_METADATA(["namespace", "foo", "bar"])% ? Just actual JSON?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Probably get better error messages that way.

:ref:`x-forwarded-proto <config_http_conn_man_headers_x-forwarded-proto>` request header.

%UPSTREAM_METADATA(namespace, key[, key...])%
Populates the header with ref:`EDS endpoint metadata <envoy_api_file_api/eds.proto>` from the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also have CDS metadata now that is an upstream concept. Do we want to include access to it in this header variable? Would we merge the metadatas together? We had this idea that the LDS/RDS metadata would all be merged before being made available to the user (at the RequestInfo level internally). I don't have a strong opinion on what it should look like, just raising these considerations.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultimately, I think CDS metadata should get merged in, probably with EDS values taking precedence over CDS. It doesn't seem to have been plumbed through to ClusterInfo yet, though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO I would just log a follow up issue for CDS metadata, but I could go either way.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
@mattklein123 mattklein123 merged commit 17d1abe into envoyproxy:master Dec 11, 2017
zuercher added a commit to envoyproxy/envoy that referenced this pull request Dec 13, 2017
Modifies the Router::RequestInfoHeaderFormatter to support extracting string, numeric, and boolean values from the upstream host provided by AccessLog::RequestInfo.

Modifies Router::HeaderParser to omit headers whose formatted value is an empty string.

Docs Changes:
envoyproxy/data-plane-api#335

Fixes #2148.

Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants