From 09b7382a10c0c029b3823b5a62b90c5c18076c83 Mon Sep 17 00:00:00 2001 From: Guillaume LEGRAIN Date: Wed, 3 Jul 2024 10:38:58 +0200 Subject: [PATCH 1/7] Add config to gitignore --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index b8a95f6e..addeb984 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,7 @@ bin/ command testing +ca/ +config/ +envoy/ + From b9e64193c0f13029edb597079b3c81ad8c1c3662 Mon Sep 17 00:00:00 2001 From: Lyonel Martinez Date: Mon, 12 Sep 2022 17:47:30 +0200 Subject: [PATCH 2/7] feat(split-time): splits timeout annotation into 3 different ones The new annotations got the priority, the simple 'timeout' annotation still sets the 3 timeouts but if there is, at least one of the new ones, it's overwriting the values set by the simple annotation. --- pkg/envoy/ingress_translator.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/pkg/envoy/ingress_translator.go b/pkg/envoy/ingress_translator.go index fca04dc1..64a3a66e 100644 --- a/pkg/envoy/ingress_translator.go +++ b/pkg/envoy/ingress_translator.go @@ -218,6 +218,18 @@ func (ing *envoyIngress) addTimeout(timeout time.Duration) { ing.vhost.PerTryTimeout = timeout } +func (ing *envoyIngress) setClusterTimeout(timeout time.Duration) { + ing.cluster.Timeout = timeout +} + +func (ing *envoyIngress) setRouteTimeout(timeout time.Duration) { + ing.vhost.Timeout = timeout +} + +func (ing *envoyIngress) setPerTryTimeout(timeout time.Duration) { + ing.vhost.PerTryTimeout = timeout +} + // hostMatch returns true if tlsHost and ruleHost match, with wildcard support // // *.a.b ruleHost accepts tlsHost *.a.b but not a.a.b or a.b or a.a.a.b @@ -324,6 +336,27 @@ func translateIngresses(ingresses []*k8s.Ingress, syncSecrets bool, secrets []*v } } + if i.Annotations["yggdrasil.uswitch.com/cluster-timeout"] != "" { + timeout, err := time.ParseDuration(i.Annotations["yggdrasil.uswitch.com/cluster-timeout"]) + if err == nil { + envoyIngress.setClusterTimeout(timeout) + } + } + + if i.Annotations["yggdrasil.uswitch.com/route-timeout"] != "" { + timeout, err := time.ParseDuration(i.Annotations["yggdrasil.uswitch.com/route-timeout"]) + if err == nil { + envoyIngress.setRouteTimeout(timeout) + } + } + + if i.Annotations["yggdrasil.uswitch.com/per-try-timeout"] != "" { + timeout, err := time.ParseDuration(i.Annotations["yggdrasil.uswitch.com/per-try-timeout"]) + if err == nil { + envoyIngress.setPerTryTimeout(timeout) + } + } + envoyIngress.addRetryOn(i) if syncSecrets && envoyIngress.vhost.TlsKey == "" && envoyIngress.vhost.TlsCert == "" { From 18edd749c13b9c5c9c1f9c1dc40dc9a88073cf5e Mon Sep 17 00:00:00 2001 From: Lyonel Martinez Date: Tue, 13 Sep 2022 14:33:44 +0200 Subject: [PATCH 3/7] feat(split-timeout): Docs --- README.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 6d2d4057..d09b6ee9 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,11 @@ Yggdrasil allows for some customisation of the route and cluster config per Ingr | Name | type | |--------------------------------------------------------------|----------| | [yggdrasil.uswitch.com/healthcheck-path](#health-check-path) | string | -| [yggdrasil.uswitch.com/timeout](#timeout) | duration | +| [yggdrasil.uswitch.com/timeout](#timeouts) | duration | +| [yggdrasil.uswitch.com/cluster-timeout](#timeouts) | duration | +| [yggdrasil.uswitch.com/route-timeout](#timeouts) | duration | +| [yggdrasil.uswitch.com/per-try-timeout](#timeouts) | duration | +| [yggdrasil.uswitch.com/weight](#weight) | uint32 | | [yggdrasil.uswitch.com/retry-on](#retries) | string | ### Health Check Path @@ -83,12 +87,17 @@ Specifies a path to configure a [HTTP health check](https://www.envoyproxy.io/do * [config.core.v3.HealthCheck.HttpHealthCheck.Path](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/core/v3/health_check.proto#envoy-v3-api-field-config-core-v3-healthcheck-httphealthcheck-path) -### Timeout -Allows for adjusting the timeout in envoy. Currently this will set the following timeouts to this value: +### Timeouts +Allows for adjusting the timeout in envoy. + +The `yggdrasil.uswitch.com/cluster-timeout` annotation will set the [config.cluster.v3.Cluster.ConnectTimeout](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout) + +The `yggdrasil.uswitch.com/route-timeout` annotation will set the [config.route.v3.RouteAction.Timeout](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-timeout) + +the `yggdrasil.uswitch.com/per-try-timeout` annotation will set the [config.route.v3.RetryPolicy.PerTryTimeout](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-retrypolicy-per-try-timeout) + +The `yggdrasil.uswitch.com/timeout` annotation will set all of the above with the same value. This annotation has the lowest priority, if it set with one of the other TO annotation, the specific one will override the general annotation. -* [config.route.v3.RouteAction.Timeout](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-timeout) -* [config.route.v3.RetryPolicy.PerTryTimeout](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-retrypolicy-per-try-timeout) -* [config.cluster.v3.Cluster.ConnectTimeout](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout) ### Retries Allows overwriting the default retry policy's [config.route.v3.RetryPolicy.RetryOn](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-retrypolicy-retry-on) set by the `--retry-on` flag (default 5xx). Accepts a comma-separated list of retry-on policies. From 5ccb29677882782a711da6ea86f35d563f814739 Mon Sep 17 00:00:00 2001 From: Lyonel Martinez Date: Tue, 13 Sep 2022 15:45:56 +0200 Subject: [PATCH 4/7] feat(split-timeout): Default Timeouts flags and config --- cmd/root.go | 8 ++++++++ pkg/envoy/configurator.go | 9 ++++++++- pkg/envoy/ingress_translator.go | 10 +++++----- pkg/envoy/options.go | 7 +++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 441e5fb1..afe73329 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -46,6 +46,7 @@ type config struct { UseRemoteAddress bool `json:"useRemoteAddress"` HttpExtAuthz envoy.HttpExtAuthz `json:"httpExtAuthz"` HttpGrpcLogger envoy.HttpGrpcLogger `json:"httpGrpcLogger"` + DefaultTimeouts envoy.DefaultTimeouts `json:"defaultTimeouts"` AccessLogger envoy.AccessLogger `json:"accessLogger"` } @@ -109,6 +110,9 @@ func init() { rootCmd.PersistentFlags().Bool("http-ext-authz-pack-as-bytes", false, "When this field is true, Envoy will send the body as raw bytes.") rootCmd.PersistentFlags().Bool("http-ext-authz-failure-mode-allow", true, "Changes filters behaviour on errors") + rootCmd.PersistentFlags().Duration("default-route-timeout", 15*time.Second, "Default timeout of the routes") + rootCmd.PersistentFlags().Duration("default-cluster-timeout", 30*time.Second, "Default timeout of the cluster") + rootCmd.PersistentFlags().Duration("default-per-try-timeout", 5*time.Second, "Default timeout of PerTry") viper.BindPFlag("debug", rootCmd.PersistentFlags().Lookup("debug")) viper.BindPFlag("configDump", rootCmd.PersistentFlags().Lookup("config-dump")) viper.BindPFlag("address", rootCmd.PersistentFlags().Lookup("address")) @@ -141,6 +145,9 @@ func init() { viper.BindPFlag("httpExtAuthz.allowPartialMessage", rootCmd.PersistentFlags().Lookup("http-ext-authz-allow-partial-message")) viper.BindPFlag("httpExtAuthz.packAsBytes", rootCmd.PersistentFlags().Lookup("http-ext-authz-pack-as-bytes")) viper.BindPFlag("httpExtAuthz.FailureModeAllow", rootCmd.PersistentFlags().Lookup("http-ext-authz-failure-mode-allow")) + viper.BindPFlag("defaultTimeouts.Route", rootCmd.PersistentFlags().Lookup("default-route-timeout")) + viper.BindPFlag("defaultTimeouts.Cluster", rootCmd.PersistentFlags().Lookup("default-cluster-timeout")) + viper.BindPFlag("defaultTimeouts.PerTry", rootCmd.PersistentFlags().Lookup("default-per-try-timeout")) } func initConfig() { @@ -241,6 +248,7 @@ func main(*cobra.Command, []string) error { envoy.WithHttpExtAuthzCluster(c.HttpExtAuthz), envoy.WithHttpGrpcLogger(c.HttpGrpcLogger), envoy.WithSyncSecrets(c.SyncSecrets), + envoy.WithDefaultTimeouts(c.DefaultTimeouts), envoy.WithDefaultRetryOn(viper.GetString("retryOn")), envoy.WithAccessLog(c.AccessLogger), envoy.WithTracingProvider(viper.GetString("tracingProvider")), diff --git a/pkg/envoy/configurator.go b/pkg/envoy/configurator.go index fc3be3c8..f659c587 100644 --- a/pkg/envoy/configurator.go +++ b/pkg/envoy/configurator.go @@ -30,6 +30,12 @@ type UpstreamHealthCheck struct { HealthyThreshold uint32 `json:"healtyThreshold"` } +type DefaultTimeouts struct { + Cluster time.Duration + Route time.Duration + PerTry time.Duration +} + type HttpExtAuthz struct { Cluster string `json:"cluster"` Timeout time.Duration `json:"timeout"` @@ -67,6 +73,7 @@ type KubernetesConfigurator struct { useRemoteAddress bool httpExtAuthz HttpExtAuthz httpGrpcLogger HttpGrpcLogger + defaultTimeouts DefaultTimeouts accessLogger AccessLogger defaultRetryOn string tracingProvider string @@ -92,7 +99,7 @@ func (c *KubernetesConfigurator) Generate(ingresses []*k8s.Ingress, secrets []*v defer c.Unlock() validIngresses := validIngressFilter(classFilter(ingresses, c.ingressClasses)) - config := translateIngresses(validIngresses, c.syncSecrets, secrets) + config := translateIngresses(validIngresses, c.syncSecrets, secrets, c.defaultTimeouts) vmatch, cmatch := config.equals(c.previousConfig) diff --git a/pkg/envoy/ingress_translator.go b/pkg/envoy/ingress_translator.go index 64a3a66e..7bc0a00f 100644 --- a/pkg/envoy/ingress_translator.go +++ b/pkg/envoy/ingress_translator.go @@ -185,14 +185,14 @@ type envoyIngress struct { cluster *cluster } -func newEnvoyIngress(host string) *envoyIngress { +func newEnvoyIngress(host string, timeouts DefaultTimeouts) *envoyIngress { clusterName := strings.Replace(host, ".", "_", -1) return &envoyIngress{ vhost: &virtualHost{ Host: host, UpstreamCluster: clusterName, - Timeout: (15 * time.Second), - PerTryTimeout: (5 * time.Second), + Timeout: timeouts.Route, + PerTryTimeout: timeouts.PerTry, }, cluster: &cluster{ Name: clusterName, @@ -310,7 +310,7 @@ func (envoyIng *envoyIngress) addRetryOn(ingress *k8s.Ingress) { } } -func translateIngresses(ingresses []*k8s.Ingress, syncSecrets bool, secrets []*v1.Secret) *envoyConfiguration { +func translateIngresses(ingresses []*k8s.Ingress, syncSecrets bool, secrets []*v1.Secret, timeouts DefaultTimeouts) *envoyConfiguration { cfg := &envoyConfiguration{} envoyIngresses := map[string]*envoyIngress{} @@ -319,7 +319,7 @@ func translateIngresses(ingresses []*k8s.Ingress, syncSecrets bool, secrets []*v for _, ruleHost := range i.RulesHosts { _, ok := envoyIngresses[ruleHost] if !ok { - envoyIngresses[ruleHost] = newEnvoyIngress(ruleHost) + envoyIngresses[ruleHost] = newEnvoyIngress(ruleHost, timeouts) } envoyIngress := envoyIngresses[ruleHost] diff --git a/pkg/envoy/options.go b/pkg/envoy/options.go index 7cc4e689..0cca3d84 100644 --- a/pkg/envoy/options.go +++ b/pkg/envoy/options.go @@ -72,6 +72,13 @@ func WithSyncSecrets(syncSecrets bool) option { } } +// WithDefaultTimeouts configures the default timeouts +func WithDefaultTimeouts(defaultTimeouts DefaultTimeouts) option { + return func(c *KubernetesConfigurator) { + c.defaultTimeouts = defaultTimeouts + } +} + // WithDefaultRetryOn configures the default retry policy func WithDefaultRetryOn(defaultRetryOn string) option { return func(c *KubernetesConfigurator) { From f139f258bc9e5d5ef894b0efc4edc12b41ce4b94 Mon Sep 17 00:00:00 2001 From: Lyonel Martinez Date: Thu, 15 Sep 2022 13:32:49 +0200 Subject: [PATCH 5/7] feat(split-timeout): fix comment and default TO for cluster --- pkg/envoy/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/envoy/options.go b/pkg/envoy/options.go index 0cca3d84..6980fabe 100644 --- a/pkg/envoy/options.go +++ b/pkg/envoy/options.go @@ -2,7 +2,7 @@ package envoy type option func(c *KubernetesConfigurator) -// WithEWithEnvoyListenerIpv4AddressnvoyPort configures envoy IPv4 listen address into a KubernetesConfigurator +// WithEnvoyListenerIpv4Address configures envoy IPv4 listen address into a KubernetesConfigurator func WithEnvoyListenerIpv4Address(address string) option { return func(c *KubernetesConfigurator) { c.envoyListenerIpv4Address = address From 54be71988915a6f111ea0f21c50357392ee2e9c5 Mon Sep 17 00:00:00 2001 From: Lowaiz <43639116+Lowaiz@users.noreply.github.com> Date: Mon, 19 Sep 2022 13:25:09 +0200 Subject: [PATCH 6/7] Update README.md Applying @Aluxima suggestion Co-authored-by: Laurent Marchaud <16262531+Aluxima@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d09b6ee9..64384e1d 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ The `yggdrasil.uswitch.com/route-timeout` annotation will set the [config.route. the `yggdrasil.uswitch.com/per-try-timeout` annotation will set the [config.route.v3.RetryPolicy.PerTryTimeout](https://www.envoyproxy.io/docs/envoy/v1.19.0/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-retrypolicy-per-try-timeout) -The `yggdrasil.uswitch.com/timeout` annotation will set all of the above with the same value. This annotation has the lowest priority, if it set with one of the other TO annotation, the specific one will override the general annotation. +The `yggdrasil.uswitch.com/timeout` annotation will set all of the above with the same value. This annotation has the lowest priority, if set with one of the other TO annotations, the specific one will override the general annotation. ### Retries From 1708910b2242eb4d9d754fd5c70f4cd3fc9f27a2 Mon Sep 17 00:00:00 2001 From: Lyonel Martinez Date: Tue, 20 Sep 2022 11:23:40 +0200 Subject: [PATCH 7/7] feat(split-timeout): Fix tests --- pkg/envoy/ingress_translator_test.go | 48 ++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/pkg/envoy/ingress_translator_test.go b/pkg/envoy/ingress_translator_test.go index b9042206..4a98a014 100644 --- a/pkg/envoy/ingress_translator_test.go +++ b/pkg/envoy/ingress_translator_test.go @@ -204,8 +204,13 @@ func TestEqualityVirtualHosts(t *testing.T) { func TestEquals(t *testing.T) { ingress := newGenericIngress("foo.app.com", "foo.cluster.com") ingress2 := newGenericIngress("bar.app.com", "foo.bar.com") - c := translateIngresses([]*k8s.Ingress{ingress, ingress2}, false, []*v1.Secret{}) - c2 := translateIngresses([]*k8s.Ingress{ingress, ingress2}, false, []*v1.Secret{}) + timeouts := DefaultTimeouts{ + Cluster: 30 * time.Second, + Route: 15 * time.Second, + PerTry: 5 * time.Second, + } + c := translateIngresses([]*k8s.Ingress{ingress, ingress2}, false, []*v1.Secret{}, timeouts) + c2 := translateIngresses([]*k8s.Ingress{ingress, ingress2}, false, []*v1.Secret{}, timeouts) vmatch, cmatch := c.equals(c2) if vmatch != true { @@ -221,8 +226,13 @@ func TestNotEquals(t *testing.T) { ingress2 := newGenericIngress("foo.app.com", "bar.cluster.com") ingress3 := newGenericIngress("foo.baz.com", "bar.cluster.com") ingress4 := newGenericIngress("foo.howdy.com", "bar.cluster.com") - c := translateIngresses([]*k8s.Ingress{ingress, ingress3, ingress2}, false, []*v1.Secret{}) - c2 := translateIngresses([]*k8s.Ingress{ingress, ingress2, ingress4}, false, []*v1.Secret{}) + timeouts := DefaultTimeouts{ + Cluster: 30 * time.Second, + Route: 15 * time.Second, + PerTry: 5 * time.Second, + } + c := translateIngresses([]*k8s.Ingress{ingress, ingress3, ingress2}, false, []*v1.Secret{}, timeouts) + c2 := translateIngresses([]*k8s.Ingress{ingress, ingress2, ingress4}, false, []*v1.Secret{}, timeouts) vmatch, cmatch := c.equals(c2) if vmatch == true { @@ -237,8 +247,13 @@ func TestNotEquals(t *testing.T) { func TestPartialEquals(t *testing.T) { ingress := newGenericIngress("foo.app.com", "bar.cluster.com") ingress2 := newGenericIngress("foo.app.com", "foo.cluster.com") - c := translateIngresses([]*k8s.Ingress{ingress2}, false, []*v1.Secret{}) - c2 := translateIngresses([]*k8s.Ingress{ingress}, false, []*v1.Secret{}) + timeouts := DefaultTimeouts{ + Cluster: 30 * time.Second, + Route: 15 * time.Second, + PerTry: 5 * time.Second, + } + c := translateIngresses([]*k8s.Ingress{ingress2}, false, []*v1.Secret{}, timeouts) + c2 := translateIngresses([]*k8s.Ingress{ingress}, false, []*v1.Secret{}, timeouts) vmatch, cmatch := c2.equals(c) if vmatch != true { @@ -252,7 +267,12 @@ func TestPartialEquals(t *testing.T) { func TestGeneratesForSingleIngress(t *testing.T) { ingress := newGenericIngress("foo.app.com", "foo.cluster.com") - c := translateIngresses([]*k8s.Ingress{ingress}, false, []*v1.Secret{}) + timeouts := DefaultTimeouts{ + Cluster: 30 * time.Second, + Route: 15 * time.Second, + PerTry: 5 * time.Second, + } + c := translateIngresses([]*k8s.Ingress{ingress}, false, []*v1.Secret{}, timeouts) if len(c.VirtualHosts) != 1 { t.Error("expected 1 virtual host") @@ -284,7 +304,12 @@ func TestGeneratesForSingleIngress(t *testing.T) { func TestGeneratesForMultipleIngressSharingSpecHost(t *testing.T) { fooIngress := newGenericIngress("app.com", "foo.com") barIngress := newGenericIngress("app.com", "bar.com") - c := translateIngresses([]*k8s.Ingress{fooIngress, barIngress}, false, []*v1.Secret{}) + timeouts := DefaultTimeouts{ + Cluster: 30 * time.Second, + Route: 15 * time.Second, + PerTry: 5 * time.Second, + } + c := translateIngresses([]*k8s.Ingress{fooIngress, barIngress}, false, []*v1.Secret{}, timeouts) if len(c.VirtualHosts) != 1 { t.Error("expected 1 virtual host") @@ -339,7 +364,12 @@ func TestFilterNonMatchingIngresses(t *testing.T) { func TestIngressWithIP(t *testing.T) { ingress := newIngressIP("app.com", "127.0.0.1") - c := translateIngresses([]*k8s.Ingress{ingress}, false, []*v1.Secret{}) + timeouts := DefaultTimeouts{ + Cluster: 30 * time.Second, + Route: 15 * time.Second, + PerTry: 5 * time.Second, + } + c := translateIngresses([]*k8s.Ingress{ingress}, false, []*v1.Secret{}, timeouts) if c.Clusters[0].Hosts[0] != "127.0.0.1" { t.Errorf("expected cluster host to be IP address, was %s", c.Clusters[0].Hosts[0]) }