Skip to content

Commit a312a4f

Browse files
authored
fix: fix KongUpstreamPolicy hash on translation (#7583) (#7590)
* fix: fix KongUpstreamPolicy hash on translation * tests: add a valid test case for hash_on uri_capture and hash_fallback with hreader
1 parent 5f15f3c commit a312a4f

16 files changed

+330
-186
lines changed

CHANGELOG.md

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,19 +106,19 @@ Adding a new version? You'll need three changes:
106106
- [0.0.5](#005)
107107
- [0.0.4 and prior](#004-and-prior)
108108

109-
## Unreleased
109+
## Unreleased
110110

111111
### Fixed
112112

113113
- Count the `attachedRoutes` of a `Gateway` in the correct way in case of `HTTPRoute`s
114114
with multiple parent references.
115115
[#7517](https://github.com/Kong/kubernetes-ingress-controller/pull/7517)
116116

117-
## [3.4.6]
117+
## [3.4.6]
118118

119-
> Release date: 2025-06-06
119+
> Release date: 2025-06-06
120120
121-
### Fixed
121+
### Fixed
122122

123123
- Keep the plugin's ID unchanged if there is already the same plugin exists
124124
when working with DB backed Kong gateways. "Same" plugin is identified by the
@@ -465,7 +465,7 @@ Please use [3.4.1] or later instead.
465465
- It is now possible to disable synchronization of consumers to Konnect through the
466466
flag `--konnect-disable-consumers-sync`.
467467
[#6313](https://github.com/Kong/kubernetes-ingress-controller/pull/6313)
468-
- Allow `KongCustomEntity` to refer to plugins in another namespace via
468+
- Allow `KongCustomEntity` to refer to plugins in another namespace via
469469
`spec.parentRef.namespace`. The reference is allowed only when there is a
470470
`ReferenceGrant` in the namespace of the `KongPlugin` to grant permissions
471471
to `KongCustomEntity` of referring to `KongPlugin`.
@@ -726,7 +726,7 @@ Please use [3.4.1] or later instead.
726726
- Do not generate invalid duplicate upstream targets when routes use multiple
727727
Services with the same endpoints.
728728
[#5817](https://github.com/Kong/kubernetes-ingress-controller/pull/5817)
729-
- Remove the constraint of items of `parentRefs` can only be empty or
729+
- Remove the constraint of items of `parentRefs` can only be empty or
730730
`gateway.network.k8s.io/Gateway` in validating `HTTPRoute`s. If an item in
731731
`parentRefs`'s group/kind is not `gateway.network.k8s.io/Gateway`, the item
732732
is seen as a parent other than the controller and ignored in parentRef check.
@@ -772,7 +772,6 @@ Please use [3.4.1] or later instead.
772772
attached to the route or service only.
773773
[#6132](https://github.com/Kong/kubernetes-ingress-controller/pull/6132)
774774

775-
776775
## [3.1.5]
777776

778777
> Release date: 2024-05-17
@@ -937,7 +936,7 @@ Please use [3.4.1] or later instead.
937936
[#5312](https://github.com/Kong/kubernetes-ingress-controller/pull/5312)
938937
- Added functionality to the `KongUpstreamPolicy` controller to properly set and
939938
enforce `KongUpstreamPolicy` status.
940-
The controller will set an ancestor status in `KongUpstreamPolicy` status for
939+
The controller will set an ancestor status in `KongUpstreamPolicy` status for
941940
each of its ancestors (i.e. `Service` or `KongServiceFacade`) with the `Accepted`
942941
and `Programmed` condition.
943942
[#5185](https://github.com/Kong/kubernetes-ingress-controller/pull/5185)

Makefile

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -792,9 +792,11 @@ uninstall-gateway-api-crds: go-mod-download-gateway-api kustomize
792792
$(KUSTOMIZE) build $(GATEWAY_API_CRDS_LOCAL_PATH) | kubectl delete -f -
793793
$(KUSTOMIZE) build $(GATEWAY_API_CRDS_LOCAL_PATH)/experimental | kubectl delete -f -
794794

795-
# Install CRDs into the K8s cluster specified in $KUBECONFIG.
796795
.PHONY: install
797-
install: manifests install-gateway-api-crds
796+
install: manifests install-gateway-api-crds install.crds.kubernetes-configuration
797+
798+
.PHONY: install.crds.kubernetes-configuration
799+
install.crds.kubernetes-configuration: kustomize
798800
$(KUSTOMIZE) build config/crd | kubectl apply -f -
799801

800802
# Uninstall CRDs from the K8s cluster specified in $KUBECONFIG.

config/crd/incubator/kustomization.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
apiVersion: kustomize.config.k8s.io/v1beta1
33
kind: Kustomization
44
resources:
5-
- https://github.com/kong/kubernetes-configuration/config/crd/ingress-controller-incubator?ref=v1.3.1 # Version is auto-updated by the generating script.
5+
- https://github.com/kong/kubernetes-configuration/config/crd/ingress-controller-incubator?ref=v1.3.3 # Version is auto-updated by the generating script.

config/crd/kustomization.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
apiVersion: kustomize.config.k8s.io/v1beta1
33
kind: Kustomization
44
resources:
5-
- https://github.com/kong/kubernetes-configuration/config/crd/ingress-controller?ref=v1.3.1 # Version is auto-updated by the generating script.
5+
- https://github.com/kong/kubernetes-configuration/config/crd/ingress-controller?ref=v1.3.3 # Version is auto-updated by the generating script.

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ go 1.24.3
1717

1818
require (
1919
cloud.google.com/go/container v1.43.0
20-
github.com/Kong/sdk-konnect-go v0.2.22
20+
github.com/Kong/sdk-konnect-go v0.2.26
2121
github.com/Masterminds/sprig/v3 v3.3.0
2222
github.com/avast/retry-go/v4 v4.6.1
2323
github.com/blang/semver/v4 v4.0.0
@@ -31,7 +31,7 @@ require (
3131
github.com/jpillora/backoff v1.0.0
3232
github.com/kong/go-database-reconciler v1.23.0
3333
github.com/kong/go-kong v0.66.1
34-
github.com/kong/kubernetes-configuration v1.3.1
34+
github.com/kong/kubernetes-configuration v1.3.3
3535
github.com/kong/kubernetes-telemetry v0.1.9
3636
github.com/kong/kubernetes-testing-framework v0.47.2
3737
github.com/lithammer/dedent v1.1.0

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ github.com/Kong/go-diff v1.2.2 h1:KKKaqHc8IxuguFVIZMNt3bi6YuC/t9r7BGD8bOOpSgM=
2020
github.com/Kong/go-diff v1.2.2/go.mod h1:nlvdwVZQk3Rm+tbI0cDmKFrOjghtcZTrZBp+UruvvA8=
2121
github.com/Kong/gojsondiff v1.3.2 h1:qIOVq2mUXt+NXy8Be5gRUee9TP3Ve0MbQSafg9bXKZE=
2222
github.com/Kong/gojsondiff v1.3.2/go.mod h1:DiIxtU59q4alK7ecP+7k56C5UjgOviJ5gQVR2esEhYw=
23-
github.com/Kong/sdk-konnect-go v0.2.22 h1:7Jm+LdqqDaypg30DIVeg9QiNS9Eg80dmsDw8Fz2v96U=
24-
github.com/Kong/sdk-konnect-go v0.2.22/go.mod h1:xsmTIkBbmVyUh1nRFjQMOhxYIPDl+sMfmRmPuZHtwLE=
23+
github.com/Kong/sdk-konnect-go v0.2.26 h1:39zLa4oExkvY4GS/TIU1i40tUjSxoe+EyH9SuWECKP8=
24+
github.com/Kong/sdk-konnect-go v0.2.26/go.mod h1:xsmTIkBbmVyUh1nRFjQMOhxYIPDl+sMfmRmPuZHtwLE=
2525
github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ=
2626
github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE=
2727
github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI=
@@ -228,8 +228,8 @@ github.com/kong/go-database-reconciler v1.23.0 h1:Hwk1DAz546DDGuOj65tnMvDEhLHg2s
228228
github.com/kong/go-database-reconciler v1.23.0/go.mod h1:FhP9xPcigtDyq7jHS0EGRpwlVwx5+27caTeg2Yqf0Fs=
229229
github.com/kong/go-kong v0.66.1 h1:UVdemzcCpfXEl6O/VHdf0rT2bXdIO5ykuJbf2z1JTko=
230230
github.com/kong/go-kong v0.66.1/go.mod h1:wRMPAXGOB3kn53TF6zN4l2JhIWPUfXDFKNHkMHBB3iQ=
231-
github.com/kong/kubernetes-configuration v1.3.1 h1:mA36ROsWLA1/1PDPYC0mbTMB3D8sLG6FbsXBoq2LNog=
232-
github.com/kong/kubernetes-configuration v1.3.1/go.mod h1:wakGUhvbgYjrE9n0y9lXvNFcP2JTDOIoIoxJTTWw4oQ=
231+
github.com/kong/kubernetes-configuration v1.3.3 h1:UpEm6S85H3eJKD/OlfxoJ3tRHAaSEY5+7fbfR0q+SPw=
232+
github.com/kong/kubernetes-configuration v1.3.3/go.mod h1:+HRrz0eeoy7d5YJYiCkk736xaSq3y9fbV5SvucYo/tY=
233233
github.com/kong/kubernetes-telemetry v0.1.9 h1:XbDMZjZZclHO4oyJGW1mmZ6bzbTnANjcRAYsBg+jpno=
234234
github.com/kong/kubernetes-telemetry v0.1.9/go.mod h1:lBSbaQdJkoMMO0d+cJWopoJiJ+QHFBttbhCp5VRibJ8=
235235
github.com/kong/kubernetes-testing-framework v0.47.2 h1:+2Z9anTpbV/hwNeN+NFQz53BMU+g3QJydkweBp3tULo=

internal/dataplane/kongstate/kongupstreampolicy.go

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ func TranslateKongUpstreamPolicy(policy configurationv1beta1.KongUpstreamPolicyS
7575
HashOn: translateHashOn(policy.HashOn),
7676
HashOnHeader: translateHashOnHeader(policy.HashOn),
7777
HashOnURICapture: translateHashOnURICapture(policy.HashOn),
78-
HashOnCookie: translateHashOnCookie(policy.HashOn),
79-
HashOnCookiePath: translateHashOnCookiePath(policy.HashOn),
78+
HashOnCookie: translateHashOnCookie(policy.HashOn, policy.HashOnFallback),
79+
HashOnCookiePath: translateHashOnCookiePath(policy.HashOn, policy.HashOnFallback),
8080
HashOnQueryArg: translateHashOnQueryArg(policy.HashOn),
8181

8282
HashFallback: translateHashOn(policy.HashOnFallback),
@@ -90,7 +90,10 @@ func translateHashOn(hashOn *configurationv1beta1.KongUpstreamHash) *string {
9090
if hashOn == nil {
9191
return nil
9292
}
93-
// CRD validations will ensure only one of hashOn fields can be set, therefore the order doesn't matter.
93+
94+
// CRD validations will ensure only one of hashOn fields can be set, therefore
95+
// the order in which we check these doesn't matter.
96+
9497
switch {
9598
case hashOn.Input != nil:
9699
return lo.ToPtr(string(*hashOn.Input))
@@ -114,11 +117,20 @@ func translateHashOnHeader(hashOn *configurationv1beta1.KongUpstreamHash) *strin
114117
return hashOn.Header
115118
}
116119

117-
func translateHashOnCookie(hashOn *configurationv1beta1.KongUpstreamHash) *string {
118-
if hashOn == nil {
120+
func translateHashOnCookie(hashOn, hashOnFallback *configurationv1beta1.KongUpstreamHash) *string {
121+
if hashOn == nil && hashOnFallback == nil {
119122
return nil
120123
}
121-
return hashOn.Cookie
124+
125+
if hashOn != nil && hashOn.Cookie != nil {
126+
return hashOn.Cookie
127+
}
128+
if hashOnFallback != nil && hashOnFallback.Cookie != nil {
129+
return hashOnFallback.Cookie
130+
}
131+
132+
// This should not happen as this should be prevented by the CRD validation.
133+
return nil
122134
}
123135

124136
func translateHashOnQueryArg(hashOn *configurationv1beta1.KongUpstreamHash) *string {
@@ -135,11 +147,20 @@ func translateHashOnURICapture(hashOn *configurationv1beta1.KongUpstreamHash) *s
135147
return hashOn.URICapture
136148
}
137149

138-
func translateHashOnCookiePath(hashOn *configurationv1beta1.KongUpstreamHash) *string {
139-
if hashOn == nil {
150+
func translateHashOnCookiePath(hashOn, hashOnFallback *configurationv1beta1.KongUpstreamHash) *string {
151+
if hashOn == nil && hashOnFallback == nil {
140152
return nil
141153
}
142-
return hashOn.CookiePath
154+
155+
if hashOn != nil && hashOn.CookiePath != nil {
156+
return hashOn.CookiePath
157+
}
158+
if hashOnFallback != nil && hashOnFallback.CookiePath != nil {
159+
return hashOnFallback.CookiePath
160+
}
161+
162+
// This should not happen as this should be prevented by the CRD validation.
163+
return nil
143164
}
144165

145166
func translateHealthchecks(healthchecks *configurationv1beta1.KongUpstreamHealthcheck) *kong.Healthcheck {

internal/dataplane/kongstate/kongupstreampolicy_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,109 @@ func TestTranslateKongUpstreamPolicy(t *testing.T) {
193193
HashFallbackHeader: lo.ToPtr("bar"),
194194
},
195195
},
196+
{
197+
name: "KongUpstreamPolicySpec with hash-on header and hash-on fallback cookie",
198+
policySpec: configurationv1beta1.KongUpstreamPolicySpec{
199+
HashOn: &configurationv1beta1.KongUpstreamHash{
200+
Header: lo.ToPtr("foo"),
201+
},
202+
HashOnFallback: &configurationv1beta1.KongUpstreamHash{
203+
Cookie: lo.ToPtr("cookie-name"),
204+
CookiePath: lo.ToPtr("/cookie-path"),
205+
},
206+
},
207+
expectedUpstream: &kong.Upstream{
208+
HashOn: lo.ToPtr("header"),
209+
HashOnHeader: lo.ToPtr("foo"),
210+
HashFallback: lo.ToPtr("cookie"),
211+
HashOnCookie: lo.ToPtr("cookie-name"),
212+
HashOnCookiePath: lo.ToPtr("/cookie-path"),
213+
},
214+
},
215+
{
216+
name: "KongUpstreamPolicySpec with hash-on query-arg and hash-on fallback cookie",
217+
policySpec: configurationv1beta1.KongUpstreamPolicySpec{
218+
HashOn: &configurationv1beta1.KongUpstreamHash{
219+
QueryArg: lo.ToPtr("foo"),
220+
},
221+
HashOnFallback: &configurationv1beta1.KongUpstreamHash{
222+
Cookie: lo.ToPtr("cookie-name"),
223+
CookiePath: lo.ToPtr("/cookie-path"),
224+
},
225+
},
226+
expectedUpstream: &kong.Upstream{
227+
HashOn: lo.ToPtr("query_arg"),
228+
HashOnQueryArg: lo.ToPtr("foo"),
229+
HashFallback: lo.ToPtr("cookie"),
230+
HashOnCookie: lo.ToPtr("cookie-name"),
231+
HashOnCookiePath: lo.ToPtr("/cookie-path"),
232+
},
233+
},
234+
{
235+
name: "KongUpstreamPolicySpec with hash-on uri-capture and hash-on fallback cookie",
236+
policySpec: configurationv1beta1.KongUpstreamPolicySpec{
237+
HashOn: &configurationv1beta1.KongUpstreamHash{
238+
URICapture: lo.ToPtr("foo"),
239+
},
240+
HashOnFallback: &configurationv1beta1.KongUpstreamHash{
241+
Cookie: lo.ToPtr("cookie-name"),
242+
CookiePath: lo.ToPtr("/cookie-path"),
243+
},
244+
},
245+
expectedUpstream: &kong.Upstream{
246+
HashOn: lo.ToPtr("uri_capture"),
247+
HashOnURICapture: lo.ToPtr("foo"),
248+
HashFallback: lo.ToPtr("cookie"),
249+
HashOnCookie: lo.ToPtr("cookie-name"),
250+
HashOnCookiePath: lo.ToPtr("/cookie-path"),
251+
},
252+
},
253+
{
254+
// This will be blocked by CRD validation rules because according to
255+
// https://developer.konghq.com/gateway/entities/upstream/#consistent-hashing
256+
// if the primary hash_on is set to cookie, the hash_fallback is invalid
257+
// and cannot be used.
258+
name: "KongUpstreamPolicySpec with hash-on cookie and hash-on fallback cookie is incorrect and should not happen",
259+
policySpec: configurationv1beta1.KongUpstreamPolicySpec{
260+
HashOn: &configurationv1beta1.KongUpstreamHash{
261+
Cookie: lo.ToPtr("cookie-name"),
262+
CookiePath: lo.ToPtr("/cookie-path"),
263+
},
264+
HashOnFallback: &configurationv1beta1.KongUpstreamHash{
265+
Cookie: lo.ToPtr("cookie-name-2"),
266+
CookiePath: lo.ToPtr("/cookie-path-2"),
267+
},
268+
},
269+
expectedUpstream: &kong.Upstream{
270+
HashOn: lo.ToPtr("cookie"),
271+
HashOnCookie: lo.ToPtr("cookie-name"),
272+
HashOnCookiePath: lo.ToPtr("/cookie-path"),
273+
HashFallback: lo.ToPtr("cookie"),
274+
},
275+
},
276+
{
277+
// This will be blocked by CRD validation rules because according to
278+
// https://developer.konghq.com/gateway/entities/upstream/#consistent-hashing
279+
// if the primary hash_on is set to cookie, the hash_fallback is invalid
280+
// and cannot be used.
281+
name: "KongUpstreamPolicySpec with hash-on cookie and hash-on fallback header is incorrect and should not happen",
282+
policySpec: configurationv1beta1.KongUpstreamPolicySpec{
283+
HashOn: &configurationv1beta1.KongUpstreamHash{
284+
Cookie: lo.ToPtr("cookie-name"),
285+
CookiePath: lo.ToPtr("/cookie-path"),
286+
},
287+
HashOnFallback: &configurationv1beta1.KongUpstreamHash{
288+
Header: lo.ToPtr("header-name"),
289+
},
290+
},
291+
expectedUpstream: &kong.Upstream{
292+
HashOn: lo.ToPtr("cookie"),
293+
HashOnCookie: lo.ToPtr("cookie-name"),
294+
HashOnCookiePath: lo.ToPtr("/cookie-path"),
295+
HashFallback: lo.ToPtr("header"),
296+
HashFallbackHeader: lo.ToPtr("header-name"),
297+
},
298+
},
196299
{
197300
name: "KongUpstreamPolicySpec with hash-on cookie",
198301
policySpec: configurationv1beta1.KongUpstreamPolicySpec{

0 commit comments

Comments
 (0)