feat: Add HTTP support for External Auth by clayton-gonsalves · Pull Request #4994 · projectcontour/contour

clayton-gonsalves · 2023-01-23T13:44:21Z

This change adds external authorization support for HTTP upstreams.
Fixes: #4954

Signed-off-by: Clayton Gonsalves claytonivorgonsalves@gmail.com

github-actions · 2023-01-23T13:46:34Z

Hi @clayton-gonsalves! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

codecov · 2023-02-14T16:35:41Z

Codecov Report

Merging #4994 (dd6a8b5) into main (9890fe9) will decrease coverage by 0.16%.
The diff coverage is 70.51%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4994      +/-   ##
==========================================
- Coverage   78.02%   77.87%   -0.16%     
==========================================
  Files         138      138              
  Lines       17545    17677     +132     
==========================================
+ Hits        13690    13766      +76     
- Misses       3591     3646      +55     
- Partials      264      265       +1

Impacted Files	Coverage Δ
internal/dag/dag.go	`96.64% <ø> (ø)`
pkg/config/parameters.go	`86.97% <ø> (ø)`
cmd/contour/serve.go	`20.92% <12.16%> (-1.35%)`	⬇️
internal/dag/httpproxy_processor.go	`92.52% <95.65%> (-0.05%)`	⬇️
cmd/contour/servecontext.go	`82.06% <100.00%> (+1.22%)`	⬆️
internal/envoy/v3/listener.go	`98.40% <100.00%> (ø)`
internal/envoy/v3/route.go	`77.27% <100.00%> (-0.07%)`	⬇️
internal/xdscache/v3/listener.go	`91.61% <100.00%> (+0.20%)`	⬆️
internal/xdscache/v3/route.go	`100.00% <100.00%> (ø)`

skriss · 2023-02-22T14:59:21Z

@clayton-gonsalves just wanted to let you know I'm working through review here, but you may not see comments until next week. Thanks for the patience!

clayton-gonsalves · 2023-02-23T11:30:31Z

Thank you, @skriss.
I have started testing this fork already, would be really nice if you could call out any major flaws in my PR. Smaller comments can wait I suppose. Thanks :)

skriss

Thanks again for the PR and the patience @clayton-gonsalves! Overall this looks good; I have some suggestions for simplifying a few things. I may have a few more minor comments down the road but I think this covers the bulk of the bigger stuff. Let me know if anything is unclear or doesn't make sense to you, happy to discuss more.

apis/projectcontour/v1/httpproxy.go

apis/projectcontour/v1/helpers.go

apis/projectcontour/v1alpha1/contourconfig.go

internal/dag/dag.go

internal/dag/httpproxy_processor.go

test/e2e/httpproxy/global_external_auth_test.go

clayton-gonsalves · 2023-03-02T12:18:12Z

@skriss Thanks for the review; I have addressed all but one of the comments left by you. Happy to chat about the test case and how to address it.

skriss

Thanks for the updates @clayton-gonsalves, couple more comments

apis/projectcontour/v1/httpproxy.go

examples/global-external-auth/01-authserver.yaml

test/e2e/httpproxy/httpproxy_test.go

internal/dag/httpproxy_processor.go

site/content/guides/external-authorization.md

changelogs/unreleased/4994-clayton-gonsalves-minor.md

skriss · 2023-03-07T23:48:39Z

@clayton-gonsalves I'm going to take one more pass through this tomorrow but I think it's looking pretty good. @sunjayBhatia will take another look as well. Thanks again for all your work on this!

sunjayBhatia

still running some stuff locally and reading tests but some small changes to make

site/content/docs/main/guides/external-authorization.md

config.yaml

sunjayBhatia

some testing and doc nits but looks great overall!

sunjayBhatia · 2023-03-08T20:51:45Z

site/content/docs/main/guides/external-authorization.md

+
+### Overriding global external authorization for a virtual host
+
+Sometimes you may want a different configuration than what is defined globally. To override the global external authorization, add the `authorization` block to your HTTPProxy as shown below


