This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 78de8bb91e harden self referencing remote run configurations, fixes
#4086 (#7555)
78de8bb91e is described below
commit 78de8bb91e86b0536d4e8c41091e793e919eae37
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Jul 17 16:00:31 2026 +0200
harden self referencing remote run configurations, fixes #4086 (#7555)
---
.../engines/remote/RemotePipelineEngine.java | 47 +++++-
.../engines/remote/RemoteWorkflowEngine.java | 47 +++++-
.../engines/remote/RemotePipelineEngineTest.java | 159 +++++++++++++++++++++
.../engines/remote/RemoteWorkflowEngineTest.java | 102 +++++++++++++
4 files changed, 343 insertions(+), 12 deletions(-)
diff --git
a/engine/src/main/java/org/apache/hop/pipeline/engines/remote/RemotePipelineEngine.java
b/engine/src/main/java/org/apache/hop/pipeline/engines/remote/RemotePipelineEngine.java
index cfb90733bf..cf867751f5 100644
---
a/engine/src/main/java/org/apache/hop/pipeline/engines/remote/RemotePipelineEngine.java
+++
b/engine/src/main/java/org/apache/hop/pipeline/engines/remote/RemotePipelineEngine.java
@@ -195,6 +195,46 @@ public class RemotePipelineEngine extends Variables
implements IPipelineEngine<P
return new RemotePipelineRunConfiguration();
}
+ /**
+ * A remote run configuration names the run configuration the pipeline is
executed with on the
+ * server. That one can be a remote run configuration again, which hands the
pipeline to yet
+ * another server. Such a chain has to end somewhere: when it leads back to
a run configuration it
+ * already passed, every server in the chain keeps handing the pipeline to
the next one and the
+ * pipeline is registered over and over again. See issue #4086.
+ *
+ * <p>Only the run configurations this client can see are followed. A chain
that continues into a
+ * run configuration that only exists on the server is left to that server
to sort out.
+ *
+ * @param runConfiguration the remote run configuration to start from
+ * @throws HopException when the chain leads back to a run configuration it
already passed
+ */
+ private void validateRunConfigurationChain(PipelineRunConfiguration
runConfiguration)
+ throws HopException {
+ List<String> chain = new ArrayList<>();
+ chain.add(runConfiguration.getName());
+
+ PipelineRunConfiguration current = runConfiguration;
+ while (current != null
+ && current.getEngineRunConfiguration() instanceof
RemotePipelineRunConfiguration remote) {
+ String linkedName = resolve(remote.getRunConfigurationName());
+ if (StringUtils.isEmpty(linkedName)) {
+ // Reported for the run configuration this engine was asked to run
with.
+ //
+ return;
+ }
+ if (chain.contains(linkedName)) {
+ chain.add(linkedName);
+ throw new HopException(
+ "The remote pipeline run configuration leads back to itself: "
+ + String.join(" -> ", chain)
+ + ". The run configuration to run the pipeline with on the
server should not lead "
+ + "back to a remote run configuration.");
+ }
+ chain.add(linkedName);
+ current =
metadataProvider.getSerializer(PipelineRunConfiguration.class).load(linkedName);
+ }
+ }
+
@Override
public void prepareExecution() throws HopException {
try {
@@ -215,18 +255,13 @@ public class RemotePipelineEngine extends Variables
implements IPipelineEngine<P
if (StringUtils.isEmpty(remoteRunConfigurationName)) {
throw new HopException("No run configuration was specified to the
remote pipeline with");
}
- if
(pipelineRunConfiguration.getName().equals(remoteRunConfigurationName)) {
- throw new HopException(
- "The remote pipeline run configuration refers to itself '"
- + remoteRunConfigurationName
- + "'");
- }
if (metadataProvider == null) {
throw new HopException(
"The remote pipeline engine didn't receive a metadata to load hop
server '"
+ hopServerName
+ "'");
}
+ validateRunConfigurationChain(pipelineRunConfiguration);
// Create a new log channel when we start the action
// It's only now that we use it
diff --git
a/engine/src/main/java/org/apache/hop/workflow/engines/remote/RemoteWorkflowEngine.java
b/engine/src/main/java/org/apache/hop/workflow/engines/remote/RemoteWorkflowEngine.java
index 31ce63a55f..973a448a7d 100644
---
a/engine/src/main/java/org/apache/hop/workflow/engines/remote/RemoteWorkflowEngine.java
+++
b/engine/src/main/java/org/apache/hop/workflow/engines/remote/RemoteWorkflowEngine.java
@@ -202,6 +202,46 @@ public class RemoteWorkflowEngine extends Variables
implements IWorkflowEngine<W
return workflowMeta == null ? null : workflowMeta.getName();
}
+ /**
+ * A remote run configuration names the run configuration the workflow is
executed with on the
+ * server. That one can be a remote run configuration again, which hands the
workflow to yet
+ * another server. Such a chain has to end somewhere: when it leads back to
a run configuration it
+ * already passed, every server in the chain keeps handing the workflow to
the next one and the
+ * workflow is registered over and over again. See issue #4086.
+ *
+ * <p>Only the run configurations this client can see are followed. A chain
that continues into a
+ * run configuration that only exists on the server is left to that server
to sort out.
+ *
+ * @param runConfiguration the remote run configuration to start from
+ * @throws HopException when the chain leads back to a run configuration it
already passed
+ */
+ void validateRunConfigurationChain(WorkflowRunConfiguration runConfiguration)
+ throws HopException {
+ List<String> chain = new ArrayList<>();
+ chain.add(runConfiguration.getName());
+
+ WorkflowRunConfiguration current = runConfiguration;
+ while (current != null
+ && current.getEngineRunConfiguration() instanceof
RemoteWorkflowRunConfiguration remote) {
+ String linkedName = resolve(remote.getRunConfigurationName());
+ if (StringUtils.isEmpty(linkedName)) {
+ // Reported for the run configuration this engine was asked to run
with.
+ //
+ return;
+ }
+ if (chain.contains(linkedName)) {
+ chain.add(linkedName);
+ throw new HopException(
+ "The remote workflow run configuration leads back to itself: "
+ + String.join(" -> ", chain)
+ + ". The run configuration to run the workflow with on the
server should not lead "
+ + "back to a remote run configuration.");
+ }
+ chain.add(linkedName);
+ current =
metadataProvider.getSerializer(WorkflowRunConfiguration.class).load(linkedName);
+ }
+ }
+
@Override
public Result startExecution() {
try {
@@ -249,18 +289,13 @@ public class RemoteWorkflowEngine extends Variables
implements IWorkflowEngine<W
if (StringUtils.isEmpty(remoteRunConfigurationName)) {
throw new HopException("No run configuration was specified to the
remote workflow with");
}
- if
(workflowRunConfiguration.getName().equals(remoteRunConfigurationName)) {
- throw new HopException(
- "The remote workflow run configuration refers to itself '"
- + remoteRunConfigurationName
- + "'");
- }
if (metadataProvider == null) {
throw new HopException(
"The remote workflow engine didn't receive a metadata to load hop
server '"
+ hopServerName
+ "'");
}
+ validateRunConfigurationChain(workflowRunConfiguration);
logChannel.logBasic(
"Executing this workflow using the Remote Workflow Engine with run
configuration '"
diff --git
a/engine/src/test/java/org/apache/hop/pipeline/engines/remote/RemotePipelineEngineTest.java
b/engine/src/test/java/org/apache/hop/pipeline/engines/remote/RemotePipelineEngineTest.java
new file mode 100644
index 0000000000..0bc094afe2
--- /dev/null
+++
b/engine/src/test/java/org/apache/hop/pipeline/engines/remote/RemotePipelineEngineTest.java
@@ -0,0 +1,159 @@
+/*
+ * 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.hop.pipeline.engines.remote;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.ArrayList;
+import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.logging.LogLevel;
+import org.apache.hop.metadata.api.IHopMetadataProvider;
+import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
+import org.apache.hop.pipeline.PipelineMeta;
+import org.apache.hop.pipeline.config.PipelineRunConfiguration;
+import org.apache.hop.pipeline.engines.local.LocalPipelineRunConfiguration;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The run configuration a remote run configuration hands the pipeline to is
used on the server.
+ * When that leads back to a remote run configuration the pipeline keeps being
handed on and
+ * registered again and again. See issue #4086.
+ */
+class RemotePipelineEngineTest {
+
+ private static final String SERVER_NAME = "a-server";
+
+ @BeforeAll
+ static void setUpBeforeClass() throws HopException {
+ HopEnvironment.init();
+ }
+
+ /** A remote run configuration that names itself never reaches a server that
would run it. */
+ @Test
+ void runConfigurationThatRefersToItselfIsRejected() throws Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ save(metadataProvider, remote("remote", "remote"));
+
+ HopException e = assertThrows(HopException.class, () ->
prepare(metadataProvider, "remote"));
+
+ assertTrue(e.getMessage().contains("remote -> remote"), "The chain should
be reported: " + e);
+ }
+
+ /**
+ * The name of the run configuration to use on the server can hold a
variable, which the check has
+ * to resolve before it can see that it leads back to itself.
+ */
+ @Test
+ void runConfigurationThatRefersToItselfThroughAVariableIsRejected() throws
Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ save(metadataProvider, remote("remote", "${RUN_CONFIG}"));
+
+ RemotePipelineEngine engine = engine(metadataProvider, "remote");
+ engine.setVariable("RUN_CONFIG", "remote");
+
+ HopException e = assertThrows(HopException.class,
engine::prepareExecution);
+
+ assertTrue(e.getMessage().contains("remote -> remote"), "The chain should
be reported: " + e);
+ }
+
+ /** A chain of remote run configurations that leads back to an earlier one
is rejected. */
+ @Test
+ void cyclicalRunConfigurationChainIsRejected() throws Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ save(metadataProvider, remote("first", "second"));
+ save(metadataProvider, remote("second", "first"));
+
+ HopException e = assertThrows(HopException.class, () ->
prepare(metadataProvider, "first"));
+
+ assertTrue(
+ e.getMessage().contains("first -> second -> first"), "The chain should
be reported: " + e);
+ }
+
+ /**
+ * A remote run configuration that hands the pipeline to a local one is what
a remote run
+ * configuration is meant to do, so it may not be rejected. It fails later
on, when it looks for
+ * the server that does not exist here.
+ */
+ @Test
+ void runConfigurationThatLeadsToALocalOneIsAccepted() throws Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ save(metadataProvider, remote("remote", "local"));
+ save(metadataProvider, local("local"));
+
+ HopException e = assertThrows(HopException.class, () ->
prepare(metadataProvider, "remote"));
+
+ assertTrue(
+ e.getMessage().contains(SERVER_NAME),
+ "It should get as far as looking for the server: " + e);
+ }
+
+ /**
+ * A run configuration that only exists on the server cannot be followed
from here, which may not
+ * be reported as a problem.
+ */
+ @Test
+ void runConfigurationThatIsUnknownHereIsAccepted() throws Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ save(metadataProvider, remote("remote", "only-known-on-the-server"));
+
+ HopException e = assertThrows(HopException.class, () ->
prepare(metadataProvider, "remote"));
+
+ assertTrue(
+ e.getMessage().contains(SERVER_NAME),
+ "It should get as far as looking for the server: " + e);
+ }
+
+ private static PipelineRunConfiguration remote(String name, String
runConfigurationName) {
+ RemotePipelineRunConfiguration engineConfiguration = new
RemotePipelineRunConfiguration();
+ engineConfiguration.setEnginePluginId("Remote");
+ engineConfiguration.setHopServerName(SERVER_NAME);
+ engineConfiguration.setRunConfigurationName(runConfigurationName);
+ return new PipelineRunConfiguration(
+ name, "", null, new ArrayList<>(), engineConfiguration, null, false);
+ }
+
+ private static PipelineRunConfiguration local(String name) {
+ LocalPipelineRunConfiguration engineConfiguration = new
LocalPipelineRunConfiguration();
+ engineConfiguration.setEnginePluginId("Local");
+ return new PipelineRunConfiguration(
+ name, "", null, new ArrayList<>(), engineConfiguration, null, false);
+ }
+
+ private static void save(IHopMetadataProvider metadataProvider,
PipelineRunConfiguration c)
+ throws HopException {
+ metadataProvider.getSerializer(PipelineRunConfiguration.class).save(c);
+ }
+
+ private static void prepare(IHopMetadataProvider metadataProvider, String
name) throws Exception {
+ engine(metadataProvider, name).prepareExecution();
+ }
+
+ private static RemotePipelineEngine engine(IHopMetadataProvider
metadataProvider, String name)
+ throws HopException {
+ RemotePipelineEngine engine = new RemotePipelineEngine();
+ engine.setLogLevel(LogLevel.BASIC);
+ engine.setMetadataProvider(metadataProvider);
+ engine.setPipelineRunConfiguration(
+
metadataProvider.getSerializer(PipelineRunConfiguration.class).load(name));
+ engine.setPipelineMeta(new PipelineMeta());
+ return engine;
+ }
+}
diff --git
a/engine/src/test/java/org/apache/hop/workflow/engines/remote/RemoteWorkflowEngineTest.java
b/engine/src/test/java/org/apache/hop/workflow/engines/remote/RemoteWorkflowEngineTest.java
index d8d38ce701..af1f8170c3 100644
---
a/engine/src/test/java/org/apache/hop/workflow/engines/remote/RemoteWorkflowEngineTest.java
+++
b/engine/src/test/java/org/apache/hop/workflow/engines/remote/RemoteWorkflowEngineTest.java
@@ -17,8 +17,11 @@
package org.apache.hop.workflow.engines.remote;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
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.anyInt;
import static org.mockito.ArgumentMatchers.eq;
@@ -26,9 +29,13 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import org.apache.hop.core.Result;
+import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.gui.WorkflowTracker;
+import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
import org.apache.hop.workflow.ActionResult;
import org.apache.hop.workflow.WorkflowMeta;
+import org.apache.hop.workflow.config.WorkflowRunConfiguration;
+import org.apache.hop.workflow.engines.local.LocalWorkflowRunConfiguration;
import org.apache.hop.www.HopServerWorkflowStatus;
import org.apache.hop.www.RemoteHopServer;
import org.junit.jupiter.api.Test;
@@ -103,6 +110,101 @@ class RemoteWorkflowEngineTest {
return engine;
}
+ /**
+ * The run configuration a remote run configuration hands the workflow to is
used on the server.
+ * When that leads back to a remote run configuration the workflow keeps
being handed on and
+ * registered again and again. See issue #4086.
+ */
+ @Test
+ void runConfigurationThatRefersToItselfIsRejected() throws Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ saveRunConfiguration(metadataProvider, remoteRunConfiguration("remote",
"remote"));
+
+ HopException e =
+ assertThrows(HopException.class, () -> validateChain(metadataProvider,
"remote"));
+
+ assertTrue(e.getMessage().contains("remote -> remote"), "The chain should
be reported: " + e);
+ }
+
+ /** The name of the run configuration to use on the server can hold a
variable. */
+ @Test
+ void runConfigurationThatRefersToItselfThroughAVariableIsRejected() throws
Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ saveRunConfiguration(metadataProvider, remoteRunConfiguration("remote",
"${RUN_CONFIG}"));
+
+ RemoteWorkflowEngine engine = new RemoteWorkflowEngine();
+ engine.setMetadataProvider(metadataProvider);
+ engine.setVariable("RUN_CONFIG", "remote");
+
+ HopException e =
+ assertThrows(
+ HopException.class,
+ () ->
+ engine.validateRunConfigurationChain(
+
metadataProvider.getSerializer(WorkflowRunConfiguration.class).load("remote")));
+
+ assertTrue(e.getMessage().contains("remote -> remote"), "The chain should
be reported: " + e);
+ }
+
+ /** A chain of remote run configurations that leads back to an earlier one
is rejected. */
+ @Test
+ void cyclicalRunConfigurationChainIsRejected() throws Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ saveRunConfiguration(metadataProvider, remoteRunConfiguration("first",
"second"));
+ saveRunConfiguration(metadataProvider, remoteRunConfiguration("second",
"first"));
+
+ HopException e =
+ assertThrows(HopException.class, () -> validateChain(metadataProvider,
"first"));
+
+ assertTrue(
+ e.getMessage().contains("first -> second -> first"), "The chain should
be reported: " + e);
+ }
+
+ /** Handing the workflow to a local run configuration is what a remote one
is meant to do. */
+ @Test
+ void runConfigurationThatLeadsToALocalOneIsAccepted() throws Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ saveRunConfiguration(metadataProvider, remoteRunConfiguration("remote",
"local"));
+ saveRunConfiguration(
+ metadataProvider,
+ new WorkflowRunConfiguration(
+ "local", "", null, new LocalWorkflowRunConfiguration(), false));
+
+ assertDoesNotThrow(() -> validateChain(metadataProvider, "remote"));
+ }
+
+ /** A run configuration that only exists on the server cannot be followed
from here. */
+ @Test
+ void runConfigurationThatIsUnknownHereIsAccepted() throws Exception {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ saveRunConfiguration(
+ metadataProvider, remoteRunConfiguration("remote",
"only-known-on-the-server"));
+
+ assertDoesNotThrow(() -> validateChain(metadataProvider, "remote"));
+ }
+
+ private static WorkflowRunConfiguration remoteRunConfiguration(
+ String name, String runConfigurationName) {
+ RemoteWorkflowRunConfiguration engineConfiguration = new
RemoteWorkflowRunConfiguration();
+ engineConfiguration.setHopServerName("a-server");
+ engineConfiguration.setRunConfigurationName(runConfigurationName);
+ return new WorkflowRunConfiguration(name, "", null, engineConfiguration,
false);
+ }
+
+ private static void saveRunConfiguration(
+ MemoryMetadataProvider metadataProvider, WorkflowRunConfiguration
runConfiguration)
+ throws HopException {
+
metadataProvider.getSerializer(WorkflowRunConfiguration.class).save(runConfiguration);
+ }
+
+ private static void validateChain(MemoryMetadataProvider metadataProvider,
String name)
+ throws HopException {
+ RemoteWorkflowEngine engine = new RemoteWorkflowEngine();
+ engine.setMetadataProvider(metadataProvider);
+ engine.validateRunConfigurationChain(
+
metadataProvider.getSerializer(WorkflowRunConfiguration.class).load(name));
+ }
+
private WorkflowTracker<WorkflowMeta> createServerTracker() {
WorkflowTracker<WorkflowMeta> serverTracker = new
WorkflowTracker<>((WorkflowMeta) null);
serverTracker.setWorkflowName(WORKFLOW_NAME);
