Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.18] listener options merge what translator generates #10578 #10635

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions changelog/v1.18.9/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
Loading