Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@

use Illuminate\Http\Request;
use ProcessMaker\Http\Controllers\Controller;
use ProcessMaker\Services\ScriptMicroserviceService;

class ScriptExecutorController extends Controller
{
public function index(Request $request)
public function index(Request $request, ScriptMicroserviceService $service)
{
if (!config('app.custom_executors')) {
abort(404);
}

return view('admin.script-executors.index');
return view('admin.script-executors.index',
[
'script_microservice_enabled' => config('script-runner-microservice.enabled'),
'script_microservice_instance_uuid' => $service->getInstanceUuid(),
]);
}
}
51 changes: 32 additions & 19 deletions ProcessMaker/Http/Controllers/Api/ScriptExecutorController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use ProcessMaker\Jobs\BuildScriptExecutor;
use ProcessMaker\Models\Script;
use ProcessMaker\Models\ScriptExecutor;
use ProcessMaker\Services\ScriptMicroserviceService;

class ScriptExecutorController extends Controller
{
Expand All @@ -26,7 +27,7 @@ class ScriptExecutorController extends Controller
* @return ResponseFactory|Response
*
*
* @OA\Get(
* @OA\Get(
* path="/script-executors",
* summary="Returns all script executors that the user has access to",
* operationId="getScriptExecutors",
Expand Down Expand Up @@ -79,7 +80,7 @@ public function index(Request $request)
* @return ResponseFactory|Response
*
*
* @OA\Post(
* @OA\Post(
* path="/script-executors",
* summary="Create a script executor",
* operationId="createScriptExecutor",
Expand Down Expand Up @@ -109,7 +110,7 @@ public function index(Request $request)
* ),
* )
*/
public function store(Request $request)
public function store(Request $request, ScriptMicroserviceService $service)
{
$request->request->add(['type' => 'custom']);
$this->checkAuth($request);
Expand All @@ -119,11 +120,14 @@ public function store(Request $request)
$request->only((new ScriptExecutor())->getFillable())
);

ScriptExecutorCreated::dispatch($scriptExecutor->getAttributes());

BuildScriptExecutor::dispatch($scriptExecutor->id, $request->user()->id);
if (!config('script-runner-microservice.enabled')) {
ScriptExecutorCreated::dispatch($scriptExecutor->getAttributes());
BuildScriptExecutor::dispatch($scriptExecutor->id, $request->user()->id);
} else {
$service->createCustomExecutor($scriptExecutor);
}

return ['status'=>'started', 'id' => $scriptExecutor->id];
return ['status' => 'started', 'id' => $scriptExecutor->id];
}

/**
Expand All @@ -135,7 +139,7 @@ public function store(Request $request)
* @return ResponseFactory|Response
*
*
* @OA\Put(
* @OA\Put(
* path="/script-executors/{script_executor}",
* summary="Update script executor",
* operationId="updateScriptExecutor",
Expand Down Expand Up @@ -173,7 +177,7 @@ public function store(Request $request)
* )
* )
*/
public function update(Request $request, ScriptExecutor $scriptExecutor)
public function update(Request $request, ScriptExecutor $scriptExecutor, ScriptMicroserviceService $service)
{
$this->checkAuth($request);
$request->validate(ScriptExecutor::rules());
Expand All @@ -184,13 +188,16 @@ public function update(Request $request, ScriptExecutor $scriptExecutor)
$request->only($scriptExecutor->getFillable())
);

if (!empty($scriptExecutor->getChanges())) {
ScriptExecutorUpdated::dispatch($scriptExecutor->id, $original, $scriptExecutor->getChanges());
if (!config('script-runner-microservice.enabled')) {
if (!empty($scriptExecutor->getChanges())) {
ScriptExecutorUpdated::dispatch($scriptExecutor->id, $original, $scriptExecutor->getChanges());
}
BuildScriptExecutor::dispatch($scriptExecutor->id, $request->user()->id);
} else {
$service->updateCustomExecutor($scriptExecutor);
}

BuildScriptExecutor::dispatch($scriptExecutor->id, $request->user()->id);

return ['status'=>'started'];
return ['status' => 'started'];
}

/**
Expand All @@ -202,7 +209,7 @@ public function update(Request $request, ScriptExecutor $scriptExecutor)
* @return ResponseFactory|Response
*
*
* @OA\Delete(
* @OA\Delete(
* path="/script-executors/{script_executor}",
* summary="Delete a script executor",
* operationId="deleteScriptExecutor",
Expand Down Expand Up @@ -233,7 +240,7 @@ public function update(Request $request, ScriptExecutor $scriptExecutor)
* ),
* )
*/
public function delete(Request $request, ScriptExecutor $scriptExecutor)
public function delete(Request $request, ScriptExecutor $scriptExecutor, ScriptMicroserviceService $service)
{
if ($scriptExecutor->scripts()->count() > 0) {
throw ValidationException::withMessages(['delete' => __('Can not delete executor when it is assigned to scripts.')]);
Expand All @@ -254,9 +261,15 @@ public function delete(Request $request, ScriptExecutor $scriptExecutor)
}
}

$scriptExecutorUUID = $scriptExecutor->uuid;

ScriptExecutor::destroy($scriptExecutor->id);

ScriptExecutorDeleted::dispatch($scriptExecutor->getAttributes());
if (!config('script-runner-microservice.enabled')) {
ScriptExecutorDeleted::dispatch($scriptExecutor->getAttributes());
} else {
$service->deleteCustomExecutor($scriptExecutorUUID);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete order causes inconsistency if microservice call fails

The delete method destroys the local database record on line 266 before attempting to delete from the microservice on line 271. If deleteCustomExecutor() throws an exception (e.g., network error, microservice unavailable), the local record is already permanently deleted while the remote executor still exists. This creates a data inconsistency where the microservice has orphaned executors that can't be managed. The microservice deletion needs to happen before or atomically with the local database deletion.

Fix in Cursor Fix in Web


return ['status' => 'done'];
}
Expand All @@ -280,7 +293,7 @@ private function checkAuth($request)
* @return ResponseFactory|Response
*
*
* @OA\Post(
* @OA\Post(
* path="/script-executors/cancel",
* summary="Cancel a script executor",
* operationId="cancelScriptExecutor",
Expand Down Expand Up @@ -327,7 +340,7 @@ public function cancel(Request $request)
* @return ResponseFactory|Response
*
*
* @OA\Get(
* @OA\Get(
* path="/script-executors/available-languages",
* summary="Returns all available languages",
* operationId="getAvailableLanguages",
Expand Down
60 changes: 16 additions & 44 deletions ProcessMaker/ScriptRunners/ScriptMicroserviceRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@

use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Log;
use ProcessMaker\Enums\ScriptExecutorType;
use ProcessMaker\Exception\ConfigurationException;
use ProcessMaker\GenerateAccessToken;
use ProcessMaker\Jobs\ErrorHandling;
use ProcessMaker\Models\EnvironmentVariable;
use ProcessMaker\Models\Script;
use ProcessMaker\Models\User;
use ProcessMaker\Services\ScriptMicroserviceService;
use stdClass;

class ScriptMicroserviceRunner
Expand All @@ -20,51 +21,26 @@ class ScriptMicroserviceRunner

private string $language;

private ScriptMicroserviceService $service;

public function __construct(protected Script $script)
{
$this->language = strtolower($script->language ?? $script->scriptExecutor->language);
}

public function getAccessToken()
{
if (Cache::has('keycloak.access_token')) {
return Cache::get('keycloak.access_token');
}

$response = Http::asForm()->post(config('script-runner-microservice.keycloak.base_url') ?? '', [
'grant_type' => 'password',
'client_id' => config('script-runner-microservice.keycloak.client_id'),
'client_secret' => config('script-runner-microservice.keycloak.client_secret'),
'username' => config('script-runner-microservice.keycloak.username'),
'password' => config('script-runner-microservice.keycloak.password'),
]);

if ($response->successful()) {
Cache::put('keycloak.access_token', $response->json()['access_token'], $response->json()['expires_in'] - 60);
}

return Cache::get('keycloak.access_token');
}

public function getScriptRunner()
{
$response = Cache::remember('script-runner-microservice.script-languages', now()->addDay(), function () {
return Http::withToken($this->getAccessToken())
->get(config('script-runner-microservice.base_url') . '/scripts')->collect();
});

return $response->filter(function ($item) {
return $item['language'] == $this->language;
})->first();
$this->service = new ScriptMicroserviceService();
}

public function run($code, array $data, array $config, $timeout, $user, $sync, $metadata)
{
Log::debug('Language: ' . $this->language);
Log::debug('Sync: ' . $sync);
Log::debug('Metadata: ' . print_r($metadata, true));
Log::debug('Script type: ' . $this->script->scriptExecutor->type?->value);

$scriptRunner = $this->getScriptRunner();
$scriptRunner = $this->service->getScriptRunner(
$this->language,
$this->script->scriptExecutor->uuid,
$this->script->scriptExecutor->type === ScriptExecutorType::Custom
);

if (!$scriptRunner) {
throw new ConfigurationException('No exists script executor for this language: ' . $this->language);
Expand All @@ -75,7 +51,7 @@ public function run($code, array $data, array $config, $timeout, $user, $sync, $
$payload = [
'version' => config('script-runner-microservice.version') ?? $this->getProcessMakerVersion(),
'language' => $scriptRunner['language'],
'metadata'=> $metadata,
'metadata' => $metadata,
'data' => !empty($data) ? $this->sanitizeCss($data) : new stdClass(),
'config' => !empty($config) ? $config : new stdClass(),
'script' => base64_encode(str_replace("'", ''', $code)),
Expand All @@ -90,14 +66,7 @@ public function run($code, array $data, array $config, $timeout, $user, $sync, $

Log::debug('Payload: ' . print_r($payload, true));

// Set a theoretical maximum timeout of 1 day (86400 seconds)
// since the laravel client must have a timeout set.
// The actual script timeout will be handled by the microservice.
$clientTimeout = 86400;

$response = Http::timeout($clientTimeout)
->withToken($this->getAccessToken())
->post(config('script-runner-microservice.base_url') . '/requests/create', $payload);
$response = $this->service->sendScriptPayload($payload);

$response->throw();

Expand Down Expand Up @@ -159,7 +128,10 @@ public function getMetadata($user)
{
return [
'script_id' => $this->script->id,
'executor_uuid' => $this->script->scriptExecutor->uuid,
'executor_type' => $this->script->scriptExecutor->type?->value,
'instance' => config('app.url'),
'instance_uuid' => $this->service->getInstanceUuid(),
'user_id' => $user->id,
'user_email' => $user->email,
];
Expand Down
2 changes: 1 addition & 1 deletion ProcessMaker/ScriptRunners/ScriptRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function run($code, array $data, array $config, $timeout, $user, $sync, $
*/
private function getScriptRunner(ScriptExecutor $executor): Base|ScriptMicroserviceRunner|MockRunner
{
if (!config('script-runner-microservice.enabled') || $executor->type === ScriptExecutorType::Custom) {
if (!config('script-runner-microservice.enabled')) {
$language = strtolower($executor->language);
$runner = config("script-runners.{$language}.runner");
if (!$runner) {
Expand Down
Loading
Loading