From f623a1e07b16493585fb71af0e9d58326792062d Mon Sep 17 00:00:00 2001 From: Paul Latzelsperger <43503240+paullatzelsperger@users.noreply.github.com> Date: Fri, 9 Aug 2024 09:50:42 +0200 Subject: [PATCH] feat: add component ID (#4402) * feat: add component ID * simplified if-statements --- .../edc/boot/BootServicesExtension.java | 5 +- .../DefaultServiceExtensionContext.java | 34 +++++++------ .../DefaultServiceExtensionContextTest.java | 48 +++++++++++++++++-- .../iam/oauth2/Oauth2ServiceExtension.java | 2 +- .../store/SqlContractNegotiationStore.java | 4 +- .../DataplaneSelfRegistrationExtension.java | 13 ++--- ...ataplaneSelfRegistrationExtensionTest.java | 12 ++--- .../spi/system/ServiceExtensionContext.java | 18 +++---- 8 files changed, 92 insertions(+), 44 deletions(-) diff --git a/core/common/boot/src/main/java/org/eclipse/edc/boot/BootServicesExtension.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/BootServicesExtension.java index f72afc3986b..94a47f78d57 100644 --- a/core/common/boot/src/main/java/org/eclipse/edc/boot/BootServicesExtension.java +++ b/core/common/boot/src/main/java/org/eclipse/edc/boot/BootServicesExtension.java @@ -42,9 +42,12 @@ public class BootServicesExtension implements ServiceExtension { @Setting(value = "Configures the participant id this runtime is operating on behalf of") public static final String PARTICIPANT_ID = "edc.participant.id"; - @Setting(value = "Configures the runtime id", defaultValue = "") + @Setting(value = "Configures the runtime id. This should be fully or partly randomized, and need not be stable across restarts. It is recommended to leave this value blank.", defaultValue = "") public static final String RUNTIME_ID = "edc.runtime.id"; + @Setting(value = "Configures this component's ID. This should be a unique, stable and deterministic identifier.", defaultValue = "") + public static final String COMPONENT_ID = "edc.component.id"; + private HealthCheckServiceImpl healthCheckService; @Override diff --git a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/DefaultServiceExtensionContext.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/DefaultServiceExtensionContext.java index 3b985526d9d..2ea8deaa856 100644 --- a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/DefaultServiceExtensionContext.java +++ b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/DefaultServiceExtensionContext.java @@ -25,6 +25,8 @@ import java.util.Map; import java.util.UUID; +import static java.util.Optional.ofNullable; +import static org.eclipse.edc.boot.BootServicesExtension.COMPONENT_ID; import static org.eclipse.edc.boot.BootServicesExtension.RUNTIME_ID; /** @@ -33,14 +35,13 @@ */ public class DefaultServiceExtensionContext implements ServiceExtensionContext { - @Deprecated(since = "0.6.2") - private static final String EDC_CONNECTOR_NAME = "edc.connector.name"; private final Map, Object> services = new HashMap<>(); private final Config config; private boolean isReadOnly = false; private String participantId; private String runtimeId; + private String componentId; public DefaultServiceExtensionContext(Monitor monitor, Config config) { this.config = config; @@ -68,6 +69,11 @@ public String getRuntimeId() { return runtimeId; } + @Override + public String getComponentId() { + return componentId; + } + @Override public boolean hasService(Class type) { return services.containsKey(type); @@ -108,22 +114,22 @@ public void initialize() { getMonitor().warning("The runtime is configured as an anonymous participant. DO NOT DO THIS IN PRODUCTION."); } - var connectorName = getSetting(EDC_CONNECTOR_NAME, null); - if (connectorName != null) { - getMonitor().warning("Setting %s has been deprecated, please use %s instead".formatted(EDC_CONNECTOR_NAME, RUNTIME_ID)); + runtimeId = getSetting(RUNTIME_ID, null); + if (runtimeId != null) { + getMonitor().warning("A configuration value for '%s' was found. Explicitly configuring this is not supported anymore and may get removed in the future. A random value will be assigned.".formatted(RUNTIME_ID)); } - runtimeId = getSetting(RUNTIME_ID, null); - if (runtimeId == null) { - if (connectorName == null) { - getMonitor().warning("%s is not configured so a random UUID is used. It is recommended to provide a static one.".formatted(RUNTIME_ID)); - runtimeId = UUID.randomUUID().toString(); - } else { - getMonitor().warning("%s is not configured and it will fallback to the deprecated %s value".formatted(RUNTIME_ID, EDC_CONNECTOR_NAME)); - runtimeId = connectorName; - } + componentId = getSetting(COMPONENT_ID, null); + if (componentId == null) { + componentId = ofNullable(runtimeId) + .orElseGet(() -> { + getMonitor().warning("%s is not configured so a random UUID is used. It is recommended to provide a static one.".formatted(COMPONENT_ID)); + return UUID.randomUUID().toString(); + }); } + // runtime-id should always be randomized to guarantee a working lease mechanism + runtimeId = UUID.randomUUID().toString(); } } diff --git a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/DefaultServiceExtensionContextTest.java b/core/common/boot/src/test/java/org/eclipse/edc/boot/system/DefaultServiceExtensionContextTest.java index 17df25e4850..b9e860170d6 100644 --- a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/DefaultServiceExtensionContextTest.java +++ b/core/common/boot/src/test/java/org/eclipse/edc/boot/system/DefaultServiceExtensionContextTest.java @@ -29,12 +29,14 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.eclipse.edc.boot.BootServicesExtension.COMPONENT_ID; import static org.eclipse.edc.boot.BootServicesExtension.RUNTIME_ID; import static org.mockito.AdditionalMatchers.and; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -83,24 +85,62 @@ void registerService_throwsWhenFrozen() { @Nested class GetRuntimeId { @Test - void shouldReturnRandomUuid_whenNotConfigured() { + void shouldReturnRandomUuid_whenNotConfigured_andComponentIdNull() { when(config.getConfig(any())).thenReturn(ConfigFactory.empty()); context.initialize(); + var runtimeId = context.getRuntimeId(); + assertThat(UUID.fromString(runtimeId)).isNotNull(); + + verify(monitor, times(2)).warning(and(isA(String.class), argThat(message -> !message.contains(RUNTIME_ID)))); + } + + + @Test + void shouldReturnRandomId_whenComponentIdNotConfigured() { + when(config.getConfig(any())).thenReturn(ConfigFactory.fromMap(Map.of(RUNTIME_ID, "runtime-id"))); + context.initialize(); + var runtimeId = context.getRuntimeId(); assertThat(UUID.fromString(runtimeId)).isNotNull(); - verify(monitor).warning(and(isA(String.class), argThat(message -> message.startsWith(RUNTIME_ID)))); + } + } + + @Nested + class GetComponentId { + @Test + void shouldReturnRandomUuid_whenNotConfigured() { + when(config.getConfig(any())).thenReturn(ConfigFactory.empty()); + context.initialize(); + + var componentId = context.getComponentId(); + assertThat(UUID.fromString(componentId)).isNotNull(); + + verify(monitor).warning(and(isA(String.class), argThat(message -> !message.contains(COMPONENT_ID)))); } + @Test - void shouldReturnConfiguredId_whenConfigured() { + void shouldUseRuntimeId_whenComponentIdNotConfigured() { when(config.getConfig(any())).thenReturn(ConfigFactory.fromMap(Map.of(RUNTIME_ID, "runtime-id"))); context.initialize(); + var componentId = context.getComponentId(); + assertThat(componentId).isEqualTo("runtime-id"); + } + + @Test + void shouldUseConfiguredValue_whenBothAreConfigured() { + when(config.getConfig(any())).thenReturn(ConfigFactory.fromMap(Map.of(RUNTIME_ID, "runtime-id", COMPONENT_ID, "component-id"))); + + context.initialize(); + var componentId = context.getComponentId(); var runtimeId = context.getRuntimeId(); - assertThat(runtimeId).isEqualTo("runtime-id"); + assertThat(UUID.fromString(runtimeId)).isNotNull(); + assertThat(componentId).isEqualTo("component-id"); + verify(monitor).warning(and(isA(String.class), argThat(message -> !message.contains(RUNTIME_ID)))); } } diff --git a/extensions/common/iam/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceExtension.java b/extensions/common/iam/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceExtension.java index b9959779af7..f52afe44ede 100644 --- a/extensions/common/iam/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceExtension.java +++ b/extensions/common/iam/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceExtension.java @@ -174,7 +174,7 @@ private Oauth2ServiceImpl createOauth2Service(Oauth2ServiceConfiguration configu } private Oauth2ServiceConfiguration createConfig(ServiceExtensionContext context) { - var providerAudience = context.getSetting(PROVIDER_AUDIENCE, context.getRuntimeId()); + var providerAudience = context.getSetting(PROVIDER_AUDIENCE, context.getComponentId()); var endpointAudience = context.getSetting(ENDPOINT_AUDIENCE, providerAudience); var tokenUrl = context.getConfig().getString(TOKEN_URL); var publicCertificateAlias = context.getConfig().getString(PUBLIC_CERTIFICATE_ALIAS); diff --git a/extensions/control-plane/store/sql/contract-negotiation-store-sql/src/main/java/org/eclipse/edc/connector/controlplane/store/sql/contractnegotiation/store/SqlContractNegotiationStore.java b/extensions/control-plane/store/sql/contract-negotiation-store-sql/src/main/java/org/eclipse/edc/connector/controlplane/store/sql/contractnegotiation/store/SqlContractNegotiationStore.java index 2409bccacb2..fa0f9ffebb8 100644 --- a/extensions/control-plane/store/sql/contract-negotiation-store-sql/src/main/java/org/eclipse/edc/connector/controlplane/store/sql/contractnegotiation/store/SqlContractNegotiationStore.java +++ b/extensions/control-plane/store/sql/contract-negotiation-store-sql/src/main/java/org/eclipse/edc/connector/controlplane/store/sql/contractnegotiation/store/SqlContractNegotiationStore.java @@ -59,12 +59,12 @@ public class SqlContractNegotiationStore extends AbstractSqlStore implements Con public SqlContractNegotiationStore(DataSourceRegistry dataSourceRegistry, String dataSourceName, TransactionContext transactionContext, ObjectMapper objectMapper, - ContractNegotiationStatements statements, String connectorId, Clock clock, + ContractNegotiationStatements statements, String leaseHolderName, Clock clock, QueryExecutor queryExecutor) { super(dataSourceRegistry, dataSourceName, transactionContext, objectMapper, queryExecutor); this.statements = statements; this.clock = clock; - leaseContext = SqlLeaseContextBuilder.with(transactionContext, connectorId, statements, clock, queryExecutor); + leaseContext = SqlLeaseContextBuilder.with(transactionContext, leaseHolderName, statements, clock, queryExecutor); } @Override diff --git a/extensions/data-plane/data-plane-self-registration/src/main/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtension.java b/extensions/data-plane/data-plane-self-registration/src/main/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtension.java index 6adb5e9a40d..37b08b7b326 100644 --- a/extensions/data-plane/data-plane-self-registration/src/main/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtension.java +++ b/extensions/data-plane/data-plane-self-registration/src/main/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtension.java @@ -46,15 +46,12 @@ @Extension(NAME) public class DataplaneSelfRegistrationExtension implements ServiceExtension { - public static final String NAME = "Dataplane Self Registration"; - private final AtomicBoolean isRegistered = new AtomicBoolean(false); - private final AtomicReference registrationError = new AtomicReference<>("Data plane self registration not complete"); - private static final boolean DEFAULT_SELF_UNREGISTRATION = false; - + public static final String NAME = "Dataplane Self Registration"; @Setting(value = "Enable data-plane un-registration at shutdown (not suggested for clustered environments)", type = "boolean", defaultValue = DEFAULT_SELF_UNREGISTRATION + "") static final String SELF_UNREGISTRATION = "edc.data.plane.self.unregistration"; - + private final AtomicBoolean isRegistered = new AtomicBoolean(false); + private final AtomicReference registrationError = new AtomicReference<>("Data plane self registration not complete"); @Inject private DataPlaneSelectorService dataPlaneSelectorService; @Inject @@ -86,7 +83,7 @@ public void start() { ); var instance = DataPlaneInstance.Builder.newInstance() - .id(context.getRuntimeId()) + .id(context.getComponentId()) .url(controlApiUrl.get().toString() + "/v1/dataflows") .allowedSourceTypes(pipelineService.supportedSourceTypes()) .allowedDestTypes(pipelineService.supportedSinkTypes()) @@ -112,7 +109,7 @@ public void start() { @Override public void shutdown() { if (context.getConfig().getBoolean(SELF_UNREGISTRATION, DEFAULT_SELF_UNREGISTRATION)) { - dataPlaneSelectorService.unregister(context.getRuntimeId()) + dataPlaneSelectorService.unregister(context.getComponentId()) .onSuccess(it -> context.getMonitor().info("data plane successfully unregistered")) .onFailure(failure -> context.getMonitor().severe("error during data plane de-registration. %s: %s" .formatted(failure.getReason(), failure.getFailureDetail()))); diff --git a/extensions/data-plane/data-plane-self-registration/src/test/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtensionTest.java b/extensions/data-plane/data-plane-self-registration/src/test/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtensionTest.java index 8f38d30a367..e8e51baee06 100644 --- a/extensions/data-plane/data-plane-self-registration/src/test/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtensionTest.java +++ b/extensions/data-plane/data-plane-self-registration/src/test/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtensionTest.java @@ -63,14 +63,14 @@ void setUp(ServiceExtensionContext context) { context.registerService(PipelineService.class, pipelineService); context.registerService(PublicEndpointGeneratorService.class, publicEndpointGeneratorService); var monitor = mock(Monitor.class); - when(monitor.withPrefix(anyString())).thenReturn(mock()); + when(monitor.withPrefix(anyString())).thenReturn(monitor); context.registerService(Monitor.class, monitor); context.registerService(HealthCheckService.class, healthCheckService); } @Test void shouldRegisterInstanceAtStartup(DataplaneSelfRegistrationExtension extension, ServiceExtensionContext context) throws MalformedURLException { - when(context.getRuntimeId()).thenReturn("runtimeId"); + when(context.getComponentId()).thenReturn("componentId"); when(controlApiUrl.get()).thenReturn(URI.create("http://control/api/url")); when(pipelineService.supportedSinkTypes()).thenReturn(Set.of("sinkType", "anotherSinkType")); when(pipelineService.supportedSourceTypes()).thenReturn(Set.of("sourceType", "anotherSourceType")); @@ -83,7 +83,7 @@ void shouldRegisterInstanceAtStartup(DataplaneSelfRegistrationExtension extensio var captor = ArgumentCaptor.forClass(DataPlaneInstance.class); verify(dataPlaneSelectorService).addInstance(captor.capture()); var dataPlaneInstance = captor.getValue(); - assertThat(dataPlaneInstance.getId()).isEqualTo("runtimeId"); + assertThat(dataPlaneInstance.getId()).isEqualTo("componentId"); assertThat(dataPlaneInstance.getUrl()).isEqualTo(new URL("http://control/api/url/v1/dataflows")); assertThat(dataPlaneInstance.getAllowedSourceTypes()).containsExactlyInAnyOrder("sourceType", "anotherSourceType"); assertThat(dataPlaneInstance.getAllowedDestTypes()).containsExactlyInAnyOrder("sinkType", "anotherSinkType"); @@ -107,7 +107,7 @@ void shouldNotStart_whenRegistrationFails(DataplaneSelfRegistrationExtension ext @Test void shouldNotUnregisterInstanceAtShutdown(DataplaneSelfRegistrationExtension extension, ServiceExtensionContext context) { - when(context.getRuntimeId()).thenReturn("runtimeId"); + when(context.getComponentId()).thenReturn("componentId"); when(dataPlaneSelectorService.unregister(any())).thenReturn(ServiceResult.success()); extension.initialize(context); @@ -118,13 +118,13 @@ void shouldNotUnregisterInstanceAtShutdown(DataplaneSelfRegistrationExtension ex @Test void shouldUnregisterInstanceAtShutdown_whenConfigured(DataplaneSelfRegistrationExtension extension, ServiceExtensionContext context) { - when(context.getRuntimeId()).thenReturn("runtimeId"); + when(context.getComponentId()).thenReturn("componentId"); when(context.getConfig()).thenReturn(ConfigFactory.fromMap(Map.of(SELF_UNREGISTRATION, "true"))); when(dataPlaneSelectorService.unregister(any())).thenReturn(ServiceResult.success()); extension.initialize(context); extension.shutdown(); - verify(dataPlaneSelectorService).unregister("runtimeId"); + verify(dataPlaneSelectorService).unregister("componentId"); } } diff --git a/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/system/ServiceExtensionContext.java b/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/system/ServiceExtensionContext.java index 88c36f8d496..6864a0390cf 100644 --- a/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/system/ServiceExtensionContext.java +++ b/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/system/ServiceExtensionContext.java @@ -40,22 +40,24 @@ default void freeze() { String getParticipantId(); /** - * Return the id of the runtime. If not configured a random UUID is used. + * Return the id of the runtime. A runtime is a physical process. If {@code edc.runtime.id} is not configured, a random UUID is used. + *
It is recommended to leave this configuration blank.. * * @return the runtime id. */ String getRuntimeId(); /** - * Fetches the unique ID of the connector. If the {@code dataspaceconnector.connector.name} config value has been set, that value is returned; otherwise a random - * name is chosen. + * Returns the id of the component. A component is a logical unit of deployment, consisting of 1...N runtimes. If it is not + * configured, but the runtime ID is configured, then the value of the runtime ID will be used. If neither runtime ID + * nor component ID are used, (individual) random values are generated. + *

+ *
It is recommended to provide a stable value for this configuration. * - * @deprecated use {{@link #getRuntimeId()}} instead. + * @return the component id. */ - @Deprecated(since = "0.6.2") - default String getConnectorId() { - return getRuntimeId(); - } + String getComponentId(); + /** * Returns the system monitor.