Skip to content

lds: Add listener_filter_chain. #400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 16 commits into from
Jan 23, 2018
Merged

lds: Add listener_filter_chain. #400

Changes from 4 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
02b4a5e
lds: Add listener_filter_chain.
jrajahalme Aug 28, 2017
74a163a
lds: Fix spacing.
jrajahalme Jan 6, 2018
113052e
lds: Rename 'listener_filter_chain' to 'listener_filters'
jrajahalme Jan 10, 2018
071ddd9
Merge branch 'master' into listener-filter-chain
jrajahalme Jan 10, 2018
6f24f4d
lds: link to filter_chains in comment.
jrajahalme Jan 11, 2018
0dd802c
lds: Rephrase the comment on 'listener_filters'
jrajahalme Jan 11, 2018
2ce8e72
lds: Define ListenerFilter message type.
jrajahalme Jan 13, 2018
138d110
Merge branch 'master' into listener-filter-chain
jrajahalme Jan 13, 2018
94773bf
Merge branch 'master' into listener-filter-chain
jrajahalme Jan 16, 2018
fd02c81
docs: Add basic listener filter documentation.
jrajahalme Jan 16, 2018
5f41ccc
Merge branch 'master' into listener-filter-chain
jrajahalme Jan 16, 2018
11d9ab4
docs: Add listener_filters/listener_filters to toctree.
jrajahalme Jan 16, 2018
362031f
docs: Add arch_overview/listener_filters.rst to the toctree.
jrajahalme Jan 16, 2018
a7e6fdf
docs: Remove proxy protocol listener filter from the docs.
jrajahalme Jan 19, 2018
d6f8842
docs: Review fixes.
jrajahalme Jan 22, 2018
e2a201a
lds: Deprecate 'use_original_dst' flag.
jrajahalme Jan 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions api/lds.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ message Listener {

// The type of draining to perform at a listener-wide level.
DrainType drain_type = 8;

// A list of individual listener filters that make up the filter chain for sockets accepted with

@wora wora Jan 11, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This sentence is a bit hard to read. Obviously this is a list of filters. What I want to know when should use use them and what action they would perform? How are they different from other filters.

If I guess correctly, this is a list of filters attached to socket listener. They do something before the listener accepts a connection.

@jrajahalme jrajahalme Jan 11, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Your guess would be almost correct. The rest of the comment states that these filters are run right after the listener accepts a connection (in the sence of the accept() system call).

I've reworded to the comment to not state the obvious and come to the point in the first sentence. Please have a look.

// the listener. These filters are run before any in the 'filter_chains', and these filters have

@htuch htuch Jan 11, 2018

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.

Optional: you could refer to the filter_chains field via:

:ref:`filter_chains <envoy_api_field_Listener.filter_chains>`

// the opportunity to manipulate and augment the connection metadata that is used in connection
// filter chain matching. Order matters as the filters are processed sequentially right after a
// socket has been accepted by the listener, and before a connection is created.
repeated Filter listener_filters = 9;

@ggreenway ggreenway Jan 12, 2018

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.

Since this is a completely different type of filter from network filters, should it use a different type to describe it? Right now they have the same set of data, but possibly in the future they could be different. Prior art: HttpFilter is a different type.

@mattklein123 mattklein123 Jan 12, 2018

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.

+1. I would do a ListenerFilter type just in case for future enhancement.

@jrajahalme jrajahalme Jan 13, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Assuming we don't need the deprecated_v1 stuff, ListenerFilter message now only has the name and config members.

}

message Filter {
Expand Down