maybe note here which things can be overridden by HTTP vs HTTPS vhosts (i.e. you can't change auth server on an HTTP vhost)

Modified the heading and added a note at the bottom of the section

internal/xdscache/v3/secret_test.go

test/e2e/deployment.go

internal/xdscache/v3/listener.go

internal/featuretests/v3/global_authorization_test.go

sunjayBhatia · 2023-03-08T21:37:30Z

should be able to merge main/rebase to fix spellcheck

cmd/contour/serve.go

Closes projectcontour#4954. Signed-off-by: claytonig <claytonivorgonsalves@gmail.com> Address review comments Signed-off-by: claytonig <claytonivorgonsalves@gmail.com>

Signed-off-by: claytonig <claytonivorgonsalves@gmail.com>

skriss

This LGTM, thanks for all the work on it @clayton-gonsalves! Will leave for @sunjayBhatia.

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>

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>

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>

clayton-gonsalves mentioned this pull request Jan 23, 2023

Design: ExtAuth HTTP support #4995

Merged

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch from 0cb4efc to 384194a Compare February 1, 2023 16:10

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch from 384194a to 8b98559 Compare February 14, 2023 16:05

clayton-gonsalves changed the title ~~WIP: Add HTTP support for External Auth~~ feat: Add HTTP support for External Auth Feb 14, 2023

clayton-gonsalves marked this pull request as ready for review February 14, 2023 16:07

clayton-gonsalves requested a review from a team as a code owner February 14, 2023 16:07

clayton-gonsalves requested review from skriss and tsaarni and removed request for a team February 14, 2023 16:07

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch from 8b98559 to e68014c Compare February 14, 2023 16:13

sunjayBhatia added the release-note/minor A minor change that needs about a paragraph of explanation in the release notes. label Feb 14, 2023

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch from d5b8150 to 1669f31 Compare February 20, 2023 14:48

skriss requested changes Feb 27, 2023

View reviewed changes

sunjayBhatia self-requested a review February 27, 2023 23:01

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch from 440143e to 383d7e9 Compare March 2, 2023 11:49

clayton-gonsalves requested review from skriss and removed request for sunjayBhatia and tsaarni March 2, 2023 12:18

skriss requested changes Mar 2, 2023

View reviewed changes

apis/projectcontour/v1/httpproxy.go Outdated Show resolved Hide resolved

examples/global-external-auth/01-authserver.yaml Show resolved Hide resolved

examples/global-external-auth/01-authserver.yaml Show resolved Hide resolved

test/e2e/httpproxy/httpproxy_test.go Show resolved Hide resolved

clayton-gonsalves requested a review from skriss March 3, 2023 13:23

skriss requested a review from sunjayBhatia March 3, 2023 19:12

skriss requested changes Mar 6, 2023

View reviewed changes

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch 3 times, most recently from d294d8a to ce2aea7 Compare March 7, 2023 12:45

clayton-gonsalves requested review from skriss and removed request for sunjayBhatia March 7, 2023 18:37

sunjayBhatia self-requested a review March 7, 2023 22:53

sunjayBhatia requested changes Mar 8, 2023

View reviewed changes

site/content/docs/main/guides/external-authorization.md Outdated Show resolved Hide resolved

config.yaml Outdated Show resolved Hide resolved

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch from ce2aea7 to 14084c0 Compare March 8, 2023 18:13

clayton-gonsalves requested review from sunjayBhatia and removed request for skriss March 8, 2023 18:14

skriss mentioned this pull request Mar 8, 2023

fix spelling errors #5154

Merged

sunjayBhatia reviewed Mar 8, 2023

View reviewed changes

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch from 14084c0 to 5a3a6d5 Compare March 10, 2023 14:17

skriss reviewed Mar 10, 2023

View reviewed changes

cmd/contour/serve.go Outdated Show resolved Hide resolved

claytonig added 3 commits March 10, 2023 16:48

feat: Add support for Global External Authorization

9b80efd

Closes projectcontour#4954. Signed-off-by: claytonig <claytonivorgonsalves@gmail.com> Address review comments Signed-off-by: claytonig <claytonivorgonsalves@gmail.com>

address review comments

672dd93

Signed-off-by: claytonig <claytonivorgonsalves@gmail.com>

fix e2e tests

dd6a8b5

Signed-off-by: claytonig <claytonivorgonsalves@gmail.com>

clayton-gonsalves force-pushed the add-support-for-http-ext-auth branch from 5a3a6d5 to dd6a8b5 Compare March 10, 2023 15:48

skriss approved these changes Mar 10, 2023

View reviewed changes

clayton-gonsalves requested a review from sunjayBhatia March 10, 2023 16:27

sunjayBhatia approved these changes Mar 20, 2023

View reviewed changes

skriss merged commit fac6a99 into projectcontour:main Mar 20, 2023

kahirokunn mentioned this pull request Mar 31, 2023

HTTP support for External Auth is not working #5237

Closed

skriss mentioned this pull request Apr 10, 2023

Support autorization for http requests (not only https) #3420

Closed

sunjayBhatia mentioned this pull request Aug 26, 2024

bump controller-runtime and k8s deps #6634

Merged


		### Overriding global external authorization for a virtual host

		Sometimes you may want a different configuration than what is defined globally. To override the global external authorization, add the `authorization` block to your HTTPProxy as shown below

Conversation

clayton-gonsalves commented Jan 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 23, 2023

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

skriss commented Feb 22, 2023

Uh oh!

clayton-gonsalves commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skriss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clayton-gonsalves commented Mar 2, 2023

Uh oh!

skriss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skriss commented Mar 7, 2023

Uh oh!

sunjayBhatia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sunjayBhatia left a comment

Choose a reason for hiding this comment

Uh oh!

sunjayBhatia Mar 8, 2023

Choose a reason for hiding this comment

Uh oh!

clayton-gonsalves Mar 10, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sunjayBhatia commented Mar 8, 2023

Uh oh!

Uh oh!

skriss left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

clayton-gonsalves commented Jan 23, 2023 •

edited

Loading

codecov bot commented Feb 14, 2023 •

edited

Loading

clayton-gonsalves commented Feb 23, 2023 •

edited

Loading