From 5f8bd9fc98fbccd345cd8adcf95b1729d16183af Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Mon, 3 Nov 2025 15:32:30 +0100 Subject: [PATCH 01/18] add validation for service id in java enabler and DS Signed-off-by: Andrea Tabone --- .../discovery/ApimlInstanceRegistry.java | 32 ++++++++++++++++--- .../util/EurekaInstanceConfigValidator.java | 15 +++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java index 2d98b8da27..e01d07beb2 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java @@ -25,6 +25,7 @@ import org.springframework.cloud.netflix.eureka.server.InstanceRegistryProperties; import org.springframework.context.ApplicationContext; import org.zowe.apiml.discovery.config.EurekaConfig; +import org.zowe.apiml.exception.MetadataValidationException; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; @@ -34,11 +35,7 @@ import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.Collections; -import java.util.HashSet; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; @@ -55,6 +52,7 @@ public class ApimlInstanceRegistry extends InstanceRegistry { private static final String EXCEPTION_MESSAGE = "Implementation of InstanceRegistry changed, please verify fix of order sending events"; + private static final Pattern SERVICE_ID_PATTERN = Pattern.compile("^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"); private MethodHandle handleRegistrationMethod; private MethodHandle handlerResolveInstanceLeaseDurationMethod; @@ -229,6 +227,7 @@ public boolean isExpired(long additionalLeaseMs) { */ @Override public void register(InstanceInfo info, int leaseDuration, boolean isReplication) { + isServiceIdConformant(info); info = changeServiceId(info); try { register3ArgsMethodHandle.invokeWithArguments(this, info, leaseDuration, isReplication); @@ -244,6 +243,7 @@ public void register(InstanceInfo info, int leaseDuration, boolean isReplication @Override public void register(InstanceInfo info, final boolean isReplication) { + isServiceIdConformant(info); info = changeServiceId(info); try { register2ArgsMethodHandle.invokeWithArguments(this, info, isReplication); @@ -257,6 +257,28 @@ public void register(InstanceInfo info, final boolean isReplication) { } } + // "-" should also be probably excluded in v4, since it's not conformant according to RFC0952 + /** + * Validate whether the service ID is conformant. + * If not, the method will throw an exception and reject registration. + * @param info the instance info + * @throws MetadataValidationException exception in case of non-conformant service ID. + */ + private void isServiceIdConformant(InstanceInfo info) { + String instanceId = info.getInstanceId(); + + String[] parts = instanceId.split(":"); + if (parts.length != 3) { + throw new MetadataValidationException( + "The instance ID '" + instanceId + "' must have the format 'hostname:serviceId:port'." + ); + } + String serviceIdPart = parts[1]; + if (!SERVICE_ID_PATTERN.matcher(serviceIdPart).matches()) { + throw new MetadataValidationException("The service ID '" + serviceIdPart + "' is not a conformant."); + } + } + @Override public long getNumOfRenewsInLastMin() { // to simulate APIML, it is not sending a heartbeat anymore diff --git a/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java b/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java index 16ce7edec6..15018d6e8a 100644 --- a/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java +++ b/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.regex.Pattern; /** * Class that validates a service configuration before the registration with API ML @@ -33,6 +34,8 @@ public class EurekaInstanceConfigValidator { private final List missingRoutesParameters = new ArrayList<>(); private final List poorlyFormedRelativeUrlParameters = new ArrayList<>(); + private static final Pattern SERVICE_ID_PATTERN = Pattern.compile("^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"); + /** * Validates mandatory and non-mandatory parameters * @@ -40,6 +43,7 @@ public class EurekaInstanceConfigValidator { * @throws MetadataValidationException if the validation fails */ public void validate(ApiMediationServiceConfig config) { + validateServiceId(config.getServiceId()); validateRoutes(config.getRoutes()); if (config.getDiscoveryServiceUrls().stream().anyMatch(url -> url.toLowerCase().startsWith("https"))) { validateSsl(config.getSsl()); @@ -55,6 +59,17 @@ public void validate(ApiMediationServiceConfig config) { } } + // "-" should be excluded also in v4 probably, as it's not conformant according to RFC0952 + private void validateServiceId(String serviceId) { + if (!SERVICE_ID_PATTERN.matcher(serviceId).matches()) { + String message = String.format( + "Invalid serviceId [%s]: must comply with RFC 952 and RFC 1123 — only lowercase letters, digits, and hyphens allowed, must not start or end with a hyphen, and must not exceed 63 characters.", + serviceId + ); + throw new MetadataValidationException(message); + } + } + private void validateRoutes(List routes) { if (routes == null || routes.isEmpty()) { throw new MetadataValidationException("Routes configuration was not provided. Try to add apiml.service.routes section."); From 896c5df2c30ba6863e851e8e5ba8286a77110ea7 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Mon, 3 Nov 2025 15:39:33 +0100 Subject: [PATCH 02/18] add validation for static definition Signed-off-by: Andrea Tabone --- .../discovery/staticdef/ServiceDefinitionProcessor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java index 6a3adf52c0..031e3a0bde 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java @@ -41,6 +41,7 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.util.*; +import java.util.regex.Pattern; import static org.zowe.apiml.constants.EurekaMetadataDefinition.*; @@ -59,6 +60,7 @@ public class ServiceDefinitionProcessor { private static final YAMLFactory YAML_FACTORY = new YAMLFactory(); private static final String ERROR_PARSING_STATIC_DEFINITION_DATA = "org.zowe.apiml.discovery.errorParsingStaticDefinitionData"; + private static final Pattern SERVICE_ID_PATTERN = Pattern.compile("^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"); public ServiceDefinitionProcessor() { } @@ -208,8 +210,8 @@ private CatalogUiTile getTile(StaticRegistrationResult context, String ymlFileNa private List createInstances(StaticRegistrationResult context, String ymlFileName, Service service, Map tiles) { try { - if (service.getServiceId() == null) { - throw new ServiceDefinitionException(String.format("ServiceId is not defined in the file '%s'. The instance will not be created.", ymlFileName)); + if (service.getServiceId() == null || !SERVICE_ID_PATTERN.matcher(service.getServiceId()).matches()) { + throw new ServiceDefinitionException(String.format("ServiceId is either not defined in the file '%s' or not conformant. The instance will not be created.", ymlFileName)); } if (service.getInstanceBaseUrls() == null) { From 1a08d397cd7cb701fe36f113aafb72458d5ad54d Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Mon, 3 Nov 2025 16:06:37 +0100 Subject: [PATCH 03/18] add tests Signed-off-by: Andrea Tabone --- .../discovery/ApimlInstanceRegistryTest.java | 75 +++++++++++++++---- 1 file changed, 62 insertions(+), 13 deletions(-) diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java index 89975c17a4..538fe2026e 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java @@ -32,6 +32,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.test.util.ReflectionTestUtils; import org.zowe.apiml.discovery.config.EurekaConfig; +import org.zowe.apiml.exception.MetadataValidationException; import java.lang.invoke.MethodHandle; import java.lang.invoke.WrongMethodTypeException; @@ -66,12 +67,13 @@ class ApimlInstanceRegistryTest { @Mock private InstanceRegistryProperties instanceRegistryProperties; @Mock private ApplicationContext appCntx; private InstanceInfo standardInstance; + private InstanceInfo instanceWithWrongServiceId; private EurekaServerConfig serverConfig; @BeforeEach void setUp() { - standardInstance = getStandardInstance(); + standardInstance = getStandardInstance("hostname:serviceclient:10010"); serverConfig = new DefaultEurekaServerConfig(); apimlInstanceRegistry = spy(new ApimlInstanceRegistry( @@ -92,6 +94,53 @@ void setUp() { ReflectionTestUtils.setField(apimlInstanceRegistry, "handleCancellationMethod", methodHandle); } + @Nested + class GivenInvalidServiceId { + @Test + void whenContainingUnderscore_thenThrowException() { + InstanceInfo wrongInstance = getStandardInstance("hostname:invalid_serviceid:10010"); + + apimlInstanceRegistry = spy(new ApimlInstanceRegistry( + serverConfig, + clientConfig, + serverCodecs, + eurekaClient, + eurekaServerHttpClientFactory, + instanceRegistryProperties, + appCntx, + new EurekaConfig.Tuple(""))); + MethodHandle methodHandle = mock(MethodHandle.class); + ReflectionTestUtils.setField(apimlInstanceRegistry,"register3ArgsMethodHandle",methodHandle); + ReflectionTestUtils.setField(apimlInstanceRegistry,"handleRegistrationMethod",methodHandle); + assertThrows(MetadataValidationException.class, () -> { + apimlInstanceRegistry.register(wrongInstance, 1, false); + }); + + } + + @Test + void whenWrongFormat_thenThrowException() { + InstanceInfo wrongInstance = getStandardInstance("invalid_serviceid:10010"); + + apimlInstanceRegistry = spy(new ApimlInstanceRegistry( + serverConfig, + clientConfig, + serverCodecs, + eurekaClient, + eurekaServerHttpClientFactory, + instanceRegistryProperties, + appCntx, + new EurekaConfig.Tuple(""))); + MethodHandle methodHandle = mock(MethodHandle.class); + ReflectionTestUtils.setField(apimlInstanceRegistry,"register3ArgsMethodHandle",methodHandle); + ReflectionTestUtils.setField(apimlInstanceRegistry,"handleRegistrationMethod",methodHandle); + assertThrows(MetadataValidationException.class, () -> { + apimlInstanceRegistry.register(wrongInstance, 1, false); + }); + + } + } + @Nested class GivenReplacerTuple { @Nested @@ -99,7 +148,7 @@ class WhenChangeServiceId { @Test void thenChangeServicePrefix() { InstanceInfo info = apimlInstanceRegistry.changeServiceId(standardInstance); - assertEquals("helloclient", info.getInstanceId()); + assertEquals("hostname:helloclient:10010", info.getInstanceId()); assertEquals("HELLOCLIENT", info.getAppName()); assertEquals("helloclient", info.getVIPAddress()); assertEquals("HELLOCLIENT", info.getAppGroupName()); @@ -112,15 +161,15 @@ void thenChangeServicePrefix() { } private static Stream tuples() { return Stream.of( - Arguments.of("service*,hello", "helloclient"), - Arguments.of("service,hello", "helloclient"), - Arguments.of("service*,hello*", "helloclient"), - Arguments.of("service*,service", "serviceclient"), - Arguments.of("service*", "serviceclient"), - Arguments.of(",service", "serviceclient"), - Arguments.of("service,", "serviceclient"), - Arguments.of(null, "serviceclient"), - Arguments.of("different*,hello", "serviceclient") + Arguments.of("service*,hello", "hostname:helloclient:10010"), + Arguments.of("service,hello", "hostname:helloclient:10010"), + Arguments.of("service*,hello*", "hostname:helloclient:10010"), + Arguments.of("service*,service", "hostname:serviceclient:10010"), + Arguments.of("service*", "hostname:serviceclient:10010"), + Arguments.of(",service", "hostname:serviceclient:10010"), + Arguments.of("service,", "hostname:serviceclient:10010"), + Arguments.of(null, "hostname:serviceclient:10010"), + Arguments.of("different*,hello", "hostname:serviceclient:10010") ); } @@ -371,10 +420,10 @@ private Stream exceptions() { } - private InstanceInfo getStandardInstance() { + private InstanceInfo getStandardInstance(String serviceId) { return InstanceInfo.Builder.newBuilder() - .setInstanceId("serviceclient") + .setInstanceId(serviceId) .setAppName("SERVICECLIENT") .setAppGroupName("SERVICECLIENT") .setIPAddr("192.168.0.1") From 0c5f508ef5190b96e19196140a1a9d22fc842998 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Mon, 3 Nov 2025 16:28:22 +0100 Subject: [PATCH 04/18] fix tests Signed-off-by: Andrea Tabone --- .../staticdef/ServiceDefinitionProcessorTest.java | 2 +- .../src/test/resources/api-defs/staticclient.yml | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java index 2e42335b36..141cb34bc4 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java @@ -331,7 +331,7 @@ void givenInstanceWithoutServiceId_whenDefinitionIsLoaded_thenErrorIsReturned() assertThatNoInstanceIsCreatedAndCorrectMessageIsProduced( result, - "ServiceId is not defined in the file 'test.yml'. The instance will not be created." + "ServiceId is either not defined in the file 'test.yml' or not conformant. The instance will not be created." ); } diff --git a/discovery-service/src/test/resources/api-defs/staticclient.yml b/discovery-service/src/test/resources/api-defs/staticclient.yml index e17d50dd7f..75531bd15b 100644 --- a/discovery-service/src/test/resources/api-defs/staticclient.yml +++ b/discovery-service/src/test/resources/api-defs/staticclient.yml @@ -23,18 +23,18 @@ services: serviceRelativeUrl: /ws - gatewayUrl: api-doc serviceRelativeUrl: /api-doc - - serviceId: toAddAuth + - serviceId: toaddauth title: Service to add auth info instanceBaseUrls: - https://localhost:10012/discoverableclient - - serviceId: toReplaceAuth + - serviceId: toreplaceauth title: Service to replace auth info instanceBaseUrls: - https://localhost:10012/discoverableclient authentication: scheme: httpBasicPassTicket applid: TSTAPPL2 - - serviceId: nowFixedAuth + - serviceId: nowfixedauth title: Service with temporary auth, althought auth is defined now instanceBaseUrls: - https://localhost:10012/discoverableclient @@ -43,16 +43,16 @@ services: applid: TSTAPPL3 additionalServiceMetadata: - - serviceId: toAddAuth + - serviceId: toaddauth authentication: scheme: httpBasicPassTicket applid: TSTAPPL4 - - serviceId: toReplaceAuth + - serviceId: toreplaceauth mode: FORCE_UPDATE authentication: scheme: httpBasicPassTicket applid: TSTAPPL5 - - serviceId: nowFixedAuth + - serviceId: nowfixedauth authentication: scheme: httpBasicPassTicket applid: TSTAPPL6 From 644f94a281969061636e3e9974a0dc454e783acb Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Mon, 3 Nov 2025 16:32:40 +0100 Subject: [PATCH 05/18] add test to check 63 length threshold Signed-off-by: Andrea Tabone --- .../discovery/ApimlInstanceRegistryTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java index 538fe2026e..90488f91ae 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java @@ -139,6 +139,29 @@ void whenWrongFormat_thenThrowException() { }); } + + @Test + void whenServiceIdLongerThan63Chars_thenThrowException() { + String longServiceId = "invalidserviceidididididididididididdididididididididididdidididididididididid"; + InstanceInfo wrongInstance = getStandardInstance("hostname:" + longServiceId + ":10010"); + + apimlInstanceRegistry = spy(new ApimlInstanceRegistry( + serverConfig, + clientConfig, + serverCodecs, + eurekaClient, + eurekaServerHttpClientFactory, + instanceRegistryProperties, + appCntx, + new EurekaConfig.Tuple(""))); + MethodHandle methodHandle = mock(MethodHandle.class); + ReflectionTestUtils.setField(apimlInstanceRegistry,"register3ArgsMethodHandle",methodHandle); + ReflectionTestUtils.setField(apimlInstanceRegistry,"handleRegistrationMethod",methodHandle); + assertThrows(MetadataValidationException.class, () -> { + apimlInstanceRegistry.register(wrongInstance, 1, false); + }); + + } } @Nested From 169295d6652424c0acaa906a781b42f9df998862 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Mon, 3 Nov 2025 16:35:54 +0100 Subject: [PATCH 06/18] enhance javadoc Signed-off-by: Andrea Tabone --- .../java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java index e01d07beb2..1be7c8ded1 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java @@ -259,7 +259,8 @@ public void register(InstanceInfo info, final boolean isReplication) { // "-" should also be probably excluded in v4, since it's not conformant according to RFC0952 /** - * Validate whether the service ID is conformant. + * Validate whether the service ID is conformant and complies with RFC 952 and RFC 1123. Only lowercase letters, digits, + * and hyphens allowed, must not start or end with a hyphen, and must not exceed 63 characters. * If not, the method will throw an exception and reject registration. * @param info the instance info * @throws MetadataValidationException exception in case of non-conformant service ID. From 76d0898d63e1cbb676d8316017459414feb076e6 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Tue, 4 Nov 2025 14:12:27 +0100 Subject: [PATCH 07/18] address comments and make instance id formt more strict Signed-off-by: Andrea Tabone --- .../java/org/zowe/apiml/util/EurekaUtils.java | 21 ++++-- .../org/zowe/apiml/util/EurekaUtilsTest.java | 8 +-- .../discovery/ApimlInstanceRegistry.java | 52 ++++++++++---- .../discovery/ApimlInstanceRegistryTest.java | 67 ++++++++----------- .../util/EurekaInstanceConfigValidator.java | 7 +- 5 files changed, 89 insertions(+), 66 deletions(-) diff --git a/common-service-core/src/main/java/org/zowe/apiml/util/EurekaUtils.java b/common-service-core/src/main/java/org/zowe/apiml/util/EurekaUtils.java index 7b83e47217..52811a50ae 100644 --- a/common-service-core/src/main/java/org/zowe/apiml/util/EurekaUtils.java +++ b/common-service-core/src/main/java/org/zowe/apiml/util/EurekaUtils.java @@ -12,11 +12,13 @@ import com.netflix.appinfo.InstanceInfo; import lombok.experimental.UtilityClass; +import org.apache.commons.lang3.StringUtils; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.zowe.apiml.constants.EurekaMetadataDefinition; import java.util.Optional; +import java.util.regex.Pattern; import static org.zowe.apiml.constants.EurekaMetadataDefinition.APIML_ID; import static org.zowe.apiml.product.constants.CoreService.GATEWAY; @@ -28,19 +30,28 @@ @UtilityClass public class EurekaUtils { + public static final Pattern SERVICE_ID_PATTERN = Pattern.compile("^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"); + /** * Extract serviceId from instanceId * @param instanceId input, instanceId in format "host:service:random number to unique instanceId" * @return second part, it means serviceId. If it doesn't exist return null; */ public String getServiceIdFromInstanceId(String instanceId) { - final int startIndex = instanceId.indexOf(':'); - if (startIndex < 0) return null; + if (StringUtils.isBlank(instanceId)) { + return null; + } + String[] parts = instanceId.split(":"); + if (parts.length != 3) { + return null; + } - final int endIndex = instanceId.indexOf(':', startIndex + 1); - if (endIndex < 0) return null; + String serviceId = parts[1].trim(); + if (serviceId.isEmpty()) { + return null; + } - return instanceId.substring(startIndex + 1, endIndex); + return serviceId; } /** diff --git a/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java b/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java index e5be5b03b6..8a4b07b9d9 100644 --- a/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java +++ b/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java @@ -32,13 +32,13 @@ class EurekaUtilsTest { @Test void test() { - assertEquals("abc", EurekaUtils.getServiceIdFromInstanceId("123:abc:def:::::xyz")); assertEquals("abc", EurekaUtils.getServiceIdFromInstanceId("123:abc:def")); - assertEquals("", EurekaUtils.getServiceIdFromInstanceId("123::def")); - assertEquals("", EurekaUtils.getServiceIdFromInstanceId("::")); + assertNull(EurekaUtils.getServiceIdFromInstanceId("123:abc:def:::::xyz")); + assertNull(EurekaUtils.getServiceIdFromInstanceId("hostname:123:")); + assertNull(EurekaUtils.getServiceIdFromInstanceId("::")); + assertNull(EurekaUtils.getServiceIdFromInstanceId("123::def")); assertNull(EurekaUtils.getServiceIdFromInstanceId(":")); assertNull(EurekaUtils.getServiceIdFromInstanceId("")); - assertNull(EurekaUtils.getServiceIdFromInstanceId("abc")); } private InstanceInfo createInstanceInfo(String host, int port, int securePort, boolean isSecureEnabled) { diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java index 1be7c8ded1..2207de45ec 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java @@ -21,11 +21,13 @@ import com.netflix.eureka.resources.ServerCodecs; import com.netflix.eureka.transport.EurekaServerHttpClientFactory; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; import org.springframework.cloud.netflix.eureka.server.InstanceRegistry; import org.springframework.cloud.netflix.eureka.server.InstanceRegistryProperties; import org.springframework.context.ApplicationContext; import org.zowe.apiml.discovery.config.EurekaConfig; import org.zowe.apiml.exception.MetadataValidationException; +import org.zowe.apiml.util.EurekaUtils; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; @@ -39,6 +41,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; +import static org.zowe.apiml.util.EurekaUtils.SERVICE_ID_PATTERN; + /** * This implementation of instance registry is solving known problem in Eureka. Discovery service notify about change * in services before it does it. From this reason listener can try to use services before they are really registered. @@ -52,7 +56,6 @@ public class ApimlInstanceRegistry extends InstanceRegistry { private static final String EXCEPTION_MESSAGE = "Implementation of InstanceRegistry changed, please verify fix of order sending events"; - private static final Pattern SERVICE_ID_PATTERN = Pattern.compile("^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"); private MethodHandle handleRegistrationMethod; private MethodHandle handlerResolveInstanceLeaseDurationMethod; @@ -257,26 +260,49 @@ public void register(InstanceInfo info, final boolean isReplication) { } } - // "-" should also be probably excluded in v4, since it's not conformant according to RFC0952 /** - * Validate whether the service ID is conformant and complies with RFC 952 and RFC 1123. Only lowercase letters, digits, - * and hyphens allowed, must not start or end with a hyphen, and must not exceed 63 characters. - * If not, the method will throw an exception and reject registration. + * Validates that the service identifiers in the {@link InstanceInfo} are conformant and mutually consistent. + * The appName and vidAddress must not be null or empty. Both must comply with RFC 952 and RFC 1123. + * Only lowercase letters, digits, and hyphens allowed, must not start or end with a hyphen, and must not exceed 63 characters. + * The instanceId must follow the format 'hostname:serviceId:port'. + * The serviceId extracted from the instanceId must match both appName and vipAddress. + * If any of these checks fail, the method will throw an exception and the registration is rejected. * @param info the instance info - * @throws MetadataValidationException exception in case of non-conformant service ID. + * @throws MetadataValidationException exception if any service identifier is invalid or inconsistent */ private void isServiceIdConformant(InstanceInfo info) { String instanceId = info.getInstanceId(); - - String[] parts = instanceId.split(":"); - if (parts.length != 3) { + String appName = info.getAppName().toLowerCase(); + String vipAddress = info.getVIPAddress(); + if (StringUtils.isBlank(appName) || + StringUtils.isBlank(vipAddress)) { + throw new MetadataValidationException( + "The service ID fields 'appName' and 'vipAddress' must not be null or empty." + ); + } + if (!SERVICE_ID_PATTERN.matcher(appName).matches()) { + throw new MetadataValidationException( + String.format("Invalid appName '%s': must comply with RFC 952/1123 (only lowercase letters, digits, hyphens, max 63 chars).", appName) + ); + } + if (!SERVICE_ID_PATTERN.matcher(vipAddress).matches()) { + throw new MetadataValidationException( + String.format("Invalid vipAddress '%s': must comply with RFC 952/1123 (only lowercase letters, digits, hyphens, max 63 chars).", vipAddress) + ); + } + String serviceId = EurekaUtils.getServiceIdFromInstanceId(instanceId); + if (serviceId == null) { throw new MetadataValidationException( - "The instance ID '" + instanceId + "' must have the format 'hostname:serviceId:port'." + "The instance ID '" + instanceId + "': must have the format 'hostname:serviceId:port'." ); } - String serviceIdPart = parts[1]; - if (!SERVICE_ID_PATTERN.matcher(serviceIdPart).matches()) { - throw new MetadataValidationException("The service ID '" + serviceIdPart + "' is not a conformant."); + if (!serviceId.equals(appName) && + !serviceId.equals(vipAddress)) { + throw new MetadataValidationException( + String.format("Inconsistent service identity: instanceId contains serviceId '%s' but appName='%s' and vipAddress='%s'.", + serviceId, appName, vipAddress) + + ); } } diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java index 90488f91ae..90e0f0b61b 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java @@ -67,13 +67,12 @@ class ApimlInstanceRegistryTest { @Mock private InstanceRegistryProperties instanceRegistryProperties; @Mock private ApplicationContext appCntx; private InstanceInfo standardInstance; - private InstanceInfo instanceWithWrongServiceId; private EurekaServerConfig serverConfig; @BeforeEach void setUp() { - standardInstance = getStandardInstance("hostname:serviceclient:10010"); + standardInstance = getStandardInstance("hostname:serviceclient:10010", "serviceclient"); serverConfig = new DefaultEurekaServerConfig(); apimlInstanceRegistry = spy(new ApimlInstanceRegistry( @@ -96,31 +95,20 @@ void setUp() { @Nested class GivenInvalidServiceId { - @Test - void whenContainingUnderscore_thenThrowException() { - InstanceInfo wrongInstance = getStandardInstance("hostname:invalid_serviceid:10010"); - - apimlInstanceRegistry = spy(new ApimlInstanceRegistry( - serverConfig, - clientConfig, - serverCodecs, - eurekaClient, - eurekaServerHttpClientFactory, - instanceRegistryProperties, - appCntx, - new EurekaConfig.Tuple(""))); - MethodHandle methodHandle = mock(MethodHandle.class); - ReflectionTestUtils.setField(apimlInstanceRegistry,"register3ArgsMethodHandle",methodHandle); - ReflectionTestUtils.setField(apimlInstanceRegistry,"handleRegistrationMethod",methodHandle); - assertThrows(MetadataValidationException.class, () -> { - apimlInstanceRegistry.register(wrongInstance, 1, false); - }); + private static Stream instanceIds() { + return Stream.of( + Arguments.of( "hostname:service_client:10010", "service_client"), + Arguments.of( "hostname:-serviceclient:10010", "-serviceclient"), + Arguments.of( "hostname:serviceclient-:10010", "serviceclient-"), + Arguments.of( "hostname:invalidserviceidididididididididididdididididididididididdidididididididididid:10010", "invalidserviceidididididididididididdididididididididididdidididididididididid") + ); } - @Test - void whenWrongFormat_thenThrowException() { - InstanceInfo wrongInstance = getStandardInstance("invalid_serviceid:10010"); + @ParameterizedTest + @MethodSource("instanceIds") + void whenContainingUnderscore_thenThrowException(String instanceId, String appName) { + InstanceInfo wrongInstance = getStandardInstance(instanceId, appName); apimlInstanceRegistry = spy(new ApimlInstanceRegistry( serverConfig, @@ -139,12 +127,14 @@ void whenWrongFormat_thenThrowException() { }); } + } - @Test - void whenServiceIdLongerThan63Chars_thenThrowException() { - String longServiceId = "invalidserviceidididididididididididdididididididididididdidididididididididid"; - InstanceInfo wrongInstance = getStandardInstance("hostname:" + longServiceId + ":10010"); + @Nested + class GivenValidServiceIdWithDash { + @Test + void thenShouldRegister() { + standardInstance = getStandardInstance("hostname:service-client:10010", "service-client"); apimlInstanceRegistry = spy(new ApimlInstanceRegistry( serverConfig, clientConfig, @@ -153,14 +143,11 @@ void whenServiceIdLongerThan63Chars_thenThrowException() { eurekaServerHttpClientFactory, instanceRegistryProperties, appCntx, - new EurekaConfig.Tuple(""))); + new EurekaConfig.Tuple(null))); MethodHandle methodHandle = mock(MethodHandle.class); - ReflectionTestUtils.setField(apimlInstanceRegistry,"register3ArgsMethodHandle",methodHandle); - ReflectionTestUtils.setField(apimlInstanceRegistry,"handleRegistrationMethod",methodHandle); - assertThrows(MetadataValidationException.class, () -> { - apimlInstanceRegistry.register(wrongInstance, 1, false); - }); - + ReflectionTestUtils.setField(apimlInstanceRegistry,"register2ArgsMethodHandle", methodHandle); + ReflectionTestUtils.setField(apimlInstanceRegistry,"handleRegistrationMethod", methodHandle); + apimlInstanceRegistry.register(standardInstance, false); } } @@ -443,18 +430,18 @@ private Stream exceptions() { } - private InstanceInfo getStandardInstance(String serviceId) { + private InstanceInfo getStandardInstance(String instanceId, String appName) { return InstanceInfo.Builder.newBuilder() - .setInstanceId(serviceId) - .setAppName("SERVICECLIENT") - .setAppGroupName("SERVICECLIENT") + .setInstanceId(instanceId) + .setAppName(appName.toUpperCase()) + .setAppGroupName(appName.toUpperCase()) .setIPAddr("192.168.0.1") .enablePort(InstanceInfo.PortType.SECURE, true) .setSecurePort(9090) .setHostName("localhost") .setSecureVIPAddress("localhost") - .setVIPAddress("serviceclient") + .setVIPAddress(appName) .setStatus(InstanceInfo.InstanceStatus.UP) .build(); } diff --git a/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java b/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java index 15018d6e8a..cbb52b4476 100644 --- a/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java +++ b/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java @@ -19,7 +19,9 @@ import java.util.ArrayList; import java.util.List; -import java.util.regex.Pattern; + +import static org.zowe.apiml.util.EurekaUtils.SERVICE_ID_PATTERN; + /** * Class that validates a service configuration before the registration with API ML @@ -34,8 +36,6 @@ public class EurekaInstanceConfigValidator { private final List missingRoutesParameters = new ArrayList<>(); private final List poorlyFormedRelativeUrlParameters = new ArrayList<>(); - private static final Pattern SERVICE_ID_PATTERN = Pattern.compile("^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"); - /** * Validates mandatory and non-mandatory parameters * @@ -59,7 +59,6 @@ public void validate(ApiMediationServiceConfig config) { } } - // "-" should be excluded also in v4 probably, as it's not conformant according to RFC0952 private void validateServiceId(String serviceId) { if (!SERVICE_ID_PATTERN.matcher(serviceId).matches()) { String message = String.format( From 0039b96b06a3d71fa2179e28c020e3261ccf30e3 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Tue, 4 Nov 2025 14:17:55 +0100 Subject: [PATCH 08/18] fix test Signed-off-by: Andrea Tabone --- .../discovery/metadata/MetadataDefaultsServiceTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/metadata/MetadataDefaultsServiceTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/metadata/MetadataDefaultsServiceTest.java index a72b29a8b8..fca1bb5150 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/metadata/MetadataDefaultsServiceTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/metadata/MetadataDefaultsServiceTest.java @@ -89,17 +89,17 @@ void updatingStaticService() { assertEquals( "TSTAPPL4", - map.get("STATIC-localhost:toAddAuth:10012").getMetadata().get(AUTHENTICATION_APPLID) + map.get("STATIC-localhost:toaddauth:10012").getMetadata().get(AUTHENTICATION_APPLID) ); assertEquals( "TSTAPPL5", - map.get("STATIC-localhost:toReplaceAuth:10012").getMetadata().get(AUTHENTICATION_APPLID) + map.get("STATIC-localhost:toreplaceauth:10012").getMetadata().get(AUTHENTICATION_APPLID) ); assertEquals( "TSTAPPL3", - map.get("STATIC-localhost:nowFixedAuth:10012").getMetadata().get(AUTHENTICATION_APPLID) + map.get("STATIC-localhost:nowfixedauth:10012").getMetadata().get(AUTHENTICATION_APPLID) ); } From 869f7e720ff05dc23c1faa83f14ec9cc9b2a431d Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Tue, 4 Nov 2025 14:38:40 +0100 Subject: [PATCH 09/18] fix integration test - make service id lowercase Signed-off-by: Andrea Tabone --- .../gateway/AuthenticationOnDeploymentTest.java | 10 +++++----- .../apiml/functional/gateway/ServiceHaModeTest.java | 8 ++++---- .../org/zowe/apiml/util/service/VirtualService.java | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/integration-tests/src/test/java/org/zowe/apiml/functional/gateway/AuthenticationOnDeploymentTest.java b/integration-tests/src/test/java/org/zowe/apiml/functional/gateway/AuthenticationOnDeploymentTest.java index 37aacbc729..5e09d2e35f 100644 --- a/integration-tests/src/test/java/org/zowe/apiml/functional/gateway/AuthenticationOnDeploymentTest.java +++ b/integration-tests/src/test/java/org/zowe/apiml/functional/gateway/AuthenticationOnDeploymentTest.java @@ -74,8 +74,8 @@ void testMultipleAuthenticationSchemes() throws Exception { List ports = RandomPorts.generateUniquePorts(2); try ( - final VirtualService service1 = new VirtualService("testService", ports.get(0)); - final VirtualService service2 = new VirtualService("testService", ports.get(1)) + final VirtualService service1 = new VirtualService("testservice", ports.get(0)); + final VirtualService service2 = new VirtualService("testservice", ports.get(1)) ) { // start first instance - without passTickets service1 @@ -158,9 +158,9 @@ void testReregistration() throws Exception { List ports = RandomPorts.generateUniquePorts(3); try ( - final VirtualService service1 = new VirtualService("testService3", ports.get(0)); - final VirtualService service2 = new VirtualService("testService3", ports.get(1)); - final VirtualService service4 = new VirtualService("testService3", ports.get(2)) + final VirtualService service1 = new VirtualService("testservice3", ports.get(0)); + final VirtualService service2 = new VirtualService("testservice3", ports.get(1)); + final VirtualService service4 = new VirtualService("testservice3", ports.get(2)) ) { List serviceList = Arrays.asList(service1, service2); diff --git a/integration-tests/src/test/java/org/zowe/apiml/functional/gateway/ServiceHaModeTest.java b/integration-tests/src/test/java/org/zowe/apiml/functional/gateway/ServiceHaModeTest.java index 3bb1bc0076..5a6dbd4ea1 100644 --- a/integration-tests/src/test/java/org/zowe/apiml/functional/gateway/ServiceHaModeTest.java +++ b/integration-tests/src/test/java/org/zowe/apiml/functional/gateway/ServiceHaModeTest.java @@ -92,8 +92,8 @@ class WhenOneIsDown { void setUp() throws LifecycleException, IOException, JSONException { List ports = RandomPorts.generateUniquePorts(2); - service1 = new VirtualService("testHaModeService1", ports.get(0)); - service2 = new VirtualService("testHaModeService1", ports.get(1)); + service1 = new VirtualService("testhamodeservice1", ports.get(0)); + service2 = new VirtualService("testhamodeservice1", ports.get(1)); service1.start(); service2.start().waitForGatewayRegistration(TIMEOUT); @@ -151,8 +151,8 @@ class OneReturns503 { void setUp() throws LifecycleException, IOException, JSONException { List ports = RandomPorts.generateUniquePorts(2); - service1 = new VirtualService("testHaModeService2", ports.get(0)); - service2 = new VirtualService("testHaModeService2", ports.get(1)); + service1 = new VirtualService("testhamodeservice2", ports.get(0)); + service2 = new VirtualService("testhamodeservice2", ports.get(1)); service1.addInstanceServlet("Http503", "/httpCode"); service2.addHttpStatusCodeServlet(HttpStatus.SC_SERVICE_UNAVAILABLE); diff --git a/integration-tests/src/test/java/org/zowe/apiml/util/service/VirtualService.java b/integration-tests/src/test/java/org/zowe/apiml/util/service/VirtualService.java index 296346429f..270f351807 100644 --- a/integration-tests/src/test/java/org/zowe/apiml/util/service/VirtualService.java +++ b/integration-tests/src/test/java/org/zowe/apiml/util/service/VirtualService.java @@ -57,7 +57,7 @@ *

* It is recommended to use try-with-resource to be sure, service will be unregistered on the end, ie.: *

- * try (final VirtualService service = new VirtualService("testService")) { + * try (final VirtualService service = new VirtualService("testservice")) { * service * // add same servlet and setting of service * .start() From c4f0a4b156123d12785fe59d1ffd723940927541 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Tue, 4 Nov 2025 16:35:20 +0100 Subject: [PATCH 10/18] address review Signed-off-by: Andrea Tabone --- .../discovery/ApimlInstanceRegistry.java | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java index 2207de45ec..237a8e092c 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java @@ -262,22 +262,20 @@ public void register(InstanceInfo info, final boolean isReplication) { /** * Validates that the service identifiers in the {@link InstanceInfo} are conformant and mutually consistent. - * The appName and vidAddress must not be null or empty. Both must comply with RFC 952 and RFC 1123. + * The appName must not be null or empty. Both must comply with RFC 952 and RFC 1123. * Only lowercase letters, digits, and hyphens allowed, must not start or end with a hyphen, and must not exceed 63 characters. * The instanceId must follow the format 'hostname:serviceId:port'. - * The serviceId extracted from the instanceId must match both appName and vipAddress. + * The serviceId extracted from the instanceId must match both appName. * If any of these checks fail, the method will throw an exception and the registration is rejected. * @param info the instance info * @throws MetadataValidationException exception if any service identifier is invalid or inconsistent */ private void isServiceIdConformant(InstanceInfo info) { String instanceId = info.getInstanceId(); - String appName = info.getAppName().toLowerCase(); - String vipAddress = info.getVIPAddress(); - if (StringUtils.isBlank(appName) || - StringUtils.isBlank(vipAddress)) { + String appName = StringUtils.lowerCase(info.getAppName()); + if (StringUtils.isBlank(appName)) { throw new MetadataValidationException( - "The service ID fields 'appName' and 'vipAddress' must not be null or empty." + "The service ID fields 'appName' must not be null or empty." ); } if (!SERVICE_ID_PATTERN.matcher(appName).matches()) { @@ -285,22 +283,22 @@ private void isServiceIdConformant(InstanceInfo info) { String.format("Invalid appName '%s': must comply with RFC 952/1123 (only lowercase letters, digits, hyphens, max 63 chars).", appName) ); } - if (!SERVICE_ID_PATTERN.matcher(vipAddress).matches()) { - throw new MetadataValidationException( - String.format("Invalid vipAddress '%s': must comply with RFC 952/1123 (only lowercase letters, digits, hyphens, max 63 chars).", vipAddress) - ); - } + String serviceId = EurekaUtils.getServiceIdFromInstanceId(instanceId); if (serviceId == null) { throw new MetadataValidationException( "The instance ID '" + instanceId + "': must have the format 'hostname:serviceId:port'." ); } - if (!serviceId.equals(appName) && - !serviceId.equals(vipAddress)) { + if (!SERVICE_ID_PATTERN.matcher(serviceId).matches()) { + throw new MetadataValidationException( + String.format("Invalid serviceId '%s' extracted from instanceId '%s': must comply with RFC 952/1123.", serviceId, instanceId) + ); + } + if (!serviceId.equals(appName)) { throw new MetadataValidationException( - String.format("Inconsistent service identity: instanceId contains serviceId '%s' but appName='%s' and vipAddress='%s'.", - serviceId, appName, vipAddress) + String.format("Inconsistent service identity: instanceId contains serviceId '%s' but appName='%s'.", + serviceId, appName) ); } From 068591f344724b1dc35629b7a4545f173406b0ce Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Tue, 4 Nov 2025 17:06:11 +0100 Subject: [PATCH 11/18] address review pt.2 Signed-off-by: Andrea Tabone --- .../java/org/zowe/apiml/util/EurekaUtils.java | 19 +++++++++++++++++++ .../discovery/ApimlInstanceRegistry.java | 10 +++++----- .../staticdef/ServiceDefinitionProcessor.java | 5 ++--- .../util/EurekaInstanceConfigValidator.java | 15 ++------------- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/common-service-core/src/main/java/org/zowe/apiml/util/EurekaUtils.java b/common-service-core/src/main/java/org/zowe/apiml/util/EurekaUtils.java index 52811a50ae..9166eced1e 100644 --- a/common-service-core/src/main/java/org/zowe/apiml/util/EurekaUtils.java +++ b/common-service-core/src/main/java/org/zowe/apiml/util/EurekaUtils.java @@ -16,6 +16,7 @@ import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.zowe.apiml.constants.EurekaMetadataDefinition; +import org.zowe.apiml.exception.MetadataValidationException; import java.util.Optional; import java.util.regex.Pattern; @@ -54,6 +55,24 @@ public String getServiceIdFromInstanceId(String instanceId) { return serviceId; } + /** + * Validate whether service ID is not null and conformant. + * @param serviceId the service ID + * @throws MetadataValidationException exception if the service ID is not conformant + */ + public void validateServiceId(String serviceId) { + if (StringUtils.isBlank(serviceId)) { + throw new MetadataValidationException("The service ID must not be null or empty. The service will not be registered."); + } + if (!SERVICE_ID_PATTERN.matcher(serviceId).matches()) { + String message = String.format( + "Invalid serviceId [%s]: must comply with RFC 952/1123 (only lowercase letters, digits, hyphens, max 63 chars). The service will not be registered.", + serviceId + ); + throw new MetadataValidationException(message); + } + } + /** * Construct base URL for specific InstanceInfo * @param instanceInfo Instance of service, for which we want to get an URL diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java index 237a8e092c..87b29b7579 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java @@ -275,29 +275,29 @@ private void isServiceIdConformant(InstanceInfo info) { String appName = StringUtils.lowerCase(info.getAppName()); if (StringUtils.isBlank(appName)) { throw new MetadataValidationException( - "The service ID fields 'appName' must not be null or empty." + "The service ID fields 'appName' must not be null or empty. The service will not be registered." ); } if (!SERVICE_ID_PATTERN.matcher(appName).matches()) { throw new MetadataValidationException( - String.format("Invalid appName '%s': must comply with RFC 952/1123 (only lowercase letters, digits, hyphens, max 63 chars).", appName) + String.format("Invalid appName '%s': must comply with RFC 952/1123 (only lowercase letters, digits, hyphens, max 63 chars). The service will not be registered.", appName) ); } String serviceId = EurekaUtils.getServiceIdFromInstanceId(instanceId); if (serviceId == null) { throw new MetadataValidationException( - "The instance ID '" + instanceId + "': must have the format 'hostname:serviceId:port'." + "The instance ID '" + instanceId + "': must have the format 'hostname:serviceId:port'. The service will not be registered." ); } if (!SERVICE_ID_PATTERN.matcher(serviceId).matches()) { throw new MetadataValidationException( - String.format("Invalid serviceId '%s' extracted from instanceId '%s': must comply with RFC 952/1123.", serviceId, instanceId) + String.format("Invalid serviceId '%s' extracted from instanceId '%s': must comply with RFC 952/1123. The service will not be registered.", serviceId, instanceId) ); } if (!serviceId.equals(appName)) { throw new MetadataValidationException( - String.format("Inconsistent service identity: instanceId contains serviceId '%s' but appName='%s'.", + String.format("Inconsistent service identity: instanceId contains serviceId '%s' but appName='%s'. The service will not be registered.", serviceId, appName) ); diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java index 031e3a0bde..beb59f4ea5 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java @@ -29,6 +29,7 @@ import org.zowe.apiml.message.log.ApimlLogger; import org.zowe.apiml.product.discovery.*; import org.zowe.apiml.product.logging.annotations.InjectApimlLogger; +import org.zowe.apiml.util.EurekaUtils; import org.zowe.apiml.util.MapUtils; import org.zowe.apiml.util.UrlUtils; @@ -210,9 +211,7 @@ private CatalogUiTile getTile(StaticRegistrationResult context, String ymlFileNa private List createInstances(StaticRegistrationResult context, String ymlFileName, Service service, Map tiles) { try { - if (service.getServiceId() == null || !SERVICE_ID_PATTERN.matcher(service.getServiceId()).matches()) { - throw new ServiceDefinitionException(String.format("ServiceId is either not defined in the file '%s' or not conformant. The instance will not be created.", ymlFileName)); - } + EurekaUtils.validateServiceId(service.getServiceId()); if (service.getInstanceBaseUrls() == null) { throw new ServiceDefinitionException(String.format("The instanceBaseUrls parameter of %s is not defined. The instance will not be created.", service.getServiceId())); diff --git a/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java b/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java index cbb52b4476..5d57d8dc3b 100644 --- a/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java +++ b/onboarding-enabler-java/src/main/java/org/zowe/apiml/eurekaservice/client/util/EurekaInstanceConfigValidator.java @@ -16,12 +16,11 @@ import org.zowe.apiml.eurekaservice.client.config.Route; import org.zowe.apiml.eurekaservice.client.config.Ssl; import org.zowe.apiml.exception.MetadataValidationException; +import org.zowe.apiml.util.EurekaUtils; import java.util.ArrayList; import java.util.List; -import static org.zowe.apiml.util.EurekaUtils.SERVICE_ID_PATTERN; - /** * Class that validates a service configuration before the registration with API ML @@ -43,7 +42,7 @@ public class EurekaInstanceConfigValidator { * @throws MetadataValidationException if the validation fails */ public void validate(ApiMediationServiceConfig config) { - validateServiceId(config.getServiceId()); + EurekaUtils.validateServiceId(config.getServiceId()); validateRoutes(config.getRoutes()); if (config.getDiscoveryServiceUrls().stream().anyMatch(url -> url.toLowerCase().startsWith("https"))) { validateSsl(config.getSsl()); @@ -59,16 +58,6 @@ public void validate(ApiMediationServiceConfig config) { } } - private void validateServiceId(String serviceId) { - if (!SERVICE_ID_PATTERN.matcher(serviceId).matches()) { - String message = String.format( - "Invalid serviceId [%s]: must comply with RFC 952 and RFC 1123 — only lowercase letters, digits, and hyphens allowed, must not start or end with a hyphen, and must not exceed 63 characters.", - serviceId - ); - throw new MetadataValidationException(message); - } - } - private void validateRoutes(List routes) { if (routes == null || routes.isEmpty()) { throw new MetadataValidationException("Routes configuration was not provided. Try to add apiml.service.routes section."); From f40ff7770e8bd4a4539be0041fffc6e51b7e83bd Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Tue, 4 Nov 2025 21:36:55 +0100 Subject: [PATCH 12/18] fix Signed-off-by: Andrea Tabone --- .../zowe/apiml/client/service/ApiMediationClientService.java | 2 +- .../apiml/discovery/staticdef/ServiceDefinitionProcessor.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/discoverable-client/src/main/java/org/zowe/apiml/client/service/ApiMediationClientService.java b/discoverable-client/src/main/java/org/zowe/apiml/client/service/ApiMediationClientService.java index 32de8e52ca..b1aa26624f 100644 --- a/discoverable-client/src/main/java/org/zowe/apiml/client/service/ApiMediationClientService.java +++ b/discoverable-client/src/main/java/org/zowe/apiml/client/service/ApiMediationClientService.java @@ -32,7 +32,7 @@ @Service public class ApiMediationClientService { private static final String PORT = "10013"; - private static final String SERVICE_ID = "registrationTest"; + private static final String SERVICE_ID = "registrationtest"; private static final String GATEWAY_URL = "api/v1"; private final DiscoverableClientConfig dcConfig; diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java index beb59f4ea5..354e7d7dca 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java @@ -61,7 +61,6 @@ public class ServiceDefinitionProcessor { private static final YAMLFactory YAML_FACTORY = new YAMLFactory(); private static final String ERROR_PARSING_STATIC_DEFINITION_DATA = "org.zowe.apiml.discovery.errorParsingStaticDefinitionData"; - private static final Pattern SERVICE_ID_PATTERN = Pattern.compile("^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"); public ServiceDefinitionProcessor() { } From 486fb4343c392b8108c556e1abc38957c30b0e8b Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Tue, 4 Nov 2025 21:56:49 +0100 Subject: [PATCH 13/18] remove unused improt Signed-off-by: Andrea Tabone --- .../apiml/discovery/staticdef/ServiceDefinitionProcessor.java | 1 - 1 file changed, 1 deletion(-) diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java index 354e7d7dca..e83731bc70 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java @@ -42,7 +42,6 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.util.*; -import java.util.regex.Pattern; import static org.zowe.apiml.constants.EurekaMetadataDefinition.*; From d376cb27ded1dfd0336153b6706ab8139b826702 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Tue, 4 Nov 2025 22:18:16 +0100 Subject: [PATCH 14/18] revert Signed-off-by: Andrea Tabone --- .../discovery/staticdef/ServiceDefinitionProcessor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java index e83731bc70..a6cf018b53 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessor.java @@ -29,7 +29,6 @@ import org.zowe.apiml.message.log.ApimlLogger; import org.zowe.apiml.product.discovery.*; import org.zowe.apiml.product.logging.annotations.InjectApimlLogger; -import org.zowe.apiml.util.EurekaUtils; import org.zowe.apiml.util.MapUtils; import org.zowe.apiml.util.UrlUtils; @@ -44,6 +43,7 @@ import java.util.*; import static org.zowe.apiml.constants.EurekaMetadataDefinition.*; +import static org.zowe.apiml.util.EurekaUtils.SERVICE_ID_PATTERN; /** * Processes static definition files and creates service instances @@ -209,7 +209,9 @@ private CatalogUiTile getTile(StaticRegistrationResult context, String ymlFileNa private List createInstances(StaticRegistrationResult context, String ymlFileName, Service service, Map tiles) { try { - EurekaUtils.validateServiceId(service.getServiceId()); + if (service.getServiceId() == null || !SERVICE_ID_PATTERN.matcher(service.getServiceId()).matches()) { + throw new ServiceDefinitionException(String.format("ServiceId is either not defined in the file '%s' or not conformant. The instance will not be created.", ymlFileName)); + } if (service.getInstanceBaseUrls() == null) { throw new ServiceDefinitionException(String.format("The instanceBaseUrls parameter of %s is not defined. The instance will not be created.", service.getServiceId())); From 5329ef3b00949400f8f4b80b24ed7754d95edf95 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Wed, 5 Nov 2025 11:46:14 +0100 Subject: [PATCH 15/18] increase coverage Signed-off-by: Andrea Tabone --- .../org/zowe/apiml/util/EurekaUtilsTest.java | 56 +++++++++++++++++++ .../discovery/ApimlInstanceRegistryTest.java | 6 +- .../ServiceDefinitionProcessorTest.java | 22 ++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java b/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java index 8a4b07b9d9..22913054dc 100644 --- a/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java +++ b/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java @@ -17,6 +17,7 @@ import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.netflix.eureka.EurekaServiceInstance; +import org.zowe.apiml.exception.MetadataValidationException; import java.util.Collections; import java.util.Map; @@ -112,4 +113,59 @@ void givenUnknownServiceId_whenGetInstanceInfo_thenReturnEmptyOptional() { } + @Nested + class WhenValidatingServiceId { + @Test + void givenServiceIdWithUnderscore_thenThrowMetadataValidationException() { + assertThrows(MetadataValidationException.class, () -> { + EurekaUtils.validateServiceId("service_id"); + }); + } + + @Test + void givenEmptyServiceId_thenThrowMetadataValidationException() { + assertThrows(MetadataValidationException.class, () -> { + EurekaUtils.validateServiceId(""); + }); + } + + @Test + void givenWhiteSpace_thenThrowMetadataValidationException() { + assertThrows(MetadataValidationException.class, () -> { + EurekaUtils.validateServiceId(" "); + }); + } + + @Test + void givenNonRFCFormat_thenThrowMetadataValidationException() { + // Testa quando il serviceId non rispetta il formato RFC + assertThrows(MetadataValidationException.class, () -> { + EurekaUtils.validateServiceId("Invalid@ServiceId"); + }); + } + + @Test + void testValidateServiceId_thenDoNotThrowException() { + assertDoesNotThrow(() -> { + EurekaUtils.validateServiceId("valid-service-id"); + }); + } + + @Test + void given63characters_thenDoNotThrowException() { + String validId = "a".repeat(63); + assertDoesNotThrow(() -> { + EurekaUtils.validateServiceId(validId); + }); + } + + @Test + void givenTooLongString_thenThrowMetadataValidationException() { + String tooLongId = "a".repeat(64); + assertThrows(MetadataValidationException.class, () -> { + EurekaUtils.validateServiceId(tooLongId); + }); + } + } + } diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java index 90e0f0b61b..346c70a69d 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java @@ -99,9 +99,13 @@ class GivenInvalidServiceId { private static Stream instanceIds() { return Stream.of( Arguments.of( "hostname:service_client:10010", "service_client"), + Arguments.of( "hostname:service_client:10010", ""), + Arguments.of( "hostname:10010", "service_client"), + Arguments.of( "hostname:service_client:10010", "different_service_client"), Arguments.of( "hostname:-serviceclient:10010", "-serviceclient"), Arguments.of( "hostname:serviceclient-:10010", "serviceclient-"), - Arguments.of( "hostname:invalidserviceidididididididididididdididididididididididdidididididididididid:10010", "invalidserviceidididididididididididdididididididididididdidididididididididid") + Arguments.of( "hostname:invalidserviceidididididididididididdididididididididididdidididididididididid:10010", "invalidserviceidididididididididididdididididididididididdidididididididididid"), + Arguments.of( null, "service") ); } diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java index 141cb34bc4..c06246def3 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java @@ -335,6 +335,28 @@ void givenInstanceWithoutServiceId_whenDefinitionIsLoaded_thenErrorIsReturned() ); } + @Test + void givenInstanceWithInvalidServiceId_whenDefinitionIsLoaded_thenErrorIsReturned() { + String yaml = + "services:\n" + + " - serviceId: seRvic_E@ \n" + + " title: Title\n" + + " description: Description\n" + + " catalogUiTileId: tileid\n" + + " instanceBaseUrls:\n" + + "catalogUiTiles:\n" + + " tileid:\n" + + " title: Tile Title\n" + + " description: Tile Description\n"; + + StaticRegistrationResult result = processServicesData(yaml); + + assertThatNoInstanceIsCreatedAndCorrectMessageIsProduced( + result, + "ServiceId is either not defined in the file 'test.yml' or not conformant. The instance will not be created." + ); + } + /** * Internal helper * Verify that no instance was created and that there was exactly one error produced with given message From c4f4df58d251ac473686a33d72e9ca919c84965e Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Wed, 5 Nov 2025 12:25:00 +0100 Subject: [PATCH 16/18] fix sonar complains Signed-off-by: Andrea Tabone --- .../discovery/ApimlInstanceRegistryTest.java | 7 ++-- .../ServiceDefinitionProcessorTest.java | 34 +++++++------------ 2 files changed, 14 insertions(+), 27 deletions(-) diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java index 346c70a69d..503a2bad5b 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/ApimlInstanceRegistryTest.java @@ -42,10 +42,7 @@ import java.util.concurrent.ConcurrentHashMap; 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.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doReturn; @@ -151,7 +148,7 @@ void thenShouldRegister() { MethodHandle methodHandle = mock(MethodHandle.class); ReflectionTestUtils.setField(apimlInstanceRegistry,"register2ArgsMethodHandle", methodHandle); ReflectionTestUtils.setField(apimlInstanceRegistry,"handleRegistrationMethod", methodHandle); - apimlInstanceRegistry.register(standardInstance, false); + assertDoesNotThrow(() -> apimlInstanceRegistry.register(standardInstance, false)); } } diff --git a/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java b/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java index c06246def3..16192c1432 100644 --- a/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java +++ b/discovery-service/src/test/java/org/zowe/apiml/discovery/staticdef/ServiceDefinitionProcessorTest.java @@ -15,7 +15,9 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.MethodSource; import org.zowe.apiml.message.core.Message; import org.zowe.apiml.message.core.MessageService; import org.zowe.apiml.message.log.ApimlLogger; @@ -30,6 +32,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; @@ -313,33 +316,20 @@ void givenNoInstanceInBaseUrls_whenDefinitionIsLoaded_thenErrorIsReturned() { ); } - @Test - void givenInstanceWithoutServiceId_whenDefinitionIsLoaded_thenErrorIsReturned() { - String yaml = - "services:\n" + - " - serviceId: \n" + - " title: Title\n" + - " description: Description\n" + - " catalogUiTileId: tileid\n" + - " instanceBaseUrls:\n" + - "catalogUiTiles:\n" + - " tileid:\n" + - " title: Tile Title\n" + - " description: Tile Description\n"; - - StaticRegistrationResult result = processServicesData(yaml); - - assertThatNoInstanceIsCreatedAndCorrectMessageIsProduced( - result, - "ServiceId is either not defined in the file 'test.yml' or not conformant. The instance will not be created." + private static Stream serviceIds() { + return Stream.of( + Arguments.of(""), + Arguments.of("seRvic_E@") ); } - @Test - void givenInstanceWithInvalidServiceId_whenDefinitionIsLoaded_thenErrorIsReturned() { + + @ParameterizedTest + @MethodSource("serviceIds") + void givenInstanceWithWrongServiceId_whenDefinitionIsLoaded_thenErrorIsReturned(String serviceId) { String yaml = "services:\n" + - " - serviceId: seRvic_E@ \n" + + " - serviceId: " + serviceId + " \n" + " title: Title\n" + " description: Description\n" + " catalogUiTileId: tileid\n" + From 462d2f0a2f2ecc0b952e7c091fdb5abca4fe2334 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Wed, 5 Nov 2025 14:14:24 +0100 Subject: [PATCH 17/18] address comments Signed-off-by: Andrea Tabone --- .../org/zowe/apiml/util/EurekaUtilsTest.java | 68 +++++++------------ .../discovery/ApimlInstanceRegistry.java | 12 +--- 2 files changed, 27 insertions(+), 53 deletions(-) diff --git a/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java b/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java index 22913054dc..64070e7002 100644 --- a/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java +++ b/common-service-core/src/test/java/org/zowe/apiml/util/EurekaUtilsTest.java @@ -14,6 +14,9 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.netflix.eureka.EurekaServiceInstance; @@ -21,6 +24,7 @@ import java.util.Collections; import java.util.Map; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -115,56 +119,34 @@ void givenUnknownServiceId_whenGetInstanceInfo_thenReturnEmptyOptional() { @Nested class WhenValidatingServiceId { - @Test - void givenServiceIdWithUnderscore_thenThrowMetadataValidationException() { - assertThrows(MetadataValidationException.class, () -> { - EurekaUtils.validateServiceId("service_id"); - }); - } - - @Test - void givenEmptyServiceId_thenThrowMetadataValidationException() { - assertThrows(MetadataValidationException.class, () -> { - EurekaUtils.validateServiceId(""); - }); - } - - @Test - void givenWhiteSpace_thenThrowMetadataValidationException() { - assertThrows(MetadataValidationException.class, () -> { - EurekaUtils.validateServiceId(" "); - }); - } - @Test - void givenNonRFCFormat_thenThrowMetadataValidationException() { - // Testa quando il serviceId non rispetta il formato RFC - assertThrows(MetadataValidationException.class, () -> { - EurekaUtils.validateServiceId("Invalid@ServiceId"); - }); + private static Stream validServiceIds() { + return Stream.of( + Arguments.of("valid-service-id"), + Arguments.of("a".repeat(63)) + ); } - @Test - void testValidateServiceId_thenDoNotThrowException() { - assertDoesNotThrow(() -> { - EurekaUtils.validateServiceId("valid-service-id"); - }); + private static Stream invalidServiceIds() { + return Stream.of( + Arguments.of("service_id"), + Arguments.of(""), + Arguments.of(" "), + Arguments.of("Invalid@ServiceId"), + Arguments.of("a".repeat(64)) + ); } - @Test - void given63characters_thenDoNotThrowException() { - String validId = "a".repeat(63); - assertDoesNotThrow(() -> { - EurekaUtils.validateServiceId(validId); - }); + @ParameterizedTest + @MethodSource("invalidServiceIds") + void givenServiceIdWithUnderscore_thenThrowMetadataValidationException(String serviceId) { + assertThrows(MetadataValidationException.class, () -> EurekaUtils.validateServiceId(serviceId)); } - @Test - void givenTooLongString_thenThrowMetadataValidationException() { - String tooLongId = "a".repeat(64); - assertThrows(MetadataValidationException.class, () -> { - EurekaUtils.validateServiceId(tooLongId); - }); + @ParameterizedTest + @MethodSource("validServiceIds") + void testValidateServiceId_thenDoNotThrowException(String serviceId) { + assertDoesNotThrow(() -> EurekaUtils.validateServiceId(serviceId)); } } diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java index 87b29b7579..fd2c117184 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java @@ -285,16 +285,8 @@ private void isServiceIdConformant(InstanceInfo info) { } String serviceId = EurekaUtils.getServiceIdFromInstanceId(instanceId); - if (serviceId == null) { - throw new MetadataValidationException( - "The instance ID '" + instanceId + "': must have the format 'hostname:serviceId:port'. The service will not be registered." - ); - } - if (!SERVICE_ID_PATTERN.matcher(serviceId).matches()) { - throw new MetadataValidationException( - String.format("Invalid serviceId '%s' extracted from instanceId '%s': must comply with RFC 952/1123. The service will not be registered.", serviceId, instanceId) - ); - } + EurekaUtils.validateServiceId(serviceId); + if (!serviceId.equals(appName)) { throw new MetadataValidationException( String.format("Inconsistent service identity: instanceId contains serviceId '%s' but appName='%s'. The service will not be registered.", From 1d50d018967a55acc66561eaf98d03d80358dce3 Mon Sep 17 00:00:00 2001 From: Andrea Tabone Date: Wed, 5 Nov 2025 14:22:21 +0100 Subject: [PATCH 18/18] use method Signed-off-by: Andrea Tabone --- .../apiml/discovery/ApimlInstanceRegistry.java | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java index fd2c117184..a440a6ad12 100644 --- a/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java +++ b/discovery-service/src/main/java/org/zowe/apiml/discovery/ApimlInstanceRegistry.java @@ -41,8 +41,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; -import static org.zowe.apiml.util.EurekaUtils.SERVICE_ID_PATTERN; - /** * This implementation of instance registry is solving known problem in Eureka. Discovery service notify about change * in services before it does it. From this reason listener can try to use services before they are really registered. @@ -273,18 +271,10 @@ public void register(InstanceInfo info, final boolean isReplication) { private void isServiceIdConformant(InstanceInfo info) { String instanceId = info.getInstanceId(); String appName = StringUtils.lowerCase(info.getAppName()); - if (StringUtils.isBlank(appName)) { - throw new MetadataValidationException( - "The service ID fields 'appName' must not be null or empty. The service will not be registered." - ); - } - if (!SERVICE_ID_PATTERN.matcher(appName).matches()) { - throw new MetadataValidationException( - String.format("Invalid appName '%s': must comply with RFC 952/1123 (only lowercase letters, digits, hyphens, max 63 chars). The service will not be registered.", appName) - ); - } + EurekaUtils.validateServiceId(appName); String serviceId = EurekaUtils.getServiceIdFromInstanceId(instanceId); + EurekaUtils.validateServiceId(serviceId); if (!serviceId.equals(appName)) {