refactor DAG and DAG consumers to support >2 Listeners by skriss · Pull Request #5128 · projectcontour/contour

skriss · 2023-02-28T18:41:17Z

Refactors DAG internals to support
any number of Listeners to prepare
for future changes to support many
Gateway Listeners.

Updates #4960.

I think this is in good shape, but as the design doc hasn't been merged yet leaving as draft for now.

codecov · 2023-02-28T18:47:15Z

Codecov Report

Merging #5128 (84d59d7) into main (5cbd2f0) will decrease coverage by 0.08%.
The diff coverage is 93.77%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5128      +/-   ##
==========================================
- Coverage   77.93%   77.86%   -0.08%     
==========================================
  Files         138      138              
  Lines       17747    17757      +10     
==========================================
- Hits        13831    13826       -5     
- Misses       3650     3663      +13     
- Partials      266      268       +2

Impacted Files	Coverage Δ
internal/dag/dag.go	`96.64% <ø> (ø)`
cmd/contour/serve.go	`21.77% <57.14%> (+0.85%)`	⬆️
internal/xdscache/v3/route.go	`96.61% <93.75%> (-3.39%)`	⬇️
internal/xdscache/v3/listener.go	`92.12% <93.87%> (+0.50%)`	⬆️
internal/dag/accessors.go	`85.78% <100.00%> (-12.72%)`	⬇️
internal/dag/builder.go	`92.98% <100.00%> (+7.26%)`	⬆️
internal/dag/gatewayapi_processor.go	`94.91% <100.00%> (ø)`
internal/dag/httpproxy_processor.go	`92.27% <100.00%> (ø)`
internal/dag/ingress_processor.go	`97.07% <100.00%> (ø)`
internal/dag/listener_processor.go	`100.00% <100.00%> (ø)`
... and 1 more

skriss · 2023-03-08T19:53:33Z

Since there's no functional change here, not including a release note. I'll include a comprehensive one with the next PR to support >2 Listeners for Gateways.

Updates projectcontour#4960. Signed-off-by: Steve Kriss <krisss@vmware.com>

sunjayBhatia

this looks great! took a look at this and how its used in your other PR 👍🏽

sunjayBhatia · 2023-03-22T18:42:14Z

internal/dag/accessors.go

