fix(meshloadbalancingstrategy): deprecate SourceIP and use Connection (…

…#12111) Signed-off-by: Lukasz Dziedziak <[email protected]> Co-authored-by: Bart Smykla <[email protected]>
kumahq · Dec 2, 2024 · 7a5db23 · 7a5db23
1 parent 4fa5da8
commit 7a5db23
Show file tree

Hide file tree

Showing 22 changed files with 187 additions and 3 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -6,6 +6,15 @@ with `x.y.z` being the version you are planning to upgrade to.
 If such a section does not exist, the upgrade you want to perform
 does not have any particular instructions.
 
+## Upgrade to `2.10.x`
+
+### MeshLoadBalancingStrategy
+
+#### Deprecation of `hashPolicies.type: SourceIP` and `maglev.type: SourceIP`
+
+The documentation did not mention the `SourceIP` type, but it was possible to create a policy using it instead of `Connection`. Since `SourceIP` 
+is not a correct value, we have decided to deprecate it. If you are using `SourceIP` in your policy, please update it to use `Connection` instead.
+
 ## Upgrade to `2.9.x`
 
 ### MeshAccessLog

diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml
@@ -3078,6 +3078,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState
@@ -3199,6 +3200,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState

diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml
@@ -3078,6 +3078,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState
@@ -3199,6 +3200,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState

diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml
@@ -3098,6 +3098,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState
@@ -3219,6 +3220,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState

diff --git a/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml b/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml
@@ -4642,6 +4642,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState
@@ -4763,6 +4764,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState

diff --git a/deployments/charts/kuma/crds/kuma.io_meshloadbalancingstrategies.yaml b/deployments/charts/kuma/crds/kuma.io_meshloadbalancingstrategies.yaml
@@ -237,6 +237,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState
@@ -358,6 +359,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState

diff --git a/docs/generated/openapi.yaml b/docs/generated/openapi.yaml
@@ -6609,6 +6609,7 @@ components:
                                       enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState
@@ -6781,6 +6782,7 @@ components:
                                       enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState

diff --git a/docs/generated/raw/crds/kuma.io_meshloadbalancingstrategies.yaml b/docs/generated/raw/crds/kuma.io_meshloadbalancingstrategies.yaml
@@ -237,6 +237,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState
@@ -358,6 +359,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState

diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/deprecated.go b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/deprecated.go
@@ -1,9 +1,44 @@
 package v1alpha1
 
 import (
+	"fmt"
+
 	"github.com/kumahq/kuma/pkg/plugins/policies/core/jsonpatch/validators"
 )
 
 func (t *MeshLoadBalancingStrategyResource) Deprecations() []string {
-	return validators.TopLevelTargetRefDeprecations(t.Spec.TargetRef)
+	deprecations := validateHashPoliciesType(t.Spec.To)
+	deprecations = append(deprecations, validators.TopLevelTargetRefDeprecations(t.Spec.TargetRef)...)
+	return deprecations
+}
+
+func validateHashPoliciesType(confs []To) []string {
+	deprecations := []string{}
+	for ruleIdx, conf := range confs {
+		if conf.Default.LoadBalancer == nil {
+			continue
+		}
+
+		switch conf.Default.LoadBalancer.Type {
+		case RingHashType:
+			if conf.Default.LoadBalancer.RingHash == nil || conf.Default.LoadBalancer.RingHash.HashPolicies == nil {
+				continue
+			}
+			for lbIdx, lbConf := range *conf.Default.LoadBalancer.RingHash.HashPolicies {
+				if lbConf.Type == SourceIPType {
+					deprecations = append(deprecations, fmt.Sprintf("%s type for 'spec.to[%d].default.loadBalancer.ringHash.hashPolicies[%d].type' is deprecated, use %s instead", SourceIPType, ruleIdx, lbIdx, ConnectionType))
+				}
+			}
+		case MaglevType:
+			if conf.Default.LoadBalancer.Maglev == nil || conf.Default.LoadBalancer.Maglev.HashPolicies == nil {
+				continue
+			}
+			for lbIdx, lbConf := range *conf.Default.LoadBalancer.Maglev.HashPolicies {
+				if lbConf.Type == SourceIPType {
+					deprecations = append(deprecations, fmt.Sprintf("%s type for 'spec.to[%d].default.loadBalancer.maglev.hashPolicies[%d].type' is deprecated, use %s instead", SourceIPType, ruleIdx, lbIdx, ConnectionType))
+				}
+			}
+		}
+	}
+	return deprecations
 }
diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/meshloadbalancingstrategy.go b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/meshloadbalancingstrategy.go
@@ -197,13 +197,14 @@ type RingHash struct {
 	HashPolicies *[]HashPolicy `json:"hashPolicies,omitempty"`
 }
 
-// +kubebuilder:validation:Enum=Header;Cookie;SourceIP;QueryParameter;FilterState
+// +kubebuilder:validation:Enum=Header;Cookie;Connection;SourceIP;QueryParameter;FilterState
 type HashPolicyType string
 
 const (
 	HeaderType         HashPolicyType = "Header"
 	CookieType         HashPolicyType = "Cookie"
 	ConnectionType     HashPolicyType = "Connection"
+	SourceIPType       HashPolicyType = "SourceIP"
 	QueryParameterType HashPolicyType = "QueryParameter"
 	FilterStateType    HashPolicyType = "FilterState"
 )

diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/schema.yaml b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/schema.yaml
@@ -199,6 +199,7 @@ properties:
                                 enum:
                                   - Header
                                   - Cookie
+                                  - Connection
                                   - SourceIP
                                   - QueryParameter
                                   - FilterState
@@ -314,6 +315,7 @@ properties:
                                 enum:
                                   - Header
                                   - Cookie
+                                  - Connection
                                   - SourceIP
                                   - QueryParameter
                                   - FilterState

diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go
@@ -249,7 +249,7 @@ func validateHashPolicies(conf *[]HashPolicy) validators.ValidationError {
 			if policy.FilterState == nil {
 				verr.AddViolationAt(path.Field("filterState"), validators.MustBeDefined)
 			}
-		case ConnectionType:
+		case ConnectionType, SourceIPType:
 			if policy.Connection == nil {
 				verr.AddViolationAt(path.Field("connection"), validators.MustBeDefined)
 			}

diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go
@@ -147,6 +147,10 @@ to:
 					Field:   "spec.to[0].default.loadBalancer.maglev.hashPolicies[1].cookie",
 					Message: "must be defined",
 				},
+				{
+					Field:   "spec.to[0].default.loadBalancer.maglev.hashPolicies[2].connection",
+					Message: "must be defined",
+				},
 				{
 					Field:   "spec.to[0].default.loadBalancer.maglev.hashPolicies[3].queryParameter",
 					Message: "must be defined",

diff --git a/...ugins/policies/meshloadbalancingstrategy/k8s/crd/kuma.io_meshloadbalancingstrategies.yaml b/...ugins/policies/meshloadbalancingstrategy/k8s/crd/kuma.io_meshloadbalancingstrategies.yaml
@@ -237,6 +237,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState
@@ -358,6 +359,7 @@ spec:
                                         enum:
                                         - Header
                                         - Cookie
+                                        - Connection
                                         - SourceIP
                                         - QueryParameter
                                         - FilterState

diff --git a/...gins/runtime/k8s/webhooks/testdata/validation/cli-user_create_mtlbs.federated.golden.yaml b/...gins/runtime/k8s/webhooks/testdata/validation/cli-user_create_mtlbs.federated.golden.yaml
@@ -0,0 +1,15 @@
+Patches: null
+allowed: false
+status:
+  code: 403
+  details:
+    causes:
+    - field: metadata.labels[kuma.io/origin]
+      message: cannot be empty
+      reason: FieldValueInvalid
+  message: Operation not allowed. Applying policies on Zone CP requires 'kuma.io/origin'
+    label to be set to 'zone'.
+  metadata: {}
+  reason: Forbidden
+  status: Failure
+uid: "12345"
diff --git a/...plugins/runtime/k8s/webhooks/testdata/validation/cli-user_create_mtlbs.global.golden.yaml b/...plugins/runtime/k8s/webhooks/testdata/validation/cli-user_create_mtlbs.global.golden.yaml
@@ -0,0 +1,11 @@
+Patches: null
+allowed: true
+status:
+  code: 200
+  metadata: {}
+uid: "12345"
+warnings:
+- SourceIP type for 'spec.to[0].default.loadBalancer.ringHash.hashPolicies[0].type'
+  is deprecated, use Connection instead
+- MeshService value for 'targetRef.kind' is deprecated, use MeshSubset with 'kuma.io/service'
+  tag instead
diff --git a/pkg/plugins/runtime/k8s/webhooks/testdata/validation/cli-user_create_mtlbs.input.yaml b/pkg/plugins/runtime/k8s/webhooks/testdata/validation/cli-user_create_mtlbs.input.yaml
@@ -0,0 +1,23 @@
+# user=cli-user,operation=CREATE,namespace=kuma-system
+apiVersion: kuma.io/v1alpha1
+kind: MeshLoadBalancingStrategy
+metadata:
+  name: mlbs-deprecated
+  labels:
+    kuma.io/mesh: default
+spec:
+  targetRef:
+    kind: MeshService
+    name: frontend
+  to:
+  - targetRef:
+      kind: MeshService
+      name: backend
+    default:
+      loadBalancer:
+        type: RingHash
+        ringHash:
+          hashPolicies:
+          - type: SourceIP
+            connection:
+              sourceIP: true
diff --git a/.../runtime/k8s/webhooks/testdata/validation/cli-user_create_mtlbs.non-federated.golden.yaml b/.../runtime/k8s/webhooks/testdata/validation/cli-user_create_mtlbs.non-federated.golden.yaml
@@ -0,0 +1,11 @@
+Patches: null
+allowed: true
+status:
+  code: 200
+  metadata: {}
+uid: "12345"
+warnings:
+- SourceIP type for 'spec.to[0].default.loadBalancer.ringHash.hashPolicies[0].type'
+  is deprecated, use Connection instead
+- MeshService value for 'targetRef.kind' is deprecated, use MeshSubset with 'kuma.io/service'
+  tag instead
diff --git a/...lugins/runtime/k8s/webhooks/testdata/validation/cp-user_create_mlbs.federated.golden.yaml b/...lugins/runtime/k8s/webhooks/testdata/validation/cp-user_create_mlbs.federated.golden.yaml
@@ -0,0 +1,11 @@
+Patches: null
+allowed: true
+status:
+  code: 200
+  metadata: {}
+uid: "12345"
+warnings:
+- SourceIP type for 'spec.to[0].default.loadBalancer.ringHash.hashPolicies[0].type'
+  is deprecated, use Connection instead
+- MeshService value for 'targetRef.kind' is deprecated, use MeshSubset with 'kuma.io/service'
+  tag instead
diff --git a/pkg/plugins/runtime/k8s/webhooks/testdata/validation/cp-user_create_mlbs.global.golden.yaml b/pkg/plugins/runtime/k8s/webhooks/testdata/validation/cp-user_create_mlbs.global.golden.yaml
@@ -0,0 +1,11 @@
+Patches: null
+allowed: true
+status:
+  code: 200
+  metadata: {}
+uid: "12345"
+warnings:
+- SourceIP type for 'spec.to[0].default.loadBalancer.ringHash.hashPolicies[0].type'
+  is deprecated, use Connection instead
+- MeshService value for 'targetRef.kind' is deprecated, use MeshSubset with 'kuma.io/service'
+  tag instead
diff --git a/pkg/plugins/runtime/k8s/webhooks/testdata/validation/cp-user_create_mlbs.input.yaml b/pkg/plugins/runtime/k8s/webhooks/testdata/validation/cp-user_create_mlbs.input.yaml
@@ -0,0 +1,24 @@
+# user=system:serviceaccount:kuma-system:kuma-control-plane,operation=CREATE
+kind: MeshLoadBalancingStrategy
+apiVersion: kuma.io/v1alpha1
+metadata:
+  name: ring-hash
+  namespace: kuma-system
+  labels:
+    kuma.io/mesh: default
+spec:
+  targetRef:
+    kind: MeshService
+    name: frontend
+  to:
+  - targetRef:
+      kind: MeshService
+      name: backend
+    default:
+      loadBalancer:
+        type: RingHash
+        ringHash:
+          hashPolicies:
+          - type: SourceIP
+            connection:
+              sourceIP: true
diff --git a/...ns/runtime/k8s/webhooks/testdata/validation/cp-user_create_mlbs.non-federated.golden.yaml b/...ns/runtime/k8s/webhooks/testdata/validation/cp-user_create_mlbs.non-federated.golden.yaml
@@ -0,0 +1,11 @@
+Patches: null
+allowed: true
+status:
+  code: 200
+  metadata: {}
+uid: "12345"
+warnings:
+- SourceIP type for 'spec.to[0].default.loadBalancer.ringHash.hashPolicies[0].type'
+  is deprecated, use Connection instead
+- MeshService value for 'targetRef.kind' is deprecated, use MeshSubset with 'kuma.io/service'
+  tag instead