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

Backport of [NET-6305] xds: Ensure v2 route match and protocol are populated for gRPC into release/1.17.x #19366

Merged
merged 2 commits into from
Oct 25, 2023
+1,631 −1,141
Merged

Backport of [NET-6305] xds: Ensure v2 route match and protocol are populated for gRPC into release/1.17.x #19366

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
229f705
backport of commit 0a8fe0d94fe3c976ab1ecd871af1f6744805b226
zalimeni Oct 23, 2023
8572f73
backport of commit 74bf2aa8b0bd3a5f6c9d9758223c6f1bed949745
zalimeni Oct 23, 2023
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
18 changes: 14 additions & 4 deletions agent/xds/proxystateconverter/clusters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ func (s *Converter) makeAppCluster(cfgSnap *proxycfg.ConfigSnapshot, name, pathP
if protocol == "" {
protocol = cfg.Protocol
}
namedCluster.cluster.Protocol = protocol
namedCluster.cluster.Protocol = protocolMap[protocol]
if cfg.MaxInboundConnections > 0 {
namedCluster.cluster.GetEndpointGroup().GetStatic().GetConfig().
CircuitBreakers = &pbproxystate.CircuitBreakers{
Expand Down Expand Up @@ -646,7 +646,7 @@ func (s *Converter) makeUpstreamClusterForPreparedQuery(upstream structs.Upstrea

if c == nil {
c = &pbproxystate.Cluster{
Protocol: cfg.Protocol,
Protocol: protocolMap[cfg.Protocol],
Group: &pbproxystate.Cluster_EndpointGroup{
EndpointGroup: &pbproxystate.EndpointGroup{
Group: &pbproxystate.EndpointGroup_Dynamic{
Expand Down Expand Up @@ -932,7 +932,7 @@ func (s *Converter) makeUpstreamClustersForDiscoveryChain(
} else {
cluster := &pbproxystate.Cluster{
AltStatName: mappedTargets.baseClusterName,
Protocol: upstreamConfig.Protocol,
Protocol: protocolMap[upstreamConfig.Protocol],
Group: &pbproxystate.Cluster_EndpointGroup{
EndpointGroup: &pbproxystate.EndpointGroup{
Group: &pbproxystate.EndpointGroup_Dynamic{
Expand All @@ -952,7 +952,7 @@ func (s *Converter) makeUpstreamClustersForDiscoveryChain(
failoverGroup.EndpointGroups = endpointGroups
cluster := &pbproxystate.Cluster{
AltStatName: mappedTargets.baseClusterName,
Protocol: upstreamConfig.Protocol,
Protocol: protocolMap[upstreamConfig.Protocol],
Group: &pbproxystate.Cluster_FailoverGroup{
FailoverGroup: failoverGroup,
},
Expand Down Expand Up @@ -1251,3 +1251,13 @@ func makeOutlierDetection(p *structs.PassiveHealthCheck, override *structs.Passi

return od
}

// protocolMap converts config entry protocols to proxystate protocol values.
// As documented on config entry protos, the valid values are "tcp", "http",
// "http2" and "grpc". Anything else is treated as tcp.
var protocolMap = map[string]pbproxystate.Protocol{
"http": pbproxystate.Protocol_PROTOCOL_HTTP,
"http2": pbproxystate.Protocol_PROTOCOL_HTTP2,
"grpc": pbproxystate.Protocol_PROTOCOL_GRPC,
"tcp": pbproxystate.Protocol_PROTOCOL_TCP,
}
20 changes: 10 additions & 10 deletions agent/xdsv2/cluster_resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (pr *ProxyResources) makeClusters(name string) ([]proto.Message, error) {
return clusters, nil
}

func (pr *ProxyResources) makeEnvoyCluster(name string, protocol string, eg *pbproxystate.EndpointGroup) (*envoy_cluster_v3.Cluster, error) {
func (pr *ProxyResources) makeEnvoyCluster(name string, protocol pbproxystate.Protocol, eg *pbproxystate.EndpointGroup) (*envoy_cluster_v3.Cluster, error) {
if eg != nil {
switch t := eg.Group.(type) {
case *pbproxystate.EndpointGroup_Dynamic:
Expand All @@ -103,7 +103,7 @@ func (pr *ProxyResources) makeEnvoyCluster(name string, protocol string, eg *pbp
return nil, fmt.Errorf("no endpoint group")
}

func (pr *ProxyResources) makeEnvoyDynamicCluster(name string, protocol string, dynamic *pbproxystate.DynamicEndpointGroup) (*envoy_cluster_v3.Cluster, error) {
func (pr *ProxyResources) makeEnvoyDynamicCluster(name string, protocol pbproxystate.Protocol, dynamic *pbproxystate.DynamicEndpointGroup) (*envoy_cluster_v3.Cluster, error) {
cluster := &envoy_cluster_v3.Cluster{
Name: name,
ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{Type: envoy_cluster_v3.Cluster_EDS},
Expand Down Expand Up @@ -153,7 +153,7 @@ func (pr *ProxyResources) makeEnvoyDynamicCluster(name string, protocol string,

}

func (pr *ProxyResources) makeEnvoyStaticCluster(name string, protocol string, static *pbproxystate.StaticEndpointGroup) (*envoy_cluster_v3.Cluster, error) {
func (pr *ProxyResources) makeEnvoyStaticCluster(name string, protocol pbproxystate.Protocol, static *pbproxystate.StaticEndpointGroup) (*envoy_cluster_v3.Cluster, error) {
cluster := &envoy_cluster_v3.Cluster{
Name: name,
ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{Type: envoy_cluster_v3.Cluster_STATIC},
Expand Down Expand Up @@ -182,11 +182,11 @@ func (pr *ProxyResources) makeEnvoyStaticCluster(name string, protocol string, s
return cluster, nil
}

func (pr *ProxyResources) makeEnvoyDnsCluster(name string, protocol string, dns *pbproxystate.DNSEndpointGroup) (*envoy_cluster_v3.Cluster, error) {
func (pr *ProxyResources) makeEnvoyDnsCluster(name string, protocol pbproxystate.Protocol, dns *pbproxystate.DNSEndpointGroup) (*envoy_cluster_v3.Cluster, error) {
return nil, nil
}

func (pr *ProxyResources) makeEnvoyPassthroughCluster(name string, protocol string, passthrough *pbproxystate.PassthroughEndpointGroup) (*envoy_cluster_v3.Cluster, error) {
func (pr *ProxyResources) makeEnvoyPassthroughCluster(name string, protocol pbproxystate.Protocol, passthrough *pbproxystate.PassthroughEndpointGroup) (*envoy_cluster_v3.Cluster, error) {
cluster := &envoy_cluster_v3.Cluster{
Name: name,
ConnectTimeout: passthrough.Config.ConnectTimeout,
Expand All @@ -207,7 +207,7 @@ func (pr *ProxyResources) makeEnvoyPassthroughCluster(name string, protocol stri
return cluster, nil
}

func (pr *ProxyResources) makeEnvoyAggregateCluster(name string, protocol string, fg *pbproxystate.FailoverGroup) ([]*envoy_cluster_v3.Cluster, error) {
func (pr *ProxyResources) makeEnvoyAggregateCluster(name string, protocol pbproxystate.Protocol, fg *pbproxystate.FailoverGroup) ([]*envoy_cluster_v3.Cluster, error) {
var clusters []*envoy_cluster_v3.Cluster
if fg != nil {
var egNames []string
Expand Down Expand Up @@ -250,8 +250,8 @@ func (pr *ProxyResources) makeEnvoyAggregateCluster(name string, protocol string
return clusters, nil
}

func addLocalAppHttpProtocolOptions(protocol string, c *envoy_cluster_v3.Cluster) error {
if !(protocol == "http2" || protocol == "grpc") {
func addLocalAppHttpProtocolOptions(protocol pbproxystate.Protocol, c *envoy_cluster_v3.Cluster) error {
if !(protocol == pbproxystate.Protocol_PROTOCOL_HTTP2 || protocol == pbproxystate.Protocol_PROTOCOL_GRPC) {
// do not error. returning nil means it won't get set.
return nil
}
Expand All @@ -274,8 +274,8 @@ func addLocalAppHttpProtocolOptions(protocol string, c *envoy_cluster_v3.Cluster
return nil
}

func addHttpProtocolOptions(protocol string, c *envoy_cluster_v3.Cluster) error {
if !(protocol == "http2" || protocol == "grpc") {
func addHttpProtocolOptions(protocol pbproxystate.Protocol, c *envoy_cluster_v3.Cluster) error {
if !(protocol == pbproxystate.Protocol_PROTOCOL_HTTP2 || protocol == pbproxystate.Protocol_PROTOCOL_GRPC) {
// do not error. returning nil means it won't get set.
return nil
}
Expand Down
8 changes: 8 additions & 0 deletions agent/xdsv2/testdata/clusters/source/l7-expose-paths.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@
]
}
]
},
"typedExtensionProtocolOptions": {
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": {
"@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions",
"explicitHttpConfig": {
"http2ProtocolOptions": {}
}
}
}
},
{
Expand Down
8 changes: 8 additions & 0 deletions agent/xdsv2/testdata/clusters/source/local-and-inbound-connections.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@
]
}
]
},
"typedExtensionProtocolOptions": {
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": {
"@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions",
"explicitHttpConfig": {
"http2ProtocolOptions": {}
}
}
}
},
{
Expand Down
8 changes: 8 additions & 0 deletions ...tdata/clusters/source/multiport-l7-multiple-workload-addresses-with-specific-ports.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@
]
}
]
},
"typedExtensionProtocolOptions": {
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": {
"@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions",
"explicitHttpConfig": {
"http2ProtocolOptions": {}
}
}
}
}
],
Expand Down
16 changes: 16 additions & 0 deletions ...v2/testdata/clusters/source/multiport-l7-multiple-workload-addresses-without-ports.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@
]
}
]
},
"typedExtensionProtocolOptions": {
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": {
"@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions",
"explicitHttpConfig": {
"http2ProtocolOptions": {}
}
}
}
},
{
Expand All @@ -71,6 +79,14 @@
]
}
]
},
"typedExtensionProtocolOptions": {
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": {
"@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions",
"explicitHttpConfig": {
"http2ProtocolOptions": {}
}
}
}
}
],
Expand Down
4 changes: 3 additions & 1 deletion agent/xdsv2/testdata/routes/destination/mixed-multi-destination.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
"name": "default/local/default/api-1:http:1.1.1.1:1234",
"virtualHosts": [
{
"domains": ["*"],
"name": "default/local/default/api-1:http:1.1.1.1:1234",
"domains": [
"*"
],
"routes": [
{
"match": {
Expand Down
8 changes: 6 additions & 2 deletions ...tdata/routes/destination/multiport-l4-and-l7-multiple-implicit-destinations-tproxy.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
"name": "default/local/default/api-app2:http",
"virtualHosts": [
{
"domains": ["*"],
"name": "default/local/default/api-app2:http",
"domains": [
"*"
],
"routes": [
{
"match": {
Expand All @@ -27,8 +29,10 @@
"name": "default/local/default/api-app:http",
"virtualHosts": [
{
"domains": ["*"],
"name": "default/local/default/api-app:http",
"domains": [
"*"
],
"routes": [
{
"match": {
Expand Down
4 changes: 3 additions & 1 deletion ...testdata/routes/destination/multiport-l4-and-l7-single-implicit-destination-tproxy.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
"name": "default/local/default/api-app:http",
"virtualHosts": [
{
"domains": ["*"],
"name": "default/local/default/api-app:http",
"domains": [
"*"
],
"routes": [
{
"match": {
Expand Down
4 changes: 3 additions & 1 deletion ...ion/multiport-l4-and-l7-single-implicit-destination-with-multiple-workloads-tproxy.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
"name": "default/local/default/api-app:http",
"virtualHosts": [
{
"domains": ["*"],
"name": "default/local/default/api-app:http",
"domains": [
"*"
],
"routes": [
{
"match": {
Expand Down
4 changes: 4 additions & 0 deletions internal/catalog/exports.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,7 @@ func ValidatePortName(name string) error {
func IsValidUnixSocketPath(host string) bool {
return types.IsValidUnixSocketPath(host)
}

func ValidateProtocol(protocol pbcatalog.Protocol) error {
return types.ValidateProtocol(protocol)
}
2 changes: 1 addition & 1 deletion internal/catalog/internal/types/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func ValidateService(res *pbresource.Resource) error {
})
}

if protoErr := validateProtocol(port.Protocol); protoErr != nil {
if protoErr := ValidateProtocol(port.Protocol); protoErr != nil {
err = multierror.Append(err, resource.ErrInvalidListElement{
Name: "ports",
Index: idx,
Expand Down
2 changes: 1 addition & 1 deletion internal/catalog/internal/types/service_endpoints.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func validateEndpoint(endpoint *pbcatalog.Endpoint, res *pbresource.Resource) er
})
}

if protoErr := validateProtocol(port.Protocol); protoErr != nil {
if protoErr := ValidateProtocol(port.Protocol); protoErr != nil {
err = multierror.Append(err, resource.ErrInvalidMapValue{
Map: "ports",
Key: portName,
Expand Down
4 changes: 3 additions & 1 deletion internal/catalog/internal/types/validators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ func ValidatePortName(name string) error {
return nil
}

func validateProtocol(protocol pbcatalog.Protocol) error {
func ValidateProtocol(protocol pbcatalog.Protocol) error {
// enumcover:pbcatalog.Protocol
switch protocol {
case pbcatalog.Protocol_PROTOCOL_UNSPECIFIED,
// means pbcatalog.FailoverMode_FAILOVER_MODE_TCP
Expand Down Expand Up @@ -240,6 +241,7 @@ func validateReference(allowedType *pbresource.Type, allowedTenancy *pbresource.
}

func validateHealth(health pbcatalog.Health) error {
// enumcover:pbcatalog.Health
switch health {
case pbcatalog.Health_HEALTH_ANY,
pbcatalog.Health_HEALTH_PASSING,
Expand Down
20 changes: 0 additions & 20 deletions internal/catalog/internal/types/validators_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,26 +373,6 @@ func TestValidatePortName(t *testing.T) {
})
}

func TestValidateProtocol(t *testing.T) {
// this test simply verifies that we accept all enum values specified in our proto
// in order to avoid validator drift.
for name, value := range pbcatalog.Protocol_value {
t.Run(name, func(t *testing.T) {
require.NoError(t, validateProtocol(pbcatalog.Protocol(value)))
})
}
}

func TestValidateHealth(t *testing.T) {
// this test simply verifies that we accept all enum values specified in our proto
// in order to avoid validator drift.
for name, value := range pbcatalog.Health_value {
t.Run(name, func(t *testing.T) {
require.NoError(t, validateHealth(pbcatalog.Health(value)))
})
}
}

func TestValidateWorkloadAddress(t *testing.T) {
type testCase struct {
addr *pbcatalog.WorkloadAddress
Expand Down
2 changes: 1 addition & 1 deletion internal/catalog/internal/types/workload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func ValidateWorkload(res *pbresource.Resource) error {
})
}

if protoErr := validateProtocol(port.Protocol); protoErr != nil {
if protoErr := ValidateProtocol(port.Protocol); protoErr != nil {
err = multierror.Append(err, resource.ErrInvalidMapValue{
Map: "ports",
Key: portName,
Expand Down
29 changes: 27 additions & 2 deletions internal/mesh/internal/controllers/sidecarproxy/builder/destination_multiport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,31 @@ func TestBuildMultiportImplicitDestinations(t *testing.T) {
},
}

multiportServiceData := &pbcatalog.Service{
Ports: []*pbcatalog.ServicePort{
{
TargetPort: "tcp",
VirtualPort: 7070,
Protocol: pbcatalog.Protocol_PROTOCOL_TCP,
},
{
TargetPort: "tcp2",
VirtualPort: 8081,
Protocol: pbcatalog.Protocol_PROTOCOL_TCP,
},
{
TargetPort: "http",
VirtualPort: 8080,
Protocol: pbcatalog.Protocol_PROTOCOL_HTTP,
},
{
TargetPort: "mesh",
VirtualPort: 20000,
Protocol: pbcatalog.Protocol_PROTOCOL_MESH,
},
},
}

multiportEndpointsData := &pbcatalog.ServiceEndpoints{
Endpoints: []*pbcatalog.Endpoint{
{
Expand All @@ -57,12 +82,12 @@ func TestBuildMultiportImplicitDestinations(t *testing.T) {
}
apiAppService := resourcetest.Resource(pbcatalog.ServiceType, apiApp).
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, serviceData).
WithData(t, multiportServiceData).
Build()

apiApp2Service := resourcetest.Resource(pbcatalog.ServiceType, apiApp2).
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, serviceData).
WithData(t, multiportServiceData).
Build()

apiAppEndpoints := resourcetest.Resource(pbcatalog.ServiceEndpointsType, apiApp).
Expand Down
Loading