Add phase-configs support to Hook model for target-app resolution by stiv03 · Pull Request #1812 · cloudfoundry/multiapps-controller

stiv03 · 2026-04-03T08:17:13Z

No description provided.

…reen deployments Route hook task execution to idle or live app based on phase-configs in the hook descriptor. Pass idleMtaColor, liveMtaColor, subprocessPhase and phase variables to all hook call activities in BPMN processes. Add determineDeploymentTypeSafely to avoid SERVICE_ID lookup failures in hook subprocesses. # Conflicts: # pom.xml

buildCurrentPhaseString was always picking the first phase from hook.getPhases(), causing wrong target-app resolution when a hook registered for multiple phases fired during a later phase. Also fixed getDeploymentTypeString returning "deploy" instead of "blue-green" inside hook subprocesses where SERVICE_ID is absent. Introduced HOOK_EXECUTION_PHASE variable set by HooksExecutor to record which phase triggered the current hook execution. ExecuteTaskStep now matches against this value to find the correct phase-config entry. Variable is passed to all hook call activities in deploy-app, undeploy-app, backup-existing-app and stop-dependent-modules BPMNs.

…-green

… phase

vkalapov · 2026-05-12T12:21:42Z

+    void appNameUnchangedWhenNoPhasesConfigMatchesCurrentPhase() {
+        context.setVariable(Variables.APP_TO_PROCESS, buildApp(APP_NAME_LIVE));
+        context.setVariable(Variables.HOOK_EXECUTION_PHASES, List.of(BEFORE_STOP_LIVE));
+        context.setVariable(Variables.HOOK_FOR_EXECUTION, buildHookWithPhasesConfig(
+            List.of(BEFORE_STOP_LIVE),
+            List.of(Map.of(PHASE_KEY, BEFORE_START_IDLE, TARGET_APP_KEY, TARGET_IDLE))
+        ));
+
+        step.execute(execution);
+
+        assertStepFinishedSuccessfully();
+        assertEquals(APP_NAME_LIVE, context.getVariable(Variables.APP_TO_PROCESS)
+                                           .getName());
+    }
+
+    @Test
+    void bgDeploy_targetIdle_resolvesToIdleColorApp() {


Stick to one naming convention for the test method names

vkalapov · 2026-05-12T12:33:03Z

    public static final String CANNOT_RETRIEVE_PARAMETERS_OF_BINDING_BETWEEN_APPLICATION_0_AND_SERVICE_INSTANCE_1 = "Cannot retrieve parameters of binding between application \"{0}\" and service instance \"{1}\"";
    public static final String CANNOT_RETRIEVE_PARAMETERS_OF_BINDING_BETWEEN_APPLICATION_0_AND_SERVICE_INSTANCE_1_FIX = "Cannot retrieve parameters of binding between application \"{0}\" and service instance \"{1}\". Got 502.";
    public static final String CANNOT_RETRIEVE_INSTANCE_OF_SERVICE = "Cannot retrieve service instance of service \"{0}\"";
+    public static final String HOOK_PHASE_BLUE_GREEN_BEFORE_UNMAP_ROUTES_IDLE_USED = "Hook \"{0}\" uses deprecated phase \"blue-green.application.before-unmap-routes.idle\" which is unreachable and will be removed in a future version";


This log message flows a bit better to me:
Hook \"{0}\" uses the deprecated phase \"blue-green.application.before-unmap-routes.idle\". This phase is unreachable and will be removed in a future version.

vkalapov · 2026-05-12T12:35:26Z

                                                                   SERVICE_BINDING_CONFIG, DELETE_SERVICE_KEY_AFTER_DEPLOYMENT);

-    public static final Set<String> MODULE_HOOK_PARAMETERS = Set.of(NAME, COMMAND, MEMORY, DISK_QUOTA, HOOK_REQUIRES);
+    public static final String PHASES_CONFIG = "phases-config";


Move PHASES_CONFIG up with the other public static final declarations.

vkalapov · 2026-05-12T12:43:05Z

