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 e386094194 fix(admin): validate discovery bindings before upstream
refresh (#7269)
e386094194 is described below
commit e386094194ecf9864f97958f8dfaaa09e5e320a5
Author: Liming Deng <[email protected]>
AuthorDate: Wed Sep 30 15:45:53 2026 +0800
fix(admin): validate discovery bindings before upstream refresh (#7269)
* fix(admin): validate discovery bindings before upstream refresh
* style(admin): retain upstream snapshot as final
* docs(admin): clarify discovery validation transaction boundaries
---
.../service/impl/DiscoveryUpstreamServiceImpl.java | 18 +++++++++++++++--
.../service/DiscoveryUpstreamServiceTest.java | 23 ++++++++++++++++++++++
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryUpstreamServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryUpstreamServiceImpl.java
index bf379d40e7..fca9a06012 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryUpstreamServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryUpstreamServiceImpl.java
@@ -34,6 +34,7 @@ import org.apache.shenyu.admin.model.entity.DiscoveryDO;
import org.apache.shenyu.admin.model.entity.DiscoveryHandlerDO;
import org.apache.shenyu.admin.model.entity.DiscoveryRelDO;
import org.apache.shenyu.admin.model.entity.DiscoveryUpstreamDO;
+import org.apache.shenyu.admin.model.entity.PluginDO;
import org.apache.shenyu.admin.model.entity.ProxySelectorDO;
import org.apache.shenyu.admin.model.entity.SelectorDO;
import org.apache.shenyu.admin.model.result.ConfigImportResult;
@@ -41,6 +42,7 @@ import org.apache.shenyu.admin.model.vo.DiscoveryUpstreamVO;
import org.apache.shenyu.admin.service.DiscoveryUpstreamService;
import org.apache.shenyu.admin.service.configs.ConfigsImportContext;
import org.apache.shenyu.admin.transfer.DiscoveryTransfer;
+import org.apache.shenyu.admin.utils.Assert;
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
import org.apache.shenyu.common.dto.DiscoverySyncData;
import org.apache.shenyu.common.dto.DiscoveryUpstreamData;
@@ -373,22 +375,34 @@ public class DiscoveryUpstreamServiceImpl implements
DiscoveryUpstreamService {
return ConfigImportResult.success(successCount);
}
+ /**
+ * Validate the binding before pushing the persisted upstream snapshot to
discovery.
+ * This method does not undo preceding database writes. Callers own the
transaction boundary:
+ * a validation failure prevents the push, but writes outside a
transaction remain persisted.
+ *
+ * @param discoveryHandlerId the handler whose upstreams should be
published
+ */
private void fetchAll(final String discoveryHandlerId) {
- List<DiscoveryUpstreamDO> discoveryUpstreamDOS =
discoveryUpstreamMapper.selectByDiscoveryHandlerId(discoveryHandlerId);
+ final List<DiscoveryUpstreamDO> discoveryUpstreamDOS =
discoveryUpstreamMapper.selectByDiscoveryHandlerId(discoveryHandlerId);
DiscoveryHandlerDO discoveryHandlerDO =
discoveryHandlerMapper.selectById(discoveryHandlerId);
+ Assert.notNull(discoveryHandlerDO, "Discovery handler does not exist:
" + discoveryHandlerId);
ProxySelectorDO proxySelectorDO =
proxySelectorMapper.selectByHandlerId(discoveryHandlerId);
ProxySelectorDTO proxySelectorDTO;
if (Objects.isNull(proxySelectorDO)) {
SelectorDO selectorDO =
selectorMapper.selectByDiscoveryHandlerId(discoveryHandlerDO.getId());
+ Assert.notNull(selectorDO, "Selector binding does not exist for
discovery handler: " + discoveryHandlerId);
+ PluginDO pluginDO =
pluginMapper.selectById(selectorDO.getPluginId());
+ Assert.notNull(pluginDO, "Plugin does not exist for selector: " +
selectorDO.getId());
proxySelectorDTO = new ProxySelectorDTO();
proxySelectorDTO.setId(selectorDO.getId());
-
proxySelectorDTO.setPluginName(pluginMapper.selectById(selectorDO.getPluginId()).getName());
+ proxySelectorDTO.setPluginName(pluginDO.getName());
proxySelectorDTO.setName(selectorDO.getSelectorName());
proxySelectorDTO.setNamespaceId(selectorDO.getNamespaceId());
} else {
proxySelectorDTO =
DiscoveryTransfer.INSTANCE.mapToDTO(proxySelectorDO);
}
DiscoveryDO discoveryDO =
discoveryMapper.selectById(discoveryHandlerDO.getDiscoveryId());
+ Assert.notNull(discoveryDO, "Discovery does not exist: " +
discoveryHandlerDO.getDiscoveryId());
List<DiscoveryUpstreamDTO> collect =
discoveryUpstreamDOS.stream().map(DiscoveryTransfer.INSTANCE::mapToDTO).collect(Collectors.toList());
DiscoveryProcessor discoveryProcessor =
discoveryProcessorHolder.chooseProcessor(discoveryDO.getDiscoveryType());
discoveryProcessor.changeUpstream(proxySelectorDTO, collect);
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryUpstreamServiceTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryUpstreamServiceTest.java
index 95615e10a0..cd1de5a753 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryUpstreamServiceTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryUpstreamServiceTest.java
@@ -22,6 +22,7 @@ import
org.springframework.transaction.support.TransactionSynchronizationManager
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;
@@ -47,6 +48,8 @@ import org.junit.jupiter.api.Assertions;
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,6 +58,7 @@ import java.sql.Timestamp;
import java.time.LocalDateTime;
import java.util.Collections;
import java.util.List;
+import java.util.Locale;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -62,6 +66,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.when;
+import org.springframework.test.util.ReflectionTestUtils;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.never;
@@ -142,6 +147,24 @@ public final class DiscoveryUpstreamServiceTest {
assertEquals(ShenyuResultMessage.DELETE_SUCCESS, delete);
}
+ @ParameterizedTest
+ @ValueSource(strings = {"handler", "selector", "plugin", "discovery"})
+ void rejectsMissingDiscoveryBindingsWithDomainErrors(final String missing)
{
+ if (!"handler".equals(missing)) {
+
when(discoveryHandlerMapper.selectById("123")).thenReturn(buildDiscoveryHandlerDO());
+ }
+ if ("plugin".equals(missing) || "discovery".equals(missing)) {
+
when(selectorMapper.selectByDiscoveryHandlerId("123")).thenReturn(buildSelectorDO());
+ }
+ if ("discovery".equals(missing)) {
+ when(pluginMapper.selectById(any())).thenReturn(buildPluginDO());
+ }
+ ValidFailException error =
Assertions.assertThrows(ValidFailException.class,
+ () ->
ReflectionTestUtils.invokeMethod(discoveryUpstreamService, "fetchAll", "123"));
+
Assertions.assertTrue(error.getMessage().toLowerCase(Locale.ROOT).contains(missing));
+ verifyNoInteractions(discoveryProcessorHolder, discoveryProcessor);
+ }
+
@Test
public void testListAll() {
List<DiscoveryHandlerDO> list =
Collections.singletonList(buildDiscoveryHandlerDO());
