Skip to content
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

internal/envoy: use consistent HTTP filter names #6124

Merged
merged 2 commits into from
Jan 29, 2024
Merged

internal/envoy: use consistent HTTP filter names #6124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
025570d
internal/envoy: use consistent HTTP filter names
skriss Jan 26, 2024
7b5a8a4
changelog
skriss Jan 26, 2024
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
1 change: 1 addition & 0 deletions changelogs/unreleased/6124-skriss-small.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Updates HTTP filter names to match between the HTTP connection manager and per-filter config on virtual hosts/routes, and to use canonical names.
33 changes: 23 additions & 10 deletions internal/envoy/v3/listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,19 @@ func Listener(name, address string, port int, perConnectionBufferLimitBytes *uin
return l
}

const (
CORSFilterName string = "envoy.filters.http.cors"
LocalRateLimitFilterName string = "envoy.filters.http.local_ratelimit"
GlobalRateLimitFilterName string = "envoy.filters.http.ratelimit"
RBACFilterName string = "envoy.filters.http.rbac"
ExtAuthzFilterName string = "envoy.filters.http.ext_authz"
JWTAuthnFilterName string = "envoy.filters.http.jwt_authn"
LuaFilterName string = "envoy.filters.http.lua"
CompressorFilterName string = "envoy.filters.http.compressor"
GRPCWebFilterName string = "envoy.filters.http.grpc_web"
GRPCStatsFilterName string = "envoy.filters.http.grpc_stats"
)