+    @SuppressWarnings("unchecked")
+    private void validateHookHasNoDuplicatePhaseConfigs(Hook hook) {
+        Object phasesConfigValue = hook.getParameters().get(SupportedParameters.PHASES_CONFIG);
+        if (phasesConfigValue == null) {
+            return;
+        }
+        if (!(phasesConfigValue instanceof List)) {
+            throw new SLException(MessageFormat.format(Messages.INVALID_PHASES_CONFIG_NOT_A_LIST, hook.getName()));
+        }
+        List<Map<String, String>> phasesConfig = (List<Map<String, String>>) phasesConfigValue;


Can we scope @SuppressWarnings("unchecked") to just the cast instead of the whole method?

vkalapov · 2026-05-12T12:53:31Z

+    @SuppressWarnings("unchecked")
+    private List<Map<String, String>> getPhasesConfig(Hook hook) {
+        Object phasesConfigValue = hook.getParameters()
+                                       .get(SupportedParameters.PHASES_CONFIG);
+        if (phasesConfigValue instanceof List) {
+            return (List<Map<String, String>>) phasesConfigValue;
+        }
+        return List.of();
+    }


We treat missing or null PHASES_CONFIG as empty, but silently ignore cases where it’s present with the wrong type. Would it make sense to log or validate that here?

vkalapov · 2026-05-12T12:58:36Z

+    private String resolveTargetAppName(ProcessContext context, CloudApplicationExtended app) {
+        Hook hook = context.getVariable(Variables.HOOK_FOR_EXECUTION);
+        if (hook == null) {
+            return app.getName();
+        }
+
+        List<Map<String, String>> phasesConfig = getPhasesConfig(hook);
+        if (phasesConfig.isEmpty()) {
+            return app.getName();
+        }
+
+        String currentPhase = buildCurrentPhaseString(context, hook);
+        String targetApp = phasesConfig.stream()
+                                       .filter(config -> currentPhase.equals(config.get(PHASE_KEY)))
+                                       .map(config -> config.get(TARGET_APP_KEY))
+                                       .findFirst()
+                                       .orElse(null);
+
+        if (targetApp == null) {
+            return app.getName();
+        }
+
+        return resolveAppNameForTarget(context, app, targetApp);
+    }


Small refactoring suggestion: app.getName() is returned in multiple branches here. Consider assigning it as the default result and only changing it when a target app is found, to reduce repetition.

vkalapov · 2026-05-12T13:03:51Z

+        if (resolvedAppName == null) {
+            context.setVariable(Variables.TASKS_TO_EXECUTE, List.of());
+            return StepPhase.DONE;
+        }
+        if (!resolvedAppName.equals(app.getName())) {
+            context.setVariable(Variables.APP_TO_PROCESS, buildAppWithName(app, resolvedAppName));
+        }
+
+        return StepPhase.DONE;
+    }


Here all branches return StepPhase.DONE. Would it be clearer to return once at the end and update the context conditionally (e.g., using an else if)?

stiv03 marked this pull request as draft April 3, 2026 08:17

stiv03 marked this pull request as ready for review April 8, 2026 12:08

stiv03 force-pushed the feature/hook-phase-target-app branch from 20deee7 to 0827e89 Compare April 20, 2026 08:15

stiv03 added 4 commits April 20, 2026 11:22

Fix phase-configs target-app routing when live app has no color suffix

ceef42a

Add phases-config hook parameter support with duplicate phase validation

eab5ad4

stiv03 force-pushed the feature/hook-phase-target-app branch from 0827e89 to c5bab3b Compare April 20, 2026 08:26

Refactoring

9165d29

stiv03 force-pushed the feature/hook-phase-target-app branch from c5bab3b to 9165d29 Compare April 20, 2026 08:30

stiv03 added 3 commits April 20, 2026 15:10

Extract hook task target app resolution into dedicated step

aab7ff0

Fix phases-config target app resolution for cf deploy --strategy blue…

695aea6

…-green

Add unit tests for ResolveHookTaskTargetAppStep

26e0d65

IvanBorislavovDimitrov reviewed Apr 21, 2026

View reviewed changes

stiv03 added 2 commits April 23, 2026 11:55

Fix comments

80b634a

Log a warning when hooks use the unreachable before-unmap-routes.idle…

7deb07c

… phase

stiv03 force-pushed the feature/hook-phase-target-app branch from 13a5600 to 7deb07c Compare May 7, 2026 07:43

Add resolveHookTaskTargetAppStep to execute-hook-tasks BPMN diagram

0ecd675

IvanBorislavovDimitrov reviewed May 8, 2026

View reviewed changes

fix comments

7f3c252

vkalapov requested changes May 12, 2026

View reviewed changes

IvanBorislavovDimitrov approved these changes May 12, 2026

View reviewed changes

Conversation

stiv03 commented Apr 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vkalapov May 12, 2026

Choose a reason for hiding this comment

Uh oh!

vkalapov May 12, 2026

Choose a reason for hiding this comment

Uh oh!

vkalapov May 12, 2026

Choose a reason for hiding this comment

Uh oh!

vkalapov May 12, 2026

Choose a reason for hiding this comment

Uh oh!

vkalapov May 12, 2026

Choose a reason for hiding this comment

Uh oh!

vkalapov May 12, 2026

Choose a reason for hiding this comment

Uh oh!

vkalapov May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants