From 554a788eea6fed21f83a167445c55f53b7454fab Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 20 Jan 2025 19:44:40 -0800 Subject: [PATCH 01/10] listener options merge what translator generates --- .../plugins/listeneroptions/listener_options_plugin.go | 8 +++++++- .../listeneroptions/listener_options_plugin_test.go | 10 ++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go index 24fd6401c02..9e037c7f2e4 100644 --- a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go +++ b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go @@ -8,6 +8,8 @@ import ( lisquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/listeneroptions/query" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + "google.golang.org/protobuf/proto" + "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -47,7 +49,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 { + proto.Merge(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..ca223818aa5 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,17 @@ var _ = Describe("ListenerOptions Plugin", func() { }, } - outputListener = &v1.Listener{} + outputListener = &v1.Listener{ + Options: &v1.ListenerOptions{ + ProxyProtocol: &proxy_protocol.ProxyProtocol{}, + }, + } expectedOptions = &v1.ListenerOptions{ PerConnectionBufferLimitBytes: &wrapperspb.UInt32Value{ Value: uint32(419), }, + ProxyProtocol: &proxy_protocol.ProxyProtocol{}, } }) JustBeforeEach(func() { @@ -118,7 +124,6 @@ var _ = Describe("ListenerOptions Plugin", func() { }) }) }) - }) func attachedListenerOption() *solokubev1.ListenerOption { @@ -144,6 +149,7 @@ func attachedListenerOption() *solokubev1.ListenerOption { }, } } + func attachedListenerOptionWithSectionName() *solokubev1.ListenerOption { listOpt := attachedListenerOption() listOpt.Spec.TargetRefs[0].SectionName = &wrapperspb.StringValue{ From 5e0ff50695c587f518d9f401e01a20a671fa31e9 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 20 Jan 2025 23:26:27 -0800 Subject: [PATCH 02/10] fix negative test case --- .../plugins/listeneroptions/listener_options_plugin_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 ca223818aa5..f1ab8a524ec 100644 --- a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin_test.go +++ b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin_test.go @@ -56,9 +56,11 @@ var _ = Describe("ListenerOptions Plugin", func() { } expectedOptions = &v1.ListenerOptions{ + // from config PerConnectionBufferLimitBytes: &wrapperspb.UInt32Value{ Value: uint32(419), }, + // base ProxyProtocol: &proxy_protocol.ProxyProtocol{}, } }) @@ -108,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()) }) }) @@ -120,7 +122,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()) }) }) }) From 167e30c6380908d521644dc25fa20ff85c8ab746 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Tue, 21 Jan 2025 14:04:08 -0800 Subject: [PATCH 03/10] changelog --- changelog/v1.19.0-beta4/merge-listener-options.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/v1.19.0-beta4/merge-listener-options.yaml diff --git a/changelog/v1.19.0-beta4/merge-listener-options.yaml b/changelog/v1.19.0-beta4/merge-listener-options.yaml new file mode 100644 index 00000000000..698e4cad468 --- /dev/null +++ b/changelog/v1.19.0-beta4/merge-listener-options.yaml @@ -0,0 +1,5 @@ + - type: NON_USER_FACING + description: >- + If we generate any ListenerOptions from a Gateway to Proxy + translator, they will no longer be overridden by user + ListenerOptions. From b563ba71b219a65a97c35aa6fe6a260860921dc7 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Wed, 22 Jan 2025 13:29:54 -0800 Subject: [PATCH 04/10] Fix changelog --- changelog/v1.19.0-beta4/merge-listener-options.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelog/v1.19.0-beta4/merge-listener-options.yaml b/changelog/v1.19.0-beta4/merge-listener-options.yaml index 698e4cad468..f873258b4ae 100644 --- a/changelog/v1.19.0-beta4/merge-listener-options.yaml +++ b/changelog/v1.19.0-beta4/merge-listener-options.yaml @@ -1,4 +1,7 @@ +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 From 0fec0e4567737883c7a5b6c2316e44fd852b58be Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 10 Feb 2025 16:15:38 -0800 Subject: [PATCH 05/10] move changelog --- .../{v1.19.0-beta4 => v1.19.0-beta6}/merge-listener-options.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.19.0-beta4 => v1.19.0-beta6}/merge-listener-options.yaml (100%) diff --git a/changelog/v1.19.0-beta4/merge-listener-options.yaml b/changelog/v1.19.0-beta6/merge-listener-options.yaml similarity index 100% rename from changelog/v1.19.0-beta4/merge-listener-options.yaml rename to changelog/v1.19.0-beta6/merge-listener-options.yaml From d178c64ac1ab31220081bb047f3272aaec947d86 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Tue, 11 Feb 2025 09:50:23 -0800 Subject: [PATCH 06/10] use same merge semantics of solo APIs --- .../listener_options_plugin.go | 5 ++-- projects/gloo/pkg/utils/merge.go | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go index 9e037c7f2e4..5ff8a686886 100644 --- a/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go +++ b/projects/gateway2/translator/plugins/listeneroptions/listener_options_plugin.go @@ -7,8 +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" - - "google.golang.org/protobuf/proto" + "github.com/solo-io/gloo/projects/gloo/pkg/utils" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -50,7 +49,7 @@ func (p *plugin) ApplyListenerPlugin( // see for more context: https://github.com/solo-io/solo-projects/issues/6313 optToUse := attachedOptions[0] if outListener.GetOptions() != nil { - proto.Merge(outListener.GetOptions(), optToUse.Spec.GetOptions()) + outListener.Options, _ = utils.ShallowMergeListenerOptions(outListener.GetOptions(), optToUse.Spec.GetOptions()) } else { outListener.Options = optToUse.Spec.GetOptions() } 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 c1d627d20dbefe4e4bc72b9533affc3e90735303 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Tue, 11 Feb 2025 09:51:23 -0800 Subject: [PATCH 07/10] changelog move --- .../{v1.19.0-beta6 => v1.19.0-beta7}/merge-listener-options.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.19.0-beta6 => v1.19.0-beta7}/merge-listener-options.yaml (100%) diff --git a/changelog/v1.19.0-beta6/merge-listener-options.yaml b/changelog/v1.19.0-beta7/merge-listener-options.yaml similarity index 100% rename from changelog/v1.19.0-beta6/merge-listener-options.yaml rename to changelog/v1.19.0-beta7/merge-listener-options.yaml From 47ccf80d7629e5794a306448cb980184cdf4bb4a Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Wed, 12 Feb 2025 15:33:41 -0800 Subject: [PATCH 08/10] changelog keeps moving --- .../{v1.19.0-beta7 => v1.19.0-beta8}/merge-listener-options.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.19.0-beta7 => v1.19.0-beta8}/merge-listener-options.yaml (100%) diff --git a/changelog/v1.19.0-beta7/merge-listener-options.yaml b/changelog/v1.19.0-beta8/merge-listener-options.yaml similarity index 100% rename from changelog/v1.19.0-beta7/merge-listener-options.yaml rename to changelog/v1.19.0-beta8/merge-listener-options.yaml From d32fbba8e6da1b30c2b5d925db0e7d5ec5290b4d Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 17 Feb 2025 07:40:46 -0800 Subject: [PATCH 09/10] changelog changelog --- .../{v1.19.0-beta8 => v1.19.0-beta9}/merge-listener-options.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.19.0-beta8 => v1.19.0-beta9}/merge-listener-options.yaml (100%) diff --git a/changelog/v1.19.0-beta8/merge-listener-options.yaml b/changelog/v1.19.0-beta9/merge-listener-options.yaml similarity index 100% rename from changelog/v1.19.0-beta8/merge-listener-options.yaml rename to changelog/v1.19.0-beta9/merge-listener-options.yaml From 2f1990de26c1c925bd2983382431e5258ff1a3f2 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Tue, 18 Feb 2025 09:33:58 -0800 Subject: [PATCH 10/10] a changelog in motion stays in motion --- .../{v1.19.0-beta9 => v1.19.0-beta10}/merge-listener-options.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.19.0-beta9 => v1.19.0-beta10}/merge-listener-options.yaml (100%) diff --git a/changelog/v1.19.0-beta9/merge-listener-options.yaml b/changelog/v1.19.0-beta10/merge-listener-options.yaml similarity index 100% rename from changelog/v1.19.0-beta9/merge-listener-options.yaml rename to changelog/v1.19.0-beta10/merge-listener-options.yaml