diff --git a/changelog/v1.18.9/merge-listener-options.yaml b/changelog/v1.18.9/merge-listener-options.yaml new file mode 100644 index 00000000000..f873258b4ae --- /dev/null +++ b/changelog/v1.18.9/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.