Skip to content

Commit

Permalink
listener options merge what translator generates (#10578)
Browse files Browse the repository at this point in the history
  • Loading branch information
stevenctl authored Feb 18, 2025
1 parent 5875e84 commit 40985ca
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 5 deletions.
8 changes: 8 additions & 0 deletions changelog/v1.19.0-beta10/merge-listener-options.yaml
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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())
})
})

Expand All @@ -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 {
Expand All @@ -144,6 +151,7 @@ func attachedListenerOption() *solokubev1.ListenerOption {
},
}
}

func attachedListenerOptionWithSectionName() *solokubev1.ListenerOption {
listOpt := attachedListenerOption()
listOpt.Spec.TargetRefs[0].SectionName = &wrapperspb.StringValue{
Expand Down
26 changes: 26 additions & 0 deletions projects/gloo/pkg/utils/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 40985ca

Please sign in to comment.