-func (d *DAG) GetSecureVirtualHost(hostname string) *SecureVirtualHost {
-	return d.SecureVirtualHosts[hostname]
+func (d *DAG) GetSecureVirtualHost(listener, hostname string) *SecureVirtualHost {
+	return d.Listeners[listener].svhostsByName[hostname]


in the future when we're doing some more dynamic listener additions/removals we may need to be more defensive here? (or make the invariant that the caller is in charge of knowing what valid listeners are so there aren't panics)

Good call, yeah this is one of those things where logically the listener should always be there if we're asking to get/ensure a virtual host for it, but hard to guarantee that, so should probably be defensive.

I'll plan to address this in the next PR when we add the dynamic listener handling

internal/dag/dag.go

Signed-off-by: Steve Kriss <krisss@vmware.com>

…rs-refactor

Signed-off-by: Steve Kriss <krisss@vmware.com>

…r#5128) Updates projectcontour#4960. Signed-off-by: Steve Kriss <krisss@vmware.com>

…r#5128) Updates projectcontour#4960. Signed-off-by: Steve Kriss <krisss@vmware.com> Signed-off-by: yy <yang.yang@daocloud.io>

Signed-off-by: yy <yang.yang@daocloud.io> add some unit test Signed-off-by: yy <yang.yang@daocloud.io> git rebase Signed-off-by: yy <yang.yang@daocloud.io> expose configuration for envoy's RateLimitedAsResourceExhausted (projectcontour#4971) The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change. Closes projectcontour#4901. Signed-off-by: Víctor Roldán Betancort <vroldanbet@authzed.com> Signed-off-by: yy <yang.yang@daocloud.io> rebase Signed-off-by: yy <yang.yang@daocloud.io> update tracing config validate Signed-off-by: yy <yang.yang@daocloud.io> make generate Signed-off-by: yy <yang.yang@daocloud.io> add chengelog Signed-off-by: yy <yang.yang@daocloud.io> update make general Signed-off-by: yy <yang.yang@daocloud.io> goimport Signed-off-by: yy <yang.yang@daocloud.io> update tracing Signed-off-by: yy <yang.yang@daocloud.io> fix golint Signed-off-by: yy <yang.yang@daocloud.io> update test Signed-off-by: yy <yang.yang@daocloud.io> delete unused code Signed-off-by: yy <yang.yang@daocloud.io> delete error file Signed-off-by: yy <yang.yang@daocloud.io> update changelog Signed-off-by: yy <yang.yang@daocloud.io> fix some mistake Signed-off-by: yy <yang.yang@daocloud.io> feat: Add HTTP support for External Auth (projectcontour#4994) Support globally configuring an external auth server which is enabled by default for all vhosts, both HTTP and HTTPS. Closes projectcontour#4954. Signed-off-by: claytonig <claytonivorgonsalves@gmail.com> Signed-off-by: yy <yang.yang@daocloud.io> refactor DAG and DAG consumers to support >2 Listeners (projectcontour#5128) Updates projectcontour#4960. Signed-off-by: Steve Kriss <krisss@vmware.com> Signed-off-by: yy <yang.yang@daocloud.io> resolve conflict Signed-off-by: yy <yang.yang@daocloud.io> fix Signed-off-by: yy <yang.yang@daocloud.io>

skriss force-pushed the pr-multiple-listeners-refactor branch from 2afed48 to 4cf3361 Compare February 28, 2023 18:52

skriss marked this pull request as ready for review March 8, 2023 19:47

skriss requested a review from a team as a code owner March 8, 2023 19:47

skriss requested review from stevesloka, sunjayBhatia and tsaarni and removed request for a team March 8, 2023 19:47

skriss force-pushed the pr-multiple-listeners-refactor branch from 4cf3361 to a17350f Compare March 8, 2023 19:48

skriss added the release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes. label Mar 8, 2023

skriss force-pushed the pr-multiple-listeners-refactor branch from a17350f to 78a040d Compare March 8, 2023 20:12

skriss mentioned this pull request Mar 9, 2023

support many Gateway Listeners #5160

Merged

skriss force-pushed the pr-multiple-listeners-refactor branch from 78a040d to 07c03b8 Compare March 20, 2023 18:24

refactor DAG and DAG consumers to support >2 Listeners

ccfd5c7

Updates projectcontour#4960. Signed-off-by: Steve Kriss <krisss@vmware.com>

skriss force-pushed the pr-multiple-listeners-refactor branch from 07c03b8 to ccfd5c7 Compare March 20, 2023 22:21

sunjayBhatia approved these changes Mar 22, 2023

View reviewed changes

skriss added 3 commits March 22, 2023 13:52

code review

ff9cd12

Signed-off-by: Steve Kriss <krisss@vmware.com>

Merge remote-tracking branch 'upstream/main' into pr-multiple-listene…

d190444

…rs-refactor

fix test

84d59d7

Signed-off-by: Steve Kriss <krisss@vmware.com>

sunjayBhatia approved these changes Mar 22, 2023

View reviewed changes

skriss merged commit 2322ca6 into projectcontour:main Mar 22, 2023

yangyy93 pushed a commit to projectsesame/contour that referenced this pull request Mar 27, 2023

refactor DAG and DAG consumers to support >2 Listeners (projectcontou…

13e334b

…r#5128) Updates projectcontour#4960. Signed-off-by: Steve Kriss <krisss@vmware.com>

skriss deleted the pr-multiple-listeners-refactor branch May 25, 2023 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor DAG and DAG consumers to support >2 Listeners#5128

refactor DAG and DAG consumers to support >2 Listeners#5128
skriss merged 4 commits intoprojectcontour:mainfrom
skriss:pr-multiple-listeners-refactor

skriss commented Feb 28, 2023 •

edited

Loading

Uh oh!

codecov bot commented Feb 28, 2023 •

edited

Loading

Uh oh!

skriss commented Mar 8, 2023

Uh oh!

sunjayBhatia left a comment

Uh oh!

sunjayBhatia Mar 22, 2023

Uh oh!

skriss Mar 22, 2023

Uh oh!

skriss Mar 22, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

skriss commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

skriss commented Mar 8, 2023

Uh oh!

sunjayBhatia left a comment

Choose a reason for hiding this comment

Uh oh!

sunjayBhatia Mar 22, 2023

Choose a reason for hiding this comment

Uh oh!

skriss Mar 22, 2023

Choose a reason for hiding this comment

Uh oh!

skriss Mar 22, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

skriss commented Feb 28, 2023 •

edited

Loading

codecov bot commented Feb 28, 2023 •

edited

Loading