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 82ad6f7f8dfe..76ff115ac8c9 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 @@ -42,6 +42,7 @@ 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.DiscoveryUpstreamPropsValidator; import org.apache.shenyu.admin.utils.Assert; import org.apache.shenyu.admin.utils.ShenyuResultMessage; import org.apache.shenyu.common.dto.DiscoverySyncData; @@ -138,6 +139,8 @@ public void createOrUpdateBatch(final List upstreams) { @Override @Transactional public int updateBatch(final String discoveryHandlerId, final List discoveryUpstreamDTOList) { + // Validate the complete replacement before deleting anything or emitting a sync event. + discoveryUpstreamDTOList.forEach(upstream -> DiscoveryUpstreamPropsValidator.validate(upstream.getProps())); discoveryUpstreamMapper.deleteByDiscoveryHandlerId(discoveryHandlerId); for (DiscoveryUpstreamDTO discoveryUpstreamDTO : discoveryUpstreamDTOList) { discoveryUpstreamDTO.setId(null); @@ -160,6 +163,7 @@ public void afterCommit() { @Override public void nativeCreateOrUpdate(final DiscoveryUpstreamDTO discoveryUpstreamDTO) { + DiscoveryUpstreamPropsValidator.validate(discoveryUpstreamDTO.getProps()); DiscoveryUpstreamDO discoveryUpstreamDO = DiscoveryUpstreamDO.buildDiscoveryUpstreamDO(discoveryUpstreamDTO); if (StringUtils.hasLength(discoveryUpstreamDTO.getId())) { discoveryUpstreamMapper.updateSelective(discoveryUpstreamDO); @@ -264,6 +268,7 @@ public void refreshBySelectorId(final String selectorId) { * @return the string */ private String create(final DiscoveryUpstreamDTO discoveryUpstreamDTO) { + DiscoveryUpstreamPropsValidator.validate(discoveryUpstreamDTO.getProps()); DiscoveryUpstreamDO discoveryUpstreamDO = DiscoveryUpstreamDO.buildDiscoveryUpstreamDO(discoveryUpstreamDTO); discoveryUpstreamMapper.insert(discoveryUpstreamDO); fetchAll(discoveryUpstreamDTO.getDiscoveryHandlerId()); @@ -277,6 +282,7 @@ private String create(final DiscoveryUpstreamDTO discoveryUpstreamDTO) { * @return the string */ private String update(final DiscoveryUpstreamDTO discoveryUpstreamDTO) { + DiscoveryUpstreamPropsValidator.validate(discoveryUpstreamDTO.getProps()); DiscoveryUpstreamDO discoveryUpstreamDO = DiscoveryUpstreamDO.buildDiscoveryUpstreamDO(discoveryUpstreamDTO); discoveryUpstreamMapper.update(discoveryUpstreamDO); fetchAll(discoveryUpstreamDTO.getDiscoveryHandlerId()); @@ -317,6 +323,7 @@ public ConfigImportResult importData(final List discoveryU if (CollectionUtils.isEmpty(discoveryUpstreamList)) { return ConfigImportResult.success(); } + discoveryUpstreamList.forEach(upstream -> DiscoveryUpstreamPropsValidator.validate(upstream.getProps())); int successCount = 0; StringBuilder errorMsgBuilder = new StringBuilder(); Map> discoveryHandlerUpstreamMap = discoveryUpstreamMapper @@ -363,6 +370,7 @@ public ConfigImportResult importData(final String namespace, final List discoveryHandlerIdMapping = context.getDiscoveryHandlerIdMapping(); + discoveryUpstreamList.forEach(upstream -> DiscoveryUpstreamPropsValidator.validate(upstream.getProps())); int successCount = 0; StringBuilder errorMsgBuilder = new StringBuilder(); Map> discoveryHandlerUpstreamMap = discoveryUpstreamMapper 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 9a8e88ebc450..27e98d60a554 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.DiscoveryUpstreamPropsValidator; import org.apache.shenyu.admin.utils.ShenyuResultMessage; import org.apache.shenyu.admin.utils.Assert; import org.apache.shenyu.common.dto.ProxySelectorData; @@ -239,6 +240,7 @@ public String delete(final List ids) { @Override @Transactional(rollbackFor = Exception.class) public String create(final ProxySelectorAddDTO proxySelectorAddDTO) { + validateUpstreamProps(proxySelectorAddDTO); Timestamp currentTime = new Timestamp(System.currentTimeMillis()); ProxySelectorDO proxySelectorDO = ProxySelectorDO.buildProxySelectorDO(proxySelectorAddDTO); String proxySelectorId = proxySelectorDO.getId(); @@ -341,6 +343,7 @@ private static DiscoveryDO buildDiscovery(final ProxySelectorAddDTO proxySelecto @Override @Transactional(rollbackFor = Exception.class) public String bindingDiscoveryHandler(final ProxySelectorAddDTO proxySelectorAddDTO) { + validateUpstreamProps(proxySelectorAddDTO); Timestamp currentTime = new Timestamp(System.currentTimeMillis()); String selectorId = proxySelectorAddDTO.getSelectorId(); final ProxySelectorAddDTO.Discovery discovery = proxySelectorAddDTO.getDiscovery(); @@ -392,6 +395,7 @@ public String bindingDiscoveryHandler(final ProxySelectorAddDTO proxySelectorAdd */ @Transactional(rollbackFor = Exception.class) public String update(final ProxySelectorAddDTO proxySelectorAddDTO) { + validateUpstreamProps(proxySelectorAddDTO); ProxySelectorAddDTO.Discovery discovery = proxySelectorAddDTO.getDiscovery(); Assert.notNull(discovery, "Discovery configuration is required"); ProxySelectorDO proxySelectorDO = ProxySelectorDO.buildProxySelectorDO(proxySelectorAddDTO); @@ -461,6 +465,12 @@ public void afterCompletion(final int status) { return ShenyuResultMessage.UPDATE_SUCCESS; } + private void validateUpstreamProps(final ProxySelectorAddDTO dto) { + if (Objects.nonNull(dto.getDiscoveryUpstreams())) { + dto.getDiscoveryUpstreams().forEach(upstream -> DiscoveryUpstreamPropsValidator.validate(upstream.getProps())); + } + } + @Override public void fetchData(final String discoveryHandlerId) { DiscoveryHandlerDO discoveryHandlerDO = discoveryHandlerMapper.selectById(discoveryHandlerId); diff --git a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java index 40e8125b9234..586327fc7516 100644 --- a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java +++ b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java @@ -30,12 +30,14 @@ import org.apache.shenyu.admin.mapper.RuleConditionMapper; import org.apache.shenyu.admin.mapper.RuleMapper; import org.apache.shenyu.admin.mapper.SelectorMapper; +import org.apache.shenyu.admin.mapper.ShenyuDictMapper; import org.apache.shenyu.admin.model.dto.RuleConditionDTO; import org.apache.shenyu.admin.model.dto.RuleDTO; import org.apache.shenyu.admin.model.entity.PluginDO; import org.apache.shenyu.admin.model.entity.RuleConditionDO; import org.apache.shenyu.admin.model.entity.RuleDO; import org.apache.shenyu.admin.model.entity.SelectorDO; +import org.apache.shenyu.admin.model.entity.ShenyuDictDO; import org.apache.shenyu.admin.model.event.rule.RuleCreatedEvent; import org.apache.shenyu.admin.model.event.selector.BatchSelectorDeletedEvent; import org.apache.shenyu.admin.model.page.CommonPager; @@ -57,6 +59,8 @@ import org.apache.shenyu.common.constant.AdminConstants; import org.apache.shenyu.common.dto.ConditionData; import org.apache.shenyu.common.dto.RuleData; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfigValidator; +import org.apache.shenyu.common.dto.convert.rule.impl.DivideRuleHandle; import org.apache.shenyu.common.enums.MatchModeEnum; import org.apache.shenyu.common.enums.PluginEnum; import org.apache.shenyu.common.utils.JsonUtils; @@ -96,16 +100,20 @@ public class RuleServiceImpl implements RuleService { private final RuleEventPublisher ruleEventPublisher; + private final ShenyuDictMapper shenyuDictMapper; + public RuleServiceImpl(final RuleMapper ruleMapper, final RuleConditionMapper ruleConditionMapper, final SelectorMapper selectorMapper, final PluginMapper pluginMapper, - final RuleEventPublisher ruleEventPublisher) { + final RuleEventPublisher ruleEventPublisher, + final ShenyuDictMapper shenyuDictMapper) { this.ruleMapper = ruleMapper; this.ruleConditionMapper = ruleConditionMapper; this.selectorMapper = selectorMapper; this.pluginMapper = pluginMapper; this.ruleEventPublisher = ruleEventPublisher; + this.shenyuDictMapper = shenyuDictMapper; } @Override @@ -156,6 +164,7 @@ public String registerDefault(final RuleDTO ruleDTO) { return ""; } validateRuleHandle(ruleDTO); + validateCanaryConfig(ruleDTO.getSelectorId(), ruleDTO.getHandle()); RuleDO ruleDO = RuleDO.buildRuleDO(ruleDTO); if (StringUtils.isEmpty(ruleDTO.getId())) { ruleMapper.insertSelective(ruleDO); @@ -181,6 +190,7 @@ public int createOrUpdate(final RuleDTO ruleDTO) { @Override public int create(final RuleDTO ruleDTO) { validateRuleHandle(ruleDTO); + validateCanaryConfig(ruleDTO.getSelectorId(), ruleDTO.getHandle()); RuleDO ruleDO = RuleDO.buildRuleDO(ruleDTO); final int ruleCount = ruleMapper.insertSelective(ruleDO); addCondition(ruleDO, ruleDTO.getRuleConditions()); @@ -195,6 +205,7 @@ public int update(final RuleDTO ruleDTO) { validateRuleHandle(ruleDTO); final RuleDO before = ruleMapper.selectById(ruleDTO.getId()); Assert.notNull(before, "the updated rule is not found"); + validateCanaryConfig(StringUtils.defaultIfBlank(ruleDTO.getSelectorId(), before.getSelectorId()), ruleDTO.getHandle()); RuleDO ruleDO = RuleDO.buildRuleDO(ruleDTO); final int ruleCount = ruleMapper.updateSelective(ruleDO); @@ -230,6 +241,38 @@ public int update(final RuleDTO ruleDTO) { return ruleCount; } + private void validateCanaryConfig(final String selectorId, final String handle) { + if (StringUtils.isBlank(handle)) { + return; + } + SelectorDO selector = selectorMapper.selectById(selectorId); + if (Objects.isNull(selector)) { + return; + } + PluginDO plugin = pluginMapper.selectById(selector.getPluginId()); + if (Objects.isNull(plugin) || !PluginEnum.DIVIDE.getName().equals(plugin.getName())) { + return; + } + try { + DivideRuleHandle divide = CanaryConfigValidator.parseHandle(handle); + if (Objects.nonNull(divide) && Objects.nonNull(divide.getCanary())) { + CanaryConfigValidator.validateExtensions(divide.getCanary(), + name -> validateCanaryDictionary("paramType", name), name -> validateCanaryDictionary("operator", name)); + } + } catch (IllegalArgumentException ex) { + throw new ShenyuAdminException(ex.getMessage(), ex); + } + } + + private void validateCanaryDictionary(final String type, final String name) { + boolean supported = shenyuDictMapper.findByType(type).stream() + .filter(dict -> Boolean.TRUE.equals(dict.getEnabled())) + .map(ShenyuDictDO::getDictValue).anyMatch(name::equals); + if (!supported) { + throw new IllegalArgumentException("Unsupported Canary " + type + ": " + name); + } + } + /** * find rule by id and namespaceId. * @@ -302,6 +345,7 @@ public ConfigImportResult importData(final List ruleList) { return ConfigImportResult.success(); } + ruleList.forEach(rule -> validateCanaryConfig(rule.getSelectorId(), rule.getHandle())); Map> selectorRuleMap = ruleMapper .selectAll() .stream() @@ -347,6 +391,8 @@ public ConfigImportResult importData(final String namespace, final List return ConfigImportResult.success(); } Map selectorIdMapping = context.getSelectorIdMapping(); + ruleList.stream().filter(rule -> selectorIdMapping.containsKey(rule.getSelectorId())) + .forEach(rule -> validateCanaryConfig(selectorIdMapping.get(rule.getSelectorId()), rule.getHandle())); Map> selectorRuleMap = ruleMapper .selectAllByNamespaceId(namespace) .stream() diff --git a/shenyu-admin/src/main/java/org/apache/shenyu/admin/transfer/DiscoveryTransfer.java b/shenyu-admin/src/main/java/org/apache/shenyu/admin/transfer/DiscoveryTransfer.java index fc8fea656c84..203e0dd8fa9a 100644 --- a/shenyu-admin/src/main/java/org/apache/shenyu/admin/transfer/DiscoveryTransfer.java +++ b/shenyu-admin/src/main/java/org/apache/shenyu/admin/transfer/DiscoveryTransfer.java @@ -17,6 +17,7 @@ package org.apache.shenyu.admin.transfer; +import com.google.gson.JsonElement; import org.apache.shenyu.admin.model.dto.DiscoveryDTO; import org.apache.shenyu.admin.model.dto.DiscoveryHandlerDTO; import org.apache.shenyu.admin.model.dto.DiscoveryRelDTO; @@ -80,11 +81,16 @@ public CommonUpstream mapToCommonUpstream(DiscoveryUpstreamData discoveryUpstrea String url = data.getUrl(); CommonUpstream commonUpstream = new CommonUpstream(data.getProtocol(), url.split(":")[0], url, false, data.getDateCreated().getTime()); - Properties properties = Optional.ofNullable(data.getProps()) - .map(props -> GsonUtils.getInstance().fromJson(props, Properties.class)) - .orElse(new Properties()); - commonUpstream - .setHealthCheckEnabled(Boolean.parseBoolean(properties.getProperty("healthCheckEnabled", "true"))); + // Other properties, such as labels, may contain nested JSON objects. + boolean healthCheckEnabled = Optional.ofNullable(data.getProps()) + .map(props -> GsonUtils.getInstance().fromJson(props, JsonElement.class)) + .filter(props -> !props.isJsonNull()) + .map(JsonElement::getAsJsonObject) + .map(props -> props.get("healthCheckEnabled")) + .filter(value -> !value.isJsonNull()) + .map(value -> Boolean.parseBoolean(value.getAsString())) + .orElse(true); + commonUpstream.setHealthCheckEnabled(healthCheckEnabled); return commonUpstream; }).orElse(null); } diff --git a/shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/DiscoveryUpstreamPropsValidator.java b/shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/DiscoveryUpstreamPropsValidator.java new file mode 100644 index 000000000000..9ebdd3263c56 --- /dev/null +++ b/shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/DiscoveryUpstreamPropsValidator.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.admin.utils; + +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParseException; +import org.apache.commons.lang3.StringUtils; +import org.apache.shenyu.admin.exception.ShenyuAdminException; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfigValidator; +import org.apache.shenyu.common.utils.GsonUtils; + +import java.util.Objects; + +/** + * Validation of submitted discovery upstream properties without changing replacement semantics. + */ +public final class DiscoveryUpstreamPropsValidator { + + private DiscoveryUpstreamPropsValidator() { + } + + /** + * Validate the submitted properties without merging or rewriting them. + * Omitted labels and an empty labels object are both valid; null labels are invalid. + * Empty properties retain the existing API behavior for optional properties. + * + * @param props the complete submitted properties + */ + public static void validate(final String props) { + if (StringUtils.isBlank(props)) { + return; + } + try { + JsonElement parsed = GsonUtils.getInstance().fromJson(props, JsonElement.class); + if (Objects.isNull(parsed) || !parsed.isJsonObject()) { + throw new IllegalArgumentException("props must be a JSON object"); + } + JsonObject object = parsed.getAsJsonObject(); + if (object.has("labels")) { + CanaryConfigValidator.validateLabels(object.get("labels"), "props.labels", true); + } + } catch (IllegalArgumentException | JsonParseException ex) { + throw new ShenyuAdminException("Invalid upstream props: " + ex.getMessage(), ex); + } + } +} diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ConfigsImportDuplicateTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ConfigsImportDuplicateTest.java index 7442bffcc9e0..9528e36049c3 100644 --- a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ConfigsImportDuplicateTest.java +++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ConfigsImportDuplicateTest.java @@ -160,7 +160,8 @@ public void testSelectorsDeduplicateAfterPluginRemapping(final boolean existing) @ParameterizedTest @ValueSource(booleans = {false, true}) public void testRulesDeduplicateAfterSelectorRemapping(final boolean existing) { - final RuleServiceImpl service = new RuleServiceImpl(ruleMapper, mock(RuleConditionMapper.class), selectorMapper, pluginMapper, mock(RuleEventPublisher.class)); + final RuleServiceImpl service = new RuleServiceImpl(ruleMapper, mock(RuleConditionMapper.class), selectorMapper, pluginMapper, + mock(RuleEventPublisher.class), dictMapper); final ConfigsImportContext context = new ConfigsImportContext(); context.getSelectorIdMapping().put("source", "target"); context.getSelectorIdMapping().put("alias", "target"); diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ConfigsServiceTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ConfigsServiceTest.java index 50422132b65b..246ca4759c53 100644 --- a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ConfigsServiceTest.java +++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ConfigsServiceTest.java @@ -22,6 +22,7 @@ import org.apache.shenyu.admin.mapper.PluginMapper; import org.apache.shenyu.admin.mapper.RuleConditionMapper; import org.apache.shenyu.admin.mapper.RuleMapper; +import org.apache.shenyu.admin.mapper.ShenyuDictMapper; import org.apache.shenyu.admin.mapper.SelectorMapper; import org.apache.shenyu.admin.model.dto.RuleDTO; import org.apache.shenyu.admin.model.entity.PluginDO; @@ -170,7 +171,7 @@ public void testRuleBeforeSelectorInZipCannotBypassHandleValidation() { SelectorMapper selectorMapper = mock(SelectorMapper.class); PluginMapper pluginMapper = mock(PluginMapper.class); RuleService ruleServiceImpl = new RuleServiceImpl(ruleMapper, mock(RuleConditionMapper.class), - selectorMapper, pluginMapper, mock(RuleEventPublisher.class)); + selectorMapper, pluginMapper, mock(RuleEventPublisher.class), mock(ShenyuDictMapper.class)); ConfigsServiceImpl importService = new ConfigsServiceImpl(appAuthService, pluginService, namespacePluginService, pluginHandleService, selectorService, ruleServiceImpl, metaDataService, shenyuDictService, proxySelectorService, discoveryService, discoveryUpstreamService, Collections.emptyList()); 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 765305847839..4ea4b908c373 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.apache.shenyu.admin.discovery.DiscoveryProcessor; import org.apache.shenyu.admin.discovery.DiscoveryProcessorHolder; +import org.apache.shenyu.admin.exception.ShenyuAdminException; import org.apache.shenyu.admin.exception.ValidFailException; import org.apache.shenyu.admin.mapper.DiscoveryHandlerMapper; import org.apache.shenyu.admin.mapper.DiscoveryMapper; @@ -50,26 +51,32 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import java.sql.Timestamp; import java.time.LocalDateTime; +import java.util.Arrays; +import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import java.util.Locale; 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.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.never; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.never; +import static org.mockito.Mockito.when; +import org.springframework.test.util.ReflectionTestUtils; /** * Test cases for DiscoveryUpstreamService. @@ -356,6 +363,128 @@ public void testRolledBackBatchDoesNotPublish() { } } + @ParameterizedTest + @ValueSource(strings = {"{\"labels\":{}}", "{\"labels\":{\"release\":\"canary\"}}", "{\"warmup\":20}", "{}", "{\"warmup\":20,\"extension\":{\"x\":1},\"labels\":{}}"}) + public void testSingleUpdateReplacesPropsAndSynchronizesLabels(final String props) { + AtomicReference stored = new AtomicReference<>(existingLabeledUpstream()); + configureSync(); + when(discoveryUpstreamMapper.update(any())).thenAnswer(invocation -> { + stored.set(invocation.getArgument(0)); + return 1; + }); + when(discoveryUpstreamMapper.selectByDiscoveryHandlerId("123")).thenAnswer(invocation -> Collections.singletonList(stored.get())); + DiscoveryUpstreamDTO dto = buildDiscoveryUpstreamDTO("123", "123", "localhost:8080"); + dto.setProtocol("http://"); + dto.setProps(props); + discoveryUpstreamService.createOrUpdate(dto); + assertEquals(props, stored.get().getProps()); + ArgumentCaptor> synced = ArgumentCaptor.forClass(List.class); + verify(discoveryProcessor).changeUpstream(any(), synced.capture()); + assertEquals(stored.get().getProps(), synced.getValue().get(0).getProps()); + when(discoveryHandlerMapper.selectBySelectorId("selector_1")).thenReturn(buildDiscoveryHandlerDO()); + assertEquals(stored.get().getProps(), discoveryUpstreamService.findBySelectorId("selector_1").get(0).getProps()); + } + + @ParameterizedTest + @ValueSource(strings = {"{\"labels\":{}}", "{\"labels\":{\"release\":\"canary\"}}", "{\"warmup\":20}", "{}", "{\"warmup\":20,\"extension\":{\"x\":1},\"labels\":{}}"}) + public void testBatchUpdateReplacesPropsAndSynchronizesLabels(final String props) { + AtomicReference stored = new AtomicReference<>(existingLabeledUpstream()); + configureSync(); + when(discoveryUpstreamMapper.selectByDiscoveryHandlerId("123")).thenAnswer(invocation -> Collections.singletonList(stored.get())); + when(discoveryUpstreamMapper.insert(any())).thenAnswer(invocation -> { + stored.set(invocation.getArgument(0)); + return 1; + }); + DiscoveryUpstreamDTO dto = buildDiscoveryUpstreamDTO("", "123", "localhost:8080"); + dto.setProtocol("http://"); + dto.setProps(props); + discoveryUpstreamService.updateBatch("123", Collections.singletonList(dto)); + assertEquals(props, stored.get().getProps()); + ArgumentCaptor> synced = ArgumentCaptor.forClass(List.class); + verify(discoveryProcessor).changeUpstream(any(), synced.capture()); + assertEquals(stored.get().getProps(), synced.getValue().get(0).getProps()); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + public void testBatchReplacementRemovesOmittedInstances(final boolean removeAll) { + List stored = new ArrayList<>(); + stored.add(existingLabeledUpstream()); + stored.add(buildDiscoveryUpstreamDO("other", "123", "localhost:8081")); + configureSync(); + doAnswer(invocation -> { + stored.clear(); + return 2; + }).when(discoveryUpstreamMapper).deleteByDiscoveryHandlerId("123"); + when(discoveryUpstreamMapper.selectByDiscoveryHandlerId("123")).thenAnswer(invocation -> new ArrayList<>(stored)); + List submitted = Collections.emptyList(); + if (!removeAll) { + DiscoveryUpstreamDTO remaining = buildDiscoveryUpstreamDTO("", "123", "localhost:8080"); + remaining.setProtocol("http://"); + remaining.setProps("{}"); + submitted = Collections.singletonList(remaining); + when(discoveryUpstreamMapper.insert(any())).thenAnswer(invocation -> { + stored.add(invocation.getArgument(0)); + return 1; + }); + } + discoveryUpstreamService.updateBatch("123", submitted); + assertEquals(removeAll ? 0 : 1, stored.size()); + ArgumentCaptor> synced = ArgumentCaptor.forClass(List.class); + verify(discoveryProcessor).changeUpstream(any(), synced.capture()); + assertEquals(stored.size(), synced.getValue().size()); + if (!removeAll) { + assertEquals("localhost:8080", synced.getValue().get(0).getUrl()); + assertEquals("{}", synced.getValue().get(0).getProps()); + } else { + verify(discoveryUpstreamMapper, never()).insert(any()); + } + } + + @Test + public void testNativeUpdateReplacesSubmittedProps() { + DiscoveryUpstreamDTO dto = buildDiscoveryUpstreamDTO("123"); + dto.setProps("{\"warmup\":20}"); + discoveryUpstreamService.nativeCreateOrUpdate(dto); + ArgumentCaptor saved = ArgumentCaptor.forClass(DiscoveryUpstreamDO.class); + verify(discoveryUpstreamMapper).updateSelective(saved.capture()); + assertEquals(dto.getProps(), saved.getValue().getProps()); + } + + @ParameterizedTest + @ValueSource(strings = {"{\"labels\":null}", "{\"labels\":[]}", "{\"labels\":{\"release\":123}}", "[]"}) + public void testInvalidPropsNeverWriteOrSynchronize(final String props) { + DiscoveryUpstreamDTO dto = buildDiscoveryUpstreamDTO(""); + dto.setProps(props); + assertThrows(ShenyuAdminException.class, () -> discoveryUpstreamService.createOrUpdate(dto)); + DiscoveryUpstreamDTO valid = buildDiscoveryUpstreamDTO(""); + valid.setProps("{\"labels\":{}}"); + assertThrows(ShenyuAdminException.class, () -> discoveryUpstreamService.updateBatch("123", Arrays.asList(valid, dto))); + assertThrows(ShenyuAdminException.class, () -> discoveryUpstreamService.importData(Arrays.asList(valid, dto))); + dto.setId("123"); + assertThrows(ShenyuAdminException.class, () -> discoveryUpstreamService.createOrUpdate(dto)); + verify(discoveryUpstreamMapper, never()).insert(any()); + verify(discoveryUpstreamMapper, never()).update(any()); + verify(discoveryUpstreamMapper, never()).deleteByDiscoveryHandlerId(any()); + verifyNoInteractions(discoveryProcessor, discoveryProcessorHolder); + } + + private DiscoveryUpstreamDO existingLabeledUpstream() { + DiscoveryUpstreamDO upstream = buildDiscoveryUpstreamDO("123", "123", "localhost:8080"); + upstream.setProtocol("http://"); + upstream.setProps("{\"warmup\":10,\"gray\":true,\"healthCheckEnabled\":false,\"extension\":{\"x\":1}," + + "\"labels\":{\"release\":\"stable\",\"region\":\"east\"}}"); + return upstream; + } + + private void configureSync() { + when(selectorMapper.selectByDiscoveryHandlerId(any())).thenReturn(buildSelectorDO()); + when(discoveryHandlerMapper.selectById(any())).thenReturn(buildDiscoveryHandlerDO()); + when(pluginMapper.selectById(any())).thenReturn(buildPluginDO()); + when(discoveryMapper.selectById(any())).thenReturn(buildDiscoveryDO()); + when(discoveryProcessorHolder.chooseProcessor(anyString())).thenReturn(discoveryProcessor); + } + private void testUpdate() { when(discoveryUpstreamMapper.update(any())).thenReturn(1); DiscoveryUpstreamDTO discoveryUpstreamDTO = buildDiscoveryUpstreamDTO("123"); 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 6f6e41f3da2f..ea456ba364b2 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 @@ import org.apache.shenyu.admin.discovery.DiscoveryProcessor; import org.apache.shenyu.admin.discovery.DiscoveryProcessorHolder; +import org.apache.shenyu.admin.exception.ShenyuAdminException; import org.apache.shenyu.admin.listener.DataChangedEvent; import org.apache.shenyu.admin.exception.ValidFailException; import org.apache.shenyu.admin.mapper.DiscoveryHandlerMapper; @@ -27,6 +28,7 @@ import org.apache.shenyu.admin.mapper.DiscoveryUpstreamMapper; import org.apache.shenyu.admin.mapper.ProxySelectorMapper; import org.apache.shenyu.admin.mapper.SelectorMapper; +import org.apache.shenyu.admin.model.dto.DiscoveryUpstreamDTO; import org.apache.shenyu.admin.model.dto.ProxySelectorAddDTO; import org.apache.shenyu.admin.model.entity.DiscoveryDO; import org.apache.shenyu.admin.model.entity.DiscoveryHandlerDO; @@ -43,9 +45,10 @@ import org.apache.shenyu.common.enums.ConfigGroupEnum; 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.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -60,6 +63,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import static org.apache.shenyu.common.constant.Constants.SYS_DEFAULT_NAMESPACE_ID; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -72,10 +76,10 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.times; import static org.junit.jupiter.api.Assertions.assertTrue; -import org.mockito.ArgumentCaptor; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -279,6 +283,7 @@ void testUpdatePublishesListenerConfigurationAfterCommit(final boolean commit) { DiscoveryDO discoveryDO = new DiscoveryDO(); discoveryDO.setNamespaceId("namespace-1"); discoveryDO.setDiscoveryType("local"); + discoveryDO.setNamespaceId("namespace-1"); given(discoveryMapper.selectById("discovery-1")).willReturn(discoveryDO); DiscoveryProcessor discoveryProcessor = mock(DiscoveryProcessor.class); @@ -449,6 +454,67 @@ void testImportData() { assertEquals(configImportResult.getSuccessCount(), proxySelectorDataList.size()); } + @ParameterizedTest + @ValueSource(strings = {"{\"labels\":{}}", "{\"labels\":{\"release\":\"stable\"}}", "{\"warmup\":20}", "{}", "{\"warmup\":20,\"extension\":{\"x\":1},\"labels\":{}}"}) + void testDiscoveryEditorReplacesPropsAndSynchronizesLabels(final String props) { + final ProxySelectorAddDTO dto = discoveryEditorRequest(props); + when(discoveryRelMapper.selectByProxySelectorId("123")).thenReturn(buildDiscoveryRelDO()); + DiscoveryUpstreamDO before = new DiscoveryUpstreamDO(); + before.setProtocol("http://"); + before.setUpstreamUrl("localhost:8080"); + before.setProps("{\"warmup\":10,\"gray\":true,\"healthCheckEnabled\":false,\"extension\":{\"x\":1}," + + "\"labels\":{\"release\":\"canary\"}}"); + AtomicReference stored = new AtomicReference<>(before); + when(discoveryUpstreamMapper.selectByDiscoveryHandlerId("123")).thenAnswer(invocation -> Collections.singletonList(stored.get())); + when(discoveryUpstreamMapper.insert(any())).thenAnswer(invocation -> { + stored.set(invocation.getArgument(0)); + return 1; + }); + DiscoveryHandlerDO handler = new DiscoveryHandlerDO(); + handler.setId("123"); + handler.setDiscoveryId("discovery"); + when(discoveryHandlerMapper.selectById("123")).thenReturn(handler); + DiscoveryDO discovery = new DiscoveryDO(); + discovery.setDiscoveryType("local"); + when(discoveryMapper.selectById("discovery")).thenReturn(discovery); + DiscoveryProcessor processor = mock(DiscoveryProcessor.class); + when(discoveryProcessorHolder.chooseProcessor("local")).thenReturn(processor); + proxySelectorService.update(dto); + assertEquals(props, stored.get().getProps()); + ArgumentCaptor> synced = ArgumentCaptor.forClass(List.class); + verify(processor).changeUpstream(any(), synced.capture()); + assertEquals(props, synced.getValue().get(0).getProps()); + } + + @Test + void testDiscoveryEditorRejectsInvalidLabelsBeforeWriting() { + final ProxySelectorAddDTO dto = discoveryEditorRequest("{\"labels\":null}"); + when(discoveryRelMapper.selectByProxySelectorId("123")).thenReturn(buildDiscoveryRelDO()); + assertThrows(ShenyuAdminException.class, () -> proxySelectorService.update(dto)); + assertThrows(ShenyuAdminException.class, () -> proxySelectorService.create(dto)); + assertThrows(ShenyuAdminException.class, () -> proxySelectorService.bindingDiscoveryHandler(dto)); + verify(proxySelectorMapper, never()).update(any()); + verify(proxySelectorMapper, never()).insert(any()); + verify(discoveryUpstreamMapper, never()).deleteByDiscoveryHandlerId(any()); + verify(discoveryUpstreamMapper, never()).insert(any()); + verifyNoInteractions(discoveryHandlerMapper, discoveryMapper, discoveryProcessorHolder); + } + + private ProxySelectorAddDTO discoveryEditorRequest(final String props) { + ProxySelectorAddDTO dto = new ProxySelectorAddDTO(); + dto.setId("123"); + dto.setName("test"); + dto.setDiscovery(new ProxySelectorAddDTO.Discovery()); + ProxySelectorAddDTO.DiscoveryUpstream upstream = new ProxySelectorAddDTO.DiscoveryUpstream(); + upstream.setProtocol("http://"); + upstream.setUrl("localhost:8080"); + upstream.setProps(props); + upstream.setStatus(0); + upstream.setWeight(50); + dto.setDiscoveryUpstreams(Collections.singletonList(upstream)); + return dto; + } + private ProxySelectorDO buildProxySelectorDO() { ProxySelectorDO proxySelectorDO = new ProxySelectorDO(); proxySelectorDO.setId("123"); diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java index 6999eae70427..a0be10a47006 100644 --- a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java +++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java @@ -21,6 +21,8 @@ import com.github.pagehelper.PageInfo; import com.google.common.collect.Lists; import com.google.common.collect.Sets; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; import org.apache.commons.lang3.StringUtils; import org.apache.shenyu.admin.exception.ShenyuAdminException; import org.apache.shenyu.admin.mapper.DataPermissionMapper; @@ -28,6 +30,7 @@ import org.apache.shenyu.admin.mapper.RuleConditionMapper; import org.apache.shenyu.admin.mapper.RuleMapper; import org.apache.shenyu.admin.mapper.SelectorMapper; +import org.apache.shenyu.admin.mapper.ShenyuDictMapper; import org.apache.shenyu.admin.model.custom.UserInfo; import org.apache.shenyu.admin.model.dto.DataPermissionDTO; import org.apache.shenyu.admin.model.dto.RuleConditionDTO; @@ -37,6 +40,7 @@ import org.apache.shenyu.admin.model.entity.RuleConditionDO; import org.apache.shenyu.admin.model.entity.RuleDO; import org.apache.shenyu.admin.model.entity.SelectorDO; +import org.apache.shenyu.admin.model.entity.ShenyuDictDO; import org.apache.shenyu.admin.model.page.CommonPager; import org.apache.shenyu.admin.model.page.PageCondition; import org.apache.shenyu.admin.model.page.PageParameter; @@ -54,6 +58,9 @@ 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.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockedStatic; @@ -85,6 +92,7 @@ import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; /** @@ -94,6 +102,9 @@ @MockitoSettings(strictness = Strictness.LENIENT) public final class RuleServiceTest { + @Mock + private ShenyuDictMapper shenyuDictMapper; + @InjectMocks private RuleServiceImpl ruleService; @@ -118,7 +129,7 @@ public final class RuleServiceTest { @BeforeEach public void setUp() { when(dataPermissionMapper.listByUserId("1")).thenReturn(Collections.singletonList(DataPermissionDO.buildPermissionDO(new DataPermissionDTO()))); - ruleService = new RuleServiceImpl(ruleMapper, ruleConditionMapper, selectorMapper, pluginMapper, ruleEventPublisher); + ruleService = new RuleServiceImpl(ruleMapper, ruleConditionMapper, selectorMapper, pluginMapper, ruleEventPublisher, shenyuDictMapper); } @Test @@ -509,6 +520,133 @@ private void publishEvent() { given(this.ruleConditionMapper.selectByRuleIdSet(Sets.newHashSet("123"))).willReturn(Collections.singletonList(buildRuleConditionDO())); } + @Test + public void testCreateRejectsOverlappingCanaryLabelsBeforeSaving() { + configureDivideSelector(); + RuleDTO dto = buildRuleDTO(""); + dto.setHandle("{\"canary\":{\"enabled\":false,\"canaryLabels\":{\"release\":\"canary\"},\"stableLabels\":{\"region\":\"east\"}}}"); + assertThrows(ShenyuAdminException.class, () -> ruleService.createOrUpdate(dto)); + verify(ruleMapper, never()).insertSelective(any()); + } + + @Test + public void testUpdateRejectsOverlappingCanaryLabelsBeforeSaving() { + configureDivideSelector(); + RuleDTO dto = buildRuleDTO("123"); + dto.setHandle("{\"canary\":{\"canaryLabels\":{\"release\":\"canary\"},\"stableLabels\":{\"release\":\"canary\"}}}"); + given(ruleMapper.selectById("123")).willReturn(buildRuleDO("123")); + assertThrows(ShenyuAdminException.class, () -> ruleService.createOrUpdate(dto)); + verify(ruleMapper, never()).updateSelective(any()); + } + + @Test + public void testSaveAllowsDisabledCanaryWithDisjointPools() { + configureDivideSelector(); + RuleDTO dto = buildRuleDTO(""); + dto.setHandle("{\"canary\":{\"enabled\":false,\"canaryLabels\":{\"release\":\"canary\"},\"stableLabels\":{\"release\":\"stable\"}}}"); + given(ruleMapper.insertSelective(any())).willReturn(1); + try (MockedStatic jwt = mockStatic(JwtUtils.class)) { + jwt.when(JwtUtils::getUserInfo).thenReturn(UserInfo.builder().userId("1").userName("admin").build()); + assertEquals(1, ruleService.createOrUpdate(dto)); + } + } + + @ParameterizedTest + @ValueSource(strings = {"{\"percentage\":101}", "{\"percentage\":20.5}", "{\"matchMode\":2}", + "{\"enabled\":true,\"percentage\":20}", "{\"stableLabels\":{\"release\":true}}", "{\"fallbackPolicy\":\"TYPO\"}", + "{\"stickyKey\":{\"paramType\":\"unregistered\"}}", + "{\"conditions\":[{\"paramType\":\"uri\",\"operator\":\"unregistered\",\"paramValue\":\"/\"}]}"}) + public void testInvalidCanaryNeverWritesOrPublishes(final String patch) { + configureDivideSelector(); + given(shenyuDictMapper.findByType("paramType")).willReturn(Collections.singletonList(dictionary("uri", true))); + RuleDTO dto = buildRuleDTO(""); + dto.setHandle(canaryHandle(patch)); + assertThrows(ShenyuAdminException.class, () -> ruleService.createOrUpdate(dto)); + dto.setId("123"); + given(ruleMapper.selectById("123")).willReturn(buildRuleDO("123")); + assertThrows(ShenyuAdminException.class, () -> ruleService.createOrUpdate(dto)); + verify(ruleMapper, never()).insertSelective(any()); + verify(ruleMapper, never()).updateSelective(any()); + verifyNoInteractions(ruleConditionMapper, ruleEventPublisher); + } + + @Test + public void testImportsValidateAllRulesBeforeInserting() { + configureDivideSelector(); + RuleDTO valid = buildRuleDTO(""); + valid.setHandle(canaryHandle("{}")); + RuleDTO invalid = buildRuleDTO(""); + invalid.setHandle(canaryHandle("{\"percentage\":101}")); + List rules = Arrays.asList(valid, invalid); + assertThrows(ShenyuAdminException.class, () -> ruleService.importData(rules)); + ConfigsImportContext context = new ConfigsImportContext(); + context.getSelectorIdMapping().put("456", "456"); + assertThrows(ShenyuAdminException.class, () -> ruleService.importData(SYS_DEFAULT_NAMESPACE_ID, rules, context)); + verify(ruleMapper, never()).insertSelective(any()); + verifyNoInteractions(ruleConditionMapper, ruleEventPublisher); + } + + @Test + public void testRegistrationRejectsInvalidCanaryBeforePublishing() { + configureDivideSelector(); + RuleDTO dto = buildRuleDTO(""); + dto.setHandle(canaryHandle("{\"percentage\":101}")); + assertThrows(ShenyuAdminException.class, () -> ruleService.registerDefault(dto)); + verify(ruleMapper, never()).insertSelective(any()); + verifyNoInteractions(ruleConditionMapper, ruleEventPublisher); + } + + @Test + public void testEnabledDictionaryExtensionIsAcceptedAndDisabledOneRejected() { + configureDivideSelector(); + RuleDTO dto = buildRuleDTO(""); + dto.setHandle(canaryHandle("{\"enabled\":true,\"percentage\":20,\"stickyKey\":{\"paramType\":\"custom-source\"}}")); + given(shenyuDictMapper.findByType("paramType")).willReturn(Collections.singletonList(dictionary("custom-source", false))); + assertThrows(ShenyuAdminException.class, () -> ruleService.createOrUpdate(dto)); + verifyNoInteractions(ruleEventPublisher); + given(shenyuDictMapper.findByType("paramType")).willReturn(Collections.singletonList(dictionary("custom-source", true))); + given(ruleMapper.insertSelective(any())).willReturn(1); + try (MockedStatic jwt = mockStatic(JwtUtils.class)) { + jwt.when(JwtUtils::getUserInfo).thenReturn(UserInfo.builder().userId("1").userName("admin").build()); + assertEquals(1, ruleService.createOrUpdate(dto)); + } + } + + @Test + public void testSavingLegacyRulePreservesHandleWithoutCanary() { + configureDivideSelector(); + RuleDTO dto = buildRuleDTO("123"); + dto.setHandle("{\"timeout\":5000,\"extension\":{\"value\":true}}"); + given(ruleMapper.selectById("123")).willReturn(buildRuleDO("123")); + given(ruleMapper.updateSelective(any())).willReturn(1); + assertEquals(1, ruleService.createOrUpdate(dto)); + ArgumentCaptor saved = ArgumentCaptor.forClass(RuleDO.class); + verify(ruleMapper).updateSelective(saved.capture()); + assertEquals(dto.getHandle(), saved.getValue().getHandle()); + verifyNoInteractions(shenyuDictMapper); + } + + private ShenyuDictDO dictionary(final String value, final boolean enabled) { + ShenyuDictDO dict = new ShenyuDictDO(); + dict.setDictValue(value); + dict.setEnabled(enabled); + return dict; + } + + private String canaryHandle(final String patch) { + JsonObject config = JsonParser.parseString("{\"stableLabels\":{\"release\":\"stable\"},\"canaryLabels\":{\"release\":\"canary\"}}") + .getAsJsonObject(); + JsonParser.parseString(patch).getAsJsonObject().entrySet().forEach(entry -> config.add(entry.getKey(), entry.getValue())); + JsonObject handle = new JsonObject(); + handle.add("canary", config); + return handle.toString(); + } + + private void configureDivideSelector() { + given(selectorMapper.selectById("456")).willReturn(buildSelectorDO()); + given(pluginMapper.selectById("789")).willReturn(PluginDO.builder().id("789").name("divide").build()); + } + private void testRegisterCreate() { RuleDTO ruleDTO = buildRuleDTO(""); RuleDO ruleDO = RuleDO.buildRuleDO(ruleDTO); diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/transfer/DiscoveryTransferTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/transfer/DiscoveryTransferTest.java index e3b809cb1fbf..af0f383ebd5c 100644 --- a/shenyu-admin/src/test/java/org/apache/shenyu/admin/transfer/DiscoveryTransferTest.java +++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/transfer/DiscoveryTransferTest.java @@ -20,16 +20,53 @@ import org.apache.shenyu.admin.model.dto.DiscoveryDTO; import org.apache.shenyu.admin.model.entity.DiscoveryDO; import org.apache.shenyu.admin.model.vo.DiscoveryVO; +import org.apache.shenyu.common.dto.DiscoveryUpstreamData; +import org.apache.shenyu.common.dto.convert.selector.CommonUpstream; import org.junit.jupiter.api.Test; +import java.sql.Timestamp; + import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; /** - * Test cases for DiscoveryTransfer. + * Test cases for discovery namespace mapping and upstream properties. */ public final class DiscoveryTransferTest { + @Test + void shouldAcceptNestedLabelsWithoutChangingProps() { + DiscoveryUpstreamData data = upstream("{\"labels\":{\"release\":\"canary\"},\"custom\":{\"tls\":true},\"warmupTime\":10,\"gray\":\"false\"}"); + String original = data.getProps(); + CommonUpstream result = DiscoveryTransfer.INSTANCE.mapToCommonUpstream(data); + assertTrue(result.isHealthCheckEnabled()); + assertEquals("127.0.0.1:18882", result.getUpstreamUrl()); + assertEquals(original, data.getProps()); + } + + @Test + void shouldPreserveBooleanAndStringHealthCheckSettings() { + for (String value : new String[]{"false", "\"false\""}) { + assertFalse(DiscoveryTransfer.INSTANCE.mapToCommonUpstream(upstream( + "{\"labels\":{\"release\":\"canary\"},\"healthCheckEnabled\":" + value + "}")).isHealthCheckEnabled()); + } + for (String value : new String[]{"true", "\"true\""}) { + assertTrue(DiscoveryTransfer.INSTANCE.mapToCommonUpstream(upstream( + "{\"healthCheckEnabled\":" + value + "}")).isHealthCheckEnabled()); + } + } + + @Test + void shouldKeepLegacyDefaults() { + for (String props : new String[]{null, "{}", "null", "{\"labels\":{}}", "{\"healthCheckEnabled\":null}"}) { + assertTrue(DiscoveryTransfer.INSTANCE.mapToCommonUpstream(upstream(props)).isHealthCheckEnabled()); + } + assertNull(DiscoveryTransfer.INSTANCE.mapToCommonUpstream(null)); + } + @Test public void testMapToDTOKeepsNamespaceId() { DiscoveryDO discoveryDO = new DiscoveryDO(); @@ -54,8 +91,18 @@ public void testMapToDTOWithoutNamespaceId() { DiscoveryDTO discoveryDTO = DiscoveryTransfer.INSTANCE.mapToDTO(discoveryDO); - assertEquals(null, discoveryDTO.getNamespaceId()); + assertNull(discoveryDTO.getNamespaceId()); } + + private DiscoveryUpstreamData upstream(final String props) { + DiscoveryUpstreamData data = new DiscoveryUpstreamData(); + data.setProtocol("http://"); + data.setUrl("127.0.0.1:18882"); + data.setDateCreated(new Timestamp(0)); + data.setProps(props); + return data; + } + @Test public void testMapToVoKeepsDiscoveryNameAndTypeSeparate() { DiscoveryDO discoveryDO = DiscoveryDO.builder() diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/utils/DiscoveryUpstreamPropsValidatorTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/utils/DiscoveryUpstreamPropsValidatorTest.java new file mode 100644 index 000000000000..75ef1ec4a9c0 --- /dev/null +++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/utils/DiscoveryUpstreamPropsValidatorTest.java @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.admin.utils; + +import org.apache.shenyu.admin.exception.ShenyuAdminException; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Tests submitted property validation, including omitted and explicitly empty labels. + */ +class DiscoveryUpstreamPropsValidatorTest { + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {"{}", "{\"labels\":{}}", "{\"labels\":{\"release\":\"canary\"}}", "{\"warmup\":20}", + "{\"extension\":{\"nested\":[1,true,null]}}"}) + void testValidProps(final String props) { + assertDoesNotThrow(() -> DiscoveryUpstreamPropsValidator.validate(props)); + } + + @ParameterizedTest + @ValueSource(strings = {"null", "[]", "broken", "{\"labels\":null}", "{\"labels\":[]}", + "{\"labels\":{\"release\":1}}", "{\"labels\":{\"release\":true}}", "{\"labels\":{\"release\":null}}", + "{\"labels\":{\" \" :\"stable\"}}"}) + void testInvalidPropsAreRejected(final String props) { + assertThrows(ShenyuAdminException.class, () -> DiscoveryUpstreamPropsValidator.validate(props)); + } +} diff --git a/shenyu-common/pom.xml b/shenyu-common/pom.xml index 379312c653bc..6c84ac31eaa4 100644 --- a/shenyu-common/pom.xml +++ b/shenyu-common/pom.xml @@ -37,6 +37,11 @@ gson + + com.google.re2j + re2j + + org.apache.commons commons-lang3 diff --git a/shenyu-common/src/main/java/org/apache/shenyu/common/constant/Constants.java b/shenyu-common/src/main/java/org/apache/shenyu/common/constant/Constants.java index 82716875f4db..97001e3065b4 100644 --- a/shenyu-common/src/main/java/org/apache/shenyu/common/constant/Constants.java +++ b/shenyu-common/src/main/java/org/apache/shenyu/common/constant/Constants.java @@ -144,6 +144,26 @@ public interface Constants { */ String DIVIDE_SELECTOR_ID = "divideSelectorId"; + /** + * Actual request partition: lowercase stable or canary. + */ + String SHENYU_CANARY_PARTITION = "shenyuCanaryPartition"; + + /** + * Immutable label constraints of the actual request partition. + */ + String SHENYU_CANARY_LABELS = "shenyuCanaryLabels"; + + /** + * Request-scoped canary observation, independent of retry routing attributes. + */ + String SHENYU_CANARY_CONTEXT = "shenyuCanaryContext"; + + /** + * Optional callback receiving the first canary routing observation. + */ + String METRICS_CANARY = "metricsCanary"; + /** * Original response Content-Type attribute name. */ diff --git a/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfig.java b/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfig.java new file mode 100644 index 000000000000..f65d0d85d0c3 --- /dev/null +++ b/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfig.java @@ -0,0 +1,265 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.common.dto.convert.rule.canary; + +import org.apache.shenyu.common.dto.ConditionData; +import org.apache.shenyu.common.enums.MatchModeEnum; + +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Canary routing configuration. + */ +public class CanaryConfig { + + /** + * Whether Canary traffic is enabled. When false, a present configuration routes to Stable. + */ + private boolean enabled; + + /** + * Canary matching conditions. Empty conditions allow all requests to participate. + */ + private List conditions; + + /** + * Condition match mode: 0 for AND (default), 1 for OR. + */ + private Integer matchMode = MatchModeEnum.AND.getCode(); + + /** + * Percentage of eligible identifiers assigned to Canary, from 0 to 100. Defaults to 0. + */ + private int percentage; + + /** + * Identifier source for sticky routing. Required when enabled with a percentage from 1 to 99. + */ + private StickyKeyConfig stickyKey; + + /** + * Labels used to select canary upstreams. + */ + private Map canaryLabels; + + /** + * Labels used to select stable upstreams. + */ + private Map stableLabels; + + /** + * Initial empty Canary pool policy: STABLE (default) or REJECT. + */ + private String fallbackPolicy = "STABLE"; + + /** + * Get enabled. + * + * @return enabled + */ + public boolean isEnabled() { + return enabled; + } + + /** + * Set enabled. + * + * @param enabled enabled + */ + public void setEnabled(final boolean enabled) { + this.enabled = enabled; + } + + /** + * Get conditions. + * + * @return conditions + */ + public List getConditions() { + return conditions; + } + + /** + * Set conditions. + * + * @param conditions conditions + */ + public void setConditions(final List conditions) { + this.conditions = conditions; + } + + /** + * Get matchMode. + * + * @return matchMode + */ + public Integer getMatchMode() { + return matchMode; + } + + /** + * Set matchMode. + * + * @param matchMode matchMode + */ + public void setMatchMode(final Integer matchMode) { + this.matchMode = matchMode; + } + + /** + * Get percentage. + * + * @return percentage + */ + public int getPercentage() { + return percentage; + } + + /** + * Set percentage. + * + * @param percentage percentage + */ + public void setPercentage(final int percentage) { + this.percentage = percentage; + } + + /** + * Get stickyKey. + * + * @return stickyKey + */ + public StickyKeyConfig getStickyKey() { + return stickyKey; + } + + /** + * Set stickyKey. + * + * @param stickyKey stickyKey + */ + public void setStickyKey(final StickyKeyConfig stickyKey) { + this.stickyKey = stickyKey; + } + + /** + * Get canaryLabels. + * + * @return canaryLabels + */ + public Map getCanaryLabels() { + return canaryLabels; + } + + /** + * Set canaryLabels. + * + * @param canaryLabels canaryLabels + */ + public void setCanaryLabels(final Map canaryLabels) { + this.canaryLabels = canaryLabels; + } + + /** + * Get stableLabels. + * + * @return stableLabels + */ + public Map getStableLabels() { + return stableLabels; + } + + /** + * Set stableLabels. + * + * @param stableLabels stableLabels + */ + public void setStableLabels(final Map stableLabels) { + this.stableLabels = stableLabels; + } + + /** + * Get fallbackPolicy. + * + * @return fallbackPolicy + */ + public String getFallbackPolicy() { + return fallbackPolicy; + } + + /** + * Set fallbackPolicy. + * + * @param fallbackPolicy fallbackPolicy + */ + public void setFallbackPolicy(final String fallbackPolicy) { + this.fallbackPolicy = fallbackPolicy; + } + + /** + * Validate that the two label selectors cannot match the same upstream. + * + * @throws IllegalArgumentException when labels are missing or selectors can overlap + */ + public void validatePartitionLabels() { + if (Objects.isNull(canaryLabels) || canaryLabels.isEmpty() || Objects.isNull(stableLabels) || stableLabels.isEmpty()) { + throw new IllegalArgumentException("Canary and Stable labels must both be configured"); + } + boolean disjoint = canaryLabels.entrySet().stream().anyMatch(entry -> stableLabels.containsKey(entry.getKey()) + && !Objects.equals(entry.getValue(), stableLabels.get(entry.getKey()))); + if (!disjoint) { + throw new IllegalArgumentException("Canary and Stable labels must have a shared key with different values"); + } + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (Objects.isNull(o) || getClass() != o.getClass()) { + return false; + } + CanaryConfig that = (CanaryConfig) o; + return enabled == that.enabled && percentage == that.percentage + && Objects.equals(conditions, that.conditions) && Objects.equals(matchMode, that.matchMode) + && Objects.equals(stickyKey, that.stickyKey) && Objects.equals(canaryLabels, that.canaryLabels) + && Objects.equals(stableLabels, that.stableLabels) && Objects.equals(fallbackPolicy, that.fallbackPolicy); + } + + @Override + public int hashCode() { + return Objects.hash(enabled, conditions, matchMode, percentage, stickyKey, canaryLabels, stableLabels, fallbackPolicy); + } + + @Override + public String toString() { + return "CanaryConfig{" + + "enabled=" + enabled + + ", conditions=" + conditions + + ", matchMode=" + matchMode + + ", percentage=" + percentage + + ", stickyKey=" + stickyKey + + ", canaryLabels=" + canaryLabels + + ", stableLabels=" + stableLabels + + ", fallbackPolicy='" + fallbackPolicy + '\'' + + '}'; + } +} diff --git a/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfigValidator.java b/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfigValidator.java new file mode 100644 index 000000000000..edb538091f27 --- /dev/null +++ b/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfigValidator.java @@ -0,0 +1,203 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.common.dto.convert.rule.canary; + +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParseException; +import com.google.re2j.Pattern; +import com.google.re2j.PatternSyntaxException; +import org.apache.commons.lang3.StringUtils; +import org.apache.shenyu.common.dto.ConditionData; +import org.apache.shenyu.common.dto.convert.rule.impl.DivideRuleHandle; +import org.apache.shenyu.common.utils.DateUtils; +import org.apache.shenyu.common.utils.GsonUtils; + +import java.util.Map; +import java.util.Objects; +import java.util.function.Consumer; + +/** + * Shared validation of Canary configuration before persistence or gateway caching. + */ +public final class CanaryConfigValidator { + + private CanaryConfigValidator() { + } + + /** + * Parse a Divide handle, checking JSON types before Gson can coerce values. + * An absent or null Canary configuration preserves legacy routing. + * + * @param handle the serialized rule handle + * @return the validated handle + * @throws IllegalArgumentException when the configuration is invalid + */ + public static DivideRuleHandle parseHandle(final String handle) { + try { + JsonObject root = object(GsonUtils.getInstance().fromJson(handle, JsonElement.class), "Divide handle"); + JsonElement canary = root.get("canary"); + if (Objects.nonNull(canary) && !canary.isJsonNull()) { + validate(object(canary, "canary")); + } + return GsonUtils.getInstance().fromJson(root, DivideRuleHandle.class); + } catch (JsonParseException ex) { + throw new IllegalArgumentException("Invalid Divide handle JSON", ex); + } + } + + /** + * Check extension names against the registry available at the caller. + * Admin uses its dictionaries; gateways use their installed SPI implementations. + * + * @param config the validated configuration + * @param parameterValidator parameter source validator + * @param operatorValidator condition operator validator + */ + public static void validateExtensions(final CanaryConfig config, final Consumer parameterValidator, + final Consumer operatorValidator) { + if (Objects.nonNull(config.getStickyKey())) { + parameterValidator.accept(config.getStickyKey().getParamType()); + } + if (Objects.nonNull(config.getConditions())) { + for (ConditionData condition : config.getConditions()) { + parameterValidator.accept(condition.getParamType()); + operatorValidator.accept(condition.getOperator()); + } + } + } + + private static void validate(final JsonObject config) { + if (config.has("enabled") && !(config.get("enabled").isJsonPrimitive() && config.getAsJsonPrimitive("enabled").isBoolean())) { + throw new IllegalArgumentException("canary.enabled must be a boolean"); + } + int percentage = integer(config, "percentage", 0); + if (percentage < 0 || percentage > 100) { + throw new IllegalArgumentException("canary.percentage must be between 0 and 100"); + } + int matchMode = integer(config, "matchMode", 0); + if (matchMode != 0 && matchMode != 1) { + throw new IllegalArgumentException("canary.matchMode must be 0 (AND) or 1 (OR)"); + } + if (config.has("fallbackPolicy")) { + String policy = string(config, "fallbackPolicy", "canary.fallbackPolicy", true); + if (!"STABLE".equals(policy) && !"REJECT".equals(policy)) { + throw new IllegalArgumentException("canary.fallbackPolicy must be STABLE or REJECT"); + } + } + validateLabels(config.get("stableLabels"), "canary.stableLabels", false); + validateLabels(config.get("canaryLabels"), "canary.canaryLabels", false); + if (config.has("stickyKey")) { + validateSource(object(config.get("stickyKey"), "canary.stickyKey"), "canary.stickyKey"); + } else if (config.has("enabled") && config.get("enabled").getAsBoolean() && percentage > 0 && percentage < 100) { + throw new IllegalArgumentException("canary.stickyKey is required when enabled with a percentage between 1 and 99"); + } + if (config.has("conditions")) { + JsonElement conditions = config.get("conditions"); + if (!conditions.isJsonArray()) { + throw new IllegalArgumentException("canary.conditions must be an array"); + } + int index = 0; + for (JsonElement element : conditions.getAsJsonArray()) { + String path = "canary.conditions[" + index++ + "]"; + JsonObject condition = object(element, path); + validateSource(condition, path); + String operator = string(condition, "operator", path + ".operator", true); + String value = string(condition, "paramValue", path + ".paramValue", !"isBlank".equals(operator)); + validateConditionValue(operator, value, path); + } + } + GsonUtils.getInstance().fromJson(config, CanaryConfig.class).validatePartitionLabels(); + } + + private static void validateConditionValue(final String operator, final String value, final String path) { + try { + if ("regex".equals(operator)) { + // Validate with the same regex engine used by the gateway's RegexPredicateJudge. + Pattern.compile(value.trim()); + } else if ("TimeBefore".equals(operator) || "TimeAfter".equals(operator)) { + DateUtils.parseLocalDateTime(value.trim()); + } + } catch (PatternSyntaxException | IllegalArgumentException | java.time.DateTimeException ex) { + throw new IllegalArgumentException(path + ".paramValue is invalid for " + operator, ex); + } + } + + private static void validateSource(final JsonObject source, final String path) { + String type = string(source, "paramType", path + ".paramType", true); + boolean named = "header".equals(type) || "cookie".equals(type) || "query".equals(type) || "post".equals(type); + string(source, "paramName", path + ".paramName", named); + } + + /** + * Validate a label map without coercing numbers or booleans to strings. + * + * @param labels the JSON labels + * @param path field name for errors + * @param allowEmpty whether an empty map is allowed (clearing instance labels) + */ + public static void validateLabels(final JsonElement labels, final String path, final boolean allowEmpty) { + JsonObject map = object(labels, path); + if (!allowEmpty && map.size() == 0) { + throw new IllegalArgumentException(path + " must not be empty"); + } + for (Map.Entry entry : map.entrySet()) { + if (StringUtils.isBlank(entry.getKey())) { + throw new IllegalArgumentException(path + " keys must not be blank"); + } + string(map, entry.getKey(), path + "." + entry.getKey(), true); + } + } + + private static JsonObject object(final JsonElement value, final String path) { + if (Objects.isNull(value) || !value.isJsonObject()) { + throw new IllegalArgumentException(path + " must be an object"); + } + return value.getAsJsonObject(); + } + + private static int integer(final JsonObject object, final String name, final int defaultValue) { + if (!object.has(name)) { + return defaultValue; + } + JsonElement value = object.get(name); + if (!(value.isJsonPrimitive() && value.getAsJsonPrimitive().isNumber())) { + throw new IllegalArgumentException("canary." + name + " must be an integer"); + } + try { + return value.getAsBigDecimal().intValueExact(); + } catch (ArithmeticException | NumberFormatException ex) { + throw new IllegalArgumentException("canary." + name + " must be an integer", ex); + } + } + + private static String string(final JsonObject object, final String name, final String path, final boolean required) { + if (!object.has(name) && !required) { + return null; + } + JsonElement value = object.get(name); + if (Objects.isNull(value) || !(value.isJsonPrimitive() && value.getAsJsonPrimitive().isString())) { + throw new IllegalArgumentException(path + " must be a string"); + } + String result = value.getAsString(); + if (required && StringUtils.isBlank(result)) { + throw new IllegalArgumentException(path + " must not be blank"); + } + return result; + } +} diff --git a/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/StickyKeyConfig.java b/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/StickyKeyConfig.java new file mode 100644 index 000000000000..3d3924809eaf --- /dev/null +++ b/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/canary/StickyKeyConfig.java @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.common.dto.convert.rule.canary; + +import java.util.Objects; + +/** + * Identifier source for stable canary grouping. + */ +public class StickyKeyConfig { + + /** + * Registered ParameterData SPI source name, such as header, cookie, query, ip or a custom extension. + */ + private String paramType; + + /** + * Source-specific parameter name, passed unchanged to the selected ParameterData implementation. + * Canary does not validate the parameter name; the source determines how it is used. + */ + private String paramName; + + /** + * Get paramType. + * + * @return paramType + */ + public String getParamType() { + return paramType; + } + + /** + * Set paramType. + * + * @param paramType paramType + */ + public void setParamType(final String paramType) { + this.paramType = paramType; + } + + /** + * Get paramName. + * + * @return paramName + */ + public String getParamName() { + return paramName; + } + + /** + * Set paramName. + * + * @param paramName paramName + */ + public void setParamName(final String paramName) { + this.paramName = paramName; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (Objects.isNull(o) || getClass() != o.getClass()) { + return false; + } + StickyKeyConfig that = (StickyKeyConfig) o; + return Objects.equals(paramType, that.paramType) && Objects.equals(paramName, that.paramName); + } + + @Override + public int hashCode() { + return Objects.hash(paramType, paramName); + } + + @Override + public String toString() { + return "StickyKeyConfig{" + + "paramType='" + paramType + '\'' + + ", paramName='" + paramName + '\'' + + '}'; + } +} diff --git a/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandle.java b/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandle.java index 84be0832c102..b8670cb0c86b 100644 --- a/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandle.java +++ b/shenyu-common/src/main/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandle.java @@ -19,6 +19,7 @@ import org.apache.shenyu.common.constant.Constants; import org.apache.shenyu.common.dto.convert.rule.RuleHandle; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfig; import org.apache.shenyu.common.enums.HttpRetryBackoffSpecEnum; import org.apache.shenyu.common.enums.LoadBalanceEnum; import org.apache.shenyu.common.enums.RetryEnum; @@ -63,7 +64,12 @@ public class DivideRuleHandle implements RuleHandle { * requestMaxSize. */ private long requestMaxSize; - + + /** + * Canary routing configuration. + */ + private CanaryConfig canary; + /** * New instance divide rule handle. * @@ -181,6 +187,24 @@ public void setRequestMaxSize(final long requestMaxSize) { this.requestMaxSize = requestMaxSize; } + /** + * Get canary configuration. + * + * @return canary configuration + */ + public CanaryConfig getCanary() { + return canary; + } + + /** + * Set canary configuration. + * + * @param canary canary configuration + */ + public void setCanary(final CanaryConfig canary) { + this.canary = canary; + } + public String getRetryBackOffSpec() { return retryBackOffSpec; } @@ -201,12 +225,12 @@ public boolean equals(final Object o) { return retry == that.retry && timeout == that.timeout && headerMaxSize == that.headerMaxSize && requestMaxSize == that.requestMaxSize && Objects.equals(loadBalance, that.loadBalance) && Objects.equals(retryStrategy, that.retryStrategy) - && Objects.equals(retryBackOffSpec, that.retryBackOffSpec); + && Objects.equals(retryBackOffSpec, that.retryBackOffSpec) && Objects.equals(canary, that.canary); } @Override public int hashCode() { - return Objects.hash(loadBalance, retryStrategy, retryBackOffSpec, retry, timeout, headerMaxSize, requestMaxSize); + return Objects.hash(loadBalance, retryStrategy, retryBackOffSpec, retry, timeout, headerMaxSize, requestMaxSize, canary); } @Override @@ -229,6 +253,8 @@ public String toString() { + headerMaxSize + ", requestMaxSize=" + requestMaxSize + + ", canary=" + + canary + '}'; } } diff --git a/shenyu-common/src/test/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfigValidatorTest.java b/shenyu-common/src/test/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfigValidatorTest.java new file mode 100644 index 000000000000..1ebff0f70b0e --- /dev/null +++ b/shenyu-common/src/test/java/org/apache/shenyu/common/dto/convert/rule/canary/CanaryConfigValidatorTest.java @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.common.dto.convert.rule.canary; + +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; +import com.google.re2j.PatternSyntaxException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Canary wire contract validation tests. + */ +class CanaryConfigValidatorTest { + + @Test + void testLegacyHandleDoesNotAcquireCanaryConfiguration() { + assertNull(CanaryConfigValidator.parseHandle("{\"timeout\":5000}").getCanary()); + assertNull(CanaryConfigValidator.parseHandle("{\"canary\":null}").getCanary()); + } + + @ParameterizedTest + @ValueSource(strings = { + "{\"percentage\":-1}", "{\"percentage\":101}", "{\"percentage\":1.5}", "{\"percentage\":\"20\"}", + "{\"percentage\":null}", "{\"percentage\":1e20}", "{\"enabled\":\"false\"}", "{\"enabled\":null}", + "{\"matchMode\":2}", "{\"matchMode\":0.5}", "{\"matchMode\":null}", "{\"fallbackPolicy\":\"stable\"}", + "{\"fallbackPolicy\":null}", "{\"stickyKey\":null}", "{\"stickyKey\":{}}", "{\"stickyKey\":{\"paramType\":\" \"}}", + "{\"stickyKey\":{\"paramType\":\"header\"}}", "{\"enabled\":true,\"percentage\":20}", + "{\"conditions\":{}}", "{\"conditions\":null}", "{\"conditions\":[null]}", "{\"conditions\":[{}]}", + "{\"conditions\":[{\"paramType\":\"header\",\"operator\":\"=\",\"paramValue\":\"east\"}]}", + "{\"conditions\":[{\"paramType\":\"uri\",\"operator\":\"=\"}]}", + "{\"conditions\":[{\"paramType\":\"uri\",\"operator\":\"regex\",\"paramValue\":\"[\"}]}", + "{\"conditions\":[{\"paramType\":\"uri\",\"operator\":\"TimeAfter\",\"paramValue\":\"yesterday\"}]}", + "{\"stableLabels\":{}}", "{\"stableLabels\":null}", "{\"stableLabels\":{\"release\":1}}", + "{\"stableLabels\":{\"release\":true}}", "{\"stableLabels\":{\"release\":null}}", + "{\"stableLabels\":{\" \" :\"stable\"}}", "{\"stableLabels\":{\"release\":\" \"}}", + "{\"stableLabels\":{\"release\":\"canary\"}}", "{\"stableLabels\":{\"region\":\"east\"}}" + }) + void testRejectsInvalidConfigurationBeforeCoercion(final String patch) { + assertThrows(IllegalArgumentException.class, () -> CanaryConfigValidator.parseHandle(handle(patch))); + } + + @ParameterizedTest + @ValueSource(strings = { + "{}", "{\"enabled\":true,\"percentage\":0}", "{\"enabled\":true,\"percentage\":100}", + "{\"enabled\":false,\"percentage\":20}", + "{\"enabled\":true,\"percentage\":20,\"stickyKey\":{\"paramType\":\"header\",\"paramName\":\"X-User-Id\"}}", + "{\"enabled\":true,\"percentage\":20,\"stickyKey\":{\"paramType\":\"ip\"}}", + "{\"stickyKey\":{\"paramType\":\"custom-extension\"}}", + "{\"conditions\":[]}", "{\"matchMode\":1,\"fallbackPolicy\":\"REJECT\"}", + "{\"conditions\":[{\"paramType\":\"header\",\"paramName\":\"X-Optional\",\"operator\":\"isBlank\"}]}", + "{\"conditions\":[{\"paramType\":\"uri\",\"operator\":\"regex\",\"paramValue\":\"/orders/.*\"}]}" + }) + void testAcceptsValidBoundariesAndExtensionNames(final String patch) { + assertDoesNotThrow(() -> CanaryConfigValidator.parseHandle(handle(patch))); + } + + @ParameterizedTest + @ValueSource(strings = {"[", "(?=a)a", "(?<=a)b", "(a)\\1"}) + void testRejectsRegexUnsupportedByGatewayWithFieldError(final String regex) { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> CanaryConfigValidator.parseHandle(regexHandle(regex))); + assertEquals("canary.conditions[0].paramValue is invalid for regex", exception.getMessage()); + assertEquals(PatternSyntaxException.class, exception.getCause().getClass()); + } + + @Test + void testDefaultsRemainStable() { + CanaryConfig config = CanaryConfigValidator.parseHandle(handle("{}")).getCanary(); + assertEquals(0, config.getPercentage()); + assertEquals(0, config.getMatchMode()); + assertEquals("STABLE", config.getFallbackPolicy()); + } + + private String regexHandle(final String regex) { + JsonObject condition = new JsonObject(); + condition.addProperty("paramType", "uri"); + condition.addProperty("operator", "regex"); + condition.addProperty("paramValue", regex); + JsonArray conditions = new JsonArray(); + conditions.add(condition); + JsonObject config = new JsonObject(); + config.add("conditions", conditions); + return handle(config.toString()); + } + + private String handle(final String patch) { + JsonObject config = JsonParser.parseString("{\"stableLabels\":{\"release\":\"stable\"},\"canaryLabels\":{\"release\":\"canary\"}}") + .getAsJsonObject(); + JsonParser.parseString(patch).getAsJsonObject().entrySet().forEach(entry -> config.add(entry.getKey(), entry.getValue())); + JsonObject handle = new JsonObject(); + handle.add("canary", config); + return handle.toString(); + } +} diff --git a/shenyu-common/src/test/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandleTest.java b/shenyu-common/src/test/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandleTest.java index e705b662698d..a4b51518dfea 100644 --- a/shenyu-common/src/test/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandleTest.java +++ b/shenyu-common/src/test/java/org/apache/shenyu/common/dto/convert/rule/impl/DivideRuleHandleTest.java @@ -18,11 +18,16 @@ package org.apache.shenyu.common.dto.convert.rule.impl; import com.google.common.collect.ImmutableSet; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfig; import org.apache.shenyu.common.enums.HttpRetryBackoffSpecEnum; import org.apache.shenyu.common.enums.LoadBalanceEnum; import org.apache.shenyu.common.enums.RetryEnum; import org.junit.jupiter.api.Test; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.Is.is; @@ -61,4 +66,43 @@ public void testEqualsAndHashCode() { assertThat(ImmutableSet.of(handle1, handle2), hasSize(1)); } + @Test + public void testCanaryConfigurationEquality() { + final DivideRuleHandle handle1 = new DivideRuleHandle(); + final DivideRuleHandle handle2 = new DivideRuleHandle(); + CanaryConfig canary1 = new CanaryConfig(); + CanaryConfig canary2 = new CanaryConfig(); + canary1.setEnabled(true); + canary1.setPercentage(10); + canary2.setEnabled(true); + canary2.setPercentage(10); + handle1.setCanary(canary1); + handle2.setCanary(canary2); + + assertThat(handle1.getCanary(), is(canary1)); + assertThat(ImmutableSet.of(handle1, handle2), hasSize(1)); + + canary2.setPercentage(20); + assertThat(handle1.equals(handle2), is(false)); + assertThat(ImmutableSet.of(handle1, handle2), hasSize(2)); + + handle2.setCanary(null); + assertThat(handle1.equals(handle2), is(false)); + } + + @Test + public void testPartitionLabelsMustBeMutuallyExclusive() { + CanaryConfig config = new CanaryConfig(); + assertThrows(IllegalArgumentException.class, config::validatePartitionLabels); + config.setCanaryLabels(Map.of("release", "canary")); + config.setStableLabels(Map.of("release", "stable")); + assertDoesNotThrow(config::validatePartitionLabels); + config.setStableLabels(Map.of("release", "canary")); + assertThrows(IllegalArgumentException.class, config::validatePartitionLabels); + config.setStableLabels(Map.of("region", "east")); + assertThrows(IllegalArgumentException.class, config::validatePartitionLabels); + config.setStableLabels(Map.of("release", "stable", "region", "east")); + assertDoesNotThrow(config::validatePartitionLabels); + } + } diff --git a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java index d6e430adb349..aaebec0ad0c5 100644 --- a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java +++ b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java @@ -27,6 +27,8 @@ import org.apache.shenyu.loadbalancer.entity.Upstream; import org.apache.shenyu.loadbalancer.entity.UpstreamIdentity; +import java.util.Collections; +import java.util.HashMap; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -47,6 +49,9 @@ public final class UpstreamCacheManager { private static final Map> UPSTREAM_MAP = Maps.newConcurrentMap(); + // Presence is based on configured nodes, including unhealthy gray nodes. + private static final Map LEGACY_GRAY_SELECTORS = Maps.newConcurrentMap(); + private UpstreamCheckTask task; /** @@ -130,6 +135,20 @@ public List findUpstreamListBySelectorId(final String selectorId) { return Objects.isNull(upstreamList) ? null : new ArrayList<>(upstreamList); } + /** + * Find healthy nodes using the legacy gray-exclusive pool policy. + * + * @param selectorId selector identifier + * @return gray healthy nodes when gray is configured, otherwise all healthy nodes + */ + public List findLegacyUpstreamListBySelectorId(final String selectorId) { + List healthy = findUpstreamListBySelectorId(selectorId); + if (Objects.isNull(healthy) || !Boolean.TRUE.equals(LEGACY_GRAY_SELECTORS.get(selectorId))) { + return healthy; + } + return healthy.stream().filter(Upstream::isGray).collect(Collectors.toList()); + } + /** * Remove by key. * @@ -137,6 +156,7 @@ public List findUpstreamListBySelectorId(final String selectorId) { */ public void removeByKey(final String key) { UPSTREAM_MAP.remove(key); + LEGACY_GRAY_SELECTORS.remove(key); task.triggerRemoveAll(key); } @@ -155,6 +175,7 @@ public void submit(final String selectorId, final List upstreamList) { return; } + LEGACY_GRAY_SELECTORS.put(selectorId, actualUpstreamList.stream().anyMatch(Upstream::isGray)); initializeUpstreamHealthStatus(actualUpstreamList); Map> partitionedUpstreams = actualUpstreamList.stream() @@ -163,6 +184,7 @@ public void submit(final String selectorId, final List upstreamList) { List offlineUpstreamList = partitionedUpstreams.get(false); List existUpstreamList = MapUtils.computeIfAbsent(UPSTREAM_MAP, selectorId, k -> Lists.newArrayList()); + updateUpstreamMetadata(selectorId, actualUpstreamList, existUpstreamList); processOfflineUpstreams(selectorId, offlineUpstreamList, existUpstreamList); processValidUpstreams(selectorId, validUpstreamList, existUpstreamList); @@ -170,6 +192,19 @@ public void submit(final String selectorId, final List upstreamList) { UPSTREAM_MAP.put(selectorId, Objects.isNull(healthyUpstreamList) ? Lists.newArrayList() : healthyUpstreamList); } + private void updateUpstreamMetadata(final String selectorId, final List upstreams, final List existingHealthy) { + Map existing = new HashMap<>(getCurrentUnhealthyMap(selectorId)); + existingHealthy.forEach(upstream -> existing.put(upstream.identity(), upstream)); + upstreams.forEach(upstream -> { + Upstream cached = existing.get(upstream.identity()); + if (Objects.nonNull(cached)) { + cached.setMetadata(Collections.unmodifiableMap(new HashMap<>(upstream.getMetadata()))); + cached.setLabels(upstream.getLabels()); + cached.setGray(upstream.isGray()); + } + }); + } + private void initializeUpstreamHealthStatus(final List upstreamList) { upstreamList.forEach(upstream -> { if (!upstream.isHealthCheckEnabled()) { diff --git a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java index d8788de44ad6..0a73f429daef 100644 --- a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java +++ b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java @@ -19,6 +19,7 @@ import org.apache.commons.lang3.StringUtils; +import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -105,14 +106,16 @@ public final class Upstream { /** * this is gray. */ - private boolean gray; + private volatile boolean gray; /** * health check enabled. */ private boolean healthCheckEnabled = true; - private Map metadata = new ConcurrentHashMap<>(); + private volatile Map labels = Collections.emptyMap(); + + private volatile Map metadata = new ConcurrentHashMap<>(); /** @@ -137,6 +140,24 @@ private Upstream(final Builder builder) { this.healthCheckEnabled = builder.healthCheckEnabled; } + /** + * Gets routing labels, independently of upstream metadata. + * + * @return routing labels + */ + public Map getLabels() { + return labels; + } + + /** + * Sets routing labels. + * + * @param labels routing labels + */ + public void setLabels(final Map labels) { + this.labels = labels; + } + /** * Gets protocol. * diff --git a/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java b/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java index 21a47d9ff6fa..b9ebbe9294c0 100644 --- a/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java +++ b/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java @@ -30,6 +30,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Objects; /** @@ -403,6 +404,10 @@ public void testSubmitPreservesLogicalUpstreamIdentity(final String secondGroup, .group("blue").version("v1").weight(30).healthCheckEnabled(false).build(); final Upstream updatedSecond = Upstream.builder().protocol("dubbo://").url("127.0.0.1:20880") .group(secondGroup).version(secondVersion).weight(40).healthCheckEnabled(false).build(); + updatedFirst.setLabels(Map.of("release", "canary")); + updatedFirst.setMetadata(Map.of("region", "east")); + updatedSecond.setLabels(Map.of("release", "stable")); + updatedSecond.setMetadata(Map.of("region", "west")); manager.submit(selectorId, List.of(updatedFirst, updatedSecond)); List updated = manager.findUpstreamListBySelectorId(selectorId); Assertions.assertNotNull(updated); @@ -411,6 +416,10 @@ public void testSubmitPreservesLogicalUpstreamIdentity(final String secondGroup, Assertions.assertSame(second, updated.get(1)); Assertions.assertEquals(30, first.getWeight()); Assertions.assertEquals(40, second.getWeight()); + Assertions.assertEquals(Map.of("release", "canary"), first.getLabels()); + Assertions.assertEquals(Map.of("region", "east"), first.getMetadata()); + Assertions.assertEquals(Map.of("release", "stable"), second.getLabels()); + Assertions.assertEquals(Map.of("region", "west"), second.getMetadata()); final Upstream offlineFirst = Upstream.builder().protocol("dubbo://").url("127.0.0.1:20880") .group("blue").version("v1").status(false).healthCheckEnabled(true).build(); @@ -472,4 +481,51 @@ private UpstreamCheckTask getUpstreamCheckTask(final UpstreamCacheManager manage return null; } } + + @Test + public void testLegacyGrayViewRetainsFullPoolAndUpdatesFlags() { + UpstreamCacheManager cache = UpstreamCacheManager.getInstance(); + String selector = "LEGACY_GRAY_VIEW"; + Upstream gray = Upstream.builder().url("gray:8080").protocol("http://").status(true).healthCheckEnabled(false).gray(true).build(); + Upstream stable = Upstream.builder().url("stable:8080").protocol("http://").status(true).healthCheckEnabled(false).build(); + try { + cache.submit(selector, List.of(gray, stable)); + Assertions.assertEquals(2, cache.findUpstreamListBySelectorId(selector).size()); + Assertions.assertEquals(List.of(gray), cache.findLegacyUpstreamListBySelectorId(selector)); + gray.getSucceeded().set(17); + Upstream updated = Upstream.builder().url("gray:8080").protocol("http://").status(true).healthCheckEnabled(false).gray(false).build(); + cache.submit(selector, List.of(updated, stable)); + Assertions.assertFalse(gray.isGray()); + Assertions.assertEquals(17, gray.getSucceeded().get()); + Assertions.assertEquals(2, cache.findLegacyUpstreamListBySelectorId(selector).size()); + updated.setGray(true); + cache.submit(selector, List.of(updated, stable)); + Assertions.assertEquals(List.of(gray), cache.findLegacyUpstreamListBySelectorId(selector)); + cache.submit(selector, List.of()); + Assertions.assertNull(cache.findLegacyUpstreamListBySelectorId(selector)); + cache.submit(selector, List.of(stable)); + Assertions.assertEquals(List.of(stable), cache.findLegacyUpstreamListBySelectorId(selector)); + } finally { + cache.removeByKey(selector); + } + } + + @Test + public void testUnhealthyGrayDoesNotEnableStableLegacyFallback() { + UpstreamCacheManager cache = UpstreamCacheManager.getInstance(); + String selector = "UNHEALTHY_GRAY_VIEW"; + Upstream gray = Upstream.builder().url("gray.invalid:8080").protocol("http://").status(false).gray(true).build(); + Upstream stable = Upstream.builder().url("stable:8080").protocol("http://").status(true).healthCheckEnabled(false).build(); + try { + cache.submit(selector, List.of(gray, stable)); + Assertions.assertEquals(List.of(stable), cache.findUpstreamListBySelectorId(selector)); + Assertions.assertTrue(cache.findLegacyUpstreamListBySelectorId(selector).isEmpty()); + gray.setGray(false); + cache.submit(selector, List.of(gray, stable)); + Assertions.assertEquals(List.of(stable), cache.findLegacyUpstreamListBySelectorId(selector)); + } finally { + cache.removeByKey(selector); + } + } + } diff --git a/shenyu-plugin/shenyu-plugin-api/src/main/java/org/apache/shenyu/plugin/api/context/CanaryContext.java b/shenyu-plugin/shenyu-plugin-api/src/main/java/org/apache/shenyu/plugin/api/context/CanaryContext.java new file mode 100644 index 000000000000..ad6357ccebf1 --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-api/src/main/java/org/apache/shenyu/plugin/api/context/CanaryContext.java @@ -0,0 +1,169 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.api.context; + +import java.util.Objects; + +/** + * Request-scoped observation populated by Divide during the first canary selection. + * Retry routing continues to use its existing exchange attributes. + */ +public final class CanaryContext { + + public static final String CANARY_POOL_EMPTY = "canary_pool_empty"; + + public static final String STABLE_POOL_EMPTY = "stable_pool_empty"; + + public static final String NO_UPSTREAM_SELECTED = "no_upstream_selected"; + + private final String selectorId; + + private final String ruleId; + + private final String intendedPartition; + + private String actualPartition; + + private String fallbackReason; + + private String rejectReason; + + private final long decisionDurationNanos; + + /** + * Creates a routing observation. An unsuccessful selection has no actual partition. + * + * @param selectorId selector configuration ID + * @param ruleId rule configuration ID + * @param intendedPartition original decision: stable or canary + * @param actualPartition selected partition, or null if no upstream was selected + * @param fallbackReason successful fallback reason, or null + * @param rejectReason routing rejection reason, or null + * @param decisionDurationNanos duration of the initial decide call + */ + public CanaryContext(final String selectorId, final String ruleId, final String intendedPartition, + final String actualPartition, final String fallbackReason, final String rejectReason, + final long decisionDurationNanos) { + this.selectorId = selectorId; + this.ruleId = ruleId; + this.intendedPartition = intendedPartition; + this.actualPartition = actualPartition; + this.fallbackReason = fallbackReason; + this.rejectReason = rejectReason; + this.decisionDurationNanos = decisionDurationNanos; + } + + /** + * Gets the selector configuration ID. + * + * @return the selector configuration ID + */ + public String getSelectorId() { + return selectorId; + } + + /** + * Gets the rule configuration ID. + * + * @return the rule configuration ID + */ + public String getRuleId() { + return ruleId; + } + + /** + * Gets the originally intended partition. + * + * @return the originally intended partition + */ + public String getIntendedPartition() { + return intendedPartition; + } + + /** + * Gets the selected partition, or null. + * + * @return the selected partition, or null + */ + public String getActualPartition() { + return actualPartition; + } + + /** + * Sets the selected partition, or null. + * + * @param actualPartition the selected partition, or null + */ + public void setActualPartition(final String actualPartition) { + this.actualPartition = actualPartition; + } + + /** + * Gets the successful fallback reason, or null. + * + * @return the successful fallback reason, or null + */ + public String getFallbackReason() { + return fallbackReason; + } + + /** + * Sets the successful fallback reason, or null. + * + * @param fallbackReason the successful fallback reason, or null + */ + public void setFallbackReason(final String fallbackReason) { + this.fallbackReason = fallbackReason; + } + + /** + * Gets the rejection reason, or null. + * + * @return the rejection reason, or null + */ + public String getRejectReason() { + return rejectReason; + } + + /** + * Sets the rejection reason, or null. + * + * @param rejectReason the rejection reason, or null + */ + public void setRejectReason(final String rejectReason) { + this.rejectReason = rejectReason; + } + + /** + * Gets the initial decision duration in nanoseconds. + * + * @return the initial decision duration in nanoseconds + */ + public long getDecisionDurationNanos() { + return decisionDurationNanos; + } + + /** + * Gets the metric partition, attributing unselected requests to their original intent. + * + * @return stable or canary + */ + public String getMetricPartition() { + return Objects.isNull(actualPartition) ? intendedPartition : actualPartition; + } +} diff --git a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/utils/UpstreamLabelUtils.java b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/utils/UpstreamLabelUtils.java new file mode 100644 index 000000000000..4530c45d9e1f --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/utils/UpstreamLabelUtils.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.base.utils; + +import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.collections4.MapUtils; +import org.apache.shenyu.loadbalancer.entity.Upstream; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +/** + * Shared label matching for initial routing and retries. + */ +public final class UpstreamLabelUtils { + + private UpstreamLabelUtils() { + } + + /** + * Select upstreams matching every label. Missing partition labels select no nodes. + * + * @param upstreams latest healthy upstreams + * @param labels partition label constraints + * @return matching upstreams without modifying the supplied list or nodes + */ + public static List filter(final List upstreams, final Map labels) { + if (CollectionUtils.isEmpty(upstreams) || MapUtils.isEmpty(labels)) { + return Collections.emptyList(); + } + return upstreams.stream().filter(upstream -> { + Map upstreamLabels = upstream.getLabels(); + return MapUtils.isNotEmpty(upstreamLabels) && upstreamLabels.entrySet().containsAll(labels.entrySet()); + }) + .collect(Collectors.toList()); + } +} diff --git a/shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/DefaultRetryStrategy.java b/shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/DefaultRetryStrategy.java index 01c9c96f3f48..74f7163b4928 100644 --- a/shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/DefaultRetryStrategy.java +++ b/shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/DefaultRetryStrategy.java @@ -26,6 +26,7 @@ import org.apache.shenyu.plugin.api.utils.RequestUrlUtils; import org.apache.shenyu.plugin.base.circuitbreaker.UpstreamCircuitBreaker; import org.apache.shenyu.plugin.base.utils.LoadbalancerUtils; +import org.apache.shenyu.plugin.base.utils.UpstreamLabelUtils; import org.apache.shenyu.plugin.httpclient.exception.ShenyuTimeoutException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,6 +42,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Locale; import java.util.Objects; import java.util.Optional; @@ -104,7 +106,7 @@ Mono executeFailover(final Mono clientResponse, final ServerWebExchange ex final Set exclude = new HashSet<>(Collections.singletonList(Objects.requireNonNull(exchange.getAttribute(Constants.HTTP_URI)))); Mono result = clientResponse; for (int i = 0; i < retryTimes; i++) { - result = result.onErrorResume(retryFilter, th -> backoff.isZero() + result = result.onErrorResume(th -> !(th instanceof FailoverExhaustedException) && retryFilter.test(th), th -> backoff.isZero() ? resend(exchange, duration, exclude) : Mono.delay(backoff).flatMap(ignored -> resend(exchange, duration, exclude))); } @@ -119,11 +121,19 @@ private Mono resend(final ServerWebExchange exchange, final String selectorId = exchange.getAttribute(Constants.DIVIDE_SELECTOR_ID); final String loadBalance = exchange.getAttribute(Constants.LOAD_BALANCE); //always query the latest available list - final List availableUpstreamList = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId(selectorId); - if (Objects.isNull(availableUpstreamList)) { - return Mono.error(new ShenyuException("CANNOT_FIND_HEALTHY_UPSTREAM_URL_AFTER_FAILOVER")); + String partition = exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION); + UpstreamCacheManager upstreamCache = UpstreamCacheManager.getInstance(); + List healthyUpstreams = Objects.isNull(partition) + ? upstreamCache.findLegacyUpstreamListBySelectorId(selectorId) + : upstreamCache.findUpstreamListBySelectorId(selectorId); + if (Objects.isNull(healthyUpstreams)) { + return Mono.error(new FailoverExhaustedException()); } - final List upstreamList = availableUpstreamList + if (Objects.nonNull(partition)) { + Map labels = exchange.getAttribute(Constants.SHENYU_CANARY_LABELS); + healthyUpstreams = UpstreamLabelUtils.filter(healthyUpstreams, labels); + } + final List upstreamList = healthyUpstreams .stream().filter(data -> { final URI upstreamUri = URI.create(data.buildDomain()); for (URI needToExclude : exclude) { @@ -139,12 +149,12 @@ private Mono resend(final ServerWebExchange exchange, .collect(Collectors.toList()); if (upstreamList.isEmpty()) { // no need to retry anymore - return Mono.error(new ShenyuException("CANNOT_FIND_HEALTHY_UPSTREAM_URL_AFTER_FAILOVER")); + return Mono.error(new FailoverExhaustedException()); } final Upstream upstream = LoadbalancerUtils.getForExchange(upstreamList, loadBalance, exchange); if (Objects.isNull(upstream)) { // no need to retry anymore - return Mono.error(new ShenyuException("CANNOT_FIND_HEALTHY_UPSTREAM_URL_AFTER_FAILOVER")); + return Mono.error(new FailoverExhaustedException()); } final URI newUri = RequestUrlUtils.buildRequestUri(exchange, upstream.buildDomain()); // in order not to affect the next retry call, newUri needs to be excluded @@ -163,4 +173,13 @@ private boolean isSameUpstream(final URI first, final URI second) { && first.getHost().equalsIgnoreCase(second.getHost()) && UriUtils.getActualPort(firstScheme, first.getPort()) == UriUtils.getActualPort(secondScheme, second.getPort()); } + + private static final class FailoverExhaustedException extends ShenyuException { + + private static final long serialVersionUID = 1L; + + private FailoverExhaustedException() { + super("CANNOT_FIND_HEALTHY_UPSTREAM_URL_AFTER_FAILOVER"); + } + } } diff --git a/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/DefaultRetryStrategyCanaryTest.java b/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/DefaultRetryStrategyCanaryTest.java new file mode 100644 index 000000000000..9219b33290b9 --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/DefaultRetryStrategyCanaryTest.java @@ -0,0 +1,261 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.httpclient; + +import org.apache.shenyu.common.constant.Constants; +import org.apache.shenyu.plugin.api.context.CanaryContext; +import org.apache.shenyu.plugin.base.circuitbreaker.UpstreamCircuitBreaker; +import org.apache.shenyu.loadbalancer.cache.UpstreamCacheManager; +import org.apache.shenyu.loadbalancer.entity.Upstream; +import org.apache.shenyu.plugin.httpclient.exception.ShenyuUpstreamStatusException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.MockedStatic; +import org.springframework.http.HttpStatus; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.web.server.ResponseStatusException; +import org.springframework.web.server.ServerWebExchange; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.net.ConnectException; +import java.net.InetSocketAddress; +import java.net.URI; +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.TimeoutException; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +/** + * Failover stays in the saved actual partition and uses current healthy nodes. + */ +class DefaultRetryStrategyCanaryTest { + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testFixedFailoverWaitsAndStaysInCanaryUntilExhausted(final boolean serverError) { + ServerWebExchange exchange = exchange("canary", "fixed-c1:8080"); + Throwable failure = serverError ? new ShenyuUpstreamStatusException(503) : new ConnectException("connection refused"); + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + when(client.doRequest(eq(exchange), anyString(), eq(URI.create("http://fixed-c2:8080/test")), any())) + .thenReturn(Mono.error(failure)); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findUpstreamListBySelectorId("selector")).thenReturn(List.of( + upstream("fixed-c1:8080", "canary"), upstream("fixed-c2:8080", "canary"), upstream("fixed-s1:8080", "stable"))); + try (MockedStatic cache = cache(manager)) { + StepVerifier.withVirtualTime(() -> new FixedRetryStrategy<>(client).execute(Mono.error(failure), exchange, Duration.ofSeconds(1), 4)) + .thenAwait(Duration.ofMillis(1999)) + .then(() -> verifyNoInteractions(client, manager)) + .thenAwait(Duration.ofMillis(1)) + .then(() -> verify(client).doRequest(eq(exchange), anyString(), eq(URI.create("http://fixed-c2:8080/test")), any())) + .thenAwait(Duration.ofSeconds(2)) + .expectErrorMatches(error -> error instanceof ResponseStatusException + && ((ResponseStatusException) error).getStatusCode() == HttpStatus.SERVICE_UNAVAILABLE) + .verify(); + } finally { + UpstreamCircuitBreaker.getInstance().reset(UpstreamCircuitBreaker.buildKey("selector", upstream("fixed-c2:8080", "canary"))); + } + verify(manager, times(2)).findUpstreamListBySelectorId("selector"); + verify(client, times(1)).doRequest(eq(exchange), anyString(), any(), any()); + verify(client, never()).doRequest(eq(exchange), anyString(), eq(URI.create("http://fixed-s1:8080/test")), any()); + assertEquals("canary", exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + } + + @Test + void testCanaryFailureRetriesOnlyCanaryAndStopsWhenExhausted() { + ServerWebExchange exchange = exchange("canary", "c1:8080"); + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + when(client.doRequest(eq(exchange), anyString(), eq(URI.create("http://c2:8080/test")), any())) + .thenReturn(Mono.error(new IllegalStateException("c2 failed"))); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findUpstreamListBySelectorId("selector")).thenReturn(List.of( + upstream("c1:8080", "canary"), upstream("c2:8080", "canary"), upstream("s1:8080", "stable"))); + try (MockedStatic cache = cache(manager)) { + verifyUnavailable(new DefaultRetryStrategy<>(client).execute(Mono.error(new IllegalStateException("c1 failed")), exchange, Duration.ofSeconds(1), 3)); + } + verify(manager, times(2)).findUpstreamListBySelectorId("selector"); + verify(client, times(1)).doRequest(eq(exchange), anyString(), any(), any()); + verify(client, never()).doRequest(eq(exchange), anyString(), eq(URI.create("http://s1:8080/test")), any()); + assertEquals("canary", exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + } + + @Test + void testInitialStableFallbackRemainsStable() { + ServerWebExchange exchange = exchange("stable", "s1:8080"); + CanaryContext observation = new CanaryContext("selector", "rule", "canary", "stable", CanaryContext.CANARY_POOL_EMPTY, null, 80_000); + exchange.getAttributes().put(Constants.SHENYU_CANARY_CONTEXT, observation); + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + when(client.doRequest(eq(exchange), anyString(), eq(URI.create("http://s2:8080/test")), any())).thenReturn(Mono.just("stable response")); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findUpstreamListBySelectorId("selector")).thenReturn(List.of( + upstream("s1:8080", "stable"), upstream("s2:8080", "stable"), upstream("c1:8080", "canary"))); + try (MockedStatic cache = cache(manager)) { + StepVerifier.create(new DefaultRetryStrategy<>(client).execute(Mono.error(new IllegalStateException("s1 failed")), exchange, Duration.ofSeconds(1), 2)) + .expectNext("stable response").verifyComplete(); + } + verify(client).doRequest(eq(exchange), anyString(), eq(URI.create("http://s2:8080/test")), any()); + assertEquals("stable", exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + assertSame(observation, exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT)); + } + + @Test + void testRetryReadsLatestLabelsAndHealthyNodes() { + ServerWebExchange exchange = exchange("canary", "c1:8080"); + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + when(client.doRequest(eq(exchange), anyString(), eq(URI.create("http://c2:8080/test")), any())) + .thenReturn(Mono.error(new IllegalStateException("c2 failed"))); + when(client.doRequest(eq(exchange), anyString(), eq(URI.create("http://c3:8080/test")), any())).thenReturn(Mono.just("new canary")); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findUpstreamListBySelectorId("selector")) + .thenReturn(List.of(upstream("c1:8080", "canary"), upstream("c2:8080", "canary"))) + .thenReturn(List.of(upstream("c2:8080", "stable"), upstream("c3:8080", "canary"), upstream("s1:8080", "stable"))); + try (MockedStatic cache = cache(manager)) { + StepVerifier.create(new DefaultRetryStrategy<>(client).execute(Mono.error(new IllegalStateException("c1 failed")), exchange, Duration.ofSeconds(1), 3)) + .expectNext("new canary").verifyComplete(); + } + verify(manager, times(2)).findUpstreamListBySelectorId("selector"); + verify(client, times(2)).doRequest(eq(exchange), anyString(), any(), any()); + } + + @Test + void testRemovedSelectorAndMissingLabelSnapshotFailClosed() { + ServerWebExchange exchange = exchange("canary", "c1:8080"); + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + try (MockedStatic cache = cache(manager)) { + verifyUnavailable(new DefaultRetryStrategy<>(client).execute(Mono.error(new IllegalStateException("failed")), exchange, Duration.ofSeconds(1), 1)); + when(manager.findUpstreamListBySelectorId("selector")).thenReturn(List.of(upstream("c2:8080", "canary"))); + exchange.getAttributes().remove(Constants.SHENYU_CANARY_LABELS); + verifyUnavailable(new DefaultRetryStrategy<>(client).execute(Mono.error(new IllegalStateException("failed")), exchange, Duration.ofSeconds(1), 1)); + } + verifyNoInteractions(client); + } + + @Test + void testSpecifyDomainKeepsLegacyFailoverWithoutPartition() { + ServerWebExchange exchange = exchange(null, "override:9090"); + exchange = exchange.mutate().request(request -> request.header(Constants.SPECIFY_DOMAIN, "override:9090")).build(); + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + when(client.doRequest(eq(exchange), anyString(), eq(URI.create("http://legacy:8080/test")), any())).thenReturn(Mono.just("legacy response")); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findLegacyUpstreamListBySelectorId("selector")).thenReturn(List.of(upstream("legacy:8080", "stable"))); + try (MockedStatic cache = cache(manager)) { + StepVerifier.create(new DefaultRetryStrategy<>(client).execute(Mono.error(new IllegalStateException("override failed")), exchange, Duration.ofSeconds(1), 1)) + .expectNext("legacy response").verifyComplete(); + } + } + + @Test + void testLegacyFailoverDoesNotUseFullCanaryView() { + ServerWebExchange exchange = exchange(null, "gray:8080"); + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findUpstreamListBySelectorId("selector")).thenReturn(List.of(upstream("stable:8080", "stable"))); + when(manager.findLegacyUpstreamListBySelectorId("selector")).thenReturn(List.of()); + try (MockedStatic cache = cache(manager)) { + verifyUnavailable(new DefaultRetryStrategy<>(client).execute(Mono.error(new IllegalStateException("gray failed")), exchange, Duration.ofSeconds(1), 3)); + } + verifyNoInteractions(client); + verify(manager, times(1)).findLegacyUpstreamListBySelectorId("selector"); + verify(manager, never()).findUpstreamListBySelectorId("selector"); + } + + @Test + void testCurrentRetryPreservesPartitionWithoutReselecting() { + ServerWebExchange exchange = exchange("canary", "c1:8080"); + exchange.getAttributes().put(Constants.RETRY_STRATEGY, "current"); + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + try (MockedStatic cache = cache(manager)) { + StepVerifier.create(new DefaultRetryStrategy<>(client).execute(Mono.error(new TimeoutException("timeout")), exchange, Duration.ofSeconds(1), 1)) + .expectErrorMatches(error -> error instanceof ResponseStatusException + && ((ResponseStatusException) error).getStatusCode() == HttpStatus.REQUEST_TIMEOUT).verify(); + } + verifyNoInteractions(client, manager); + assertEquals("canary", exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + } + + @Test + void testRetryPreservesDefaultPortExclusionAndCircuitBreakerWithinPartition() { + ServerWebExchange exchange = exchange("canary", "C1"); + Upstream blocked = upstream("c2:8080", "canary"); + UpstreamCircuitBreaker breaker = UpstreamCircuitBreaker.getInstance(); + String key = UpstreamCircuitBreaker.buildKey("selector", blocked); + for (int i = 0; i < UpstreamCircuitBreaker.DEFAULT_FAILURE_THRESHOLD; i++) { + breaker.recordFailure(key); + } + AbstractHttpClientPlugin client = mock(AbstractHttpClientPlugin.class); + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findUpstreamListBySelectorId("selector")).thenReturn(List.of( + upstream("c1:80", "canary"), blocked, upstream("s1:8080", "stable"))); + try (MockedStatic cache = cache(manager)) { + verifyUnavailable(new DefaultRetryStrategy<>(client).execute(Mono.error(new IllegalStateException("failed")), exchange, Duration.ofSeconds(1), 2)); + verifyNoInteractions(client); + } finally { + breaker.reset(key); + } + } + + private void verifyUnavailable(final Mono result) { + StepVerifier.create(result).expectErrorMatches(error -> error instanceof ResponseStatusException + && ((ResponseStatusException) error).getStatusCode() == HttpStatus.SERVICE_UNAVAILABLE).verify(); + } + + private ServerWebExchange exchange(final String partition, final String initialHost) { + ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/test") + .remoteAddress(new InetSocketAddress("127.0.0.1", 12345))); + exchange.getAttributes().put(Constants.DIVIDE_SELECTOR_ID, "selector"); + exchange.getAttributes().put(Constants.LOAD_BALANCE, "random"); + exchange.getAttributes().put(Constants.RETRY_STRATEGY, "failover"); + exchange.getAttributes().put(Constants.REWRITE_URI, "/test"); + exchange.getAttributes().put(Constants.HTTP_URI, URI.create("http://" + initialHost + "/test")); + if (Objects.nonNull(partition)) { + exchange.getAttributes().put(Constants.SHENYU_CANARY_PARTITION, partition); + exchange.getAttributes().put(Constants.SHENYU_CANARY_LABELS, Map.of("release", partition)); + } + return exchange; + } + + private MockedStatic cache(final UpstreamCacheManager manager) { + MockedStatic cache = mockStatic(UpstreamCacheManager.class); + cache.when(UpstreamCacheManager::getInstance).thenReturn(manager); + return cache; + } + + private Upstream upstream(final String url, final String partition) { + Upstream upstream = Upstream.builder().url(url).protocol("http://").weight(100).build(); + upstream.setLabels(Map.of("release", partition)); + return upstream; + } +} diff --git a/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/RequestBodyReplayRetryTest.java b/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/RequestBodyReplayRetryTest.java index ebf27e61dcc9..becee66b84bc 100644 --- a/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/RequestBodyReplayRetryTest.java +++ b/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/RequestBodyReplayRetryTest.java @@ -116,7 +116,7 @@ void testBodyReplayedOnFailoverRetry() { Upstream standby = Upstream.builder().url("localhost:8081").build(); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); - when(cacheManager.findUpstreamListBySelectorId(anyString())).thenReturn(Collections.singletonList(standby)); + when(cacheManager.findLegacyUpstreamListBySelectorId(anyString())).thenReturn(Collections.singletonList(standby)); try (MockedStatic cacheMock = org.mockito.Mockito.mockStatic(UpstreamCacheManager.class); MockedStatic lbMock = org.mockito.Mockito.mockStatic(LoadbalancerUtils.class); diff --git a/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/RetryStrategyTest.java b/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/RetryStrategyTest.java index 6649a340f7a9..799bd000577a 100644 --- a/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/RetryStrategyTest.java +++ b/shenyu-plugin/shenyu-plugin-httpclient/src/test/java/org/apache/shenyu/plugin/httpclient/RetryStrategyTest.java @@ -154,7 +154,7 @@ void testFailoverExcludesSameUpstream(final String currentUri, final String prot ServerWebExchange exchange = createFailoverExchange(currentUri); Upstream upstream = Upstream.builder().protocol(protocol).url(url).build(); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); - when(cacheManager.findUpstreamListBySelectorId("selector-7475")).thenReturn(Collections.singletonList(upstream)); + when(cacheManager.findLegacyUpstreamListBySelectorId("selector-7475")).thenReturn(Collections.singletonList(upstream)); try (MockedStatic cacheMock = mockStatic(UpstreamCacheManager.class)) { cacheMock.when(UpstreamCacheManager::getInstance).thenReturn(cacheManager); @@ -178,7 +178,7 @@ void testFailoverSelectsDifferentUpstream(final String protocol, final String ur Upstream failed = Upstream.builder().url("localhost:8080").build(); Upstream standby = Upstream.builder().protocol(protocol).url(url).build(); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); - when(cacheManager.findUpstreamListBySelectorId("selector-7475")).thenReturn(Arrays.asList(failed, standby)); + when(cacheManager.findLegacyUpstreamListBySelectorId("selector-7475")).thenReturn(Arrays.asList(failed, standby)); when(plugin.getCachedRequestBody(exchange)).thenReturn(Flux.empty()); when(plugin.doRequest(eq(exchange), eq("GET"), any(URI.class), any())).thenReturn(Mono.just("success")); @@ -222,7 +222,7 @@ void testFailoverSkipsBlockedUpstream() { Upstream failed = Upstream.builder().url("localhost:8080").build(); Upstream standby = Upstream.builder().protocol("http://").url("localhost:8081").build(); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); - when(cacheManager.findUpstreamListBySelectorId("selector-7475")).thenReturn(Arrays.asList(failed, standby)); + when(cacheManager.findLegacyUpstreamListBySelectorId("selector-7475")).thenReturn(Arrays.asList(failed, standby)); final String standbyKey = UpstreamCircuitBreaker.buildKey("selector-7475", standby); UpstreamCircuitBreaker breaker = UpstreamCircuitBreaker.getInstance(); for (int i = 0; i < 3; i++) { @@ -247,7 +247,7 @@ void testFailoverResendRecordsFailureOutcome() { Upstream failed = Upstream.builder().url("localhost:8080").build(); Upstream standby = Upstream.builder().protocol("http://").url("localhost:8081").build(); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); - when(cacheManager.findUpstreamListBySelectorId("selector-7475")).thenReturn(Arrays.asList(failed, standby)); + when(cacheManager.findLegacyUpstreamListBySelectorId("selector-7475")).thenReturn(Arrays.asList(failed, standby)); when(plugin.getCachedRequestBody(exchange)).thenReturn(Flux.empty()); when(plugin.doRequest(eq(exchange), eq("GET"), any(URI.class), any())).thenReturn(Mono.error(new TimeoutException("resend failed"))); final String standbyKey = UpstreamCircuitBreaker.buildKey("selector-7475", standby); @@ -274,7 +274,7 @@ void testFailoverResendRecordsSuccessOutcome() { Upstream failed = Upstream.builder().url("localhost:8080").build(); Upstream standby = Upstream.builder().protocol("http://").url("localhost:8081").build(); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); - when(cacheManager.findUpstreamListBySelectorId("selector-7475")).thenReturn(Arrays.asList(failed, standby)); + when(cacheManager.findLegacyUpstreamListBySelectorId("selector-7475")).thenReturn(Arrays.asList(failed, standby)); when(plugin.getCachedRequestBody(exchange)).thenReturn(Flux.empty()); when(plugin.doRequest(eq(exchange), eq("GET"), any(URI.class), any())).thenReturn(Mono.just("success")); final String standbyKey = UpstreamCircuitBreaker.buildKey("selector-7475", standby); @@ -362,7 +362,7 @@ void testFixedFailoverWaitsBeforeSendingToStandby(final boolean serverError) { uri -> primaryUri.equals(uri) ? Mono.error(failure) : Mono.just("success")); Upstream standby = Upstream.builder().url("localhost:8081").build(); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); - when(cacheManager.findUpstreamListBySelectorId("fixed-failover")).thenReturn(Collections.singletonList(standby)); + when(cacheManager.findLegacyUpstreamListBySelectorId("fixed-failover")).thenReturn(Collections.singletonList(standby)); try (MockedStatic cacheMock = mockStatic(UpstreamCacheManager.class)) { cacheMock.when(UpstreamCacheManager::getInstance).thenReturn(cacheManager); @@ -406,7 +406,7 @@ void testFixedFailoverRetriesAcrossDistinctUpstreams() { Upstream standby = Upstream.builder().url("localhost:8081").build(); Upstream healthy = Upstream.builder().url("localhost:8082").build(); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); - when(cacheManager.findUpstreamListBySelectorId("fixed-failover")) + when(cacheManager.findLegacyUpstreamListBySelectorId("fixed-failover")) .thenReturn(Arrays.asList(primary, standby), Arrays.asList(primary, standby, healthy)); try (MockedStatic cacheMock = mockStatic(UpstreamCacheManager.class)) { diff --git a/shenyu-plugin/shenyu-plugin-metrics/CANARY_OBSERVABILITY.md b/shenyu-plugin/shenyu-plugin-metrics/CANARY_OBSERVABILITY.md new file mode 100644 index 000000000000..ace2b53eb694 --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-metrics/CANARY_OBSERVABILITY.md @@ -0,0 +1,188 @@ + + +# Divide 灰度分流观测 + +本实现复用现有 Metrics 插件及 Prometheus 输出端点。启用 Metrics 插件并配置现有 Prometheus reporter 后，新指标自动注册；无须配置额外服务。 +Divide 不依赖 Metrics 模块。即使未启用 Metrics，首次选路仍会保存观测记录，原有路由、重试和错误响应行为保持不变。 + +## 请求内的数据流 + +1. 沿用 GlobalPlugin 调用的 DefaultShenyuContextBuilder 已有逻辑，在 ShenyuContext 的 `startDateTime` 中保存请求上下文创建时间；不新增全局计时属性。 +2. MetricsPlugin 安装可选的选路回调，沿用 `chain.execute(...).doOnSuccess(...).doOnError(...)` 执行链，并通过 `doFinally` 结算灰度请求结果。 +3. Divide 调用一次 `decide()` 后，将 `CanaryContext` 放在 `SHENYU_CANARY_CONTEXT` 中；执行候选池过滤、初始回退和负载均衡后，补充实际分区和拒绝/回退原因。 +4. Divide 通知可选回调，立即记录决策耗时和成功回退。长连接不必等到结束才能看到这些指标。 +5. 下游链正常完成或报错后，沿用原来的 responseCommitted：响应已提交则立即记录延迟，否则注册 `beforeCommit` 回调等待提交。 +6. MetricsPlugin 包围的下游链完成、出错或取消时，记录一次外部请求结果。 + +`CanaryContext` 位于 plugin-api，包含 selector/rule ID、期望分区、实际分区、成功回退原因、拒绝原因和决策纳秒耗时。 +埋点沿用 Sentinel、RateLimiter、Resilience4j 的 exchange Consumer 回调方式：MetricsPlugin 注册 `METRICS_CANARY`，Divide 在选路结束或拒绝时调用。没有独立的 Recorder 或 Observer；指标上报、请求计时和去重统一在 MetricsPlugin 内完成。 +CanaryContext 只由 Divide 在首次选路期间填充，MetricsPlugin 读取，重试不修改它。分流决策计时紧邻 Divide 的 decide 调用。 +它不保存 Header/Cookie、sticky key、用户 ID、请求 ID、路径或上游地址。 +原有 `SHENYU_CANARY_PARTITION` 和 `SHENYU_CANARY_LABELS` 继续供 HTTP 重试使用；观测上下文不会改变其含义。 + +## 指标契约 + +| 名称 | 类型 | 标签 | +| --- | --- | --- | +| `shenyu_canary_requests_total` | Counter | `selector, rule, partition, outcome` | +| `shenyu_canary_fallback_total` | Counter | `selector, rule, reason` | +| `shenyu_canary_decision_duration_seconds` | Histogram | `selector, rule` | +| `shenyu_execute_latency_millis`（复用现有指标） | Histogram | `partition` | + +三个新增指标的名称、标签及顺序统一定义在 `CanaryMetric`，注册和上报使用同一份定义。 +`CanaryMetric` 只提供指标定义及按定义排列的标签值，不执行注册或上报。`MetricsReporter.register` 统一组织注册，具体创建指标由 `MetricsRegister` 实现；MetricsPlugin 直接通过 MetricsReporter 记录计数和耗时。 +请求延迟复用 `MetricsPlugin.responseCommitted/recordTime → MetricsReporter.recordTime`，保留 `shenyu_execute_latency_millis` 的名称和毫秒单位，增加 `partition=stable/canary/none` 标签。 +该延迟指标仅区分分区，不含 selector/rule，因此只能比较跨规则聚合的分区延迟。请求量和错误率仍可按规则比较。 + +### 分区与结果 + +- 已选出节点：`partition` 使用实际分区，Stable 回退后的请求归 Stable。 +- 未选出节点：`partition` 使用最初期望分区。例如 Canary 和 Stable 都为空时，拒绝仍归最初期望的 Canary。 +- 没有灰度观测上下文：现有请求延迟使用 `partition=none`，不产生三个新增灰度指标。 +- `success`：下游链正常完成，最终 HTTP 状态为 2xx/3xx；没有显式状态时按默认成功响应处理。 +- `error`：转发链异常，或正常完成但 HTTP 状态不属于 2xx/3xx。4xx 纳入错误，因此该指标不是纯服务端故障率。 +- `reject`：Divide 显式标记的选路拒绝。拒绝原因优先于普通状态/错误分类，不能仅凭 HTTP 503 推断。 +- `cancelled`：下游链取消，优先于其他分类；响应头已经提交也仍可取消。 + +每次外部请求最多记录一个 outcome。重试后的成功只计一个 success，重试耗尽只计一个 error。 +这里统计的是外部请求归属，不是后端收到的实际网络请求次数；未选出节点的异常请求也可能按期望分区归属。 +长连接的 requests 计数要等到终止时才出现；进程退出或崩溃时尚未结束的请求不会结算。 + +### 回退与拒绝原因 + +只有首次选路从 Canary 成功选中 Stable 节点，才增加 fallback，reason 固定为 `canary_pool_empty`。 +Canary 和 Stable 都空、Stable 负载均衡未选中节点，以及同分区重试，都不会增加 fallback。 +成功回退后再发生转发失败或取消，不撤销已发生的回退事件。 + +观测上下文中的拒绝原因包括 `canary_pool_empty`、`stable_pool_empty`、`no_upstream_selected`。 +它们用于受 DEBUG 级别控制的选路日志；第一版请求计数不增加 reason 标签。 + +### 耗时 + +- 决策耗时以 `System.nanoTime()` 的差值计量，仅包围首次 `decide()`，不含候选池过滤、负载均衡或网络转发。 +- 请求延迟沿用 `DateUtils.acquireMillisBetween(startDateTime, LocalDateTime.now())`，起点为 ShenyuContext 创建时间。这里是每个请求的上下文初始化，不是网关进程启动时间。 +- 结束点沿用既有逻辑：下游链正常完成或报错时，响应已经提交则在此刻记录；尚未提交则在随后执行的 `beforeCommit` 回调中记录。因此不能统一称为“响应头提交延迟”，它可能包含响应体处理时间，也不保证字节已发送到客户端。 +- 请求延迟包含此前的重试等待。沿用的墙上时钟计算可能受系统时钟调整影响。 +- 请求上下文未设置 startDateTime 时，沿用 MetricsPlugin 进入时刻作为起点。 +- 取消不触发 doOnSuccess/doOnError：无论响应头是否已提交，此路径只结算 cancelled 请求计数，不新增延迟样本。 +- 响应已提交但下游链尚未结束时，不提前记录延迟或 success；之后完成或报错时再记录延迟和最终结果。 +- 外层错误处理器在转发异常后提交错误响应，可触发 doOnError 安装的延迟回调。异常后始终没有响应提交，则没有延迟样本。请求结果描述 MetricsPlugin 下游链，不包含外层错误处理器写错误响应时的二次失败。 +- 延迟直方图包含已提交的拒绝及错误响应，不能按最终 outcome 过滤，不是成功请求专用延迟。 + +本实现保留原 execute 的写法和延迟记录时点，只在执行链末尾增加 doFinally 结算灰度请求结果。默认 DefaultShenyuPluginChain 本身已通过 Mono.defer 延迟执行插件；MetricsPlugin 不再额外包一层。自定义链若在返回 Mono 前直接抛异常，仍沿用原有的直接抛出行为，不经过这里的响应式统计回调。请求延迟保留一次性记录标记，防止同一 exchange 重入后重复上报。 + +仅为决策耗时新增 `MetricsReporter.observe(..., double)` 和相应 SPI，纳秒除以 `1e9` 后以小数秒上报。 +请求延迟继续使用旧 `recordTime(..., long)`。自定义 MetricsRegister 可以继续编译和运行，但须覆写 `observe` 才能输出决策直方图；默认跳过，避免截断为零。 +支持显式桶边界，Prometheus 使用以下初始设置： + +- 决策秒：`0.000001, 0.000005, 0.00001, 0.00005, 0.0001, 0.0005, 0.001, 0.005, 0.01, 0.05, 0.1`。 +- 请求毫秒：`1, 5, 10, 25, 50, 100, 250, 500, 1000, 2500, 5000, 10000, 30000`。 + +决策桶定义在 `CanaryMetric`；现有请求延迟桶在 `MetricsReporter.register` 中按毫秒设置。可按实际分布调整；部署时同名直方图须保持桶配置一致。 + +## 统计范围与兼容性 + +- 灰度配置存在但 enabled=false、percentage=0 或条件未命中：仍执行分区决策，归 Stable。 +- 无灰度配置、指定域名绕过灰度：不产生灰度指标。 +- 请求大小校验失败、整个 selector 的可用上游列表为空：仍沿用决策之前的返回路径，不为观测额外调用 decide，因此不产生灰度指标。 +- decide 自身抛出异常时尚无分区结果，不产生灰度指标；原有全局异常计数继续适用。 +- 普通请求计数沿用原有口径，本改动没有给此前以成功 HTTP 状态返回的业务拒绝重新分类。现有请求延迟增加 partition 标签，保留原来的记录时点。 +- 现有延迟指标的标签和桶边界发生变化，需要同步调整看板聚合，不能宣称所有旧查询结果不变。完成进程升级后再比较新口径；滚动升级期间不要将新旧桶配置混合计算分位数。 +- MetricsReporter 沿用既有异常传播行为，不统一捕获上报异常；未配置 reporter 时仍跳过上报。Divide 的首次选路通知保留本地回调异常处理；注册阶段仍暴露配置错误。 +- 每组配置会产生多个标签组合和直方图时序。标签只使用配置 ID 和固定枚举，仍需关注大量规则及规则 ID 频繁更替造成的时序增长。 + +## 查询与看板示例 + +以下请求量、错误率和回退查询跨实例聚合并保留规则维度，延迟查询仅保留分区。实际部署时添加 job/环境过滤，避免不同网关配置的 ID 混在一起。 +请求计数按终止时间记录，回退和决策按首次选路时间记录，不保证短窗口内逐项相等。 + +各分区、结果的请求速率： + +```promql +sum by (selector, rule, partition, outcome) ( + rate(shenyu_canary_requests_total[5m]) +) +``` + +已完成且非拒绝、非取消的请求中 Canary 的归属占比： + +```promql +( + sum by (selector, rule) ( + rate(shenyu_canary_requests_total{partition="canary", outcome=~"success|error"}[5m]) + ) + or on (selector, rule) + 0 * sum by (selector, rule) ( + rate(shenyu_canary_requests_total{outcome=~"success|error"}[5m]) + ) +) +/ +sum by (selector, rule) ( + rate(shenyu_canary_requests_total{outcome=~"success|error"}[5m]) +) +``` + +该比例描述请求归属，不等于底层发送次数，也不能直接校验配置百分比；条件匹配、sticky key 缺失和空池回退都会影响它。 +未创建过的 Canary 时序通过已有请求时序补零；总流量为零时显示无数据，不应当作 0% 的健康结论。 + +各分区的请求错误率（不含拒绝和取消）： + +```promql +( + sum by (selector, rule, partition) ( + rate(shenyu_canary_requests_total{outcome="error"}[5m]) + ) + or on (selector, rule, partition) + 0 * sum by (selector, rule, partition) ( + rate(shenyu_canary_requests_total{outcome=~"success|error"}[5m]) + ) +) +/ +sum by (selector, rule, partition) ( + rate(shenyu_canary_requests_total{outcome=~"success|error"}[5m]) +) +``` + +Stable/Canary 分区请求延迟 P95（毫秒，跨规则聚合）： + +```promql +histogram_quantile(0.95, + sum by (partition, le) ( + rate(shenyu_execute_latency_millis_bucket{partition=~"stable|canary"}[5m]) + ) +) +``` + +现有看板如需继续查看所有请求的平均延迟，应聚合新增的分区序列（毫秒，包含 none）： + +```promql +sum(rate(shenyu_execute_latency_millis_sum[5m])) +/ +sum(rate(shenyu_execute_latency_millis_count[5m])) +``` + +十分钟内的成功回退次数： + +```promql +sum by (selector, rule, reason) ( + increase(shenyu_canary_fallback_total[10m]) +) +``` + +建议看板并列展示请求速率及样本量、错误率、拒绝率、取消率、请求延迟和回退次数。 +缺失时序与真实零值应区分；低样本窗口不据此判断 Canary 优劣。第一版不设置未经流量基线校准的自动回滚阈值。 diff --git a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/MetricsPlugin.java b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/MetricsPlugin.java index e20f28c9517e..c22a2eccb2c3 100644 --- a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/MetricsPlugin.java +++ b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/MetricsPlugin.java @@ -23,13 +23,17 @@ import org.apache.shenyu.common.utils.DateUtils; import org.apache.shenyu.plugin.api.ShenyuPlugin; import org.apache.shenyu.plugin.api.ShenyuPluginChain; +import org.apache.shenyu.plugin.api.context.CanaryContext; import org.apache.shenyu.plugin.api.context.ShenyuContext; +import org.apache.shenyu.plugin.metrics.constant.CanaryMetric; import org.apache.shenyu.plugin.metrics.constant.LabelNames; import org.apache.shenyu.plugin.metrics.reporter.MetricsReporter; import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Mono; +import reactor.core.publisher.SignalType; import java.time.LocalDateTime; import java.util.Objects; @@ -42,6 +46,12 @@ */ public class MetricsPlugin implements ShenyuPlugin { + private static final String CANARY_ROUTING_RECORDED = "canaryRoutingRecorded"; + + private static final String CANARY_REQUEST_RECORDED = "canaryRequestRecorded"; + + private static final String EXECUTE_LATENCY_RECORDED = "executeLatencyRecorded"; + @Override public Mono execute(final ServerWebExchange exchange, final ShenyuPluginChain chain) { MetricsReporter.counterIncrement(LabelNames.REQUEST_TOTAL); @@ -54,7 +64,8 @@ public Mono execute(final ServerWebExchange exchange, final ShenyuPluginCh .doOnError(throwable -> { MetricsReporter.counterIncrement(LabelNames.REQUEST_THROW_TOTAL); responseCommitted(exchange, startDateTime); - }); + }) + .doFinally(signal -> recordCanaryResult(exchange, signal)); } private void setMetricsCallbacks(final ServerWebExchange exchange) { @@ -88,6 +99,39 @@ private void setMetricsCallbacks(final ServerWebExchange exchange) { MetricsReporter.counterIncrement(LabelNames.RATELIMITER_REQUEST_RESTRICT_TOTAL); } }); + exchange.getAttributes().put(Constants.METRICS_CANARY, (Consumer) context -> recordCanaryRouting(exchange, context)); + } + + private void recordCanaryRouting(final ServerWebExchange exchange, final CanaryContext context) { + if (Objects.isNull(exchange.getAttributes().putIfAbsent(CANARY_ROUTING_RECORDED, true))) { + MetricsReporter.observe(CanaryMetric.DECISION_DURATION.getName(), CanaryMetric.DECISION_DURATION.getLabelValues(context, null), + context.getDecisionDurationNanos() / 1_000_000_000.0); + if (Objects.nonNull(context.getFallbackReason())) { + MetricsReporter.counterIncrement(CanaryMetric.FALLBACK.getName(), CanaryMetric.FALLBACK.getLabelValues(context, null)); + } + } + } + + private void recordCanaryResult(final ServerWebExchange exchange, final SignalType signal) { + CanaryContext context = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + if (Objects.nonNull(context) && Objects.isNull(exchange.getAttributes().putIfAbsent(CANARY_REQUEST_RECORDED, true))) { + recordCanaryRouting(exchange, context); + MetricsReporter.counterIncrement(CanaryMetric.REQUESTS.getName(), CanaryMetric.REQUESTS.getLabelValues(context, canaryOutcome(exchange, context, signal))); + } + } + + private String canaryOutcome(final ServerWebExchange exchange, final CanaryContext context, final SignalType signal) { + if (signal == SignalType.CANCEL) { + return "cancelled"; + } + if (Objects.nonNull(context.getRejectReason())) { + return "reject"; + } + if (signal == SignalType.ON_ERROR) { + return "error"; + } + HttpStatusCode status = exchange.getResponse().getStatusCode(); + return Objects.isNull(status) || status.is2xxSuccessful() || status.is3xxRedirection() ? "success" : "error"; } @Override @@ -103,17 +147,19 @@ public String named() { private void responseCommitted(final ServerWebExchange exchange, final LocalDateTime startDateTime) { ServerHttpResponse response = exchange.getResponse(); if (response.isCommitted()) { - recordTime(startDateTime); + recordTime(exchange, startDateTime); } else { - response.beforeCommit(() -> { - recordTime(startDateTime); - return Mono.empty(); - }); + response.beforeCommit(() -> Mono.fromRunnable(() -> recordTime(exchange, startDateTime))); } } - private void recordTime(final LocalDateTime startDateTime) { + private void recordTime(final ServerWebExchange exchange, final LocalDateTime startDateTime) { + if (Objects.nonNull(exchange.getAttributes().putIfAbsent(EXECUTE_LATENCY_RECORDED, true))) { + return; + } long millisBetween = DateUtils.acquireMillisBetween(startDateTime, LocalDateTime.now()); - MetricsReporter.recordTime(LabelNames.EXECUTE_LATENCY_NAME, millisBetween); + CanaryContext context = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + String partition = Objects.isNull(context) ? "none" : context.getMetricPartition(); + MetricsReporter.recordTime(LabelNames.EXECUTE_LATENCY_NAME, new String[]{partition}, millisBetween); } } diff --git a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/constant/CanaryMetric.java b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/constant/CanaryMetric.java new file mode 100644 index 000000000000..8926e2eff03d --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/constant/CanaryMetric.java @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.metrics.constant; + +import org.apache.shenyu.plugin.api.context.CanaryContext; + +import java.util.Arrays; +import java.util.Objects; + +/** + * Shared canary metric definitions for registration and label value ordering. + */ +public enum CanaryMetric { + + REQUESTS("shenyu_canary_requests_total", "External requests attributed to canary routing", null, + "selector", "rule", "partition", "outcome"), + + FALLBACK("shenyu_canary_fallback_total", "Successful initial fallback from canary to stable", null, + "selector", "rule", "reason"), + + DECISION_DURATION("shenyu_canary_decision_duration_seconds", "Initial canary decide duration in seconds", + new double[]{0.000001, 0.000005, 0.00001, 0.00005, 0.0001, 0.0005, 0.001, 0.005, 0.01, 0.05, 0.1}, + "selector", "rule"); + + private final String name; + + private final String description; + + private final double[] buckets; + + private final String[] labelNames; + + CanaryMetric(final String name, final String description, final double[] buckets, final String... labelNames) { + this.name = name; + this.description = description; + this.buckets = buckets; + this.labelNames = labelNames; + } + + /** + * Gets the exported metric name. + * + * @return metric name + */ + public String getName() { + return name; + } + + /** + * Gets the metric description. + * + * @return metric description + */ + public String getDescription() { + return description; + } + + /** + * Gets the label names in registration order. + * + * @return a copy of the label names + */ + public String[] getLabelNames() { + return labelNames.clone(); + } + + /** + * Gets the histogram bucket boundaries, or null for a counter. + * + * @return a copy of the bucket boundaries, or null + */ + public double[] getBuckets() { + return Objects.isNull(buckets) ? null : buckets.clone(); + } + + /** + * Gets label values in registration order. + * + * @param context routing observation + * @param outcome request outcome, unused by other metrics + * @return label values + */ + public String[] getLabelValues(final CanaryContext context, final String outcome) { + return Arrays.stream(labelNames).map(label -> labelValue(label, context, outcome)).toArray(String[]::new); + } + + private String labelValue(final String label, final CanaryContext context, final String outcome) { + switch (label) { + case "selector": + return context.getSelectorId(); + case "rule": + return context.getRuleId(); + case "partition": + return context.getMetricPartition(); + case "outcome": + return outcome; + case "reason": + return context.getFallbackReason(); + default: + throw new IllegalArgumentException("Unknown canary metric label: " + label); + } + } +} diff --git a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/prometheus/PrometheusMetricsRegister.java b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/prometheus/PrometheusMetricsRegister.java index 1f0d7d8f8086..0bdca8defb6a 100644 --- a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/prometheus/PrometheusMetricsRegister.java +++ b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/prometheus/PrometheusMetricsRegister.java @@ -63,11 +63,19 @@ public void registerGauge(final String name, final String[] labelNames, final St @Override public void registerHistogram(final String name, final String[] labelNames, final String document) { + registerHistogram(name, labelNames, document, null); + } + + @Override + public void registerHistogram(final String name, final String[] labelNames, final String document, final double[] buckets) { if (!HISTOGRAM_MAP.containsKey(name)) { Histogram.Builder builder = Histogram.build().name(name).help(document); if (Objects.nonNull(labelNames)) { builder.labelNames(labelNames); } + if (Objects.nonNull(buckets)) { + builder.buckets(buckets); + } HISTOGRAM_MAP.putIfAbsent(name, builder.register()); } } @@ -113,14 +121,19 @@ public void gaugeDecrement(final String name, final String[] labelValues) { @Override public void recordTime(final String name, final String[] labelValues, final long duration) { + observe(name, labelValues, duration); + } + + @Override + public void observe(final String name, final String[] labelValues, final double value) { Histogram histogram = HISTOGRAM_MAP.get(name); if (Objects.isNull(histogram)) { return; } if (Objects.nonNull(labelValues)) { - histogram.labels(labelValues).observe(duration); + histogram.labels(labelValues).observe(value); } else { - histogram.observe(duration); + histogram.observe(value); } } diff --git a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporter.java b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporter.java index 5e4b035340fa..265e1918d5b5 100644 --- a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporter.java +++ b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporter.java @@ -18,11 +18,13 @@ package org.apache.shenyu.plugin.metrics.reporter; import org.apache.shenyu.plugin.metrics.config.Metric; +import org.apache.shenyu.plugin.metrics.constant.CanaryMetric; import org.apache.shenyu.plugin.metrics.constant.LabelNames; import org.apache.shenyu.plugin.metrics.spi.MetricsRegister; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.Optional; /** @@ -42,7 +44,8 @@ public static void register(final MetricsRegister metricsRegister) { MetricsReporter.registerCounter(LabelNames.REQUEST_TOTAL, "shenyu request total count"); MetricsReporter.registerCounter(LabelNames.REQUEST_TYPE_TOTAL, new String[]{"type"}, "shenyu http request type total count"); MetricsReporter.registerCounter(LabelNames.REQUEST_THROW_TOTAL, "shenyu request error total count"); - MetricsReporter.registerHistogram(LabelNames.EXECUTE_LATENCY_NAME, "the shenyu executor latency millis"); + MetricsReporter.registerHistogram(LabelNames.EXECUTE_LATENCY_NAME, new String[]{"partition"}, "the shenyu executor latency millis", + new double[]{1, 5, 10, 25, 50, 100, 250, 500, 1000, 2500, 5000, 10000, 30000}); MetricsReporter.registerCounter(LabelNames.AGENT_MCP_CALLS_TOTAL, new String[]{"outcome"}, "Terminal authenticated parsed MCP logical calls"); MetricsReporter.registerHistogram(LabelNames.AGENT_MCP_CALL_LATENCY, new String[]{"outcome"}, "MCP logical call terminal latency millis"); MetricsReporter.registerCounter(LabelNames.SENTINEL_REQUEST_RESTRICT_TOTAL, "shenyu sentinel request restrict total count"); @@ -51,6 +54,14 @@ public static void register(final MetricsRegister metricsRegister) { MetricsReporter.registerCounter(LabelNames.RESILIENCE4J_REQUEST_CIRCUITBREAKER_TOTAL, "shenyu resilience4j circuitbreaker request total count"); MetricsReporter.registerCounter(LabelNames.HYSTRIX_REQUEST_CIRCUITBREAKER_TOTAL, "shenyu hystrix circuitbreaker request total count"); MetricsReporter.registerCounter(LabelNames.RATELIMITER_REQUEST_RESTRICT_TOTAL, "shenyu ratelimiter request restrict total count"); + for (CanaryMetric metric : CanaryMetric.values()) { + double[] buckets = metric.getBuckets(); + if (Objects.isNull(buckets)) { + MetricsReporter.registerCounter(metric.getName(), metric.getLabelNames(), metric.getDescription()); + } else { + MetricsReporter.registerHistogram(metric.getName(), metric.getLabelNames(), metric.getDescription(), buckets); + } + } } /** @@ -129,6 +140,18 @@ public static void registerHistogram(final String name, final String[] labelName Optional.ofNullable(metricsRegister).ifPresent(register -> register.registerHistogram(name, labelNames, document)); } + /** + * Registers a histogram with explicit bucket boundaries. + * + * @param name metric name + * @param labelNames label names + * @param document metric description + * @param buckets bucket boundaries in the metric's unit + */ + public static void registerHistogram(final String name, final String[] labelNames, final String document, final double[] buckets) { + Optional.ofNullable(metricsRegister).ifPresent(register -> register.registerHistogram(name, labelNames, document, buckets)); + } + /** * Register histogram. * @@ -228,6 +251,17 @@ public static void recordTime(final String name, final long duration) { recordTime(name, null, duration); } + /** + * Records a fractional histogram observation without truncation or unit conversion. + * + * @param name metric name + * @param labelValues label values + * @param value observation in the metric's unit + */ + public static void observe(final String name, final String[] labelValues, final double value) { + Optional.ofNullable(metricsRegister).ifPresent(register -> register.observe(name, labelValues, value)); + } + /** * Clean. */ diff --git a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/spi/MetricsRegister.java b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/spi/MetricsRegister.java index b8a8418bf9b0..155245854e06 100644 --- a/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/spi/MetricsRegister.java +++ b/shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/spi/MetricsRegister.java @@ -51,6 +51,19 @@ public interface MetricsRegister { * @param document document for histogram */ void registerHistogram(String name, String[] labelNames, String document); + + /** + * Registers a histogram with explicit bucket boundaries, in the metric's unit. + * Existing providers may continue to use their default buckets. + * + * @param name metric name + * @param labelNames label names + * @param document metric description + * @param buckets bucket boundaries + */ + default void registerHistogram(final String name, final String[] labelNames, final String document, final double[] buckets) { + registerHistogram(name, labelNames, document); + } /** * Counter increment by count. @@ -85,6 +98,19 @@ public interface MetricsRegister { * @param duration duration */ void recordTime(String name, String[] labelValues, long duration); + + /** + * Records a floating-point histogram observation without changing its unit. + * Providers must override this optional operation to support fractional observations. + * The default deliberately skips the sample rather than truncating it to zero. + * + * @param name metric name + * @param labelValues label values + * @param value observation in the metric's unit + */ + default void observe(final String name, final String[] labelValues, final double value) { + // Optional for existing MetricsRegister implementations. + } /** * Clean. diff --git a/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/CanaryMetricsPluginTest.java b/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/CanaryMetricsPluginTest.java new file mode 100644 index 000000000000..08d5fafc9f6c --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/CanaryMetricsPluginTest.java @@ -0,0 +1,411 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.metrics; + +import io.prometheus.client.CollectorRegistry; +import org.apache.shenyu.common.config.ShenyuConfig.MetricsConfig; +import org.apache.shenyu.common.constant.Constants; +import org.apache.shenyu.plugin.api.context.CanaryContext; +import org.apache.shenyu.plugin.api.context.ShenyuContext; +import org.apache.shenyu.plugin.metrics.constant.CanaryMetric; +import org.apache.shenyu.plugin.metrics.constant.LabelNames; +import org.apache.shenyu.plugin.metrics.prometheus.PrometheusMetricsRegister; +import org.apache.shenyu.plugin.metrics.prometheus.PrometheusMetricsService; +import org.apache.shenyu.plugin.metrics.reporter.MetricsReporter; +import org.apache.shenyu.plugin.metrics.spi.MetricsRegister; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpCookie; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import reactor.core.Disposable; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.time.Duration; +import java.time.LocalDateTime; +import java.util.List; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; + +/** + * Verifies exported samples across routing, commit, error, cancellation and retry boundaries. + */ +class CanaryMetricsPluginTest { + + private final MetricsPlugin plugin = new MetricsPlugin(); + + private MockServerWebExchange exchange; + + @BeforeEach + void setUp() { + MetricsReporter.clean(); + new PrometheusMetricsRegister().clean(); + CollectorRegistry.defaultRegistry.clear(); + MetricsReporter.register(new PrometheusMetricsRegister()); + exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/test")); + ShenyuContext context = new ShenyuContext(); + context.setStartDateTime(LocalDateTime.now().minusSeconds(1)); + context.setRpcType("http"); + exchange.getAttributes().put(Constants.CONTEXT, context); + } + + @AfterEach + void clean() { + MetricsReporter.clean(); + new PrometheusMetricsRegister().clean(); + CollectorRegistry.defaultRegistry.clear(); + } + + @ParameterizedTest + @ValueSource(ints = {200, 204, 302, 404, 503}) + void recordsHttpOutcomeAndFractionalDecision(final int status) { + plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + current.getResponse().setStatusCode(HttpStatus.valueOf(status)); + return current.getResponse().setComplete(); + }).block(); + assertRequest("canary", status < 400 ? "success" : "error"); + assertEquals(0.00008, ruleSample(CanaryMetric.DECISION_DURATION, "_sum"), 0.000000001); + assertEquals(1.0, ruleSample(CanaryMetric.DECISION_DURATION, "_count")); + assertEquals(1.0, latency("canary", "_count")); + assertTrue(latency("canary", "_sum") >= 1000.0); + assertEquals(1.0, CollectorRegistry.defaultRegistry.getSampleValue(CanaryMetric.DECISION_DURATION.getName() + "_bucket", + new String[]{"selector", "rule", "le"}, new String[]{"selector-a", "rule-a", "1.0E-4"})); + } + + @Test + void recordsFallbackAtRoutingBeforeTheRequestFinishesAndOnlyOnce() { + StepVerifier.create(plugin.execute(exchange, current -> { + CanaryContext context = context("stable", CanaryContext.CANARY_POOL_EMPTY, null); + publish(context); + publish(context); + assertEquals(1.0, fallback()); + assertEquals(1.0, ruleSample(CanaryMetric.DECISION_DURATION, "_count")); + return Mono.never(); + })).thenCancel().verify(); + assertRequest("stable", "cancelled"); + assertEquals(1.0, fallback()); + assertNull(latency("stable", "_count")); + } + + @ParameterizedTest + @ValueSource(strings = {CanaryContext.CANARY_POOL_EMPTY, CanaryContext.STABLE_POOL_EMPTY, CanaryContext.NO_UPSTREAM_SELECTED}) + void recordsExplicitRejectionEvenWhenHttpStatusDefaultsToSuccess(final String reason) { + plugin.execute(exchange, current -> { + publish(context(null, null, reason)); + return current.getResponse().setComplete(); + }).block(); + assertRequest("canary", "reject"); + assertNull(fallback()); + } + + @Test + void recordsStableIntentRejectionAsStable() { + plugin.execute(exchange, current -> { + publish(new CanaryContext("selector-a", "rule-a", "stable", null, null, CanaryContext.STABLE_POOL_EMPTY, 80_000)); + return current.getResponse().setComplete(); + }).block(); + assertRequest("stable", "reject"); + } + + @Test + void recordsErrorBeforeAnOuterHandlerCommitsItsResponse() { + IllegalStateException failure = new IllegalStateException("backend failed"); + plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + return Mono.error(failure); + }).onErrorResume(error -> { + assertEquals(failure, error); + exchange.getResponse().setStatusCode(HttpStatus.BAD_GATEWAY); + return exchange.getResponse().setComplete(); + }).block(); + assertRequest("canary", "error"); + assertEquals(1.0, latency("canary", "_count")); + } + + @Test + void capturesDownstreamPluginFailureFromDeferredChain() { + IllegalStateException failure = new IllegalStateException("selection failed"); + StepVerifier.create(plugin.execute(exchange, current -> Mono.defer(() -> { + publish(context(null, null, null)); + throw failure; + }))).expectErrorMatches(error -> error == failure).verify(); + assertRequest("canary", "error"); + assertEquals(1.0, CollectorRegistry.defaultRegistry.getSampleValue(LabelNames.REQUEST_THROW_TOTAL)); + assertNull(latency("canary", "_count")); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void committedHeadersWaitForChainTerminationToRecordLatencyAndOutcome(final boolean bodyFails) { + Mono result = plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + return current.getResponse().setComplete().then(Mono.defer(() -> { + assertTrue(current.getResponse().isCommitted()); + assertNull(latency("canary", "_count")); + assertNull(request("canary", "success")); + return bodyFails ? Mono.error(new IllegalStateException("response body failed")) : Mono.empty(); + })); + }); + if (bodyFails) { + StepVerifier.create(result).expectError(IllegalStateException.class).verify(); + } else { + StepVerifier.create(result).verifyComplete(); + } + assertRequest("canary", bodyFails ? "error" : "success"); + assertNull(request("canary", bodyFails ? "success" : "error")); + assertEquals(1.0, latency("canary", "_count")); + } + + @Test + void cancellationAfterCommitRecordsCancelledRequestWithoutLatency() { + Disposable subscription = plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + return current.getResponse().setComplete().then(Mono.never()); + }).subscribe(); + assertTrue(exchange.getResponse().isCommitted()); + subscription.dispose(); + assertRequest("canary", "cancelled"); + assertNull(request("canary", "success")); + assertNull(latency("canary", "_count")); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void internalRetriesProduceOneFinalRequest(final boolean eventuallySucceeds) { + AtomicInteger attempts = new AtomicInteger(); + Mono result = plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + return Mono.defer(() -> { + int attempt = attempts.incrementAndGet(); + return eventuallySucceeds && attempt == 3 ? current.getResponse().setComplete() : Mono.error(new IllegalStateException("retry")); + }).retry(2); + }); + if (eventuallySucceeds) { + StepVerifier.create(result).verifyComplete(); + } else { + StepVerifier.create(result).expectError(IllegalStateException.class).verify(); + } + assertEquals(3, attempts.get()); + assertRequest("canary", eventuallySucceeds ? "success" : "error"); + assertEquals(1.0, ruleSample(CanaryMetric.DECISION_DURATION, "_count")); + assertNull(fallback()); + } + + @Test + void reenteringMetricsWithTheSameExchangeDoesNotDuplicateCanarySamples() { + plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + return plugin.execute(current, inner -> inner.getResponse().setComplete()); + }).block(); + assertRequest("canary", "success"); + assertEquals(1.0, ruleSample(CanaryMetric.DECISION_DURATION, "_count")); + assertEquals(1.0, latency("canary", "_count")); + } + + @Test + void legacyRequestOnlyRecordsExistingMetrics() { + plugin.execute(exchange, current -> current.getResponse().setComplete()).block(); + assertEquals(1.0, CollectorRegistry.defaultRegistry.getSampleValue(LabelNames.REQUEST_TOTAL)); + assertEquals(1.0, latency("none", "_count")); + assertNull(latency("canary", "_count")); + assertNull(ruleSample(CanaryMetric.DECISION_DURATION, "_count")); + assertNull(request("canary", "success")); + } + + @Test + void existingLatencyHistogramSeparatesStableAndCanary() { + plugin.execute(exchange, current -> { + publish(context("stable", CanaryContext.CANARY_POOL_EMPTY, null)); + return current.getResponse().setComplete(); + }).block(); + setUpExchange(); + plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + return current.getResponse().setComplete(); + }).block(); + assertEquals(1.0, latency("stable", "_count")); + assertEquals(1.0, latency("canary", "_count")); + assertNull(latency("none", "_count")); + assertEquals(1.0, fallback()); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void distinctSelectorsOrRulesKeepSeparateCounters(final boolean differentSelector) { + plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + return current.getResponse().setComplete(); + }).block(); + setUpExchange(); + String selector = differentSelector ? "selector-b" : "selector-a"; + String rule = differentSelector ? "rule-a" : "rule-b"; + plugin.execute(exchange, current -> { + publish(new CanaryContext(selector, rule, "canary", "canary", null, null, 80_000)); + current.getResponse().setStatusCode(HttpStatus.BAD_GATEWAY); + return current.getResponse().setComplete(); + }).block(); + assertRequest("canary", "success"); + assertNull(request("canary", "error")); + assertEquals(1.0, CollectorRegistry.defaultRegistry.getSampleValue(CanaryMetric.REQUESTS.getName(), + new String[]{"selector", "rule", "partition", "outcome"}, new String[]{selector, rule, "canary", "error"})); + assertNull(CollectorRegistry.defaultRegistry.getSampleValue(CanaryMetric.REQUESTS.getName(), + new String[]{"selector", "rule", "partition", "outcome"}, new String[]{selector, rule, "canary", "success"})); + assertEquals(1.0, ruleSample(CanaryMetric.DECISION_DURATION, "_count")); + assertEquals(1.0, CollectorRegistry.defaultRegistry.getSampleValue(CanaryMetric.DECISION_DURATION.getName() + "_count", + new String[]{"selector", "rule"}, new String[]{selector, rule})); + assertEquals(2.0, latency("canary", "_count")); + } + + @Test + void httpExportKeepsCanaryLabelsBoundedAcrossDifferentRequests() throws Exception { + for (int index = 0; index < 3; index++) { + exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/sensitive-path-" + index) + .header("Authorization", "Bearer sensitive-token-" + index) + .header("X-Request-ID", "sensitive-request-" + index) + .header("X-Sticky-Key", "sensitive-sticky-" + index) + .cookie(new HttpCookie("session", "sensitive-cookie-" + index))); + ShenyuContext context = new ShenyuContext(); + context.setStartDateTime(LocalDateTime.now().minusSeconds(1)); + context.setRpcType("http"); + exchange.getAttributes().put(Constants.CONTEXT, context); + plugin.execute(exchange, current -> { + publish(context("stable", CanaryContext.CANARY_POOL_EMPTY, null)); + return current.getResponse().setComplete(); + }).block(); + } + MetricsConfig config = new MetricsConfig(); + config.setHost("127.0.0.1"); + config.setPort(0); + config.setProps(new Properties()); + PrometheusMetricsService service = new PrometheusMetricsService(); + try { + service.start(config, new PrometheusMetricsRegister()); + assertNotNull(service.getServer()); + URI uri = URI.create("http://127.0.0.1:" + service.getServer().getPort() + "/metrics"); + HttpRequest request = HttpRequest.newBuilder(uri).timeout(Duration.ofSeconds(5)).GET().build(); + HttpResponse response = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(5)).build() + .send(request, HttpResponse.BodyHandlers.ofString()); + assertEquals(200, response.statusCode()); + List canarySamples = response.body().lines().filter(line -> line.startsWith("shenyu_canary_")).toList(); + assertFalse(canarySamples.isEmpty()); + assertFalse(String.join("\n", canarySamples).contains("sensitive-")); + List requests = canarySamples.stream().filter(line -> line.startsWith("shenyu_canary_requests_total{")).toList(); + assertEquals(1, requests.size()); + assertTrue(requests.get(0).contains("selector=\"selector-a\"")); + assertTrue(requests.get(0).contains("rule=\"rule-a\"")); + assertTrue(requests.get(0).contains("partition=\"stable\"")); + assertTrue(requests.get(0).contains("outcome=\"success\"")); + assertEquals(4, requests.get(0).chars().filter(character -> character == '=').count()); + assertTrue(requests.get(0).endsWith(" 3.0")); + assertTrue(canarySamples.stream().anyMatch(line -> line.startsWith("shenyu_canary_fallback_total{") + && line.contains("reason=\"canary_pool_empty\"") && line.endsWith(" 3.0"))); + assertTrue(canarySamples.stream().anyMatch(line -> line.startsWith("shenyu_canary_decision_duration_seconds_count{") && line.endsWith(" 3.0"))); + assertTrue(response.body().lines().anyMatch(line -> line.startsWith("shenyu_execute_latency_millis_count{") + && line.contains("partition=\"stable\"") && line.endsWith(" 3.0"))); + String decisionSum = canarySamples.stream().filter(line -> line.startsWith("shenyu_canary_decision_duration_seconds_sum{")).findFirst().orElseThrow(); + assertEquals(0.00024, Double.parseDouble(decisionSum.substring(decisionSum.lastIndexOf(' ') + 1)), 0.000000001); + } finally { + service.stop(); + } + } + + @Test + void missingReporterDoesNotAffectTheResponse() { + MetricsReporter.clean(); + StepVerifier.create(plugin.execute(exchange, current -> { + publish(context("canary", null, null)); + return current.getResponse().setComplete(); + })).verifyComplete(); + assertTrue(exchange.getResponse().isCommitted()); + } + + @Test + void reporterFailurePropagatesFromRequestCounter() { + MetricsRegister failing = mock(MetricsRegister.class); + MetricsReporter.register(failing); + IllegalStateException failure = new IllegalStateException("metrics unavailable"); + doThrow(failure).when(failing).counterIncrement(anyString(), any(), anyLong()); + assertSame(failure, assertThrows(IllegalStateException.class, + () -> plugin.execute(exchange, current -> current.getResponse().setComplete()))); + } + + private void setUpExchange() { + exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/test")); + ShenyuContext context = new ShenyuContext(); + context.setRpcType("http"); + exchange.getAttributes().put(Constants.CONTEXT, context); + } + + private CanaryContext context(final String actual, final String fallbackReason, final String rejectReason) { + return new CanaryContext("selector-a", "rule-a", "canary", actual, fallbackReason, rejectReason, 80_000); + } + + private void publish(final CanaryContext context) { + exchange.getAttributes().put(Constants.SHENYU_CANARY_CONTEXT, context); + Consumer consumer = exchange.getAttribute(Constants.METRICS_CANARY); + consumer.accept(context); + } + + private void assertRequest(final String partition, final String outcome) { + assertEquals(1.0, request(partition, outcome)); + } + + private Double request(final String partition, final String outcome) { + return CollectorRegistry.defaultRegistry.getSampleValue(CanaryMetric.REQUESTS.getName(), + new String[]{"selector", "rule", "partition", "outcome"}, new String[]{"selector-a", "rule-a", partition, outcome}); + } + + private Double ruleSample(final CanaryMetric metric, final String suffix) { + return CollectorRegistry.defaultRegistry.getSampleValue(metric.getName() + suffix, + new String[]{"selector", "rule"}, new String[]{"selector-a", "rule-a"}); + } + + private Double fallback() { + return CollectorRegistry.defaultRegistry.getSampleValue(CanaryMetric.FALLBACK.getName(), + new String[]{"selector", "rule", "reason"}, new String[]{"selector-a", "rule-a", CanaryContext.CANARY_POOL_EMPTY}); + } + + private Double latency(final String partition, final String suffix) { + return CollectorRegistry.defaultRegistry.getSampleValue(LabelNames.EXECUTE_LATENCY_NAME + suffix, + new String[]{"partition"}, new String[]{partition}); + } +} diff --git a/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/prometheus/PrometheusMetricsRegisterTest.java b/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/prometheus/PrometheusMetricsRegisterTest.java index 417f76aa1d44..c94000857167 100644 --- a/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/prometheus/PrometheusMetricsRegisterTest.java +++ b/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/prometheus/PrometheusMetricsRegisterTest.java @@ -17,10 +17,13 @@ package org.apache.shenyu.plugin.metrics.prometheus; +import io.prometheus.client.CollectorRegistry; import io.prometheus.client.Counter; import io.prometheus.client.Gauge; import io.prometheus.client.Histogram; import org.apache.shenyu.common.utils.ReflectUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.util.CollectionUtils; @@ -36,6 +39,13 @@ public final class PrometheusMetricsRegisterTest { private final PrometheusMetricsRegister prometheusMetricsRegister = new PrometheusMetricsRegister(); + @BeforeEach + @AfterEach + void resetRegistry() { + prometheusMetricsRegister.clean(); + CollectorRegistry.defaultRegistry.clear(); + } + @Test @SuppressWarnings("unchecked") public void assertCounter() { @@ -86,6 +96,18 @@ public void testRegisterGauge() throws Exception { prometheusMetricsRegister.clean(); } + @Test + void testFractionalObservationAndExplicitBuckets() { + String name = "test_fractional_duration_seconds"; + prometheusMetricsRegister.registerHistogram(name, new String[]{"rule"}, "fractional duration", new double[]{0.00001, 0.0001, 0.001}); + prometheusMetricsRegister.observe(name, new String[]{"rule-a"}, 0.00008); + Assertions.assertEquals(0.00008, CollectorRegistry.defaultRegistry.getSampleValue(name + "_sum", new String[]{"rule"}, new String[]{"rule-a"}), 0.000000001); + Assertions.assertEquals(0.0, CollectorRegistry.defaultRegistry.getSampleValue(name + "_bucket", + new String[]{"rule", "le"}, new String[]{"rule-a", "1.0E-5"})); + Assertions.assertEquals(1.0, CollectorRegistry.defaultRegistry.getSampleValue(name + "_bucket", + new String[]{"rule", "le"}, new String[]{"rule-a", "1.0E-4"})); + } + @Test public void testClean() throws Exception { prometheusMetricsRegister.clean(); diff --git a/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporterTest.java b/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporterTest.java index 057aaa77d0d2..893e05ebe73b 100644 --- a/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporterTest.java +++ b/shenyu-plugin/shenyu-plugin-metrics/src/test/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporterTest.java @@ -17,21 +17,21 @@ package org.apache.shenyu.plugin.metrics.reporter; +import io.prometheus.client.CollectorRegistry; import io.prometheus.client.Counter; import io.prometheus.client.Gauge; import io.prometheus.client.Histogram; import org.apache.shenyu.common.utils.ReflectUtils; import org.apache.shenyu.plugin.metrics.config.Metric; +import org.apache.shenyu.plugin.metrics.constant.CanaryMetric; import org.apache.shenyu.plugin.metrics.constant.LabelNames; import org.apache.shenyu.plugin.metrics.enums.MetricType; import org.apache.shenyu.plugin.metrics.prometheus.PrometheusMetricsRegister; import org.apache.shenyu.plugin.metrics.spi.MetricsRegister; -import org.junit.FixMethodOrder; -import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.runners.MethodSorters; import org.springframework.util.CollectionUtils; import java.lang.reflect.Field; @@ -43,15 +43,17 @@ /** * The Test Case For MetricsReporter. */ -@FixMethodOrder(MethodSorters.NAME_ASCENDING) public final class MetricsReporterTest { private static final String DOCUMENT = "testDocument"; private static MetricsRegister metricsRegister; - @BeforeAll - public static void setUp() { + @BeforeEach + public void setUp() { + MetricsReporter.clean(); + new PrometheusMetricsRegister().clean(); + CollectorRegistry.defaultRegistry.clear(); metricsRegister = new PrometheusMetricsRegister(); MetricsReporter.register(metricsRegister); } @@ -62,12 +64,14 @@ public void testRegister() throws Exception { for (String name : List.of(LabelNames.REQUEST_TOTAL, LabelNames.REQUEST_TYPE_TOTAL, LabelNames.REQUEST_THROW_TOTAL, LabelNames.AGENT_MCP_CALLS_TOTAL, LabelNames.SENTINEL_REQUEST_RESTRICT_TOTAL, LabelNames.SENTINEL_REQUEST_CIRCUITBREAKER_TOTAL, LabelNames.RESILIENCE4J_REQUEST_RESTRICT_TOTAL, LabelNames.RESILIENCE4J_REQUEST_CIRCUITBREAKER_TOTAL, - LabelNames.HYSTRIX_REQUEST_CIRCUITBREAKER_TOTAL, LabelNames.RATELIMITER_REQUEST_RESTRICT_TOTAL)) { + LabelNames.HYSTRIX_REQUEST_CIRCUITBREAKER_TOTAL, LabelNames.RATELIMITER_REQUEST_RESTRICT_TOTAL, + CanaryMetric.REQUESTS.getName(), CanaryMetric.FALLBACK.getName())) { Assertions.assertNotNull(map1.get(name), name); } Map map2 = getPrivateField(metricsRegister, "HISTOGRAM_MAP", Map.class); Assertions.assertNotNull(map2.get(LabelNames.EXECUTE_LATENCY_NAME)); Assertions.assertNotNull(map2.get(LabelNames.AGENT_MCP_CALL_LATENCY)); + Assertions.assertNotNull(map2.get(CanaryMetric.DECISION_DURATION.getName())); List labels = new ArrayList<>(); labels.add("shenyu_request_total"); Collection metrics = new ArrayList<>(); @@ -170,9 +174,10 @@ public void testZClean() throws Exception { Assertions.assertTrue(CollectionUtils.isEmpty(counterMap2)); } - @AfterAll - public static void clean() { + @AfterEach + public void clean() { metricsRegister.clean(); MetricsReporter.clean(); + CollectorRegistry.defaultRegistry.clear(); } } diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/DividePlugin.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/DividePlugin.java index ef8deefe997c..5ffdda13fe7d 100644 --- a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/DividePlugin.java +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/DividePlugin.java @@ -22,6 +22,7 @@ import org.apache.shenyu.common.constant.Constants; import org.apache.shenyu.common.dto.RuleData; import org.apache.shenyu.common.dto.SelectorData; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfig; import org.apache.shenyu.common.dto.convert.rule.impl.DivideRuleHandle; import org.apache.shenyu.common.enums.HttpRetryBackoffSpecEnum; import org.apache.shenyu.common.enums.LoadBalanceEnum; @@ -31,6 +32,7 @@ import org.apache.shenyu.loadbalancer.cache.UpstreamCacheManager; import org.apache.shenyu.loadbalancer.entity.Upstream; import org.apache.shenyu.plugin.api.ShenyuPluginChain; +import org.apache.shenyu.plugin.api.context.CanaryContext; import org.apache.shenyu.plugin.api.context.ShenyuContext; import org.apache.shenyu.plugin.api.result.ShenyuResultEnum; import org.apache.shenyu.plugin.api.result.ShenyuResultWrap; @@ -40,9 +42,14 @@ import org.apache.shenyu.plugin.base.circuitbreaker.UpstreamCircuitBreaker; import org.apache.shenyu.plugin.base.utils.CacheKeyUtils; import org.apache.shenyu.plugin.base.utils.LoadbalancerUtils; +import org.apache.shenyu.plugin.base.utils.UpstreamLabelUtils; +import org.apache.shenyu.plugin.divide.canary.CanaryDecision; +import org.apache.shenyu.plugin.divide.canary.CanaryDecisionService; +import org.apache.shenyu.plugin.divide.canary.DefaultCanaryDecisionService; import org.apache.shenyu.plugin.divide.handler.DividePluginDataHandler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.http.HttpStatus; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Mono; @@ -51,7 +58,10 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.function.Consumer; /** * Divide Plugin. @@ -64,6 +74,16 @@ public class DividePlugin extends AbstractShenyuPlugin { private static final String SHORTEST_RESPONSE = "shortestResponse"; + private final CanaryDecisionService canaryDecisionService; + + public DividePlugin() { + this(new DefaultCanaryDecisionService()); + } + + public DividePlugin(final CanaryDecisionService canaryDecisionService) { + this.canaryDecisionService = Objects.requireNonNull(canaryDecisionService); + } + @Override protected String getRawPath(final ServerWebExchange exchange) { return RequestUrlUtils.getRewrittenRawPath(exchange); @@ -99,37 +119,14 @@ protected Mono doExecute(final ServerWebExchange exchange, final ShenyuPlu Object error = ShenyuResultWrap.error(exchange, ShenyuResultEnum.CANNOT_FIND_HEALTHY_UPSTREAM_URL); return WebFluxResultUtils.result(exchange, error); } - final List breakeredUpstreamList = filterByCircuitBreaker(selector.getId(), upstreamList); - if (CollectionUtils.isEmpty(breakeredUpstreamList)) { - LOG.warn("all upstreams of selector {} are blocked by the circuit breaker, fail fast", selector.getId()); - Object error = ShenyuResultWrap.error(exchange, ShenyuResultEnum.CANNOT_FIND_HEALTHY_UPSTREAM_URL); - return WebFluxResultUtils.result(exchange, error); - } - List specifyDomains = exchange.getRequest().getHeaders().get(Constants.SPECIFY_DOMAIN); - Upstream upstream; - if (CollectionUtils.isNotEmpty(specifyDomains)) { - String requested = specifyDomains.get(0); - upstream = breakeredUpstreamList.stream() - .filter(u -> u.getUrl().equals(requested)) - .findFirst() - .map(u -> Upstream.builder() - .url(u.getUrl()) - .protocol(u.getProtocol()) - .weight(u.getWeight()) - .warmup(u.getWarmup()) - .status(u.isStatus()) - .build()) - .orElseGet(() -> LoadbalancerUtils.getForExchange(breakeredUpstreamList, ruleHandle.getLoadBalance(), exchange)); - } else { - upstream = LoadbalancerUtils.getForExchange(breakeredUpstreamList, ruleHandle.getLoadBalance(), exchange); - } + Upstream upstream = selectUpstream(exchange, selector.getId(), rule, ruleHandle, upstreamList); if (Objects.isNull(upstream)) { LOG.error("divide has no upstream"); Object error = ShenyuResultWrap.error(exchange, ShenyuResultEnum.CANNOT_FIND_HEALTHY_UPSTREAM_URL); return WebFluxResultUtils.result(exchange, error); } - // set domain String domain = upstream.buildDomain(); + // set domain exchange.getAttributes().put(Constants.HTTP_DOMAIN, domain); // set the http timeout exchange.getAttributes().put(Constants.HTTP_TIME_OUT, ruleHandle.getTimeout()); @@ -167,6 +164,85 @@ protected Mono handleRuleIfNull(final String pluginName, final ServerWebEx return WebFluxResultUtils.noRuleResult(pluginName, exchange); } + private Upstream selectUpstream(final ServerWebExchange exchange, final String selectorId, final RuleData rule, + final DivideRuleHandle ruleHandle, final List upstreams) { + CanaryConfig config = ruleHandle.getCanary(); + if (CollectionUtils.isNotEmpty(exchange.getRequest().getHeaders().get(Constants.SPECIFY_DOMAIN))) { + return selectLegacyUpstream(exchange, selectorId, ruleHandle); + } + if (Objects.isNull(config)) { + return selectLegacyUpstream(exchange, selectorId, ruleHandle); + } + long start = System.nanoTime(); + CanaryDecision actual = canaryDecisionService.decide(exchange, rule.getId(), config); + CanaryContext context = new CanaryContext(selectorId, rule.getId(), actual.getName(), null, null, null, System.nanoTime() - start); + exchange.getAttributes().putIfAbsent(Constants.SHENYU_CANARY_CONTEXT, context); + Map labels = partitionLabels(actual, config); + List candidates = UpstreamLabelUtils.filter(upstreams, labels); + if (candidates.isEmpty() && actual == CanaryDecision.CANARY && "STABLE".equals(config.getFallbackPolicy())) { + actual = CanaryDecision.STABLE; + labels = partitionLabels(actual, config); + candidates = UpstreamLabelUtils.filter(upstreams, labels); + } + if (candidates.isEmpty()) { + context.setRejectReason(actual == CanaryDecision.CANARY ? CanaryContext.CANARY_POOL_EMPTY : CanaryContext.STABLE_POOL_EMPTY); + reportCanary(exchange, context); + exchange.getResponse().setStatusCode(HttpStatus.SERVICE_UNAVAILABLE); + return null; + } + // The pool is resolved before load balancing. A backend failure must not change this partition. + exchange.getAttributes().put(Constants.SHENYU_CANARY_PARTITION, actual.getName()); + exchange.getAttributes().put(Constants.SHENYU_CANARY_LABELS, labels); + candidates = filterByCircuitBreaker(selectorId, candidates); + Upstream upstream = candidates.isEmpty() ? null : LoadbalancerUtils.getForExchange(candidates, ruleHandle.getLoadBalance(), exchange); + context.setActualPartition(Objects.isNull(upstream) ? null : actual.getName()); + context.setRejectReason(Objects.isNull(upstream) ? CanaryContext.NO_UPSTREAM_SELECTED : null); + if (Objects.nonNull(upstream) && !context.getIntendedPartition().equals(actual.getName())) { + context.setFallbackReason(CanaryContext.CANARY_POOL_EMPTY); + } + reportCanary(exchange, context); + return upstream; + } + + private void reportCanary(final ServerWebExchange exchange, final CanaryContext context) { + if (exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT) != context) { + return; + } + LOG.debug("canary routing selector={} rule={} intended={} actual={} fallback={} reject={}", context.getSelectorId(), + context.getRuleId(), context.getIntendedPartition(), context.getActualPartition(), context.getFallbackReason(), context.getRejectReason()); + final Consumer consumer = exchange.getAttribute(Constants.METRICS_CANARY); + try { + Optional.ofNullable(consumer).ifPresent(c -> c.accept(context)); + } catch (RuntimeException ex) { + LOG.debug("Unable to report canary routing observation", ex); + } + } + + private Upstream selectLegacyUpstream(final ServerWebExchange exchange, final String selectorId, final DivideRuleHandle ruleHandle) { + List candidates = UpstreamCacheManager.getInstance().findLegacyUpstreamListBySelectorId(selectorId); + if (CollectionUtils.isEmpty(candidates)) { + return null; + } + candidates = filterByCircuitBreaker(selectorId, candidates); + if (candidates.isEmpty()) { + return null; + } + List specifyDomains = exchange.getRequest().getHeaders().get(Constants.SPECIFY_DOMAIN); + if (CollectionUtils.isNotEmpty(specifyDomains)) { + String requested = specifyDomains.get(0); + Optional specified = candidates.stream().filter(upstream -> upstream.getUrl().equals(requested)).findFirst(); + if (specified.isPresent()) { + return specified.get(); + } + } + return LoadbalancerUtils.getForExchange(candidates, ruleHandle.getLoadBalance(), exchange); + } + + private Map partitionLabels(final CanaryDecision partition, final CanaryConfig config) { + Map labels = partition == CanaryDecision.CANARY ? config.getCanaryLabels() : config.getStableLabels(); + return Objects.isNull(labels) ? Map.of() : Map.copyOf(labels); + } + /** * Forward the request through the plugin chain and record its outcome into * the built-in circuit breaker, keeping the load-balance specific triggers. diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/CanaryDecision.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/CanaryDecision.java new file mode 100644 index 000000000000..df732c4f9a12 --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/CanaryDecision.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.divide.canary; + +/** + * Expected request partition, independent of upstream availability. + */ +public enum CanaryDecision { + + /** + * Production partition. + */ + STABLE("stable"), + + /** + * Canary partition. + */ + CANARY("canary"); + + private final String name; + + CanaryDecision(final String name) { + this.name = name; + } + + /** + * Get the lowercase partition name stored on the exchange. + * + * @return partition name + */ + public String getName() { + return name; + } +} diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/CanaryDecisionService.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/CanaryDecisionService.java new file mode 100644 index 000000000000..0b3bf16f4bca --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/CanaryDecisionService.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.divide.canary; + +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfig; +import org.springframework.web.server.ServerWebExchange; + +/** + * Assign requests to a partition without selecting or invoking upstreams. + */ +public interface CanaryDecisionService { + + /** + * Decide the expected partition; a disabled Canary configuration selects Stable. + * + * @param exchange current request + * @param ruleId stable rule identifier + * @param config present Canary configuration + * @return expected Stable or Canary partition + */ + CanaryDecision decide(ServerWebExchange exchange, String ruleId, CanaryConfig config); +} diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/DefaultCanaryDecisionService.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/DefaultCanaryDecisionService.java new file mode 100644 index 000000000000..acf39a4e8a34 --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/canary/DefaultCanaryDecisionService.java @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.divide.canary; + +import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfig; +import org.apache.shenyu.common.dto.convert.rule.canary.StickyKeyConfig; +import org.apache.shenyu.common.enums.MatchModeEnum; +import org.apache.shenyu.plugin.base.condition.data.ParameterDataFactory; +import org.apache.shenyu.plugin.base.condition.strategy.MatchStrategyFactory; +import org.springframework.web.server.ServerWebExchange; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Objects; + +/** + * Deterministic request partitioning using conditions and SHA-256 buckets. + */ +public class DefaultCanaryDecisionService implements CanaryDecisionService { + + @Override + public CanaryDecision decide(final ServerWebExchange exchange, final String ruleId, final CanaryConfig config) { + Objects.requireNonNull(config, "Canary configuration is required"); + if (!config.isEnabled()) { + return CanaryDecision.STABLE; + } + int percentage = config.getPercentage(); + if (percentage < 0 || percentage > 100) { + throw new IllegalArgumentException("Canary percentage must be between 0 and 100"); + } + if (percentage == 0) { + return CanaryDecision.STABLE; + } + int matchMode = Objects.isNull(config.getMatchMode()) ? MatchModeEnum.AND.getCode() : config.getMatchMode(); + if (CollectionUtils.isNotEmpty(config.getConditions()) + && !MatchStrategyFactory.match(matchMode, config.getConditions(), exchange)) { + return CanaryDecision.STABLE; + } + if (percentage == 100) { + return CanaryDecision.CANARY; + } + String key = readStickyKey(exchange, config.getStickyKey()); + if (StringUtils.isBlank(key)) { + return CanaryDecision.STABLE; + } + return hashToBucket(ruleId, key) < percentage * 100 ? CanaryDecision.CANARY : CanaryDecision.STABLE; + } + + private String readStickyKey(final ServerWebExchange exchange, final StickyKeyConfig stickyKey) { + if (Objects.isNull(stickyKey) || StringUtils.isBlank(stickyKey.getParamType())) { + throw new IllegalArgumentException("Canary sticky key parameter type is required"); + } + // Pass the parameter name through unchanged; the selected source owns its extraction semantics. + return ParameterDataFactory.builderData(stickyKey.getParamType(), stickyKey.getParamName(), exchange); + } + + /** + * Encode each UTF-8 field with a four-byte big-endian byte-length prefix. + * Interpret the first four SHA-256 bytes as an unsigned big-endian integer. + * The percentage and upstream topology deliberately do not enter the hash. + * + * @param ruleId stable rule identifier + * @param key request identifier + * @return bucket from 0 to 9999 + */ + int hashToBucket(final String ruleId, final String key) { + Objects.requireNonNull(ruleId, "Canary rule identifier is required"); + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + updateField(digest, ruleId); + updateField(digest, key); + return (int) (Integer.toUnsignedLong(ByteBuffer.wrap(digest.digest()).getInt()) % 10000); + } catch (NoSuchAlgorithmException ex) { + throw new IllegalStateException("SHA-256 is unavailable", ex); + } + } + + private void updateField(final MessageDigest digest, final String value) { + byte[] bytes = value.getBytes(StandardCharsets.UTF_8); + digest.update(ByteBuffer.allocate(Integer.BYTES).putInt(bytes.length).array()); + digest.update(bytes); + } +} diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DividePluginDataHandler.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DividePluginDataHandler.java index acd2f3642090..95e2929ab1c7 100644 --- a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DividePluginDataHandler.java +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DividePluginDataHandler.java @@ -20,12 +20,14 @@ import org.apache.shenyu.common.constant.Constants; import org.apache.shenyu.common.dto.RuleData; import org.apache.shenyu.common.dto.SelectorData; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfigValidator; import org.apache.shenyu.common.dto.convert.rule.impl.DivideRuleHandle; import org.apache.shenyu.common.enums.PluginEnum; -import org.apache.shenyu.common.utils.GsonUtils; import org.apache.shenyu.loadbalancer.cache.UpstreamCacheManager; import org.apache.shenyu.plugin.base.cache.CommonHandleCache; import org.apache.shenyu.plugin.base.cache.MetaDataCache; +import org.apache.shenyu.plugin.base.condition.data.ParameterDataFactory; +import org.apache.shenyu.plugin.base.condition.judge.PredicateJudgeFactory; import org.apache.shenyu.plugin.base.handler.PluginDataHandler; import org.apache.shenyu.plugin.base.utils.BeanHolder; import org.apache.shenyu.plugin.base.utils.CacheKeyUtils; @@ -64,7 +66,11 @@ public void removeSelector(final SelectorData selectorData) { @Override public void handlerRule(final RuleData ruleData) { Optional.ofNullable(ruleData.getHandle()).ifPresent(s -> { - DivideRuleHandle divideRuleHandle = GsonUtils.getInstance().fromJson(s, DivideRuleHandle.class); + DivideRuleHandle divideRuleHandle = CanaryConfigValidator.parseHandle(s); + if (Objects.nonNull(divideRuleHandle.getCanary())) { + CanaryConfigValidator.validateExtensions(divideRuleHandle.getCanary(), + ParameterDataFactory::newInstance, PredicateJudgeFactory::newInstance); + } CACHED_HANDLE.get().cachedHandle(CacheKeyUtils.INST.getKey(ruleData), divideRuleHandle); // the update is also need to clean, but there is no way to // distinguish between crate and update, so it is always clean diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandler.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandler.java index 1413785cfcd7..50010e5ae12a 100644 --- a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandler.java +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandler.java @@ -17,19 +17,25 @@ package org.apache.shenyu.plugin.divide.handler; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; +import org.apache.shenyu.common.dto.DiscoverySyncData; import org.apache.shenyu.common.dto.DiscoveryUpstreamData; import org.apache.shenyu.common.enums.PluginEnum; +import org.apache.shenyu.common.utils.GsonUtils; import org.apache.shenyu.loadbalancer.cache.UpstreamCacheManager; import org.apache.shenyu.loadbalancer.entity.Upstream; import org.apache.shenyu.plugin.base.cache.MetaDataCache; import org.apache.shenyu.plugin.base.handler.AbstractDiscoveryUpstreamDataHandler; -import org.apache.shenyu.plugin.base.utils.UpstreamProps; import org.springframework.util.ObjectUtils; import java.sql.Timestamp; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; @@ -38,6 +44,18 @@ */ public class DivideUpstreamDataHandler extends AbstractDiscoveryUpstreamDataHandler { + @Override + public void handlerDiscoveryUpstreamData(final DiscoverySyncData discoverySyncData) { + if (Objects.isNull(discoverySyncData) || Objects.isNull(discoverySyncData.getSelectorId())) { + return; + } + List upstreamList = discoverySyncData.getUpstreamDataList(); + final List upstreams = convertUpstreamList(upstreamList); + // A selector may serve both Canary and legacy rules; keep all nodes for per-request routing. + submitUpstreamData(discoverySyncData.getSelectorId(), upstreams); + afterSubmitUpstreamData(discoverySyncData.getSelectorId()); + } + @Override public String pluginName() { return PluginEnum.DIVIDE.getName(); @@ -49,23 +67,66 @@ protected List convertUpstreamList(final List u return Collections.emptyList(); } return upstreamList.stream().map(u -> { - UpstreamProps props = UpstreamProps.parse(u.getProps()); - Map metadata = props.toMap(); + JsonObject properties = Optional.ofNullable(u.getProps()) + .map(ps -> GsonUtils.getInstance().fromJson(ps, JsonObject.class)).orElseGet(JsonObject::new); + Map labels = readLabels(properties); Upstream upstream = Upstream.builder() .protocol(u.getProtocol()) .url(u.getUrl()) .weight(u.getWeight()) - .warmup(props.getWarmup()) - .gray(props.isGray()) - .healthCheckEnabled(props.isHealthCheckEnabled()) + .warmup(Integer.parseInt(readScalar(properties, "warmup", "10"))) + .gray(readBoolean(properties, "gray", false)) + .healthCheckEnabled(readBoolean(properties, "healthCheckEnabled", true)) .status(0 == u.getStatus()) .timestamp(Optional.ofNullable(u.getDateCreated()).map(Timestamp::getTime).orElse(System.currentTimeMillis())) .build(); + Map metadata = new HashMap<>(); + properties.entrySet().stream().filter(entry -> entry.getValue().isJsonPrimitive()) + .forEach(entry -> metadata.put(entry.getKey(), entry.getValue().getAsString())); upstream.setMetadata(metadata); + upstream.setLabels(labels); return upstream; }).collect(Collectors.toList()); } + private String readScalar(final JsonObject properties, final String name, final String defaultValue) { + JsonElement value = properties.get(name); + if (Objects.isNull(value)) { + return defaultValue; + } + if (!value.isJsonPrimitive()) { + throw new IllegalArgumentException("Upstream property " + name + " must be a scalar"); + } + return value.getAsString(); + } + + private boolean readBoolean(final JsonObject properties, final String name, final boolean defaultValue) { + String value = readScalar(properties, name, Boolean.toString(defaultValue)); + if (!"true".equalsIgnoreCase(value) && !"false".equalsIgnoreCase(value)) { + throw new IllegalArgumentException("Upstream property " + name + " must be a boolean"); + } + return Boolean.parseBoolean(value); + } + + private Map readLabels(final JsonObject properties) { + JsonElement labels = properties.get("labels"); + if (Objects.isNull(labels)) { + return Collections.emptyMap(); + } + if (!labels.isJsonObject()) { + throw new IllegalArgumentException("Upstream labels must be an object of strings"); + } + Map result = new HashMap<>(); + for (Map.Entry entry : labels.getAsJsonObject().entrySet()) { + JsonElement value = entry.getValue(); + if (!(value instanceof JsonPrimitive) || !value.getAsJsonPrimitive().isString()) { + throw new IllegalArgumentException("Upstream label values must be strings"); + } + result.put(entry.getKey(), value.getAsString()); + } + return Collections.unmodifiableMap(result); + } + @Override protected boolean isGray(final Upstream upstream) { return upstream.isGray(); diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/DivideCanaryRoutingTest.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/DivideCanaryRoutingTest.java new file mode 100644 index 000000000000..b5fac04fb735 --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/DivideCanaryRoutingTest.java @@ -0,0 +1,423 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.divide; + +import org.apache.shenyu.common.constant.Constants; +import org.apache.shenyu.common.dto.DiscoverySyncData; +import org.apache.shenyu.common.dto.DiscoveryUpstreamData; +import org.apache.shenyu.common.dto.RuleData; +import org.apache.shenyu.common.dto.SelectorData; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfig; +import org.apache.shenyu.common.dto.convert.rule.impl.DivideRuleHandle; +import org.apache.shenyu.common.utils.GsonUtils; +import org.apache.shenyu.loadbalancer.cache.UpstreamCacheManager; +import org.apache.shenyu.loadbalancer.entity.Upstream; +import org.apache.shenyu.plugin.api.ShenyuPluginChain; +import org.apache.shenyu.plugin.api.context.CanaryContext; +import org.apache.shenyu.plugin.api.context.ShenyuContext; +import org.apache.shenyu.plugin.api.result.DefaultShenyuResult; +import org.apache.shenyu.plugin.api.result.ShenyuResult; +import org.apache.shenyu.plugin.api.utils.SpringBeanUtils; +import org.apache.shenyu.plugin.base.circuitbreaker.UpstreamCircuitBreaker; +import org.apache.shenyu.plugin.base.utils.CacheKeyUtils; +import org.apache.shenyu.plugin.base.utils.LoadbalancerUtils; +import org.apache.shenyu.plugin.divide.canary.CanaryDecision; +import org.apache.shenyu.plugin.divide.canary.CanaryDecisionService; +import org.apache.shenyu.plugin.divide.canary.DefaultCanaryDecisionService; +import org.apache.shenyu.plugin.divide.handler.DividePluginDataHandler; +import org.apache.shenyu.plugin.divide.handler.DivideUpstreamDataHandler; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.http.HttpStatus; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.net.InetSocketAddress; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +/** + * Verifies initial routing, fallback, exchange snapshots and legacy bypass. + */ +class DivideCanaryRoutingTest { + + private RuleData rule; + + private SelectorData selector; + + private DivideRuleHandle handle; + + private CanaryConfig config; + + private CanaryDecisionService decisions; + + private DividePlugin plugin; + + @BeforeEach + void setUp() { + decisions = mock(CanaryDecisionService.class); + plugin = new DividePlugin(decisions); + rule = new RuleData(); + rule.setId("canary-routing-rule"); + rule.setSelectorId("canary-routing-selector"); + selector = new SelectorData(); + selector.setId("canary-routing-selector"); + DivideRuleHandle initial = new DivideRuleHandle(); + rule.setHandle(GsonUtils.getGson().toJson(initial)); + new DividePluginDataHandler().handlerRule(rule); + handle = DividePluginDataHandler.CACHED_HANDLE.get().obtainHandle(CacheKeyUtils.INST.getKey(rule)); + config = new CanaryConfig(); + config.setEnabled(true); + config.setCanaryLabels(new HashMap<>(Map.of("release", "canary", "region", "east"))); + config.setStableLabels(new HashMap<>(Map.of("release", "stable"))); + handle.setCanary(config); + when(decisions.decide(any(), anyString(), any())).thenReturn(CanaryDecision.CANARY); + ConfigurableApplicationContext context = mock(ConfigurableApplicationContext.class); + when(context.getBean(ShenyuResult.class)).thenReturn(new DefaultShenyuResult()); + SpringBeanUtils.getInstance().setApplicationContext(context); + } + + @Test + void testAllLabelsAndSnapshotBeforeBackendFailure() { + Upstream canary = upstream("canary:8080", Map.of("release", "canary", "region", "east", "extra", "value")); + Upstream wrongRegion = upstream("other:8080", Map.of("release", "canary", "region", "west")); + ServerWebExchange exchange = exchange(null); + ShenyuPluginChain chain = current -> { + assertEquals("canary", current.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + assertEquals("http://canary:8080", current.getAttribute(Constants.HTTP_DOMAIN)); + CanaryContext observation = current.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + assertEquals(rule.getId(), observation.getRuleId()); + assertEquals(selector.getId(), observation.getSelectorId()); + assertEquals("canary", observation.getIntendedPartition()); + assertEquals("canary", observation.getActualPartition()); + assertNull(observation.getFallbackReason()); + assertNull(observation.getRejectReason()); + assertTrue(observation.getDecisionDurationNanos() >= 0); + return Mono.error(new IllegalStateException("backend failed")); + }; + try (MockedStatic cache = cache(List.of(wrongRegion, canary))) { + StepVerifier.create(plugin.doExecute(exchange, chain, selector, rule)).expectError(IllegalStateException.class).verify(); + } + Map labels = exchange.getAttribute(Constants.SHENYU_CANARY_LABELS); + config.getCanaryLabels().put("region", "west"); + assertEquals("east", labels.get("region")); + assertThrows(UnsupportedOperationException.class, () -> labels.put("region", "south")); + assertEquals("canary", exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + } + + @Test + void testInitialFallbackSavesStable() { + ServerWebExchange exchange = exchange(null); + try (MockedStatic cache = cache(List.of(upstream("stable:8080", Map.of("release", "stable"))))) { + StepVerifier.create(plugin.doExecute(exchange, current -> Mono.empty(), selector, rule)).verifyComplete(); + } + assertEquals("stable", exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + assertEquals(Map.of("release", "stable"), exchange.getAttribute(Constants.SHENYU_CANARY_LABELS)); + CanaryContext observation = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + assertEquals("canary", observation.getIntendedPartition()); + assertEquals("stable", observation.getActualPartition()); + assertEquals(CanaryContext.CANARY_POOL_EMPTY, observation.getFallbackReason()); + assertNull(observation.getRejectReason()); + } + + @Test + void testRejectAndEmptyStableNeverPublishPartition() { + config.setFallbackPolicy("REJECT"); + assertRejected(List.of(upstream("stable:8080", Map.of("release", "stable")))); + config.setFallbackPolicy("STABLE"); + assertRejected(List.of(upstream("unlabelled:8080", Map.of()))); + when(decisions.decide(any(), anyString(), any())).thenReturn(CanaryDecision.STABLE); + assertRejected(List.of(upstream("canary:8080", Map.of("release", "canary", "region", "east")))); + } + + @Test + void testMissingLabelsDoNotSelectWholePool() { + config.setCanaryLabels(null); + config.setStableLabels(Map.of()); + assertRejected(List.of(upstream("unlabelled:8080", Map.of()))); + } + + @Test + void testNoHealthyUpstreamDoesNotDecide() { + assertRejected(List.of()); + verifyNoInteractions(decisions); + } + + @Test + void testPoolIsSavedBeforeLoadBalancerAndNullNodeKeepsLegacyError() { + ServerWebExchange exchange = exchange(null); + ShenyuPluginChain chain = mock(ShenyuPluginChain.class); + try (MockedStatic balance = mockStatic(LoadbalancerUtils.class); + MockedStatic cache = cache(List.of(upstream("canary:8080", Map.of("release", "canary", "region", "east"))))) { + balance.when(() -> LoadbalancerUtils.getForExchange(anyList(), anyString(), any())).thenAnswer(invocation -> { + assertEquals("canary", exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + return null; + }); + StepVerifier.create(plugin.doExecute(exchange, chain, selector, rule)).verifyComplete(); + } + verifyNoInteractions(chain); + assertNull(exchange.getResponse().getStatusCode()); + CanaryContext observation = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + assertNull(observation.getActualPartition()); + assertNull(observation.getFallbackReason()); + assertEquals(CanaryContext.NO_UPSTREAM_SELECTED, observation.getRejectReason()); + } + + @Test + void testAbsentConfigurationKeepsLegacyGrayPool() { + Upstream gray = upstream("gray:8080", Map.of()); + gray.setGray(true); + try (MockedStatic cache = cache(List.of(gray, upstream("normal:8080", Map.of())))) { + handle.setCanary(null); + assertLegacy(exchange(null), "http://gray:8080"); + } + verifyNoInteractions(decisions); + } + + @Test + void testNewAndLegacyRulesShareSelectorWithoutLosingStableNodes() { + DiscoverySyncData data = new DiscoverySyncData(); + data.setSelectorId(selector.getId()); + data.setUpstreamDataList(List.of( + DiscoveryUpstreamData.builder().url("gray:8080").protocol("http://").status(0) + .props("{\"gray\":true,\"healthCheckEnabled\":false,\"labels\":{\"release\":\"canary\",\"region\":\"east\"}}").build(), + DiscoveryUpstreamData.builder().url("stable:8080").protocol("http://").status(0) + .props("{\"healthCheckEnabled\":false,\"labels\":{\"release\":\"stable\"}}").build())); + new DivideUpstreamDataHandler().handlerDiscoveryUpstreamData(data); + try { + plugin = new DividePlugin(new DefaultCanaryDecisionService()); + config.setEnabled(false); + ServerWebExchange stableRequest = exchange(null); + StepVerifier.create(plugin.doExecute(stableRequest, current -> Mono.empty(), selector, rule)).verifyComplete(); + assertEquals("http://stable:8080", stableRequest.getAttribute(Constants.HTTP_DOMAIN)); + assertEquals("stable", stableRequest.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + CanaryContext observation = stableRequest.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + assertEquals("stable", observation.getIntendedPartition()); + assertEquals("stable", observation.getActualPartition()); + config.setEnabled(true); + config.setPercentage(100); + ServerWebExchange canaryRequest = exchange(null); + StepVerifier.create(plugin.doExecute(canaryRequest, current -> Mono.empty(), selector, rule)).verifyComplete(); + assertEquals("http://gray:8080", canaryRequest.getAttribute(Constants.HTTP_DOMAIN)); + assertEquals("canary", canaryRequest.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + handle.setCanary(null); + assertLegacy(exchange(null), "http://gray:8080"); + assertEquals(2, UpstreamCacheManager.getInstance().findUpstreamListBySelectorId(selector.getId()).size()); + } finally { + UpstreamCacheManager.getInstance().removeByKey(selector.getId()); + } + } + + @Test + void testSpecifyDomainAndLegacyDoNotEscapeUnavailableGrayPool() { + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findUpstreamListBySelectorId(selector.getId())).thenReturn(List.of(upstream("stable:8080", Map.of("release", "stable")))); + when(manager.findLegacyUpstreamListBySelectorId(selector.getId())).thenReturn(List.of()); + ShenyuPluginChain chain = mock(ShenyuPluginChain.class); + try (MockedStatic cache = mockStatic(UpstreamCacheManager.class)) { + cache.when(UpstreamCacheManager::getInstance).thenReturn(manager); + ServerWebExchange specified = exchange("override:9090"); + StepVerifier.create(plugin.doExecute(specified, chain, selector, rule)).verifyComplete(); + assertNull(specified.getAttribute(Constants.HTTP_DOMAIN)); + assertNull(specified.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + handle.setCanary(null); + ServerWebExchange legacy = exchange(null); + StepVerifier.create(plugin.doExecute(legacy, chain, selector, rule)).verifyComplete(); + assertNull(legacy.getAttribute(Constants.HTTP_DOMAIN)); + } + verifyNoInteractions(chain, decisions); + } + + @Test + void testSpecifyDomainBypassesCanaryWithoutMutatingSharedNode() { + Upstream shared = upstream("original:8080", Map.of()); + Upstream specified = upstream("override:9090", Map.of()); + try (MockedStatic cache = cache(List.of(shared, specified))) { + assertLegacy(exchange("override:9090"), "http://override:9090"); + assertEquals("original:8080", shared.getUrl()); + } + try (MockedStatic cache = cache(List.of(shared))) { + handle.setCanary(null); + assertLegacy(exchange("unconfigured:9090"), "http://original:8080"); + assertLegacy(exchange(null), "http://original:8080"); + } + verifyNoInteractions(decisions); + } + + @Test + void testSpecifyDomainStillRequiresAvailableNode() { + ShenyuPluginChain chain = mock(ShenyuPluginChain.class); + try (MockedStatic cache = cache(List.of())) { + StepVerifier.create(plugin.doExecute(exchange("override:9090"), chain, selector, rule)).verifyComplete(); + } + verifyNoInteractions(chain, decisions); + } + + @Test + void testCircuitBreakerDoesNotSelectOutsideCanaryPartition() { + Upstream blocked = upstream("blocked-canary:8080", Map.of("release", "canary", "region", "east")); + Upstream available = upstream("available-canary:8080", Map.of("release", "canary", "region", "east")); + Upstream stable = upstream("stable:8080", Map.of("release", "stable")); + UpstreamCircuitBreaker breaker = UpstreamCircuitBreaker.getInstance(); + String key = UpstreamCircuitBreaker.buildKey(selector.getId(), blocked); + for (int i = 0; i < UpstreamCircuitBreaker.DEFAULT_FAILURE_THRESHOLD; i++) { + breaker.recordFailure(key); + } + try (MockedStatic cache = cache(List.of(blocked, available, stable))) { + ServerWebExchange exchange = exchange(null); + StepVerifier.create(plugin.doExecute(exchange, current -> Mono.empty(), selector, rule)).verifyComplete(); + assertEquals("http://available-canary:8080", exchange.getAttribute(Constants.HTTP_DOMAIN)); + assertEquals("canary", exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + } + try (MockedStatic cache = cache(List.of(blocked, stable))) { + ServerWebExchange exchange = exchange(null); + ShenyuPluginChain chain = mock(ShenyuPluginChain.class); + StepVerifier.create(plugin.doExecute(exchange, chain, selector, rule)).verifyComplete(); + verifyNoInteractions(chain); + CanaryContext observation = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + assertEquals("canary", observation.getMetricPartition()); + assertNull(observation.getFallbackReason()); + assertEquals(CanaryContext.NO_UPSTREAM_SELECTED, observation.getRejectReason()); + } finally { + breaker.reset(key); + } + } + + @Test + void testObservationCallbackCannotBreakRouting() { + ServerWebExchange exchange = exchange(null); + AtomicInteger callbacks = new AtomicInteger(); + exchange.getAttributes().put(Constants.METRICS_CANARY, (Consumer) observation -> { + callbacks.incrementAndGet(); + throw new IllegalStateException("metrics failed"); + }); + try (MockedStatic cache = cache(List.of(upstream("stable:8080", Map.of("release", "stable"))))) { + StepVerifier.create(plugin.doExecute(exchange, current -> Mono.empty(), selector, rule)).verifyComplete(); + } + assertEquals(1, callbacks.get()); + assertEquals("http://stable:8080", exchange.getAttribute(Constants.HTTP_DOMAIN)); + CanaryContext observation = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + assertEquals(CanaryContext.CANARY_POOL_EMPTY, observation.getFallbackReason()); + } + + @Test + void testFailedStableFallbackKeepsOriginalCanaryIntent() { + ServerWebExchange exchange = exchange(null); + try (MockedStatic cache = cache(List.of(upstream("unlabelled:8080", Map.of())))) { + StepVerifier.create(plugin.doExecute(exchange, current -> Mono.empty(), selector, rule)).verifyComplete(); + } + CanaryContext observation = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + assertEquals("canary", observation.getMetricPartition()); + assertNull(observation.getActualPartition()); + assertNull(observation.getFallbackReason()); + assertEquals(CanaryContext.STABLE_POOL_EMPTY, observation.getRejectReason()); + } + + @Test + void testLoadBalancerExceptionStillPublishesDecision() { + ServerWebExchange exchange = exchange(null); + try (MockedStatic balance = mockStatic(LoadbalancerUtils.class); + MockedStatic cache = cache(List.of(upstream("stable:8080", Map.of("release", "stable"))))) { + balance.when(() -> LoadbalancerUtils.getForExchange(anyList(), anyString(), any())).thenThrow(new IllegalStateException("balance failed")); + assertThrows(IllegalStateException.class, () -> plugin.doExecute(exchange, current -> Mono.empty(), selector, rule)); + } + CanaryContext observation = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + assertNotNull(observation); + assertNull(observation.getFallbackReason()); + assertNull(observation.getRejectReason()); + assertEquals("canary", observation.getIntendedPartition()); + } + + private void assertRejected(final List upstreams) { + ServerWebExchange exchange = exchange(null); + ShenyuPluginChain chain = mock(ShenyuPluginChain.class); + try (MockedStatic cache = cache(upstreams)) { + StepVerifier.create(plugin.doExecute(exchange, chain, selector, rule)).verifyComplete(); + } + verifyNoInteractions(chain); + assertNull(exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + assertNull(exchange.getAttribute(Constants.SHENYU_CANARY_LABELS)); + CanaryContext observation = exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT); + if (!upstreams.isEmpty()) { + assertEquals(HttpStatus.SERVICE_UNAVAILABLE, exchange.getResponse().getStatusCode()); + assertNotNull(observation); + assertNull(observation.getActualPartition()); + assertNull(observation.getFallbackReason()); + assertNotNull(observation.getRejectReason()); + } else { + assertNull(observation); + } + } + + private void assertLegacy(final ServerWebExchange exchange, final String domain) { + StepVerifier.create(plugin.doExecute(exchange, current -> Mono.empty(), selector, rule)).verifyComplete(); + assertEquals(domain, exchange.getAttribute(Constants.HTTP_DOMAIN)); + assertNull(exchange.getAttribute(Constants.SHENYU_CANARY_CONTEXT)); + assertNull(exchange.getAttribute(Constants.SHENYU_CANARY_PARTITION)); + assertNull(exchange.getAttribute(Constants.SHENYU_CANARY_LABELS)); + } + + private MockedStatic cache(final List upstreams) { + UpstreamCacheManager manager = mock(UpstreamCacheManager.class); + when(manager.findUpstreamListBySelectorId(selector.getId())).thenReturn(upstreams); + List gray = upstreams.stream().filter(Upstream::isGray).toList(); + when(manager.findLegacyUpstreamListBySelectorId(selector.getId())).thenReturn(gray.isEmpty() ? upstreams : gray); + MockedStatic cache = mockStatic(UpstreamCacheManager.class); + cache.when(UpstreamCacheManager::getInstance).thenReturn(manager); + return cache; + } + + private ServerWebExchange exchange(final String domain) { + MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest.get("/test") + .remoteAddress(new InetSocketAddress("127.0.0.1", 12345)); + if (Objects.nonNull(domain)) { + request.header(Constants.SPECIFY_DOMAIN, domain); + } + ServerWebExchange exchange = MockServerWebExchange.from(request); + exchange.getAttributes().put(Constants.CONTEXT, new ShenyuContext()); + return exchange; + } + + private Upstream upstream(final String url, final Map labels) { + Upstream upstream = Upstream.builder().url(url).protocol("http://").weight(100).build(); + upstream.setLabels(labels); + return upstream; + } +} diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/DividePluginCircuitBreakerTest.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/DividePluginCircuitBreakerTest.java index 69e67575e816..866b6bbd977c 100644 --- a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/DividePluginCircuitBreakerTest.java +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/DividePluginCircuitBreakerTest.java @@ -102,6 +102,7 @@ public void setup() { when(applicationContext.getBean(ShenyuResult.class)).thenReturn(new DefaultShenyuResult()); UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); when(cacheManager.findUpstreamListBySelectorId(SELECTOR_ID)).thenReturn(Collections.singletonList(upstream)); + when(cacheManager.findLegacyUpstreamListBySelectorId(SELECTOR_ID)).thenReturn(Collections.singletonList(upstream)); this.cacheMock = mockStatic(UpstreamCacheManager.class); cacheMock.when(UpstreamCacheManager::getInstance).thenReturn(cacheManager); } @@ -164,6 +165,7 @@ public void testHealthyUpstreamsBypassBreaker() { UpstreamCacheManager cacheManager = mock(UpstreamCacheManager.class); List upstreams = List.of(upstream, healthy); when(cacheManager.findUpstreamListBySelectorId(SELECTOR_ID)).thenReturn(upstreams); + when(cacheManager.findLegacyUpstreamListBySelectorId(SELECTOR_ID)).thenReturn(upstreams); cacheMock.when(UpstreamCacheManager::getInstance).thenReturn(cacheManager); // the blocked upstream is filtered out, the request still goes through the healthy one when(chain.execute(exchange)).thenReturn(Mono.empty()); diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/canary/DefaultCanaryDecisionServiceTest.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/canary/DefaultCanaryDecisionServiceTest.java new file mode 100644 index 000000000000..0000aa4a89bb --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/canary/DefaultCanaryDecisionServiceTest.java @@ -0,0 +1,248 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.divide.canary; + +import org.apache.shenyu.common.constant.Constants; +import org.apache.shenyu.common.dto.ConditionData; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfig; +import org.apache.shenyu.common.dto.convert.rule.canary.StickyKeyConfig; +import org.apache.shenyu.plugin.api.context.ShenyuContext; +import org.apache.shenyu.plugin.base.utils.HostAddressUtils; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.springframework.http.HttpCookie; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mockStatic; + +/** + * Tests the partition contract, including portable hash vectors and rollout stability. + */ +class DefaultCanaryDecisionServiceTest { + + private final DefaultCanaryDecisionService service = new DefaultCanaryDecisionService(); + + @Test + void testPortableHashVectors() { + // Independently calculated SHA-256 vectors, including unsigned values and UTF-8 byte lengths. + assertEquals(3033, service.hashToBucket("rule-1", "user-1")); + assertEquals(9925, service.hashToBucket("规则甲", "用户乙")); + assertEquals(912, service.hashToBucket("ab", "c")); + assertEquals(6726, service.hashToBucket("a", "bc")); + } + + @Test + void testDisabledConfigurationAlwaysSelectsStable() { + CanaryConfig config = config(100, "unknown"); + config.setEnabled(false); + config.setConditions(List.of(condition("user-1"))); + assertEquals(CanaryDecision.STABLE, service.decide(exchange("user-1"), "rule-1", config)); + config.setStickyKey(null); + assertEquals(CanaryDecision.STABLE, service.decide(exchange("user-1"), "rule-1", config)); + } + + @Test + void testPercentageBoundariesAndEmptyOrConditions() { + CanaryConfig config = config(0, "header"); + config.setStickyKey(null); + ServerWebExchange exchange = exchange("user-1"); + assertEquals(CanaryDecision.STABLE, service.decide(exchange, "rule-1", config)); + config.setPercentage(100); + config.setMatchMode(1); + config.setConditions(List.of()); + assertEquals(CanaryDecision.CANARY, service.decide(exchange, "rule-1", config)); + } + + @Test + void testConditionsAreEligibilityBeforePercentage() { + CanaryConfig config = config(100, "header"); + config.setConditions(List.of(condition("user-1"), condition("another-user"))); + ServerWebExchange exchange = exchange("user-1"); + assertEquals(CanaryDecision.STABLE, service.decide(exchange, "rule-1", config)); + config.setMatchMode(1); + assertEquals(CanaryDecision.CANARY, service.decide(exchange, "rule-1", config)); + config.setMatchMode(null); + assertEquals(CanaryDecision.STABLE, service.decide(exchange, "rule-1", config)); + config.setConditions(List.of(condition("user-1"))); + config.setPercentage(30); + assertEquals(CanaryDecision.STABLE, service.decide(exchange, "rule-1", config)); + config.setPercentage(31); + assertEquals(CanaryDecision.CANARY, service.decide(exchange, "rule-1", config)); + } + + @Test + void testStickySourcesAndMissingIdentifiers() { + ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/?user=user-1") + .header("user", "user-1").cookie(new HttpCookie("user", "user-1"))); + for (String type : List.of("header", "cookie", "query")) { + assertEquals(CanaryDecision.CANARY, service.decide(exchange, "rule-1", config(31, type))); + assertEquals(CanaryDecision.STABLE, service.decide(MockServerWebExchange.from(MockServerHttpRequest.get("/")), "rule-1", config(31, type))); + } + try (MockedStatic addresses = mockStatic(HostAddressUtils.class)) { + addresses.when(() -> HostAddressUtils.acquireIp(exchange)).thenReturn("user-1"); + CanaryConfig config = config(31, "ip"); + config.getStickyKey().setParamName(null); + assertEquals(CanaryDecision.CANARY, service.decide(exchange, "rule-1", config)); + } + assertEquals(CanaryDecision.STABLE, service.decide(exchange(" "), "rule-1", config(99, "header"))); + } + + @Test + void testAdditionalBuiltinSourcesWithoutParameterName() { + ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("http://example.com/orders?ignored=value")); + // Thresholds come from independently calculated buckets: /orders=9824, GET=4747, example.com=391. + assertSourceThreshold(exchange, "uri", null, 99); + assertSourceThreshold(exchange, "req_method", null, 48); + assertSourceThreshold(exchange, "domain", null, 4); + try (MockedStatic addresses = mockStatic(HostAddressUtils.class)) { + addresses.when(() -> HostAddressUtils.acquireHost(exchange)).thenReturn("client.example.com"); + assertSourceThreshold(exchange, "host", null, 55); + } + } + + @Test + void testPostSourceReusesShenyuContextFields() { + ServerWebExchange exchange = exchange("ignored"); + ShenyuContext context = new ShenyuContext(); + context.setModule("user-1"); + exchange.getAttributes().put(Constants.CONTEXT, context); + assertSourceThreshold(exchange, "post", "module", 31); + } + + @Test + void testCustomSourceLoadedFromSpiResource() { + ServerWebExchange exchange = exchange("ignored"); + exchange.getAttributes().put("authenticatedUserId", "user-1"); + exchange.getAttributes().put("tenantUser", "user-2"); + // Neither the source name nor its optional parameter name is built into the decision service. + assertSourceThreshold(exchange, "test_attribute", null, 31); + assertSourceThreshold(exchange, "test_attribute", "tenantUser", 77); + } + + @Test + void testCustomSourceMissingValueAndExtractionFailure() { + ServerWebExchange exchange = exchange("ignored"); + CanaryConfig config = config(99, "test_attribute"); + config.getStickyKey().setParamName(null); + assertEquals(CanaryDecision.STABLE, service.decide(exchange, "rule-1", config)); + exchange.getAttributes().put("authenticatedUserId", " "); + assertEquals(CanaryDecision.STABLE, service.decide(exchange, "rule-1", config)); + exchange.getAttributes().put("authenticatedUserId", new Object()); + assertThrows(ClassCastException.class, () -> service.decide(exchange, "rule-1", config)); + } + + @Test + void testRequiredSourceType() { + ServerWebExchange exchange = exchange("user-1"); + CanaryConfig missingType = config(50, null); + assertThrows(IllegalArgumentException.class, () -> service.decide(exchange, "rule-1", missingType)); + for (String type : List.of("", " ")) { + assertThrows(IllegalArgumentException.class, () -> service.decide(exchange, "rule-1", config(50, type))); + } + } + + @Test + void testBlankParameterNamesUseBuiltinExtraction() { + ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/?user=user-1") + .header("user", "user-1").cookie(new HttpCookie("user", "user-1"))); + ShenyuContext context = new ShenyuContext(); + context.setModule("user-1"); + exchange.getAttributes().put(Constants.CONTEXT, context); + for (String type : List.of("header", "cookie", "query", "post")) { + CanaryConfig missingName = config(50, type); + for (String paramName : new String[] {null, "", " "}) { + missingName.getStickyKey().setParamName(paramName); + assertEquals(CanaryDecision.STABLE, service.decide(exchange, "rule-1", missingName)); + } + } + } + + @Test + void testRolloutExpansionPreservesUsersAcrossInstances() { + CanaryConfig tenPercent = config(10, "header"); + CanaryConfig twentyPercent = config(20, "header"); + DefaultCanaryDecisionService otherInstance = new DefaultCanaryDecisionService(); + int originalCanaries = 0; + int newCanaries = 0; + for (int i = 0; i < 1000; i++) { + ServerWebExchange exchange = exchange("user-" + i); + CanaryDecision before = service.decide(exchange, "rule-1", tenPercent); + assertEquals(before, otherInstance.decide(exchange, "rule-1", tenPercent)); + CanaryDecision after = otherInstance.decide(exchange, "rule-1", twentyPercent); + if (before == CanaryDecision.CANARY) { + originalCanaries++; + assertEquals(CanaryDecision.CANARY, after); + } + if (after == CanaryDecision.CANARY) { + newCanaries++; + } + } + assertTrue(originalCanaries > 0); + assertTrue(newCanaries > originalCanaries); + } + + @Test + void testInvalidConfigurationIsNotARequestKeyMiss() { + ServerWebExchange exchange = exchange("user-1"); + assertThrows(IllegalArgumentException.class, () -> service.decide(exchange, "rule-1", config(-1, "header"))); + assertThrows(IllegalArgumentException.class, () -> service.decide(exchange, "rule-1", config(101, "header"))); + assertThrows(IllegalArgumentException.class, () -> service.decide(exchange, "rule-1", config(50, "unknown"))); + CanaryConfig missing = config(50, "header"); + missing.setStickyKey(null); + assertThrows(IllegalArgumentException.class, () -> service.decide(exchange, "rule-1", missing)); + } + + private void assertSourceThreshold(final ServerWebExchange exchange, final String type, final String paramName, final int percentage) { + CanaryConfig config = config(percentage, type); + config.getStickyKey().setParamName(paramName); + assertEquals(CanaryDecision.CANARY, service.decide(exchange, "rule-1", config)); + config.setPercentage(percentage - 1); + assertEquals(CanaryDecision.STABLE, service.decide(exchange, "rule-1", config)); + } + + private CanaryConfig config(final int percentage, final String type) { + CanaryConfig config = new CanaryConfig(); + config.setEnabled(true); + config.setPercentage(percentage); + StickyKeyConfig stickyKey = new StickyKeyConfig(); + stickyKey.setParamType(type); + stickyKey.setParamName("user"); + config.setStickyKey(stickyKey); + return config; + } + + private ServerWebExchange exchange(final String user) { + return MockServerWebExchange.from(MockServerHttpRequest.get("/").header("user", user)); + } + + private ConditionData condition(final String value) { + ConditionData condition = new ConditionData(); + condition.setParamType("header"); + condition.setParamName("user"); + condition.setOperator("equals"); + condition.setParamValue(value); + return condition; + } +} diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/canary/fixture/AttributeParameterData.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/canary/fixture/AttributeParameterData.java new file mode 100644 index 000000000000..f8ef8be37030 --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/canary/fixture/AttributeParameterData.java @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.divide.canary.fixture; + +import org.apache.commons.lang3.StringUtils; +import org.apache.shenyu.plugin.base.condition.data.ParameterData; +import org.apache.shenyu.spi.Join; +import org.springframework.web.server.ServerWebExchange; + +/** + * Test-only extension that reads a string attribute and defines its own default parameter name. + */ +@Join +public class AttributeParameterData implements ParameterData { + + @Override + public String builder(final String paramName, final ServerWebExchange exchange) { + return exchange.getAttribute(StringUtils.defaultIfBlank(paramName, "authenticatedUserId")); + } +} diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DividePluginDataHandlerTest.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DividePluginDataHandlerTest.java index 70e496b9f493..fa7f998d2d5a 100644 --- a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DividePluginDataHandlerTest.java +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DividePluginDataHandlerTest.java @@ -18,6 +18,8 @@ package org.apache.shenyu.plugin.divide.handler; import org.apache.shenyu.common.dto.RuleData; +import org.apache.shenyu.common.dto.convert.rule.canary.CanaryConfig; +import org.apache.shenyu.common.dto.convert.rule.impl.DivideRuleHandle; import org.apache.shenyu.common.dto.SelectorData; import org.apache.shenyu.common.dto.convert.selector.DivideUpstream; import org.apache.shenyu.common.enums.PluginEnum; @@ -25,9 +27,12 @@ import org.apache.shenyu.common.utils.UpstreamCheckUtils; import org.apache.shenyu.loadbalancer.cache.UpstreamCacheManager; import org.apache.shenyu.loadbalancer.entity.Upstream; +import org.apache.shenyu.plugin.base.utils.CacheKeyUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.MockedStatic; @@ -36,11 +41,14 @@ import org.mockito.quality.Strictness; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; @@ -111,4 +119,61 @@ public void pluginNamedTest() { public void removeRuleTest() { dividePluginDataHandler.removeRule(ruleData); } + + @ParameterizedTest + @ValueSource(strings = {"\"percentage\":101", "\"percentage\":20.5", "\"enabled\":true,\"percentage\":20", + "\"fallbackPolicy\":\"TYPO\"", "\"stickyKey\":{\"paramType\":\"missing-extension\"}"}) + public void testInvalidConfigurationDoesNotReplaceCachedRule(final String fields) { + RuleData rule = new RuleData(); + rule.setId("invalid-config-rule"); + rule.setSelectorId("invalid-config-selector"); + rule.setHandle("{\"timeout\":5000}"); + dividePluginDataHandler.handlerRule(rule); + DivideRuleHandle previous = DividePluginDataHandler.CACHED_HANDLE.get().obtainHandle(CacheKeyUtils.INST.getKey(rule)); + rule.setHandle("{\"canary\":{\"stableLabels\":{\"release\":\"stable\"},\"canaryLabels\":{\"release\":\"canary\"}," + fields + "}}"); + try { + assertThrows(IllegalArgumentException.class, () -> dividePluginDataHandler.handlerRule(rule)); + assertSame(previous, DividePluginDataHandler.CACHED_HANDLE.get().obtainHandle(CacheKeyUtils.INST.getKey(rule))); + } finally { + dividePluginDataHandler.removeRule(rule); + } + } + + @Test + public void testInstalledCustomParameterSourceCanBeCached() { + RuleData rule = new RuleData(); + rule.setId("custom-source-rule"); + rule.setSelectorId("custom-source-selector"); + rule.setHandle("{\"canary\":{\"enabled\":true,\"percentage\":20,\"stickyKey\":{\"paramType\":\"test_attribute\"}," + + "\"stableLabels\":{\"release\":\"stable\"},\"canaryLabels\":{\"release\":\"canary\"}}}"); + try { + dividePluginDataHandler.handlerRule(rule); + DivideRuleHandle cached = DividePluginDataHandler.CACHED_HANDLE.get().obtainHandle(CacheKeyUtils.INST.getKey(rule)); + assertEquals("test_attribute", cached.getCanary().getStickyKey().getParamType()); + } finally { + dividePluginDataHandler.removeRule(rule); + } + } + + @Test + public void testRejectOverlappingLabelsBeforeReplacingCachedRule() { + RuleData rule = new RuleData(); + rule.setId("label-validation-rule"); + rule.setSelectorId("label-validation-selector"); + CanaryConfig config = new CanaryConfig(); + config.setEnabled(false); + config.setCanaryLabels(Map.of("release", "canary")); + config.setStableLabels(Map.of("release", "stable")); + DivideRuleHandle handle = new DivideRuleHandle(); + handle.setCanary(config); + rule.setHandle(GsonUtils.getGson().toJson(handle)); + dividePluginDataHandler.handlerRule(rule); + final DivideRuleHandle previous = DividePluginDataHandler.CACHED_HANDLE.get().obtainHandle(CacheKeyUtils.INST.getKey(rule)); + config.setStableLabels(Map.of("region", "east")); + rule.setHandle(GsonUtils.getGson().toJson(handle)); + assertThrows(IllegalArgumentException.class, () -> dividePluginDataHandler.handlerRule(rule)); + assertSame(previous, DividePluginDataHandler.CACHED_HANDLE.get().obtainHandle(CacheKeyUtils.INST.getKey(rule))); + dividePluginDataHandler.removeRule(rule); + } + } diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandlerTest.java b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandlerTest.java index a4e39078a0a7..f0048753d9c3 100644 --- a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandlerTest.java +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandlerTest.java @@ -24,23 +24,32 @@ import org.apache.shenyu.common.utils.GsonUtils; import org.apache.shenyu.common.utils.UpstreamCheckUtils; import org.apache.shenyu.loadbalancer.cache.UpstreamCacheManager; +import org.apache.shenyu.loadbalancer.cache.UpstreamCheckTask; import org.apache.shenyu.loadbalancer.entity.Upstream; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; +import org.springframework.test.util.ReflectionTestUtils; import java.sql.Timestamp; import java.util.List; +import java.util.Map; import java.util.Properties; import java.util.stream.Collectors; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.anyInt; @@ -81,6 +90,7 @@ public void setUp() { public void tearDown() { UpstreamCacheManager.getInstance().removeByKey("handler"); mockCheckUtils.close(); + UpstreamCacheManager.getInstance().removeByKey("handler"); } /** @@ -143,4 +153,119 @@ public void handlerDiscoveryUpstreamDataWithPropsMetadataTest() { public void pluginNamedTest() { assertEquals(divideUpstreamDataHandler.pluginName(), PluginEnum.DIVIDE.getName()); } + + @ParameterizedTest + @ValueSource(strings = { + "{\"warmup\":\"12\",\"gray\":\"true\",\"healthCheckEnabled\":\"false\",\"labels\":{\"release\":\"canary\"}}", + "{\"warmup\":12,\"gray\":true,\"healthCheckEnabled\":false,\"labels\":{\"release\":\"canary\"}}" + }) + public void testScalarCompatibilityAndSeparateLegacyGrayView(final String props) { + publish(List.of(instance("canary:8080", props), instance("stable:8080", "{\"labels\":{\"release\":\"stable\"}}"))); + List upstreams = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler"); + assertEquals(2, upstreams.size()); + List legacy = UpstreamCacheManager.getInstance().findLegacyUpstreamListBySelectorId("handler"); + assertEquals(1, legacy.size()); + Upstream canary = legacy.get(0); + assertEquals(12, canary.getWarmup()); + assertTrue(canary.isGray()); + assertFalse(canary.isHealthCheckEnabled()); + assertEquals(Map.of("release", "canary"), canary.getLabels()); + } + + @Test + public void testRoutingLabelsDoNotOverwriteFlatMetadata() { + publish(List.of(instance("backend:8080", "{\"release\":\"metadata-value\",\"labels\":{\"release\":\"canary\"},\"custom\":{\"nested\":true}}"))); + Upstream original = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler").get(0); + assertEquals("metadata-value", original.getMetadata().get("release")); + assertEquals(Map.of("release", "canary"), original.getLabels()); + publish(List.of(instance("backend:8080", "{\"release\":\"new-metadata\"}"))); + Upstream updated = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler").get(0); + assertSame(original, updated); + assertTrue(updated.getLabels().isEmpty()); + assertEquals("new-metadata", updated.getMetadata().get("release")); + } + + @Test + public void testLabelUpdatesAndDeletionRetainStatistics() { + publish(List.of(instance("backend:8080", "{\"labels\":{\"release\":\"canary\",\"region\":\"shanghai\"}}"))); + Upstream original = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler").get(0); + original.getSucceeded().set(17); + original.setLag(42); + publish(List.of(instance("backend:8080", "{\"labels\":{\"release\":\"stable\"}}"))); + Upstream updated = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler").get(0); + assertSame(original, updated); + assertEquals(17, updated.getSucceeded().get()); + assertEquals(42, updated.getLag()); + assertFalse(updated.isGray()); + assertEquals(Map.of("release", "stable"), updated.getLabels()); + publish(List.of(instance("backend:8080", "{\"labels\":{}}"))); + assertTrue(original.getLabels().isEmpty()); + publish(List.of(instance("backend:8080", "{\"labels\":{\"release\":\"canary\"}}"))); + publish(List.of(instance("backend:8080", "{}"))); + assertTrue(original.getLabels().isEmpty()); + } + + @ParameterizedTest + @ValueSource(strings = {"[]", "{\"labels\":[]}", "{\"labels\":null}", "{\"labels\":{\"release\":1}}", + "{\"labels\":{\"release\":true}}", "{\"labels\":{\"release\":null}}", "{\"labels\":{\"release\":{}}}", + "{\"gray\":1}", "{\"gray\":\"invalid\"}", "{\"healthCheckEnabled\":null}", "{\"warmup\":1.5}"}) + public void testInvalidUpdateDoesNotPublishPartialState(final String invalidProps) { + publish(List.of(instance("backend:8080", "{\"gray\":true,\"labels\":{\"release\":\"canary\"}}"))); + final List before = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler"); + assertThrows(RuntimeException.class, () -> publish(List.of(instance("backend:8080", "{}"), instance("bad:8080", invalidProps)))); + List after = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler"); + assertEquals(before, after); + assertSame(before.get(0), after.get(0)); + assertEquals(1, before.size()); + assertTrue(before.get(0).isGray()); + assertEquals(Map.of("release", "canary"), before.get(0).getLabels()); + } + + @Test + public void testMissingPropsKeepLegacyDefaults() { + publish(List.of(instance("backend:8080", null))); + Upstream upstream = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler").get(0); + assertEquals(10, upstream.getWarmup()); + assertFalse(upstream.isGray()); + assertTrue(upstream.isHealthCheckEnabled()); + assertTrue(upstream.getLabels().isEmpty()); + } + + @Test + public void testLabelsDoNotFilterInstancesWithoutLegacyGray() { + publish(List.of(instance("canary:8080", "{\"labels\":{\"release\":\"canary\"}}"), + instance("stable:8080", "{\"labels\":{\"release\":\"stable\"}}"))); + List upstreams = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler"); + assertEquals(2, upstreams.size()); + assertEquals(Map.of("release", "canary"), upstreams.get(0).getLabels()); + assertEquals(Map.of("release", "stable"), upstreams.get(1).getLabels()); + } + + @Test + public void testLabelDeletionAlsoUpdatesUnhealthyInstance() { + publish(List.of(instance("backend:8080", "{\"labels\":{\"release\":\"canary\"}}"))); + final Upstream original = UpstreamCacheManager.getInstance().findUpstreamListBySelectorId("handler").get(0); + final UpstreamCheckTask task = (UpstreamCheckTask) ReflectionTestUtils.getField(UpstreamCacheManager.getInstance(), "task"); + original.setHealthy(false); + original.setLastUnhealthyTimestamp(123); + original.getSucceeded().set(17); + task.putToMap(task.getUnhealthyUpstream(), "handler", original); + task.removeFromMap(task.getHealthyUpstream(), "handler", original); + publish(List.of(instance("backend:8080", "{}"))); + assertSame(original, task.getUnhealthyUpstream().get("handler").get(0)); + assertTrue(original.getLabels().isEmpty()); + assertFalse(original.isHealthy()); + assertEquals(123, original.getLastUnhealthyTimestamp()); + assertEquals(17, original.getSucceeded().get()); + } + + private DiscoveryUpstreamData instance(final String url, final String props) { + return DiscoveryUpstreamData.builder().protocol("http://").url(url).status(0).props(props).build(); + } + + private void publish(final List upstreams) { + when(discoverySyncData.getUpstreamDataList()).thenReturn(upstreams); + divideUpstreamDataHandler.handlerDiscoveryUpstreamData(discoverySyncData); + } + } diff --git a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/resources/META-INF/shenyu/org.apache.shenyu.plugin.base.condition.data.ParameterData b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/resources/META-INF/shenyu/org.apache.shenyu.plugin.base.condition.data.ParameterData new file mode 100644 index 000000000000..82e8a708a0eb --- /dev/null +++ b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/resources/META-INF/shenyu/org.apache.shenyu.plugin.base.condition.data.ParameterData @@ -0,0 +1,18 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +test_attribute=org.apache.shenyu.plugin.divide.canary.fixture.AttributeParameterData diff --git a/shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-divide/src/main/java/org/apache/shenyu/springboot/plugin/divide/DividePluginConfiguration.java b/shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-divide/src/main/java/org/apache/shenyu/springboot/plugin/divide/DividePluginConfiguration.java index ba75f15eccba..7e43448bfd94 100644 --- a/shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-divide/src/main/java/org/apache/shenyu/springboot/plugin/divide/DividePluginConfiguration.java +++ b/shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-divide/src/main/java/org/apache/shenyu/springboot/plugin/divide/DividePluginConfiguration.java @@ -22,10 +22,13 @@ import org.apache.shenyu.plugin.base.handler.MetaDataHandler; import org.apache.shenyu.plugin.base.handler.PluginDataHandler; import org.apache.shenyu.plugin.divide.DividePlugin; +import org.apache.shenyu.plugin.divide.canary.CanaryDecisionService; +import org.apache.shenyu.plugin.divide.canary.DefaultCanaryDecisionService; import org.apache.shenyu.plugin.divide.context.DivideShenyuContextDecorator; import org.apache.shenyu.plugin.divide.handler.DividePluginDataHandler; import org.apache.shenyu.plugin.divide.handler.DivideMetaDataHandler; import org.apache.shenyu.plugin.divide.handler.DivideUpstreamDataHandler; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -40,11 +43,23 @@ public class DividePluginConfiguration { /** * init dividePlugin. * + * @param canaryDecisionService request partition decision service * @return {@linkplain DividePlugin} */ @Bean - public ShenyuPlugin dividePlugin() { - return new DividePlugin(); + public ShenyuPlugin dividePlugin(final CanaryDecisionService canaryDecisionService) { + return new DividePlugin(canaryDecisionService); + } + + /** + * Default request partition decision service. + * + * @return Canary decision service + */ + @Bean + @ConditionalOnMissingBean(CanaryDecisionService.class) + public CanaryDecisionService canaryDecisionService() { + return new DefaultCanaryDecisionService(); } /** diff --git a/shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-divide/src/test/java/org/apache/shenyu/springboot/plugin/divide/DividePluginConfigurationTest.java b/shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-divide/src/test/java/org/apache/shenyu/springboot/plugin/divide/DividePluginConfigurationTest.java index 619dd2dad3b8..c9f5767e99ed 100644 --- a/shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-divide/src/test/java/org/apache/shenyu/springboot/plugin/divide/DividePluginConfigurationTest.java +++ b/shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-divide/src/test/java/org/apache/shenyu/springboot/plugin/divide/DividePluginConfigurationTest.java @@ -21,6 +21,8 @@ import org.apache.shenyu.plugin.api.context.ShenyuContextDecorator; import org.apache.shenyu.plugin.base.handler.MetaDataHandler; import org.apache.shenyu.plugin.base.handler.PluginDataHandler; +import org.apache.shenyu.plugin.divide.canary.CanaryDecisionService; +import org.apache.shenyu.plugin.divide.canary.DefaultCanaryDecisionService; import org.apache.shenyu.plugin.divide.handler.DivideUpstreamDataHandler; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -30,6 +32,9 @@ import org.springframework.context.annotation.Configuration; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; /** * Test case for {@link DividePluginConfiguration}. @@ -57,6 +62,20 @@ public void testDividePlugin() { ); } + @Test + public void testDefaultCanaryDecisionService() { + applicationContextRunner.run(context -> assertTrue(context.getBean(CanaryDecisionService.class) instanceof DefaultCanaryDecisionService)); + } + + @Test + public void testCustomCanaryDecisionService() { + CanaryDecisionService custom = mock(CanaryDecisionService.class); + applicationContextRunner.withBean(CanaryDecisionService.class, () -> custom).run(context -> { + assertSame(custom, context.getBean(CanaryDecisionService.class)); + assertNotNull(context.getBean("dividePlugin", ShenyuPlugin.class)); + }); + } + @Test public void testDividePluginDataHandler() { applicationContextRunner.run(context -> {