Skip to content

Adds AuthenticationFilter Support to IR.#813

Merged
danehans merged 3 commits intoenvoyproxy:mainfrom
danehans:authen_ir
Jan 10, 2023
Merged

Adds AuthenticationFilter Support to IR.#813
danehans merged 3 commits intoenvoyproxy:mainfrom
danehans:authen_ir

Conversation

@danehans
Copy link
Copy Markdown
Contributor

@danehans danehans commented Dec 15, 2022

Adds AuthenticationFilter support to IR:

  • api/v1alpha1/validation/authenticationfilter.go: Adds validation package for validating the AuthenticationFilter type.
  • internal/ir/xds.go: Adds internal Request Authentication types with initial support for JWT.

Requires #804

xref: #790

@danehans danehans requested a review from a team as a code owner December 15, 2022 22:53
@danehans danehans added this to the 0.3.0-rc.1 milestone Dec 15, 2022
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.

It might be so, that this call is triggered multiple times for the same authentication filter, as multiple httproutes could use the same authn filter.

If we just store authentication filter NamespacedName as key in resourceMap then after processing all httproutes we can do the getAuthenticationFilter call for unique filters.

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.

Commit 07e3398 resolves this comment. I took a slightly different approach than what you suggested, PTAL.

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.

shouldnt this be per HTTPRoute instead of per HTTPListener

Copy link
Copy Markdown
Contributor Author

@danehans danehans Dec 20, 2022

Choose a reason for hiding this comment

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

JWT is configured as an Envoy listener filter, specifically envoy.filters.http.jwt_authn. The request authentication design provides examples of how the AuthentictionFilter<>HTTPRoutre resources are translated into an xDS config. The authenticationfilter.spec.jwtProviders[] are translated into jwt providers of the xDS IR. The httproute.spec.rules[].matches[] associated to httproute.spec.rules[].filters[] (of type AuthenticationFilter) are translated into jwt matches of the xDS IR. The JWT filter associated to a listener processes the request before routing.

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.

would recommend looking into the route config here https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#config-route-v3-route which allows you to the same thing using the typed_per_filter_config field within Route. This eliminates the need to duplicate the info in httproute.spec.rules[].matches[] since it is already applicable to the route

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.

@arkodg thanks for the review and pointer. Commit cf98e95 updates RequestAuthentication to be a field of HTTPRoute instead of HTTPListener.

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.

thanks for incorporating this, this will continue to keep the mapping of GAPI resources to xDS resources easy to maintain

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 5, 2023

Codecov Report

Merging #813 (f6a5c0f) into main (8dd60b1) will decrease coverage by 0.71%.
The diff coverage is 36.41%.

@@            Coverage Diff             @@
##             main     #813      +/-   ##
==========================================
- Coverage   64.10%   63.38%   -0.72%     
==========================================
  Files          52       53       +1     
  Lines        7170     7330     +160     
==========================================
+ Hits         4596     4646      +50     
- Misses       2290     2396     +106     
- Partials      284      288       +4     
Impacted Files Coverage Δ
internal/ir/zz_generated.deepcopy.go 12.52% <0.00%> (-2.33%) ⬇️
internal/ir/xds.go 76.02% <52.17%> (-2.04%) ⬇️
internal/provider/kubernetes/controller.go 46.74% <66.66%> (+0.09%) ⬆️
internal/provider/kubernetes/routes.go 28.62% <73.68%> (+1.98%) ⬆️
api/v1alpha1/validation/authenticationfilter.go 79.54% <79.54%> (ø)
internal/gatewayapi/helpers.go 78.39% <100.00%> (ø)
internal/provider/kubernetes/helpers.go 77.86% <0.00%> (-1.64%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

arkodg
arkodg previously approved these changes Jan 5, 2023
@danehans
Copy link
Copy Markdown
Contributor Author

danehans commented Jan 6, 2023

@arkodg since this PR depends on and includes #804, can you /approve that PR as well?

Signed-off-by: danehans <daneyonhansen@gmail.com>
Signed-off-by: danehans <daneyonhansen@gmail.com>
Signed-off-by: danehans <daneyonhansen@gmail.com>
@danehans danehans merged commit fb0c155 into envoyproxy:main Jan 10, 2023
@danehans danehans deleted the authen_ir branch January 10, 2023 23:56
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.

4 participants