From 199b62369e63414f3032f50fe1ab5f3573ff15cb Mon Sep 17 00:00:00 2001
From: Kent Dong <ch3cho@qq.com>
Date: Sun, 29 Sep 2024 16:06:56 +0800
Subject: [PATCH] fix: Fix some bugs in header control configurations (#348)

---
 .../kubernetes/KubernetesModelConverter.java  | 20 +++++++++---
 frontend/src/locales/en-US/translation.json   |  6 +++-
 frontend/src/locales/zh-CN/translation.json   |  6 +++-
 .../plugin/components/HeaderModify/index.tsx  | 31 ++++++++++++++++---
 4 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/backend/sdk/src/main/java/com/alibaba/higress/sdk/service/kubernetes/KubernetesModelConverter.java b/backend/sdk/src/main/java/com/alibaba/higress/sdk/service/kubernetes/KubernetesModelConverter.java
index 792946ff..824122ab 100644
--- a/backend/sdk/src/main/java/com/alibaba/higress/sdk/service/kubernetes/KubernetesModelConverter.java
+++ b/backend/sdk/src/main/java/com/alibaba/higress/sdk/service/kubernetes/KubernetesModelConverter.java
@@ -32,6 +32,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import javax.naming.InvalidNameException;
 import javax.naming.ldap.LdapName;
@@ -1208,21 +1209,32 @@ private void fillIngressHeaderControlStageConfig(V1ObjectMeta metadata, HeaderCo
         }
         if (CollectionUtils.isNotEmpty(config.getAdd())) {
             setFunctionalAnnotation(metadata, addKey,
-                StringUtils.join(config.getAdd().stream().map(this::getHeaderConfig).toList(), Separators.NEW_LINE),
+                StringUtils.join(config.getAdd().stream().map(this::getHeaderConfig).filter(Objects::nonNull).toList(),
+                    Separators.NEW_LINE),
                 enabled);
         }
         if (CollectionUtils.isNotEmpty(config.getSet())) {
             setFunctionalAnnotation(metadata, setKey,
-                StringUtils.join(config.getSet().stream().map(this::getHeaderConfig).toList(), Separators.NEW_LINE),
+                StringUtils.join(config.getSet().stream().map(this::getHeaderConfig).filter(Objects::nonNull).toList(),
+                    Separators.NEW_LINE),
                 enabled);
         }
         if (CollectionUtils.isNotEmpty(config.getRemove())) {
-            setFunctionalAnnotation(metadata, removeKey, StringUtils.join(config.getRemove(), Separators.COMMA),
+            setFunctionalAnnotation(metadata, removeKey,
+                StringUtils.join(
+                    config.getRemove().stream().filter(StringUtils::isNotEmpty).collect(Collectors.toList()),
+                    Separators.COMMA),
                 enabled);
         }
     }
 
     private String getHeaderConfig(Header header) {
+        if (StringUtils.isEmpty(header.getKey())) {
+            return null;
+        }
+        if (StringUtils.isEmpty(header.getValue())) {
+            return header.getKey() + Separators.SPACE;
+        }
         return header.getKey() + Separators.SPACE + header.getValue();
     }
 
@@ -1273,7 +1285,7 @@ private void fillIngressTls(V1ObjectMeta metadata, V1IngressSpec spec, Route rou
             }
 
             V1IngressTLS tls = new V1IngressTLS();
-            if (!HigressConstants.DEFAULT_DOMAIN.equals(domainName)){
+            if (!HigressConstants.DEFAULT_DOMAIN.equals(domainName)) {
                 tls.setHosts(Collections.singletonList(domainName));
             }
             tls.setSecretName(domain.getCertIdentifier());
diff --git a/frontend/src/locales/en-US/translation.json b/frontend/src/locales/en-US/translation.json
index 0e9ac1c1..e7b90696 100644
--- a/frontend/src/locales/en-US/translation.json
+++ b/frontend/src/locales/en-US/translation.json
@@ -145,7 +145,11 @@
         "set": "Set",
         "remove": "Remove",
         "action": "Action",
-        "addNewRule": "Add New Rule"
+        "addNewRule": "Add New Rule",
+        "headerTypeRequired": "Please select header type",
+        "actionTypeRequired": "Please select action type",
+        "keyRequired": "Please input header key",
+        "valueRequired": "Please input header value"
       },
       "cors": {
         "allowOrigins": "Allow Origins",
diff --git a/frontend/src/locales/zh-CN/translation.json b/frontend/src/locales/zh-CN/translation.json
index 6dd0a786..afff4d2f 100644
--- a/frontend/src/locales/zh-CN/translation.json
+++ b/frontend/src/locales/zh-CN/translation.json
@@ -145,7 +145,11 @@
         "set": "更新",
         "remove": "删除",
         "action": "操作",
-        "addNewRule": "添加新规则"
+        "addNewRule": "添加新规则",
+        "headerTypeRequired": "请选择Header类型",
+        "actionTypeRequired": "请选择操作类型",
+        "keyRequired": "请输入Header Key",
+        "valueRequired": "请输入Header Value"
       },
       "cors": {
         "allowOrigins": "允许的访问来源",
diff --git a/frontend/src/pages/plugin/components/HeaderModify/index.tsx b/frontend/src/pages/plugin/components/HeaderModify/index.tsx
index f020f84f..6e1ef3b6 100644
--- a/frontend/src/pages/plugin/components/HeaderModify/index.tsx
+++ b/frontend/src/pages/plugin/components/HeaderModify/index.tsx
@@ -48,7 +48,10 @@ const HeaderModify = forwardRef((props, ref) => {
           return (
             <Form.Item
               shouldUpdate
-              rules={[{ required: true }]}
+              rules={[{
+                required: true,
+                message: t('plugins.builtIns.headerControl.headerTypeRequired') || '',
+              }]}
               name={[field.name, 'headerType']}
               fieldKey={[field.fieldKey, 'headerType']}
             >
@@ -69,7 +72,10 @@ const HeaderModify = forwardRef((props, ref) => {
           return (
             <Form.Item
               shouldUpdate
-              rules={[{ required: true }]}
+              rules={[{
+                required: true,
+                message: t('plugins.builtIns.headerControl.actionTypeRequired') || '',
+              }]}
               name={[field.name, 'actionType']}
               fieldKey={[field.fieldKey, 'actionType']}
             >
@@ -90,7 +96,10 @@ const HeaderModify = forwardRef((props, ref) => {
           return (
             <Form.Item
               shouldUpdate
-              rules={[{ required: true }]}
+              rules={[{
+                required: true,
+                message: t('plugins.builtIns.headerControl.valueRequired') || '',
+              }]}
               name={[field.name, 'key']}
               fieldKey={[field.fieldKey, 'key']}
             >
@@ -104,11 +113,23 @@ const HeaderModify = forwardRef((props, ref) => {
         title: 'Header Value',
         key: 'value',
         dataIndex: 'value',
-        render(text, field, record) {
+        render(text, field, index) {
           return (
             <Form.Item
               shouldUpdate
-              rules={[{ required: true }]}
+              rules={[{
+                validator(_, value) {
+                  const headerList = form.getFieldValue('headerList');
+                  if (!Array.isArray(headerList) || headerList.length === 0) {
+                    return '';
+                  }
+                  const header = headerList[index];
+                  if (header.actionType !== 'remove' && !value) {
+                    return Promise.reject(new Error(t('plugins.builtIns.headerControl.valueRequired') || ''));
+                  }
+                  return Promise.resolve();
+                },
+              }]}
               name={[field.name, 'value']}
               fieldKey={[field.fieldKey, 'value']}
             >