type httpConnectionManagerBuilder struct {
routeConfigName string
metricsPrefix string
Expand Down Expand Up @@ -296,7 +309,7 @@ func (b *httpConnectionManagerBuilder) DefaultFilters() *httpConnectionManagerBu
// identified by the TypeURL of each filter.
b.filters = append(b.filters,
&http.HttpFilter{
Name: "compressor",
Name: CompressorFilterName,
Copy link
Member Author

@skriss skriss Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think it should be fine to change the filter names here as they're added to the HCM (making them all match the canonical filter names for consistency), but an alternate option would be to only update the per-filter config within the RouteConfigurations to match these names, without changing them here. Any thoughts?

ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_compressor_v3.Compressor{
CompressorLibrary: &envoy_core_v3.TypedExtensionConfig{
Expand All @@ -323,13 +336,13 @@ func (b *httpConnectionManagerBuilder) DefaultFilters() *httpConnectionManagerBu
},
},
&http.HttpFilter{
Name: "grpcweb",
Name: GRPCWebFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_grpc_web_v3.GrpcWeb{}),
},
},
&http.HttpFilter{
Name: "grpc_stats",
Name: GRPCStatsFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(
&envoy_config_filter_http_grpc_stats_v3.FilterConfig{
Expand All @@ -340,13 +353,13 @@ func (b *httpConnectionManagerBuilder) DefaultFilters() *httpConnectionManagerBu
},
},
&http.HttpFilter{
Name: "cors",
Name: CORSFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_cors_v3.Cors{}),
},
},
&http.HttpFilter{
Name: "local_ratelimit",
Name: LocalRateLimitFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(
&envoy_config_filter_http_local_ratelimit_v3.LocalRateLimit{
Expand All @@ -358,7 +371,7 @@ func (b *httpConnectionManagerBuilder) DefaultFilters() *httpConnectionManagerBu
},
},
&http.HttpFilter{
Name: "envoy.filters.http.lua",
Name: LuaFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&lua.Lua{
DefaultSourceCode: &envoy_core_v3.DataSource{
Expand All @@ -370,7 +383,7 @@ func (b *httpConnectionManagerBuilder) DefaultFilters() *httpConnectionManagerBu
},
},
&http.HttpFilter{
Name: "envoy.filters.http.rbac",
Name: RBACFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_rbac_v3.RBAC{}),
},
Expand Down Expand Up @@ -761,7 +774,7 @@ end
`

return &http.HttpFilter{
Name: "envoy.filters.http.lua",
Name: LuaFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&lua.Lua{
DefaultSourceCode: &envoy_core_v3.DataSource{
Expand Down Expand Up @@ -805,7 +818,7 @@ func FilterExternalAuthz(externalAuthorization *dag.ExternalAuthorization) *http
}

return &http.HttpFilter{
Name: "envoy.filters.http.ext_authz",
Name: ExtAuthzFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&authConfig),
},
Expand Down Expand Up @@ -863,7 +876,7 @@ func FilterJWTAuthN(jwtProviders []dag.JWTProvider) *http.HttpFilter {
}

return &http.HttpFilter{
Name: "envoy.filters.http.jwt_authn",
Name: JWTAuthnFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&jwtConfig),
},
Expand Down
28 changes: 14 additions & 14 deletions internal/envoy/v3/listener_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ func TestDownstreamTLSContext(t *testing.T) {
func TestHTTPConnectionManager(t *testing.T) {
defaultHTTPFilters := []*http.HttpFilter{
{
Name: "compressor",
Name: CompressorFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_compressor_v3.Compressor{
CompressorLibrary: &envoy_core_v3.TypedExtensionConfig{
Expand All @@ -584,12 +584,12 @@ func TestHTTPConnectionManager(t *testing.T) {
}),
},
}, {
Name: "grpcweb",
Name: GRPCWebFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_grpc_web_v3.GrpcWeb{}),
},
}, {
Name: "grpc_stats",
Name: GRPCStatsFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(
&envoy_config_filter_http_grpc_stats_v3.FilterConfig{
Expand All @@ -599,12 +599,12 @@ func TestHTTPConnectionManager(t *testing.T) {
),
},
}, {
Name: "cors",
Name: CORSFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_cors_v3.Cors{}),
},
}, {
Name: "local_ratelimit",
Name: LocalRateLimitFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(
&envoy_config_filter_http_local_ratelimit_v3.LocalRateLimit{
Expand All @@ -613,7 +613,7 @@ func TestHTTPConnectionManager(t *testing.T) {
),
},
}, {
Name: "envoy.filters.http.lua",
Name: LuaFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&lua.Lua{
DefaultSourceCode: &envoy_core_v3.DataSource{
Expand All @@ -624,7 +624,7 @@ func TestHTTPConnectionManager(t *testing.T) {
}),
},
}, {
Name: "envoy.filters.http.rbac",
Name: RBACFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_rbac_v3.RBAC{}),
},
Expand Down Expand Up @@ -1744,20 +1744,20 @@ func TestAddFilter(t *testing.T) {
},
}
grpcWebFilter := &http.HttpFilter{
Name: "grpcweb",
Name: GRPCWebFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_grpc_web_v3.GrpcWeb{}),
},
}
corsFilter := &http.HttpFilter{
Name: "cors",
Name: CORSFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_cors_v3.Cors{}),
},
}

grpcStatsFilter := &http.HttpFilter{
Name: "grpc_stats",
Name: GRPCStatsFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(
&envoy_config_filter_http_grpc_stats_v3.FilterConfig{
Expand All @@ -1768,7 +1768,7 @@ func TestAddFilter(t *testing.T) {
},
}
luaFilter := &http.HttpFilter{
Name: "envoy.filters.http.lua",
Name: LuaFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&lua.Lua{
DefaultSourceCode: &envoy_core_v3.DataSource{
Expand All @@ -1780,14 +1780,14 @@ func TestAddFilter(t *testing.T) {
},
}
rbacFilter := &http.HttpFilter{
Name: "envoy.filters.http.rbac",
Name: RBACFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_rbac_v3.RBAC{}),
},
}

localRateLimitFilter := &http.HttpFilter{
Name: "local_ratelimit",
Name: LocalRateLimitFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(
&envoy_config_filter_http_local_ratelimit_v3.LocalRateLimit{
Expand All @@ -1798,7 +1798,7 @@ func TestAddFilter(t *testing.T) {
}

compressFilter := &http.HttpFilter{
Name: "compressor",
Name: CompressorFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(&envoy_compressor_v3.Compressor{
CompressorLibrary: &envoy_core_v3.TypedExtensionConfig{
Expand Down
22 changes: 11 additions & 11 deletions internal/envoy/v3/route.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@
if evh.TypedPerFilterConfig == nil {
evh.TypedPerFilterConfig = map[string]*anypb.Any{}
}
evh.TypedPerFilterConfig["envoy.filters.http.cors"] = protobuf.MustMarshalAny(corsPolicy(vh.CORSPolicy))
evh.TypedPerFilterConfig[CORSFilterName] = protobuf.MustMarshalAny(corsPolicy(vh.CORSPolicy))
}
if vh.RateLimitPolicy != nil && vh.RateLimitPolicy.Local != nil {
if evh.TypedPerFilterConfig == nil {
evh.TypedPerFilterConfig = map[string]*anypb.Any{}
}
evh.TypedPerFilterConfig["envoy.filters.http.local_ratelimit"] = LocalRateLimitConfig(vh.RateLimitPolicy.Local, "vhost."+vh.Name)
evh.TypedPerFilterConfig[LocalRateLimitFilterName] = LocalRateLimitConfig(vh.RateLimitPolicy.Local, "vhost."+vh.Name)
}

if vh.RateLimitPolicy != nil && vh.RateLimitPolicy.Global != nil {
Expand All @@ -74,7 +74,7 @@
if evh.TypedPerFilterConfig == nil {
evh.TypedPerFilterConfig = map[string]*anypb.Any{}
}
evh.TypedPerFilterConfig["envoy.filters.http.rbac"] = protobuf.MustMarshalAny(
evh.TypedPerFilterConfig[RBACFilterName] = protobuf.MustMarshalAny(
ipFilterConfig(vh.IPFilterAllow, vh.IPFilterRules),
)
}
Expand Down Expand Up @@ -142,32 +142,32 @@

// Apply per-route local rate limit policy.
if dagRoute.RateLimitPolicy != nil && dagRoute.RateLimitPolicy.Local != nil {
route.TypedPerFilterConfig["envoy.filters.http.local_ratelimit"] = LocalRateLimitConfig(dagRoute.RateLimitPolicy.Local, "vhost."+vhostName)
route.TypedPerFilterConfig[LocalRateLimitFilterName] = LocalRateLimitConfig(dagRoute.RateLimitPolicy.Local, "vhost."+vhostName)
}

if dagRoute.RateLimitPerRoute != nil {
route.TypedPerFilterConfig["envoy.filters.http.ratelimit"] = rateLimitPerRoute(dagRoute.RateLimitPerRoute)
route.TypedPerFilterConfig[GlobalRateLimitFilterName] = rateLimitPerRoute(dagRoute.RateLimitPerRoute)

Check warning on line 149 in internal/envoy/v3/route.go

View check run for this annotation

Codecov / codecov/patch

internal/envoy/v3/route.go#L149 

Added line #L149 was not covered by tests
}

// Apply per-route authorization policy modifications.
if dagRoute.AuthDisabled {
route.TypedPerFilterConfig["envoy.filters.http.ext_authz"] = routeAuthzDisabled()
route.TypedPerFilterConfig[ExtAuthzFilterName] = routeAuthzDisabled()
} else if len(dagRoute.AuthContext) > 0 {
route.TypedPerFilterConfig["envoy.filters.http.ext_authz"] = routeAuthzContext(dagRoute.AuthContext)
route.TypedPerFilterConfig[ExtAuthzFilterName] = routeAuthzContext(dagRoute.AuthContext)
}

// If JWT verification is enabled, add per-route filter
// config referencing a requirement in the main filter
// config.
if len(dagRoute.JWTProvider) > 0 {
route.TypedPerFilterConfig["envoy.filters.http.jwt_authn"] = protobuf.MustMarshalAny(&envoy_jwt_v3.PerRouteConfig{
route.TypedPerFilterConfig[JWTAuthnFilterName] = protobuf.MustMarshalAny(&envoy_jwt_v3.PerRouteConfig{
RequirementSpecifier: &envoy_jwt_v3.PerRouteConfig_RequirementName{RequirementName: dagRoute.JWTProvider},
})
}

// If IP filtering is enabled, add per-route filtering
if len(dagRoute.IPFilterRules) > 0 {
route.TypedPerFilterConfig["envoy.filters.http.rbac"] = protobuf.MustMarshalAny(
route.TypedPerFilterConfig[RBACFilterName] = protobuf.MustMarshalAny(
ipFilterConfig(dagRoute.IPFilterAllow, dagRoute.IPFilterRules),
)
}
Expand Down Expand Up @@ -638,7 +638,7 @@
if c.TypedPerFilterConfig == nil {
c.TypedPerFilterConfig = map[string]*anypb.Any{}
}
c.TypedPerFilterConfig["envoy.filters.http.lua"] = cookieRewriteConfig(route.CookieRewritePolicies, cluster.CookieRewritePolicies)
c.TypedPerFilterConfig[LuaFilterName] = cookieRewriteConfig(route.CookieRewritePolicies, cluster.CookieRewritePolicies)

Check warning on line 641 in internal/envoy/v3/route.go

View check run for this annotation

Codecov / codecov/patch

internal/envoy/v3/route.go#L641 

Added line #L641 was not covered by tests
}
wc.Clusters = append(wc.Clusters, c)
}
Expand Down Expand Up @@ -667,7 +667,7 @@
vh := VirtualHost(hostname, routes...)
if corspolicy != nil {
vh.TypedPerFilterConfig = map[string]*anypb.Any{
"envoy.filters.http.cors": protobuf.MustMarshalAny(corspolicy),
CORSFilterName: protobuf.MustMarshalAny(corspolicy),
}
}
return vh
Expand Down
2 changes: 1 addition & 1 deletion internal/envoy/v3/route_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1254,7 +1254,7 @@ func TestCORSVirtualHost(t *testing.T) {
Name: "www.example.com",
Domains: []string{"www.example.com"},
TypedPerFilterConfig: map[string]*anypb.Any{
"envoy.filters.http.cors": protobuf.MustMarshalAny(&envoy_cors_v3.CorsPolicy{
CORSFilterName: protobuf.MustMarshalAny(&envoy_cors_v3.CorsPolicy{
AllowOriginStringMatch: []*matcher.StringMatcher{
{
MatchPattern: &matcher.StringMatcher_Exact{
Expand Down
4 changes: 2 additions & 2 deletions internal/featuretests/v3/authorization_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func authzOverrideDisabled(t *testing.T, rh ResourceEventHandlerWrapper, c *Cont
// same authorization enablement as the root proxy, and
// the other path should have the opposite enablement.

disabledConfig := withFilterConfig("envoy.filters.http.ext_authz",
disabledConfig := withFilterConfig(envoy_v3.ExtAuthzFilterName,
&envoy_config_filter_http_ext_authz_v3.ExtAuthzPerRoute{
Override: &envoy_config_filter_http_ext_authz_v3.ExtAuthzPerRoute_Disabled{
Disabled: true,
Expand Down Expand Up @@ -390,7 +390,7 @@ func authzMergeRouteContext(t *testing.T, rh ResourceEventHandlerWrapper, c *Con
&envoy_route_v3.Route{
Match: routePrefix("/"),
Action: routeCluster("default/app-server/80/da39a3ee5e"),
TypedPerFilterConfig: withFilterConfig("envoy.filters.http.ext_authz",
TypedPerFilterConfig: withFilterConfig(envoy_v3.ExtAuthzFilterName,
&envoy_config_filter_http_ext_authz_v3.ExtAuthzPerRoute{
Override: &envoy_config_filter_http_ext_authz_v3.ExtAuthzPerRoute_CheckSettings{
CheckSettings: &envoy_config_filter_http_ext_authz_v3.CheckSettings{
Expand Down
4 changes: 2 additions & 2 deletions internal/featuretests/v3/envoy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ func authzFilterFor(
AddFilter(envoy_v3.FilterMisdirectedRequests(vhost)).
DefaultFilters().
AddFilter(&http.HttpFilter{
Name: "envoy.filters.http.ext_authz",
Name: envoy_v3.ExtAuthzFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(authz),
},
Expand All @@ -533,7 +533,7 @@ func jwtAuthnFilterFor(
AddFilter(envoy_v3.FilterMisdirectedRequests(vhost)).
DefaultFilters().
AddFilter(&http.HttpFilter{
Name: "envoy.filters.http.jwt_authn",
Name: envoy_v3.JWTAuthnFilterName,
ConfigType: &http.HttpFilter_TypedConfig{
TypedConfig: protobuf.MustMarshalAny(jwt),
},
Expand Down
4 changes: 2 additions & 2 deletions internal/featuretests/v3/global_authorization_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func globalExternalAuthorizationWithMergedAuthPolicy(t *testing.T, rh ResourceEv
Match: routePrefix("/"),
Action: routeCluster("default/s1/80/da39a3ee5e"),
TypedPerFilterConfig: map[string]*anypb.Any{
"envoy.filters.http.ext_authz": protobuf.MustMarshalAny(
envoy_v3.ExtAuthzFilterName: protobuf.MustMarshalAny(
&envoy_config_filter_http_ext_authz_v3.ExtAuthzPerRoute{
Override: &envoy_config_filter_http_ext_authz_v3.ExtAuthzPerRoute_CheckSettings{
CheckSettings: &envoy_config_filter_http_ext_authz_v3.CheckSettings{
Expand Down Expand Up @@ -355,7 +355,7 @@ func globalExternalAuthorizationWithMergedAuthPolicyTLS(t *testing.T, rh Resourc
Match: routePrefix("/"),
Action: routeCluster("default/s1/80/da39a3ee5e"),
TypedPerFilterConfig: map[string]*anypb.Any{
"envoy.filters.http.ext_authz": protobuf.MustMarshalAny(
envoy_v3.ExtAuthzFilterName: protobuf.MustMarshalAny(
&envoy_config_filter_http_ext_authz_v3.ExtAuthzPerRoute{
Override: &envoy_config_filter_http_ext_authz_v3.ExtAuthzPerRoute_CheckSettings{
CheckSettings: &envoy_config_filter_http_ext_authz_v3.CheckSettings{
Expand Down
6 changes: 3 additions & 3 deletions internal/featuretests/v3/ipfilter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestIPFilterPolicy(t *testing.T) {
Match: routePrefix("/"),
Action: routeCluster("default/backend/80/da39a3ee5e"),
},
), withFilterConfig("envoy.filters.http.rbac", &envoy_rbac_v3.RBACPerRoute{Rbac: &envoy_rbac_v3.RBAC{
), withFilterConfig(envoy_v3.RBACFilterName, &envoy_rbac_v3.RBACPerRoute{Rbac: &envoy_rbac_v3.RBAC{
Rules: &envoy_config_rbac_v3.RBAC{
Action: envoy_config_rbac_v3.RBAC_ALLOW,
Policies: map[string]*envoy_config_rbac_v3.Policy{
Expand Down Expand Up @@ -131,7 +131,7 @@ func TestIPFilterPolicy(t *testing.T) {
&envoy_route_v3.Route{
Match: routePrefix("/"),
Action: routeCluster("default/backend/80/da39a3ee5e"),
TypedPerFilterConfig: withFilterConfig("envoy.filters.http.rbac", &envoy_rbac_v3.RBACPerRoute{
TypedPerFilterConfig: withFilterConfig(envoy_v3.RBACFilterName, &envoy_rbac_v3.RBACPerRoute{
Rbac: &envoy_rbac_v3.RBAC{
Rules: &envoy_config_rbac_v3.RBAC{
Action: envoy_config_rbac_v3.RBAC_DENY,
Expand All @@ -156,7 +156,7 @@ func TestIPFilterPolicy(t *testing.T) {
},
}),
},
), withFilterConfig("envoy.filters.http.rbac", &envoy_rbac_v3.RBACPerRoute{Rbac: &envoy_rbac_v3.RBAC{
), withFilterConfig(envoy_v3.RBACFilterName, &envoy_rbac_v3.RBACPerRoute{Rbac: &envoy_rbac_v3.RBAC{
Rules: &envoy_config_rbac_v3.RBAC{
Action: envoy_config_rbac_v3.RBAC_ALLOW,
Policies: map[string]*envoy_config_rbac_v3.Policy{
Expand Down
Loading
Loading