fix meshtimeout policy default timeouts on outbound

Signed-off-by: Icarus Wu <[email protected]>
kumahq · Dec 23, 2024 · 0516c46 · 0516c46
1 parent 8035df6
commit 0516c46
Show file tree

Hide file tree

Showing 16 changed files with 50 additions and 31 deletions.
diff --git a/pkg/core/xds/inspect/rules.go b/pkg/core/xds/inspect/rules.go
@@ -119,7 +119,7 @@ func getOutboundRuleAttachments(rules core_rules.Rules, networking *mesh_proto.D
 		}
 		attachment := byUniqueClusterName[name]
 		if attachment == nil {
-			computedRule := rules.NewCompute(core_rules.Element(outboundTags))
+			computedRule := rules.Compute(core_rules.Element(outboundTags))
 			if computedRule == nil {
 				continue
 			}

diff --git a/pkg/plugins/policies/core/rules/rules.go b/pkg/plugins/policies/core/rules/rules.go
@@ -353,7 +353,8 @@ func (r *Rule) GetBackendRefOrigin(hash common_api.MatchesHash) (core_model.Reso
 
 type Rules []*Rule
 
-func (rs Rules) NewCompute(element Element) *Rule {
+// Compute returns Rule for the given element.
+func (rs Rules) Compute(element Element) *Rule {
 	for _, rule := range rs {
 		if rule.Subset.ContainsElement(element) {
 			return rule
@@ -362,17 +363,19 @@ func (rs Rules) NewCompute(element Element) *Rule {
 	return nil
 }
 
-func NewComputeConf[T any](rs Rules, element Element) *T {
-	computed := rs.NewCompute(element)
+// ComputeConf returns configuration for the given element.
+func ComputeConf[T any](rs Rules, element Element) *T {
+	computed := rs.Compute(element)
 	if computed != nil {
 		return pointer.To(computed.Conf.(T))
 	}
 
 	return nil
 }
 
-// Compute returns configuration for the given subset.
-func (rs Rules) Compute(sub Subset) *Rule {
+// LegacyCompute returns Rule for the given subset.
+// Deprecated: use Compute instead
+func (rs Rules) LegacyCompute(sub Subset) *Rule {
 	for _, rule := range rs {
 		if rule.Subset.IsSubset(sub) {
 			return rule
@@ -381,8 +384,10 @@ func (rs Rules) Compute(sub Subset) *Rule {
 	return nil
 }
 
-func ComputeConf[T any](rs Rules, sub Subset) *T {
-	if computed := rs.Compute(sub); computed != nil {
+// LegacyComputeConf returns configuration for the given subset.
+// Deprecated: use ComputeConf instead
+func LegacyComputeConf[T any](rs Rules, sub Subset) *T {
+	if computed := rs.LegacyCompute(sub); computed != nil {
 		return pointer.To(computed.Conf.(T))
 	}
 	return nil

diff --git a/pkg/plugins/policies/core/rules/rules_test.go b/pkg/plugins/policies/core/rules/rules_test.go
@@ -491,7 +491,7 @@ var _ = Describe("Rules", func() {
 
 		DescribeTable("should compute conf for subset based on rules",
 			func(given testCase) {
-				conf := given.rules.NewCompute(given.element)
+				conf := given.rules.Compute(given.element)
 				if given.confYAML == nil {
 					Expect(conf).To(BeNil())
 				} else {

diff --git a/pkg/plugins/policies/meshaccesslog/plugin/v1alpha1/plugin.go b/pkg/plugins/policies/meshaccesslog/plugin/v1alpha1/plugin.go
@@ -239,7 +239,7 @@ func configureInbound(
 	serviceName := dataplane.Spec.GetIdentifyingService()
 
 	// `from` section of MeshAccessLog only allows Mesh targetRef
-	conf := core_rules.NewComputeConf[api.Conf](fromRules, core_rules.MeshElement())
+	conf := core_rules.ComputeConf[api.Conf](fromRules, core_rules.MeshElement())
 	if conf == nil {
 		return nil
 	}
@@ -276,7 +276,7 @@ func configureOutbound(
 ) error {
 	sourceService := dataplane.Spec.GetIdentifyingService()
 
-	conf := core_rules.NewComputeConf[api.Conf](toRules, element)
+	conf := core_rules.ComputeConf[api.Conf](toRules, element)
 	if conf == nil {
 		return nil
 	}

diff --git a/pkg/plugins/policies/meshcircuitbreaker/plugin/v1alpha1/plugin.go b/pkg/plugins/policies/meshcircuitbreaker/plugin/v1alpha1/plugin.go
@@ -195,7 +195,7 @@ func configure(
 	element core_rules.Element,
 	cluster *envoy_cluster.Cluster,
 ) error {
-	if computed := rules.NewCompute(element); computed != nil {
+	if computed := rules.Compute(element); computed != nil {
 		return plugin_xds.NewConfigurer(computed.Conf.(api.Conf)).ConfigureCluster(cluster)
 	}
 

diff --git a/pkg/plugins/policies/meshhealthcheck/plugin/v1alpha1/plugin.go b/pkg/plugins/policies/meshhealthcheck/plugin/v1alpha1/plugin.go
@@ -170,7 +170,7 @@ func configure(
 	protocol core_mesh.Protocol,
 	cluster *envoy_cluster.Cluster,
 ) error {
-	conf := core_rules.NewComputeConf[api.Conf](rules, element)
+	conf := core_rules.ComputeConf[api.Conf](rules, element)
 	if conf == nil {
 		return nil
 	}

diff --git a/pkg/plugins/policies/meshhttproute/plugin/v1alpha1/listeners.go b/pkg/plugins/policies/meshhttproute/plugin/v1alpha1/listeners.go
@@ -145,7 +145,7 @@ func ComputeHTTPRouteConf(toRules rules.ToRules, svc meshroute_xds.DestinationSe
 	var conf *api.PolicyDefault
 	backendRefOrigin := map[common_api.MatchesHash]core_model.ResourceMeta{}
 
-	ruleHTTP := toRules.Rules.NewCompute(core_rules.MeshServiceElement(svc.ServiceName))
+	ruleHTTP := toRules.Rules.Compute(core_rules.MeshServiceElement(svc.ServiceName))
 	if ruleHTTP != nil {
 		conf = pointer.To(ruleHTTP.Conf.(api.PolicyDefault))
 		for hash := range ruleHTTP.BackendRefOriginIndex {

diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/plugin.go b/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/plugin.go
@@ -96,7 +96,7 @@ func (p plugin) configureDPP(
 		oface := proxy.Dataplane.Spec.Networking.ToOutboundInterface(outbound.LegacyOutbound)
 		serviceName := outbound.LegacyOutbound.GetService()
 
-		computed := toRules.Rules.NewCompute(core_rules.MeshServiceElement(serviceName))
+		computed := toRules.Rules.Compute(core_rules.MeshServiceElement(serviceName))
 		if computed == nil {
 			continue
 		}
@@ -294,7 +294,7 @@ func (p plugin) configureGateway(
 					}
 
 					serviceName := dest.Destination[mesh_proto.ServiceTag]
-					if localityConf := core_rules.NewComputeConf[api.Conf](rules.Rules, core_rules.MeshServiceElement(serviceName)); localityConf != nil {
+					if localityConf := core_rules.ComputeConf[api.Conf](rules.Rules, core_rules.MeshServiceElement(serviceName)); localityConf != nil {
 						perServiceConfiguration[serviceName] = localityConf
 
 						if err := p.configureCluster(cluster, *localityConf); err != nil {
@@ -412,7 +412,7 @@ func (p plugin) computeFrom(fr core_rules.FromRules) *core_rules.Rule {
 	if len(rules) == 0 {
 		return nil
 	}
-	return rules[0].NewCompute(core_rules.MeshElement())
+	return rules[0].Compute(core_rules.MeshElement())
 }
 
 func (p plugin) configureListener(

diff --git a/pkg/plugins/policies/meshproxypatch/plugin/v1alpha1/plugin.go b/pkg/plugins/policies/meshproxypatch/plugin/v1alpha1/plugin.go
@@ -38,7 +38,7 @@ func (p plugin) Apply(rs *core_xds.ResourceSet, _ xds_context.Context, proxy *co
 	if len(policies.SingleItemRules.Rules) == 0 {
 		return nil
 	}
-	rule := policies.SingleItemRules.Rules.NewCompute(rules.MeshElement())
+	rule := policies.SingleItemRules.Rules.Compute(rules.MeshElement())
 	conf := rule.Conf.(api.Conf)
 	if err := ApplyMods(rs, conf.AppendModifications); err != nil {
 		return err

diff --git a/pkg/plugins/policies/meshratelimit/plugin/xds/configurer.go b/pkg/plugins/policies/meshratelimit/plugin/xds/configurer.go
@@ -248,5 +248,5 @@ func (c *Configurer) getConf(element core_rules.Element) *api.Conf {
 	if c.Rules == nil {
 		return &api.Conf{}
 	}
-	return core_rules.NewComputeConf[api.Conf](c.Rules, element)
+	return core_rules.ComputeConf[api.Conf](c.Rules, element)
 }
diff --git a/pkg/plugins/policies/meshretry/plugin/xds/configurer.go b/pkg/plugins/policies/meshretry/plugin/xds/configurer.go
@@ -423,7 +423,7 @@ func (c *DeprecatedConfigurer) getConf(element core_rules.Element) *api.Conf {
 	if c.Rules == nil {
 		return nil
 	}
-	return core_rules.NewComputeConf[api.Conf](c.Rules, element)
+	return core_rules.ComputeConf[api.Conf](c.Rules, element)
 }
 
 type Configurer struct {

diff --git a/pkg/plugins/policies/meshtcproute/plugin/v1alpha1/listeners_backendrefs.go b/pkg/plugins/policies/meshtcproute/plugin/v1alpha1/listeners_backendrefs.go
@@ -16,7 +16,7 @@ func computeConf(toRules core_xds.ToRules, svc meshroute_xds.DestinationService,
 	var tcpConf *api.Rule
 	var origin core_model.ResourceMeta
 
-	ruleTCP := toRules.Rules.NewCompute(core_xds.MeshServiceElement(svc.ServiceName))
+	ruleTCP := toRules.Rules.Compute(core_xds.MeshServiceElement(svc.ServiceName))
 	if ruleTCP != nil {
 		tcpConf = pointer.To(ruleTCP.Conf.(api.Rule))
 		if o, ok := ruleTCP.GetBackendRefOrigin(core_xds.EmptyMatches); ok {

diff --git a/pkg/plugins/policies/meshtimeout/plugin/v1alpha1/plugin.go b/pkg/plugins/policies/meshtimeout/plugin/v1alpha1/plugin.go
@@ -137,6 +137,7 @@ func applyToOutbounds(
 		configurer := plugin_xds.DeprecatedListenerConfigurer{
 			Rules:    rules.Rules,
 			Protocol: meshCtx.GetServiceProtocol(serviceName),
+			Subset:   core_rules.MeshService(serviceName),
 			Element:  core_rules.MeshServiceElement(serviceName),
 		}
 
@@ -264,7 +265,7 @@ func getConf(
 	if rules == nil {
 		return &api.Conf{}
 	} else {
-		if computed := rules.NewCompute(element); computed != nil {
+		if computed := rules.Compute(element); computed != nil {
 			return pointer.To(computed.Conf.(api.Conf))
 		} else {
 			return nil

diff --git a/pkg/plugins/policies/meshtimeout/plugin/xds/configurer.go b/pkg/plugins/policies/meshtimeout/plugin/xds/configurer.go
@@ -31,6 +31,7 @@ import (
 type DeprecatedListenerConfigurer struct {
 	Rules    rules.Rules
 	Protocol core_mesh.Protocol
+	Subset   rules.Subset
 	Element  rules.Element
 }
 
@@ -94,6 +95,13 @@ func (c *DeprecatedListenerConfigurer) configureRequestTimeout(routeConfiguratio
 }
 
 func (c *DeprecatedListenerConfigurer) configureRequestHeadersTimeout(hcm *envoy_hcm.HttpConnectionManager) {
+	// For backward, once a user upgrades from an older version we shouldn't set default timeouts.
+	// Refer to https://github.com/kumahq/kuma/issues/12033
+	deprecatedGetConf := c.legacyGetConf(c.Subset)
+	if deprecatedGetConf == nil {
+		return
+	}
+
 	if conf := c.getConf(c.Element); conf != nil {
 		hcm.RequestHeadersTimeout = toProtoDurationOrDefault(
 			pointer.Deref(conf.Http).RequestHeadersTimeout,
@@ -106,7 +114,14 @@ func (c *DeprecatedListenerConfigurer) getConf(element rules.Element) *api.Conf
 	if c.Rules == nil {
 		return &api.Conf{}
 	}
-	return rules.NewComputeConf[api.Conf](c.Rules, element)
+	return rules.ComputeConf[api.Conf](c.Rules, element)
+}
+
+func (c *DeprecatedListenerConfigurer) legacyGetConf(subset rules.Subset) *api.Conf {
+	if c.Rules == nil {
+		return &api.Conf{}
+	}
+	return rules.LegacyComputeConf[api.Conf](c.Rules, subset)
 }
 
 type ClusterConfigurer struct {

diff --git a/pkg/plugins/policies/meshtls/plugin/v1alpha1/plugin.go b/pkg/plugins/policies/meshtls/plugin/v1alpha1/plugin.go
@@ -97,7 +97,7 @@ func applyToInbounds(
 		if !ok {
 			continue
 		}
-		conf := core_rules.NewComputeConf[api.Conf](fromRules.Rules[listenerKey], core_rules.MeshElement())
+		conf := core_rules.ComputeConf[api.Conf](fromRules.Rules[listenerKey], core_rules.MeshElement())
 		if conf == nil {
 			continue
 		}
@@ -138,7 +138,7 @@ func applyToOutbounds(
 		// there is only one rule always because we're in `Mesh/Mesh`
 		var conf *api.Conf
 		for _, r := range fromRules.Rules {
-			conf = core_rules.NewComputeConf[api.Conf](r, core_rules.MeshElement())
+			conf = core_rules.ComputeConf[api.Conf](r, core_rules.MeshElement())
 			break
 		}
 		if conf == nil {
@@ -166,7 +166,7 @@ func applyToGateways(
 		// there is only one rule always because we're in `Mesh/Mesh`
 		var conf *api.Conf
 		for _, r := range gatewayRules.FromRules {
-			conf = core_rules.NewComputeConf[api.Conf](r, core_rules.MeshElement())
+			conf = core_rules.ComputeConf[api.Conf](r, core_rules.MeshElement())
 			break
 		}
 		if conf == nil {
@@ -187,7 +187,7 @@ func applyToRealResources(
 		// there is only one rule always because we're in `Mesh/Mesh`
 		var conf *api.Conf
 		for _, r := range fromRules.Rules {
-			conf = core_rules.NewComputeConf[api.Conf](r, core_rules.MeshElement())
+			conf = core_rules.ComputeConf[api.Conf](r, core_rules.MeshElement())
 			break
 		}
 		if conf == nil {

diff --git a/pkg/plugins/policies/meshtrafficpermission/graph/reachable_graph.go b/pkg/plugins/policies/meshtrafficpermission/graph/reachable_graph.go
@@ -29,8 +29,7 @@ func (r *Graph) CanReach(fromTags map[string]string, toTags map[string]string) b
 		// we cannot compute graph for cross mesh, so it's better to allow the traffic
 		return true
 	}
-	// rule := r.rules[toTags[mesh_proto.ServiceTag]].Compute(core_rules.SubsetFromTags(fromTags))
-	rule := r.rules[toTags[mesh_proto.ServiceTag]].NewCompute(core_rules.Element(fromTags))
+	rule := r.rules[toTags[mesh_proto.ServiceTag]].Compute(core_rules.Element(fromTags))
 	if rule == nil {
 		return false
 	}
@@ -46,8 +45,7 @@ func (r *Graph) CanReachBackend(fromTags map[string]string, backendIdentifier co
 		ResourceIdentifier: backendIdentifier.ResourceIdentifier,
 		ResourceType:       backendIdentifier.ResourceType,
 	}
-	// rule := r.backendRules[noPort].Compute(core_rules.SubsetFromTags(fromTags))
-	rule := r.backendRules[noPort].NewCompute(core_rules.Element(fromTags))
+	rule := r.backendRules[noPort].Compute(core_rules.Element(fromTags))
 	if rule == nil {
 		return false
 	}