This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new e6cc09e010 fix(admin): validate proxy discovery bindings before writes
(#7271)
e6cc09e010 is described below
commit e6cc09e0102533f4709f49226cbace347d700584
Author: Liming Deng <[email protected]>
AuthorDate: Wed Sep 30 11:27:35 2026 +0800
fix(admin): validate proxy discovery bindings before writes (#7271)
* fix(admin): validate proxy discovery bindings before writes
* fix(admin): validate discovery before binding processor lookup
---
.../service/impl/ProxySelectorServiceImpl.java | 19 +++++----
.../admin/service/ProxySelectorServiceTest.java | 47 ++++++++++++++++++++++
2 files changed, 59 insertions(+), 7 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java
index ed44ff4807..3c128165de 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java
@@ -49,6 +49,7 @@ import org.apache.shenyu.admin.service.ProxySelectorService;
import org.apache.shenyu.admin.service.configs.ConfigsImportContext;
import org.apache.shenyu.admin.transfer.DiscoveryTransfer;
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
+import org.apache.shenyu.admin.utils.Assert;
import org.apache.shenyu.common.dto.ProxySelectorData;
import org.apache.shenyu.common.utils.UUIDUtils;
import org.jetbrains.annotations.NotNull;
@@ -301,8 +302,9 @@ public class ProxySelectorServiceImpl implements
ProxySelectorService {
public String bindingDiscoveryHandler(final ProxySelectorAddDTO
proxySelectorAddDTO) {
Timestamp currentTime = new Timestamp(System.currentTimeMillis());
String selectorId = proxySelectorAddDTO.getSelectorId();
- DiscoveryProcessor discoveryProcessor =
discoveryProcessorHolder.chooseProcessor(proxySelectorAddDTO.getDiscovery().getDiscoveryType());
- ProxySelectorAddDTO.Discovery discovery =
proxySelectorAddDTO.getDiscovery();
+ final ProxySelectorAddDTO.Discovery discovery =
proxySelectorAddDTO.getDiscovery();
+ Assert.notNull(discovery, "Discovery configuration is required for
selector: " + selectorId);
+ DiscoveryProcessor discoveryProcessor =
discoveryProcessorHolder.chooseProcessor(discovery.getDiscoveryType());
String discoveryId = discovery.getId();
if (!StringUtils.hasLength(discoveryId)) {
discoveryId = UUIDUtils.getInstance().generateShortUuid();
@@ -349,13 +351,18 @@ public class ProxySelectorServiceImpl implements
ProxySelectorService {
*/
@Transactional(rollbackFor = Exception.class)
public String update(final ProxySelectorAddDTO proxySelectorAddDTO) {
- // update proxy selector
+ ProxySelectorAddDTO.Discovery discovery =
proxySelectorAddDTO.getDiscovery();
+ Assert.notNull(discovery, "Discovery configuration is required");
ProxySelectorDO proxySelectorDO =
ProxySelectorDO.buildProxySelectorDO(proxySelectorAddDTO);
- proxySelectorMapper.update(proxySelectorDO);
- // DiscoveryRelDO
DiscoveryRelDO discoveryRelDO =
discoveryRelMapper.selectByProxySelectorId(proxySelectorDO.getId());
+ Assert.notNull(discoveryRelDO, "Discovery binding does not exist for
proxy selector: " + proxySelectorDO.getId());
String discoveryHandlerId = discoveryRelDO.getDiscoveryHandlerId();
DiscoveryHandlerDO discoveryHandlerDO =
discoveryHandlerMapper.selectById(discoveryHandlerId);
+ Assert.notNull(discoveryHandlerDO, "Discovery handler does not exist:
" + discoveryHandlerId);
+ DiscoveryDO discoveryDO =
discoveryMapper.selectById(discoveryHandlerDO.getDiscoveryId());
+ Assert.notNull(discoveryDO, "Discovery does not exist: " +
discoveryHandlerDO.getDiscoveryId());
+ // Validate all related records before performing any update.
+ proxySelectorMapper.update(proxySelectorDO);
// update discovery handler
Timestamp currentTime = new Timestamp(System.currentTimeMillis());
discoveryHandlerDO.setHandler(proxySelectorAddDTO.getHandler());
@@ -364,8 +371,6 @@ public class ProxySelectorServiceImpl implements
ProxySelectorService {
discoveryHandlerDO.setDateUpdated(currentTime);
discoveryHandlerMapper.updateSelective(discoveryHandlerDO);
// update discovery
- DiscoveryDO discoveryDO =
discoveryMapper.selectById(discoveryHandlerDO.getDiscoveryId());
- ProxySelectorAddDTO.Discovery discovery =
proxySelectorAddDTO.getDiscovery();
discoveryDO.setServerList(discovery.getServerList());
discoveryDO.setDateUpdated(currentTime);
discoveryDO.setProps(discovery.getProps());
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ProxySelectorServiceTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ProxySelectorServiceTest.java
index 0751ee66c9..cdad6d53e3 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ProxySelectorServiceTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ProxySelectorServiceTest.java
@@ -19,6 +19,7 @@ package org.apache.shenyu.admin.service;
import org.apache.shenyu.admin.discovery.DiscoveryProcessor;
import org.apache.shenyu.admin.discovery.DiscoveryProcessorHolder;
+import org.apache.shenyu.admin.exception.ValidFailException;
import org.apache.shenyu.admin.mapper.DiscoveryHandlerMapper;
import org.apache.shenyu.admin.mapper.DiscoveryMapper;
import org.apache.shenyu.admin.mapper.DiscoveryRelMapper;
@@ -40,6 +41,8 @@ import org.apache.shenyu.common.dto.ProxySelectorData;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -55,11 +58,13 @@ import static
org.apache.shenyu.common.constant.Constants.SYS_DEFAULT_NAMESPACE_
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
@@ -157,6 +162,48 @@ class ProxySelectorServiceTest {
verify(discoveryUpstreamMapper,
never()).deleteByDiscoveryHandlerId(any());
}
+ @ParameterizedTest
+ @ValueSource(strings = {"configuration", "relation", "handler",
"discovery"})
+ void validatesAllBindingsBeforeUpdatingAnyRecord(final String missing) {
+ ProxySelectorAddDTO dto = new ProxySelectorAddDTO();
+ dto.setId("proxy");
+ dto.setName("proxy");
+ dto.setPluginName("tcp");
+ dto.setForwardPort(8080);
+ dto.setHandler("new-handler");
+ if (!"configuration".equals(missing)) {
+ dto.setDiscovery(new ProxySelectorAddDTO.Discovery());
+ }
+ DiscoveryRelDO relation = new DiscoveryRelDO();
+ relation.setDiscoveryHandlerId("handler");
+ DiscoveryHandlerDO handler = new DiscoveryHandlerDO();
+ handler.setId("handler");
+ handler.setDiscoveryId("discovery");
+ handler.setHandler("original");
+
given(discoveryRelMapper.selectByProxySelectorId("proxy")).willReturn("relation".equals(missing)
? null : relation);
+
given(discoveryHandlerMapper.selectById("handler")).willReturn("handler".equals(missing)
? null : handler);
+
given(discoveryMapper.selectById("discovery")).willReturn("discovery".equals(missing)
? null : new DiscoveryDO());
+
+ assertThrows(ValidFailException.class, () ->
proxySelectorService.update(dto));
+
+ verify(proxySelectorMapper, never()).update(any());
+ verify(discoveryHandlerMapper, never()).updateSelective(any());
+ verify(discoveryMapper, never()).updateSelective(any());
+ verifyNoInteractions(discoveryUpstreamMapper,
discoveryProcessorHolder);
+ assertEquals("original", handler.getHandler());
+ }
+
+ @Test
+ void bindingRejectsMissingConfigurationBeforeLookingUpProcessorOrWriting()
{
+ ProxySelectorAddDTO dto = new ProxySelectorAddDTO();
+ dto.setSelectorId("selector-without-discovery");
+
+ ValidFailException failure = assertThrows(ValidFailException.class, ()
-> proxySelectorService.bindingDiscoveryHandler(dto));
+
+ assertEquals("Discovery configuration is required for selector:
selector-without-discovery", failure.getMessage());
+ verifyNoInteractions(discoveryProcessorHolder, discoveryMapper,
discoveryHandlerMapper, discoveryRelMapper, discoveryUpstreamMapper);
+ }
+
@Test
void testFetchDataWithProxySelector() {
DiscoveryHandlerDO discoveryHandlerDO = new DiscoveryHandlerDO();
