chore: clean codes

Signed-off-by: bitliu <bitliu@tencent.com>
envoyproxy · Feb 23, 2024 · cea05e6 · cea05e6
1 parent 66d2866
commit cea05e6
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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
@@ -1595,15 +1595,6 @@ _Appears in:_
 
 
 
-#### MergeType
-
-_Underlying type:_ _string_
-
-MergeType defines the type of merge operation
-
-_Appears in:_
-- [KubernetesPatchSpec](#kubernetespatchspec)
-
 
 
 #### MetricSinkType