From cea05e657bc26f86c1f1c3fffe2aeb2c4d100d15 Mon Sep 17 00:00:00 2001 From: bitliu Date: Fri, 23 Feb 2024 11:17:18 +0800 Subject: [PATCH] chore: clean codes Signed-off-by: bitliu --- api/v1alpha1/kubernetes_helpers.go | 10 +++---- api/v1alpha1/shared_types.go | 2 +- .../validation/envoyproxy_validate.go | 4 +-- .../validation/envoyproxy_validate_test.go | 28 +++++++++++++++++-- api/v1alpha1/zz_generated.deepcopy.go | 5 ++++ .../kubernetes/proxy/resource_provider.go | 1 + .../proxy/resource_provider_test.go | 3 +- .../ratelimit/resource_provider_test.go | 3 +- site/content/en/latest/api/extension_types.md | 9 ------ 9 files changed, 43 insertions(+), 22 deletions(-) diff --git a/api/v1alpha1/kubernetes_helpers.go b/api/v1alpha1/kubernetes_helpers.go index 650d84e73d55..a35c8899f115 100644 --- a/api/v1alpha1/kubernetes_helpers.go +++ b/api/v1alpha1/kubernetes_helpers.go @@ -142,15 +142,13 @@ func (deployment *KubernetesDeploymentSpec) ApplyMergePatch(old *appv1.Deploymen return nil, fmt.Errorf("error marshaling original deployment: %w", err) } - switch deployment.Patch.Type { - case StrategicMerge, "": + if deployment.Patch.Type == nil || *deployment.Patch.Type == StrategicMerge { patchedJSON, err = strategicpatch.StrategicMergePatch(originalJSON, deployment.Patch.Value.Raw, appv1.Deployment{}) - case JSONMerge: + } else if *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) + } else { + return nil, fmt.Errorf("unsupported merge type: %s", *deployment.Patch.Type) } - if err != nil { return nil, fmt.Errorf("error applying merge patch: %w", err) } diff --git a/api/v1alpha1/shared_types.go b/api/v1alpha1/shared_types.go index 65183eb4e556..23cc2a1ef090 100644 --- a/api/v1alpha1/shared_types.go +++ b/api/v1alpha1/shared_types.go @@ -393,7 +393,7 @@ type KubernetesPatchSpec struct { // // By default, StrategicMerge is used as the patch type. // +optional - Type MergeType `json:"type,omitempty"` + Type *MergeType `json:"type,omitempty"` // Object contains the raw configuration for merged object Value apiextensionsv1.JSON `json:"value"` diff --git a/api/v1alpha1/validation/envoyproxy_validate.go b/api/v1alpha1/validation/envoyproxy_validate.go index 1298dc3f0be4..6dc7e116c895 100644 --- a/api/v1alpha1/validation/envoyproxy_validate.go +++ b/api/v1alpha1/validation/envoyproxy_validate.go @@ -92,8 +92,8 @@ func validateDeployment(spec *egv1a1.EnvoyProxySpec) []error { if patch.Value.Raw == nil { errs = append(errs, fmt.Errorf("envoy deployment patch object cannot be empty")) } - if patch.Type != "" && patch.Type != egv1a1.JSONMerge && patch.Type != egv1a1.StrategicMerge { - errs = append(errs, fmt.Errorf("unsupported envoy deployment patch type %s", patch.Type)) + if patch.Type != nil && *patch.Type != egv1a1.JSONMerge && *patch.Type != egv1a1.StrategicMerge { + errs = append(errs, fmt.Errorf("unsupported envoy deployment patch type %s", *patch.Type)) } } } diff --git a/api/v1alpha1/validation/envoyproxy_validate_test.go b/api/v1alpha1/validation/envoyproxy_validate_test.go index c69c42d9ab5e..53ab043f3e9a 100644 --- a/api/v1alpha1/validation/envoyproxy_validate_test.go +++ b/api/v1alpha1/validation/envoyproxy_validate_test.go @@ -16,6 +16,7 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" "k8s.io/utils/ptr" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -484,7 +485,7 @@ func TestValidateEnvoyProxy(t *testing.T) { Kubernetes: &egv1a1.EnvoyProxyKubernetesProvider{ EnvoyDeployment: &egv1a1.KubernetesDeploymentSpec{ Patch: &egv1a1.KubernetesPatchSpec{ - Type: egv1a1.JSONMerge, + Type: (*egv1a1.MergeType)(pointer.String(string(egv1a1.StrategicMerge))), }, }, }, @@ -505,7 +506,30 @@ func TestValidateEnvoyProxy(t *testing.T) { Kubernetes: &egv1a1.EnvoyProxyKubernetesProvider{ EnvoyDeployment: &egv1a1.KubernetesDeploymentSpec{ Patch: &egv1a1.KubernetesPatchSpec{ - Type: egv1a1.JSONMerge, + Type: (*egv1a1.MergeType)(pointer.String(string(egv1a1.JSONMerge))), + Value: v1.JSON{ + Raw: []byte("{}"), + }, + }, + }, + }, + }, + }, + }, + expected: true, + }, { + name: "should valid when patch type is empty and object is not empty", + proxy: &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egv1a1.EnvoyProxySpec{ + Provider: &egv1a1.EnvoyProxyProvider{ + Type: egv1a1.ProviderTypeKubernetes, + Kubernetes: &egv1a1.EnvoyProxyKubernetesProvider{ + EnvoyDeployment: &egv1a1.KubernetesDeploymentSpec{ + Patch: &egv1a1.KubernetesPatchSpec{ Value: v1.JSON{ Raw: []byte("{}"), }, diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 6d01673235d3..7579fb4a5a8b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -2189,6 +2189,11 @@ func (in *KubernetesHorizontalPodAutoscalerSpec) DeepCopy() *KubernetesHorizonta // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KubernetesPatchSpec) DeepCopyInto(out *KubernetesPatchSpec) { *out = *in + if in.Type != nil { + in, out := &in.Type, &out.Type + *out = new(MergeType) + **out = **in + } in.Value.DeepCopyInto(&out.Value) } diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider.go b/internal/infrastructure/kubernetes/proxy/resource_provider.go index 725a1202b455..ebe90a796a0a 100644 --- a/internal/infrastructure/kubernetes/proxy/resource_provider.go +++ b/internal/infrastructure/kubernetes/proxy/resource_provider.go @@ -268,6 +268,7 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) { // apply merge patch to deployment if merged, err := deploymentConfig.ApplyMergePatch(deployment); err == nil { deployment = merged + } else { } return deployment, nil diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go index 57d96ad037e3..6d7e12f3f640 100644 --- a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go +++ b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go @@ -19,6 +19,7 @@ import ( v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" "k8s.io/utils/ptr" "sigs.k8s.io/yaml" @@ -139,7 +140,7 @@ func TestDeployment(t *testing.T) { infra: newTestInfra(), deploy: &egv1a1.KubernetesDeploymentSpec{ Patch: &egv1a1.KubernetesPatchSpec{ - Type: egv1a1.StrategicMerge, + Type: (*egv1a1.MergeType)(pointer.String(string(egv1a1.StrategicMerge))), Value: v1.JSON{ Raw: []byte("{\"spec\":{\"template\":{\"spec\":{\"hostNetwork\":true,\"dnsPolicy\":\"ClusterFirstWithHostNet\"}}}}"), }, diff --git a/internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go b/internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go index 69076d91f223..7200a30840a9 100644 --- a/internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go +++ b/internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go @@ -18,6 +18,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" "k8s.io/utils/ptr" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/yaml" @@ -152,7 +153,7 @@ func TestDeployment(t *testing.T) { rateLimit: rateLimit, deploy: &egv1a1.KubernetesDeploymentSpec{ Patch: &egv1a1.KubernetesPatchSpec{ - Type: egv1a1.StrategicMerge, + Type: (*egv1a1.MergeType)(pointer.String(string(egv1a1.StrategicMerge))), Value: v1.JSON{ Raw: []byte("{\"spec\":{\"template\":{\"spec\":{\"hostNetwork\":true,\"dnsPolicy\":\"ClusterFirstWithHostNet\"}}}}"), }, diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 815d915fd458..a5f0bb653158 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1595,15 +1595,6 @@ _Appears in:_ -#### MergeType - -_Underlying type:_ _string_ - -MergeType defines the type of merge operation - -_Appears in:_ -- [KubernetesPatchSpec](#kubernetespatchspec) - #### MetricSinkType