Repository: envoyproxy/gateway

Issue:
adpot internals/utils/merge.Merge
              non blocking comment: would be good to dedup this and https://github.com/envoyproxy/gateway/blob/645b65fa16c6ed924bcb5c9abbdcd4a60e64e787/api/v1alpha1/kubernetes_helpers.go#L177 in a follow up

_Originally posted by @arkodg in https://github.com/envoyproxy/gateway/pull/5795#discussion_r2063956492_
            

Hints from issue comments:

Solution patch:
diff --git a/api/v1alpha1/kubernetes_helpers.go b/api/v1alpha1/kubernetes_helpers.go
index 6dd6b5fbf..e944df1c2 100644
--- a/api/v1alpha1/kubernetes_helpers.go
+++ b/api/v1alpha1/kubernetes_helpers.go
@@ -11,7 +11,6 @@ import (
 
 	jsonpatch "github.com/evanphx/json-patch"
 	appsv1 "k8s.io/api/apps/v1"
-	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
 	policyv1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -158,150 +157,6 @@ func (hpa *KubernetesHorizontalPodAutoscalerSpec) setDefault() {
 	}
 }
 
-// ApplyMergePatch applies a merge patch to a deployment based on the merge type
-func (deployment *KubernetesDeploymentSpec) ApplyMergePatch(old *appsv1.Deployment) (*appsv1.Deployment, error) {
-	if deployment.Patch == nil {
-		return old, nil
-	}
-
-	var patchedJSON []byte
-	var err error
-
-	// Serialize the current deployment to JSON
-	originalJSON, err := json.Marshal(old)
-	if err != nil {
-		return nil, fmt.Errorf("error marshaling original deployment: %w", err)
-	}
-
-	switch {
-	case deployment.Patch.Type == nil || *deployment.Patch.Type == StrategicMerge:
-		patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, deployment.Patch.Value.Raw, appsv1.Deployment{})
-	case *deployment.Patch.Type == JSONMerge:
-		patchedJSON, err = jsonpatch.MergePatch(originalJSON, deployment.Patch.Value.Raw)
-	default:
-		return nil, fmt.Errorf("unsupported merge type: %s", *deployment.Patch.Type)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("error applying merge patch: %w", err)
-	}
-
-	// Deserialize the patched JSON into a new deployment object
-	var patchedDeployment appsv1.Deployment
-	if err := json.Unmarshal(patchedJSON, &patchedDeployment); err != nil {
-		return nil, fmt.Errorf("error unmarshaling patched deployment: %w", err)
-	}
-
-	return &patchedDeployment, nil
-}
-
-// ApplyMergePatch applies a merge patch to a daemonset based on the merge type
-func (daemonset *KubernetesDaemonSetSpec) ApplyMergePatch(old *appsv1.DaemonSet) (*appsv1.DaemonSet, error) {
-	if daemonset.Patch == nil {
-		return old, nil
-	}
-
-	var patchedJSON []byte
-	var err error
-
-	// Serialize the current daemonset to JSON
-	originalJSON, err := json.Marshal(old)
-	if err != nil {
-		return nil, fmt.Errorf("error marshaling original daemonset: %w", err)
-	}
-
-	switch {
-	case daemonset.Patch.Type == nil || *daemonset.Patch.Type == StrategicMerge:
-		patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, daemonset.Patch.Value.Raw, appsv1.DaemonSet{})
-	case *daemonset.Patch.Type == JSONMerge:
-		patchedJSON, err = jsonpatch.MergePatch(originalJSON, daemonset.Patch.Value.Raw)
-	default:
-		return nil, fmt.Errorf("unsupported merge type: %s", *daemonset.Patch.Type)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("error applying merge patch: %w", err)
-	}
-
-	// Deserialize the patched JSON into a new daemonset object
-	var patchedDaemonSet appsv1.DaemonSet
-	if err := json.Unmarshal(patchedJSON, &patchedDaemonSet); err != nil {
-		return nil, fmt.Errorf("error unmarshaling patched daemonset: %w", err)
-	}
-
-	return &patchedDaemonSet, nil
-}
-
-// ApplyMergePatch applies a merge patch to a service based on the merge type
-func (service *KubernetesServiceSpec) ApplyMergePatch(old *corev1.Service) (*corev1.Service, error) {
-	if service.Patch == nil {
-		return old, nil
-	}
-
-	var patchedJSON []byte
-	var err error
-
-	// Serialize the current service to JSON
-	originalJSON, err := json.Marshal(old)
-	if err != nil {
-		return nil, fmt.Errorf("error marshaling original service: %w", err)
-	}
-
-	switch {
-	case service.Patch.Type == nil || *service.Patch.Type == StrategicMerge:
-		patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, service.Patch.Value.Raw, corev1.Service{})
-	case *service.Patch.Type == JSONMerge:
-		patchedJSON, err = jsonpatch.MergePatch(originalJSON, service.Patch.Value.Raw)
-	default:
-		return nil, fmt.Errorf("unsupported merge type: %s", *service.Patch.Type)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("error applying merge patch: %w", err)
-	}
-
-	// Deserialize the patched JSON into a new service object
-	var patchedService corev1.Service
-	if err := json.Unmarshal(patchedJSON, &patchedService); err != nil {
-		return nil, fmt.Errorf("error unmarshaling patched service: %w", err)
-	}
-
-	return &patchedService, nil
-}
-
-// ApplyMergePatch applies a merge patch to a HorizontalPodAutoscaler based on the merge type
-func (hpa *KubernetesHorizontalPodAutoscalerSpec) ApplyMergePatch(old *autoscalingv2.HorizontalPodAutoscaler) (*autoscalingv2.HorizontalPodAutoscaler, error) {
-	if hpa.Patch == nil {
-		return old, nil
-	}
-
-	var patchedJSON []byte
-	var err error
-
-	// Serialize the current HPA to JSON
-	originalJSON, err := json.Marshal(old)
-	if err != nil {
-		return nil, fmt.Errorf("error marshaling original HorizontalPodAutoscaler: %w", err)
-	}
-
-	switch {
-	case hpa.Patch.Type == nil || *hpa.Patch.Type == StrategicMerge:
-		patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, hpa.Patch.Value.Raw, autoscalingv2.HorizontalPodAutoscaler{})
-	case *hpa.Patch.Type == JSONMerge:
-		patchedJSON, err = jsonpatch.MergePatch(originalJSON, hpa.Patch.Value.Raw)
-	default:
-		return nil, fmt.Errorf("unsupported merge type: %s", *hpa.Patch.Type)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("error applying merge patch: %w", err)
-	}
-
-	// Deserialize the patched JSON into a new HorizontalPodAutoscaler object
-	var patchedHpa autoscalingv2.HorizontalPodAutoscaler
-	if err := json.Unmarshal(patchedJSON, &patchedHpa); err != nil {
-		return nil, fmt.Errorf("error unmarshaling patched HorizontalPodAutoscaler: %w", err)
-	}
-
-	return &patchedHpa, nil
-}
-
 // ApplyMergePatch applies a merge patch to a PodDisruptionBudget based on the merge type
 func (pdb *KubernetesPodDisruptionBudgetSpec) ApplyMergePatch(old *policyv1.PodDisruptionBudget) (*policyv1.PodDisruptionBudget, error) {
 	if pdb.Patch == nil {
diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider.go b/internal/infrastructure/kubernetes/proxy/resource_provider.go
index 002e08a46..ca4543d07 100644
--- a/internal/infrastructure/kubernetes/proxy/resource_provider.go
+++ b/internal/infrastructure/kubernetes/proxy/resource_provider.go
@@ -24,6 +24,7 @@ import (
 	"github.com/envoyproxy/gateway/internal/infrastructure/common"
 	"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"
 	"github.com/envoyproxy/gateway/internal/ir"
+	"github.com/envoyproxy/gateway/internal/utils"
 	"github.com/envoyproxy/gateway/internal/xds/bootstrap"
 )
 
@@ -38,7 +39,7 @@ const (
 	// trusted CA certificate.
 	XdsTLSCaFilepath = "/certs/ca.crt"
 
-	// XdsTLSCertFileName is the file name of the xDS server TLS certificate.
+	// XdsTLSCaFileName is the file name of the xDS server TLS certificate.
 	XdsTLSCaFileName = "ca.crt"
 )
 
@@ -211,7 +212,7 @@ func (r *ResourceRender) Service() (*corev1.Service, error) {
 
 	// apply merge patch to service
 	var err error
-	if svc, err = envoyServiceConfig.ApplyMergePatch(svc); err != nil {
+	if svc, err = utils.MergeWithPatch(svc, envoyServiceConfig.Patch); err != nil {
 		return nil, err
 	}
 
@@ -344,7 +345,7 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
 	}
 
 	// apply merge patch to deployment
-	if deployment, err = deploymentConfig.ApplyMergePatch(deployment); err != nil {
+	if deployment, err = utils.MergeWithPatch(deployment, deploymentConfig.Patch); err != nil {
 		return nil, err
 	}
 
@@ -413,8 +414,8 @@ func (r *ResourceRender) DaemonSet() (*appsv1.DaemonSet, error) {
 		daemonSet.Name = r.Name()
 	}
 
-	// apply merge patch to daemonset
-	if daemonSet, err = daemonSetConfig.ApplyMergePatch(daemonSet); err != nil {
+	// apply merge patch to DaemonSet
+	if daemonSet, err = utils.MergeWithPatch(daemonSet, daemonSetConfig.Patch); err != nil {
 		return nil, err
 	}
 
@@ -516,8 +517,8 @@ func (r *ResourceRender) HorizontalPodAutoscaler() (*autoscalingv2.HorizontalPod
 		hpa.Spec.ScaleTargetRef.Name = r.Name()
 	}
 
-	hpa, err := hpaConfig.ApplyMergePatch(hpa)
-	if err != nil {
+	var err error
+	if hpa, err = utils.MergeWithPatch(hpa, hpaConfig.Patch); err != nil {
 		return nil, err
 	}
 
diff --git a/internal/infrastructure/kubernetes/ratelimit/resource_provider.go b/internal/infrastructure/kubernetes/ratelimit/resource_provider.go
index a4016ac85..66219636c 100644
--- a/internal/infrastructure/kubernetes/ratelimit/resource_provider.go
+++ b/internal/infrastructure/kubernetes/ratelimit/resource_provider.go
@@ -22,6 +22,7 @@ import (
 
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
 	"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"
+	"github.com/envoyproxy/gateway/internal/utils"
 )
 
 // ResourceKind indicates the main resources of envoy-ratelimit,
@@ -282,7 +283,8 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
 
 	// apply merge patch to deployment
 	var err error
-	if deployment, err = r.rateLimitDeployment.ApplyMergePatch(deployment); err != nil {
+	deploymentConfig := r.rateLimitDeployment
+	if deployment, err = utils.MergeWithPatch(deployment, deploymentConfig.Patch); err != nil {
 		return nil, err
 	}
 
@@ -335,7 +337,7 @@ func (r *ResourceRender) HorizontalPodAutoscaler() (*autoscalingv2.HorizontalPod
 		hpa.Spec.ScaleTargetRef.Name = r.Name()
 	}
 
-	if hpa, err = hpaConfig.ApplyMergePatch(hpa); err != nil {
+	if hpa, err = utils.MergeWithPatch(hpa, hpaConfig.Patch); err != nil {
 		return nil, err
 	}
 
diff --git a/internal/utils/merge.go b/internal/utils/merge.go
index cb5d890dd..337147ed0 100644
--- a/internal/utils/merge.go
+++ b/internal/utils/merge.go
@@ -16,11 +16,23 @@ import (
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
 )
 
-func Merge[T client.Object](original, patch T, mergeType egv1a1.MergeType) (T, error) {
+func MergeWithPatch[T client.Object](original T, patch *egv1a1.KubernetesPatchSpec) (T, error) {
+	if patch == nil {
+		return original, nil
+	}
+
+	mergeType := egv1a1.StrategicMerge
+	if patch.Type != nil {
+		mergeType = *patch.Type
+	}
+
+	return mergeInternal(original, patch.Value.Raw, mergeType)
+}
+
+func mergeInternal[T client.Object](original T, patchJSON []byte, mergeType egv1a1.MergeType) (T, error) {
 	var (
 		patchedJSON  []byte
 		originalJSON []byte
-		patchJSON    []byte
 		err          error
 		empty        T
 	)
@@ -29,13 +41,9 @@ func Merge[T client.Object](original, patch T, mergeType egv1a1.MergeType) (T, e
 	if err != nil {
 		return empty, fmt.Errorf("error marshaling original service: %w", err)
 	}
-	patchJSON, err = json.Marshal(patch)
-	if err != nil {
-		return empty, fmt.Errorf("error marshaling original service: %w", err)
-	}
 	switch mergeType {
 	case egv1a1.StrategicMerge:
-		patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, patchJSON, egv1a1.BackendTrafficPolicy{})
+		patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, patchJSON, empty)
 		if err != nil {
 			return empty, fmt.Errorf("error during strategic merge: %w", err)
 		}
@@ -45,7 +53,7 @@ func Merge[T client.Object](original, patch T, mergeType egv1a1.MergeType) (T, e
 			return empty, fmt.Errorf("error during JSON merge: %w", err)
 		}
 	default:
-		return empty, fmt.Errorf("unsupported merge type: %s", mergeType)
+		return empty, fmt.Errorf("unsupported merge type: %v", mergeType)
 	}
 
 	res := new(T)
@@ -55,3 +63,17 @@ func Merge[T client.Object](original, patch T, mergeType egv1a1.MergeType) (T, e
 
 	return *res, nil
 }
+
+func Merge[T client.Object](original, patch T, mergeType egv1a1.MergeType) (T, error) {
+	var (
+		patchJSON []byte
+		err       error
+		empty     T
+	)
+
+	patchJSON, err = json.Marshal(patch)
+	if err != nil {
+		return empty, fmt.Errorf("error marshaling original service: %w", err)
+	}
+	return mergeInternal(original, patchJSON, mergeType)
+}



Test patch:
diff --git a/site/content/en/latest/tasks/traffic/global-rate-limit.md b/site/content/en/latest/tasks/traffic/global-rate-limit.md
index 47eac33bc..63f940b15 100644
--- a/site/content/en/latest/tasks/traffic/global-rate-limit.md
+++ b/site/content/en/latest/tasks/traffic/global-rate-limit.md
@@ -493,6 +493,9 @@ spec:
         - headers:
           - type: Distinct
             name: x-user-id
+          - name: x-user-id
+            value: admin
+            invert: true
         limit:
           requests: 3
           unit: Hour