From 3c35003253e5c92c89b52b8292786ac172ad57ba Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Tue, 18 Feb 2025 10:10:53 -0800 Subject: [PATCH 1/2] listener options merge what translator generates (#10578) --- .../merge-listener-options.yaml | 8 ++++++ .../listener_options_plugin.go | 7 ++++- .../listener_options_plugin_test.go | 16 +++++++++--- projects/gloo/pkg/utils/merge.go | 26 +++++++++++++++++++ 4 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 changelog/v1.19.0-beta10/merge-listener-options.yaml diff --git a/changelog/v1.19.0-beta10/merge-listener-options.yaml b/changelog/v1.19.0-beta10/merge-listener-options.yaml new file mode 100644 index 00000000000..f873258b4ae --- /dev/null +++ b/changelog/v1.19.0-beta10/merge-listener-options.yaml @@ -0,0 +1,8 @@ +changelog: + - type: NON_USER_FACING + issueLink: https://github.com/solo-io/solo-projects/issues/7300 + resolvesIssue: true + description: >- + If we generate any ListenerOptions from a Gateway to Proxy + translator, they will no longer be overridden by user + ListenerOptions. diff --git a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go index 24fd6401c02..5ff8a686886 100644 --- a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go +++ b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go @@ -7,6 +7,7 @@ import ( "github.com/solo-io/gloo/projects/gateway2/translator/plugins" lisquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/listeneroptions/query" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + "github.com/solo-io/gloo/projects/gloo/pkg/utils" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -47,7 +48,11 @@ func (p *plugin) ApplyListenerPlugin( // use the first option (highest in priority) // see for more context: https://github.com/solo-io/solo-projects/issues/6313 optToUse := attachedOptions[0] - outListener.Options = optToUse.Spec.GetOptions() + if outListener.GetOptions() != nil { + outListener.Options, _ = utils.ShallowMergeListenerOptions(outListener.GetOptions(), optToUse.Spec.GetOptions()) + } else { + outListener.Options = optToUse.Spec.GetOptions() + } return nil } diff --git a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin_test.go b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin_test.go index 569b76b2092..f1ab8a524ec 100644 --- a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin_test.go +++ b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin_test.go @@ -16,6 +16,7 @@ import ( "github.com/solo-io/gloo/projects/gateway2/translator/testutils" "github.com/solo-io/gloo/projects/gateway2/wellknown" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/proxy_protocol" corev1 "github.com/solo-io/skv2/pkg/api/core.skv2.solo.io/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -48,12 +49,19 @@ var _ = Describe("ListenerOptions Plugin", func() { }, } - outputListener = &v1.Listener{} + outputListener = &v1.Listener{ + Options: &v1.ListenerOptions{ + ProxyProtocol: &proxy_protocol.ProxyProtocol{}, + }, + } expectedOptions = &v1.ListenerOptions{ + // from config PerConnectionBufferLimitBytes: &wrapperspb.UInt32Value{ Value: uint32(419), }, + // base + ProxyProtocol: &proxy_protocol.ProxyProtocol{}, } }) JustBeforeEach(func() { @@ -102,7 +110,7 @@ var _ = Describe("ListenerOptions Plugin", func() { It("does not add buffer limit", func() { err := plugin.ApplyListenerPlugin(ctx, listenerCtx, outputListener) Expect(err).ToNot(HaveOccurred()) - Expect(outputListener.GetOptions()).To(BeNil()) + Expect(outputListener.GetOptions().GetPerConnectionBufferLimitBytes()).To(BeNil()) }) }) @@ -114,11 +122,10 @@ var _ = Describe("ListenerOptions Plugin", func() { It("does not add buffer limit", func() { err := plugin.ApplyListenerPlugin(ctx, listenerCtx, outputListener) Expect(err).ToNot(HaveOccurred()) - Expect(outputListener.GetOptions()).To(BeNil()) + Expect(outputListener.GetOptions().GetPerConnectionBufferLimitBytes()).To(BeNil()) }) }) }) - }) func attachedListenerOption() *solokubev1.ListenerOption { @@ -144,6 +151,7 @@ func attachedListenerOption() *solokubev1.ListenerOption { }, } } + func attachedListenerOptionWithSectionName() *solokubev1.ListenerOption { listOpt := attachedListenerOption() listOpt.Spec.TargetRefs[0].SectionName = &wrapperspb.StringValue{ diff --git a/projects/gloo/pkg/utils/merge.go b/projects/gloo/pkg/utils/merge.go index d129529b12a..39370a0a83c 100644 --- a/projects/gloo/pkg/utils/merge.go +++ b/projects/gloo/pkg/utils/merge.go @@ -80,6 +80,32 @@ func isEmptyValue(v reflect.Value) bool { return false } +// ShallowMergeListenerOptions merges the top-level fields of src into dst. +// The fields in dst that have non-zero values will not be overwritten. +// It performs a shallow merge of top-level fields only. +// It returns a boolean indicating whether any fields in src overwrote dst. +func ShallowMergeListenerOptions(dst, src *v1.ListenerOptions) (*v1.ListenerOptions, bool) { + if src == nil { + return dst, false + } + + if dst == nil { + return src.Clone().(*v1.ListenerOptions), true + } + + dstValue, srcValue := reflect.ValueOf(dst).Elem(), reflect.ValueOf(src).Elem() + + overwrote := false + for i := range dstValue.NumField() { + dstField, srcField := dstValue.Field(i), srcValue.Field(i) + if srcOverride := ShallowMerge(dstField, srcField); srcOverride { + overwrote = true + } + } + + return dst, overwrote +} + // ShallowMergeRouteOptions merges the top-level fields of src into dst. // The fields in dst that have non-zero values will not be overwritten. // It performs a shallow merge of top-level fields only. From c29d9db6a503404077f6fac4f61b7d87a9814248 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Tue, 18 Feb 2025 10:17:31 -0800 Subject: [PATCH 2/2] changelog for backport --- changelog/{v1.19.0-beta10 => v1.18.9}/merge-listener-options.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.19.0-beta10 => v1.18.9}/merge-listener-options.yaml (100%) diff --git a/changelog/v1.19.0-beta10/merge-listener-options.yaml b/changelog/v1.18.9/merge-listener-options.yaml similarity index 100% rename from changelog/v1.19.0-beta10/merge-listener-options.yaml rename to changelog/v1.18.9/merge-listener-options.yaml