From 624560f405db3021f00d4dac75d09e213593396a Mon Sep 17 00:00:00 2001 From: Stefan Yonkov <156345010+s-yonkov-yonkov@users.noreply.github.com> Date: Thu, 8 May 2025 11:58:59 +0300 Subject: [PATCH 1/5] Add support for cnb (#1629) * add CNB type * add validation for cnb * add validation and tests * update lifecycle to be nullable * refactoring * add tests * update validation --- .../client/lib/domain/DropletInfoFactory.java | 6 +- .../core/model/SupportedParameters.java | 1 + .../core/parser/StagingParametersParser.java | 36 ++++++ .../parser/StagingParametersParserTest.java | 72 ++++++++++++ ...CreateOrUpdateStepWithExistingAppTest.java | 103 +++++++++--------- 5 files changed, 165 insertions(+), 53 deletions(-) create mode 100644 multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java diff --git a/multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/client/lib/domain/DropletInfoFactory.java b/multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/client/lib/domain/DropletInfoFactory.java index bba3f61b15..af168d2ed7 100644 --- a/multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/client/lib/domain/DropletInfoFactory.java +++ b/multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/client/lib/domain/DropletInfoFactory.java @@ -1,13 +1,13 @@ package org.cloudfoundry.multiapps.controller.client.lib.domain; +import java.util.List; + import com.sap.cloudfoundry.client.facade.CloudControllerClient; import com.sap.cloudfoundry.client.facade.domain.CloudApplication; import com.sap.cloudfoundry.client.facade.domain.DockerData; import com.sap.cloudfoundry.client.facade.domain.LifecycleType; import com.sap.cloudfoundry.client.facade.domain.Staging; -import java.util.List; - public class DropletInfoFactory { public DropletInfo createDropletInfo(Staging staging) { @@ -20,7 +20,7 @@ public DropletInfo createDropletInfo(Staging staging) { public DropletInfo createDropletInfo(CloudApplication app, CloudControllerClient client) { var lifecycle = app.getLifecycle(); - if (lifecycle.getType() == LifecycleType.BUILDPACK) { + if (lifecycle.getType() == LifecycleType.BUILDPACK || lifecycle.getType() == LifecycleType.CNB) { var buildpacks = (List) lifecycle.getData() .get("buildpacks"); var stack = (String) lifecycle.getData() diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java index a9b173c3ce..994662c611 100644 --- a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java @@ -58,6 +58,7 @@ public class SupportedParameters { public static final String BUILDPACK = "buildpack"; public static final String BUILDPACKS = "buildpacks"; public static final String STACK = "stack"; + public static final String LIFECYCLE = "lifecycle"; public static final String HEALTH_CHECK_INVOCATION_TIMEOUT = "health-check-invocation-timeout"; public static final String HEALTH_CHECK_TIMEOUT = "health-check-timeout"; public static final String HEALTH_CHECK_TYPE = "health-check-type"; diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java index 141f91d20e..9e1787e188 100644 --- a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java @@ -3,11 +3,13 @@ import java.util.List; import java.util.Map; +import org.cloudfoundry.multiapps.common.SLException; import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters; import org.cloudfoundry.multiapps.mta.util.PropertiesUtil; import com.sap.cloudfoundry.client.facade.domain.DockerInfo; import com.sap.cloudfoundry.client.facade.domain.ImmutableStaging; +import com.sap.cloudfoundry.client.facade.domain.LifecycleType; import com.sap.cloudfoundry.client.facade.domain.Staging; public class StagingParametersParser implements ParametersParser { @@ -32,6 +34,10 @@ public Staging parse(List> parametersList) { getDefaultHealthCheckHttpEndpoint(healthCheckType)); Boolean isSshEnabled = (Boolean) PropertiesUtil.getPropertyValue(parametersList, SupportedParameters.ENABLE_SSH, null); DockerInfo dockerInfo = new DockerInfoParser().parse(parametersList); + LifecycleType lifecycleType = parseLifecycleType(parametersList); + + validateLifecycleType(lifecycleType, buildpacks, dockerInfo); + return ImmutableStaging.builder() .command(command) .buildpacks(buildpacks) @@ -42,9 +48,39 @@ public Staging parse(List> parametersList) { .healthCheckHttpEndpoint(healthCheckHttpEndpoint) .isSshEnabled(isSshEnabled) .dockerInfo(dockerInfo) + .lifecycleType(lifecycleType) .build(); } + private LifecycleType parseLifecycleType(List> parametersList) { + String lifecycleValue = (String) PropertiesUtil.getPropertyValue(parametersList, SupportedParameters.LIFECYCLE, null); + if (lifecycleValue == null) { + return null; + } + try { + return LifecycleType.valueOf(lifecycleValue.toUpperCase()); + } catch (IllegalArgumentException e) { + throw new SLException("Unsupported lifecycle value: " + lifecycleValue); + } + } + + private void validateLifecycleType(LifecycleType lifecycleType, List buildpacks, DockerInfo dockerInfo) { + if (lifecycleType == LifecycleType.CNB && (buildpacks == null || buildpacks.isEmpty())) { + throw new SLException("Buildpacks must be provided when lifecycle is set to 'cnb'."); + } + // Validate Docker-specific conditions + if (lifecycleType == LifecycleType.DOCKER) { + if (dockerInfo == null) { + throw new SLException("Docker information must be provided when lifecycle is set to 'docker'."); + } + if (buildpacks != null && !buildpacks.isEmpty()) { + throw new SLException("Buildpacks must not be provided when lifecycle is set to 'docker'."); + } + } else if (dockerInfo != null && lifecycleType != null) { + throw new SLException("Docker information must not be provided when lifecycle is set to " + lifecycleType + "'."); + } + } + private String getDefaultHealthCheckHttpEndpoint(String healthCheckType) { return HTTP_HEALTH_CHECK_TYPE.equals(healthCheckType) ? DEFAULT_HEALTH_CHECK_HTTP_ENDPOINT : null; } diff --git a/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java new file mode 100644 index 0000000000..4920af01a5 --- /dev/null +++ b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java @@ -0,0 +1,72 @@ +package org.cloudfoundry.multiapps.controller.core.parser; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import org.cloudfoundry.multiapps.common.SLException; +import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class StagingParametersParserTest { + + private StagingParametersParser parser; + private List> parametersList; + + @BeforeEach + void setup() { + parser = new StagingParametersParser(); + parametersList = new ArrayList<>(); + } + + @Test + void testValidateLifecycleWithCnbAndValidBuildpacks() { + parametersList.add(Collections.singletonMap("lifecycle", "cnb")); + parametersList.add(Collections.singletonMap("buildpacks", List.of("custom-buildpack-url"))); + assertDoesNotThrow(() -> parser.parse(parametersList)); + } + + @Test + void testValidateLifecycleWithCnbAndNoBuildpacks() { + parametersList.add(Collections.singletonMap(SupportedParameters.LIFECYCLE, "cnb")); + Exception exception = assertThrows(SLException.class, () -> parser.parse(parametersList)); + assertEquals("Buildpacks must be provided when lifecycle is set to 'cnb'.", exception.getMessage()); + } + + @Test + void testValidateLifecycleWithDockerAndValidDockerInfo() { + parametersList.add(getDockerParams()); + assertDoesNotThrow(() -> parser.parse(parametersList)); + } + + @Test + void testValidateLifecycleWithDockerAndBuildpacksProvided() { + parametersList.add(getDockerParams()); + parametersList.add(Collections.singletonMap("buildpacks", List.of("some-buildpack"))); + + Exception exception = assertThrows(SLException.class, () -> parser.parse(parametersList)); + assertEquals("Buildpacks must not be provided when lifecycle is set to 'docker'.", exception.getMessage()); + } + + @Test + void testValidateLifecycleWithBuildpackAndNoBuildpacks() { + parametersList.add(Collections.singletonMap("lifecycle", "buildpack")); + assertDoesNotThrow(() -> parser.parse(parametersList)); + } + + @Test + void testValidateLifecycleWithInvalidLifecycleValue() { + parametersList.add(Collections.singletonMap("lifecycle", "invalid_value")); + Exception exception = assertThrows(SLException.class, () -> parser.parse(parametersList)); + assertEquals("Unsupported lifecycle value: invalid_value", exception.getMessage()); + } + + private static Map getDockerParams() { + return Map.of(SupportedParameters.LIFECYCLE, "docker", SupportedParameters.DOCKER, Map.of("image", "cloudfoundry/test-app")); + } + +} diff --git a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateStepWithExistingAppTest.java b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateStepWithExistingAppTest.java index 652c316304..cfe4220fb6 100644 --- a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateStepWithExistingAppTest.java +++ b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateStepWithExistingAppTest.java @@ -2,21 +2,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; 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.anyMap; -import static org.mockito.ArgumentMatchers.anySet; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.UUID; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -32,20 +21,7 @@ import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mock; -import com.sap.cloudfoundry.client.facade.domain.CloudApplication; -import com.sap.cloudfoundry.client.facade.domain.CloudRoute; -import com.sap.cloudfoundry.client.facade.domain.HealthCheckType; -import com.sap.cloudfoundry.client.facade.domain.ImmutableCloudApplication; -import com.sap.cloudfoundry.client.facade.domain.ImmutableCloudMetadata; -import com.sap.cloudfoundry.client.facade.domain.ImmutableCloudPackage; -import com.sap.cloudfoundry.client.facade.domain.ImmutableCloudProcess; -import com.sap.cloudfoundry.client.facade.domain.ImmutableDockerData; -import com.sap.cloudfoundry.client.facade.domain.ImmutableDockerInfo; -import com.sap.cloudfoundry.client.facade.domain.ImmutableDropletInfo; -import com.sap.cloudfoundry.client.facade.domain.ImmutableLifecycle; -import com.sap.cloudfoundry.client.facade.domain.ImmutableStaging; -import com.sap.cloudfoundry.client.facade.domain.LifecycleType; -import com.sap.cloudfoundry.client.facade.domain.Staging; +import com.sap.cloudfoundry.client.facade.domain.*; import com.sap.cloudfoundry.client.facade.util.JsonUtil; class CreateOrUpdateStepWithExistingAppTest extends SyncFlowableStepTest { @@ -65,16 +41,16 @@ static Stream testHandleStagingApplicationAttributes() { return Stream.of( //@formatter:off Arguments.of(ImmutableStaging.builder().addBuildpack("buildpack-1").command("command1").build(), - ImmutableStaging.builder().addBuildpack("buildpack-1").command("command2").build(), true), + ImmutableStaging.builder().addBuildpack("buildpack-1").command("command2").build(), true, LifecycleType.BUILDPACK), Arguments.of(ImmutableStaging.builder().addBuildpack("buildpack-1").build(), - ImmutableStaging.builder().addBuildpack("buildpack-1").build(), false), + ImmutableStaging.builder().addBuildpack("buildpack-1").build(), false, LifecycleType.BUILDPACK), Arguments.of( ImmutableStaging.builder().addBuildpack("buildpack-1").command("command1").stackName("stack1") .healthCheckTimeout(5).healthCheckType("process").isSshEnabled(false).build(), ImmutableStaging.builder().addBuildpack("buildpack-2").command("command2").stackName("stack2") .healthCheckTimeout(10).healthCheckType("port").healthCheckHttpEndpoint("/test") .isSshEnabled(true).build(), - true), + true, LifecycleType.BUILDPACK), Arguments.of( ImmutableStaging.builder().addBuildpack("buildpack-2").command("command2").stackName("stack2") .healthCheckTimeout(10).healthCheckType("process").healthCheckHttpEndpoint("/test") @@ -82,50 +58,56 @@ static Stream testHandleStagingApplicationAttributes() { ImmutableStaging.builder().addBuildpack("buildpack-2").command("command2").stackName("stack2") .healthCheckTimeout(10).healthCheckType("process").healthCheckHttpEndpoint("/test") .isSshEnabled(true).build(), - false), + false, LifecycleType.BUILDPACK), Arguments.of(ImmutableStaging.builder() .dockerInfo(ImmutableDockerInfo.builder().image("cloudfoundry/test-app").build()).build(), ImmutableStaging.builder() .dockerInfo(ImmutableDockerInfo.builder().image("cloudfoundry/test-app2").build()) .build(), - true), + true, LifecycleType.DOCKER), Arguments.of(ImmutableStaging.builder() .dockerInfo(ImmutableDockerInfo.builder().image("cloudfoundry/test-app").build()).build(), ImmutableStaging.builder() .dockerInfo(ImmutableDockerInfo.builder().image("cloudfoundry/test-app").build()) .build(), - false)); + false, LifecycleType.DOCKER), + Arguments.of(ImmutableStaging.builder().build(), + ImmutableStaging.builder().lifecycleType(LifecycleType.CNB).build(), + false, LifecycleType.CNB), + Arguments.of(ImmutableStaging.builder().addBuildpack("buildpack-1").command("command1").build(), + ImmutableStaging.builder().addBuildpack("buildpack-1").command("command2").lifecycleType(LifecycleType.CNB).build(), + true, LifecycleType.CNB), + Arguments.of(ImmutableStaging.builder().addBuildpack("buildpack-333").build(), + ImmutableStaging.builder().addBuildpacks("buildpack-4", "buildpack-8").lifecycleType(LifecycleType.CNB).build(), + true, LifecycleType.CNB)); //@formatter:on } @ParameterizedTest @MethodSource - void testHandleStagingApplicationAttributes(Staging existingStaging, Staging staging, boolean expectedPropertiesChanged) { + void testHandleStagingApplicationAttributes(Staging existingStaging, Staging staging, boolean expectedPropertiesChanged, + LifecycleType expectedLifecycleType) { CloudApplication existingApplication = getApplicationBuilder(false).staging(existingStaging) .build(); - if (staging.getCommand() == null) { - staging = ImmutableStaging.copyOf(staging) - .withCommand(DEFAULT_COMMAND); - } - if (staging.getStackName() == null) { - staging = ImmutableStaging.copyOf(staging) - .withStackName(DEFAULT_STACK); - } + staging = ImmutableStaging.copyOf(applyDefaultsToStaging(staging)); CloudApplicationExtended application = getApplicationBuilder(false).staging(staging) .build(); - if (staging.getDockerInfo() != null) { - application = ImmutableCloudApplicationExtended.copyOf(application) - .withLifecycle(ImmutableLifecycle.builder() - .type(LifecycleType.DOCKER) - .build()); - } + LifecycleType lifecycleTypeToApply = determineLifecycleType(staging); + application = ImmutableCloudApplicationExtended.copyOf(application) + .withLifecycle(ImmutableLifecycle.builder() + .type(lifecycleTypeToApply) + .build()); + prepareContext(application, false); prepareClientWithStaging(existingApplication, existingStaging); step.execute(execution); assertStepFinishedSuccessfully(); + assertEquals(expectedLifecycleType, application.getLifecycle() + .getType()); assertEquals(expectedPropertiesChanged, context.getVariable(Variables.VCAP_APP_PROPERTIES_CHANGED)); + if (expectedPropertiesChanged) { verify(client).updateApplicationStaging(APP_NAME, staging); return; @@ -133,6 +115,27 @@ void testHandleStagingApplicationAttributes(Staging existingStaging, Staging sta verify(client, never()).updateApplicationStaging(eq(APP_NAME), any()); } + private LifecycleType determineLifecycleType(Staging staging) { + if (staging.getLifecycleType() != null) { + return staging.getLifecycleType(); + } else if (staging.getDockerInfo() != null) { + return LifecycleType.DOCKER; + } + return LifecycleType.BUILDPACK; + } + + private Staging applyDefaultsToStaging(Staging staging) { + if (staging.getCommand() == null) { + staging = ImmutableStaging.copyOf(staging) + .withCommand(DEFAULT_COMMAND); + } + if (staging.getStackName() == null) { + staging = ImmutableStaging.copyOf(staging) + .withStackName(DEFAULT_STACK); + } + return staging; + } + private ImmutableCloudApplicationExtended.Builder getApplicationBuilder(boolean shouldKeepExistingEnv) { return ImmutableCloudApplicationExtended.builder() .metadata(ImmutableCloudMetadata.of(UUID.randomUUID())) From 629713fd74357b52f2fd6b878d5e0e8e9480fe7d Mon Sep 17 00:00:00 2001 From: Stefan Yonkov Date: Fri, 9 May 2025 16:34:38 +0300 Subject: [PATCH 2/5] Apply formatting --- .../core/parser/StagingParametersParser.java | 7 +-- .../parser/StagingParametersParserTest.java | 6 +- ...CreateOrUpdateStepWithExistingAppTest.java | 56 ++++++++++++++----- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java index 9e1787e188..92512ee566 100644 --- a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java @@ -3,14 +3,13 @@ import java.util.List; import java.util.Map; -import org.cloudfoundry.multiapps.common.SLException; -import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters; -import org.cloudfoundry.multiapps.mta.util.PropertiesUtil; - import com.sap.cloudfoundry.client.facade.domain.DockerInfo; import com.sap.cloudfoundry.client.facade.domain.ImmutableStaging; import com.sap.cloudfoundry.client.facade.domain.LifecycleType; import com.sap.cloudfoundry.client.facade.domain.Staging; +import org.cloudfoundry.multiapps.common.SLException; +import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters; +import org.cloudfoundry.multiapps.mta.util.PropertiesUtil; public class StagingParametersParser implements ParametersParser { diff --git a/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java index 4920af01a5..a85e99aa67 100644 --- a/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java +++ b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java @@ -1,7 +1,5 @@ package org.cloudfoundry.multiapps.controller.core.parser; -import static org.junit.jupiter.api.Assertions.*; - import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -12,6 +10,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + class StagingParametersParserTest { private StagingParametersParser parser; diff --git a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateStepWithExistingAppTest.java b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateStepWithExistingAppTest.java index cfe4220fb6..4d3060127c 100644 --- a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateStepWithExistingAppTest.java +++ b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateStepWithExistingAppTest.java @@ -1,14 +1,29 @@ package org.cloudfoundry.multiapps.controller.process.steps; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; - -import java.util.*; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.Stream; +import com.sap.cloudfoundry.client.facade.domain.CloudApplication; +import com.sap.cloudfoundry.client.facade.domain.CloudRoute; +import com.sap.cloudfoundry.client.facade.domain.HealthCheckType; +import com.sap.cloudfoundry.client.facade.domain.ImmutableCloudApplication; +import com.sap.cloudfoundry.client.facade.domain.ImmutableCloudMetadata; +import com.sap.cloudfoundry.client.facade.domain.ImmutableCloudPackage; +import com.sap.cloudfoundry.client.facade.domain.ImmutableCloudProcess; +import com.sap.cloudfoundry.client.facade.domain.ImmutableDockerData; +import com.sap.cloudfoundry.client.facade.domain.ImmutableDockerInfo; +import com.sap.cloudfoundry.client.facade.domain.ImmutableDropletInfo; +import com.sap.cloudfoundry.client.facade.domain.ImmutableLifecycle; +import com.sap.cloudfoundry.client.facade.domain.ImmutableStaging; +import com.sap.cloudfoundry.client.facade.domain.LifecycleType; +import com.sap.cloudfoundry.client.facade.domain.Staging; +import com.sap.cloudfoundry.client.facade.util.JsonUtil; import org.cloudfoundry.multiapps.controller.client.lib.domain.CloudApplicationExtended; import org.cloudfoundry.multiapps.controller.client.lib.domain.ImmutableCloudApplicationExtended; import org.cloudfoundry.multiapps.controller.core.Constants; @@ -21,8 +36,16 @@ import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mock; -import com.sap.cloudfoundry.client.facade.domain.*; -import com.sap.cloudfoundry.client.facade.util.JsonUtil; +import static org.junit.jupiter.api.Assertions.assertEquals; +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.anyMap; +import static org.mockito.ArgumentMatchers.anySet; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; class CreateOrUpdateStepWithExistingAppTest extends SyncFlowableStepTest { @@ -39,7 +62,7 @@ class CreateOrUpdateStepWithExistingAppTest extends SyncFlowableStepTest testHandleStagingApplicationAttributes() { return Stream.of( -//@formatter:off + //@formatter:off Arguments.of(ImmutableStaging.builder().addBuildpack("buildpack-1").command("command1").build(), ImmutableStaging.builder().addBuildpack("buildpack-1").command("command2").build(), true, LifecycleType.BUILDPACK), Arguments.of(ImmutableStaging.builder().addBuildpack("buildpack-1").build(), @@ -153,9 +176,11 @@ private ImmutableCloudApplicationExtended.Builder getApplicationBuilder(boolean .healthCheckType("port") .isSshEnabled(false) .build()) - .attributesUpdateStrategy(ImmutableCloudApplicationExtended.AttributeUpdateStrategy.builder() - .shouldKeepExistingEnv(shouldKeepExistingEnv) - .build()); + .attributesUpdateStrategy( + ImmutableCloudApplicationExtended.AttributeUpdateStrategy.builder() + .shouldKeepExistingEnv( + shouldKeepExistingEnv) + .build()); } private void prepareContext(CloudApplicationExtended application, boolean shouldSkipServiceRebinding) { @@ -203,10 +228,11 @@ private void prepareClient(CloudApplication application, Set routes, .command(command) .diskInMb(disk == null ? 1024 : disk) .memoryInMb(memory == null ? 1024 - : memory) + : memory) .healthCheckType(hcType == null - ? HealthCheckType.PORT - : HealthCheckType.valueOf(hcType.toUpperCase())) + ? HealthCheckType.PORT + : HealthCheckType.valueOf( + hcType.toUpperCase())) .healthCheckTimeout(hcTimeout) .healthCheckHttpEndpoint(hcEndpoint) .instances(1) From f89fffb55ee87d8f718c6d5466b78ec65fcc6c25 Mon Sep 17 00:00:00 2001 From: Stefan Yonkov Date: Wed, 4 Jun 2025 13:28:48 +0300 Subject: [PATCH 3/5] Add lifecycle to supported parameters --- .../multiapps/controller/core/model/SupportedParameters.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java index 994662c611..dd49ce935e 100644 --- a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java @@ -180,7 +180,7 @@ public class SupportedParameters { @Deprecated public static final String DEPRECATED_CONFIG_MTA_PROVIDES_DEPENDENCY = "mta-provides-dependency"; - public static final Set MODULE_PARAMETERS = Set.of(APP_NAME, APPLY_NAMESPACE, BUILDPACK, BUILDPACKS, COMMAND, + public static final Set MODULE_PARAMETERS = Set.of(APP_NAME, APPLY_NAMESPACE, BUILDPACK, BUILDPACKS, LIFECYCLE, COMMAND, CREATE_SERVICE_BROKER, DEFAULT_APP_NAME, DEFAULT_HOST, DEFAULT_INSTANCES, DEFAULT_LIVE_APP_NAME, DEFAULT_LIVE_DOMAIN, DEFAULT_LIVE_HOST, DEFAULT_LIVE_URI, DEFAULT_LIVE_URL, DEFAULT_URI, DEFAULT_URL, From 20d0dfff52e6d413cbc4c4ec48792477d4a51c6f Mon Sep 17 00:00:00 2001 From: Stefan Yonkov Date: Fri, 6 Jun 2025 15:48:23 +0300 Subject: [PATCH 4/5] Changes on comments --- .../multiapps/controller/core/Messages.java | 5 + .../core/parser/StagingParametersParser.java | 54 ++++++--- .../parser/StagingParametersParserTest.java | 113 +++++++++++++++--- 3 files changed, 138 insertions(+), 34 deletions(-) diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java index 74eb64d9d1..feeca5e1bb 100644 --- a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java @@ -86,6 +86,11 @@ public final class Messages { public static final String OBJECT_STORE_FILE_STORAGE_HEALTH_DATABASE_HEALTH = "Object store file storage health: \"{0}\", Database health: \"{1}\""; public static final String ERROR_OCCURRED_DURING_OBJECT_STORE_HEALTH_CHECKING_FOR_INSTANCE = "Error occurred during object store health checking for instance: \"{0}\""; public static final String ERROR_OCCURRED_WHILE_CHECKING_DATABASE_INSTANCE_0 = "Error occurred while checking database instance: \"{0}\""; + public static final String DOCKER_INFO_NOT_ALLOWED_WITH_LIFECYCLE = "Docker information must not be provided when lifecycle is set to \"{0}\""; + public static final String UNSUPPORTED_LIFECYCLE_VALUE = "Unsupported lifecycle value: \"{0}\""; + public static final String BUILDPACKS_REQUIRED_FOR_CNB = "Buildpacks must be provided when lifecycle is set to 'cnb'."; + public static final String DOCKER_INFO_REQUIRED = "Docker information must be provided when lifecycle is set to 'docker'."; + public static final String BUILDPACKS_NOT_ALLOWED_WITH_DOCKER = "Buildpacks must not be provided when lifecycle is set to 'docker'."; // Warning messages public static final String ENVIRONMENT_VARIABLE_IS_NOT_SET_USING_DEFAULT = "Environment variable \"{0}\" is not set. Using default \"{1}\"..."; diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java index 92512ee566..b882a48fce 100644 --- a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java @@ -1,5 +1,6 @@ package org.cloudfoundry.multiapps.controller.core.parser; +import java.text.MessageFormat; import java.util.List; import java.util.Map; @@ -7,12 +8,18 @@ import com.sap.cloudfoundry.client.facade.domain.ImmutableStaging; import com.sap.cloudfoundry.client.facade.domain.LifecycleType; import com.sap.cloudfoundry.client.facade.domain.Staging; -import org.cloudfoundry.multiapps.common.SLException; +import org.cloudfoundry.multiapps.common.ContentException; import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters; import org.cloudfoundry.multiapps.mta.util.PropertiesUtil; +import org.springframework.util.CollectionUtils; -public class StagingParametersParser implements ParametersParser { +import static org.cloudfoundry.multiapps.controller.core.Messages.BUILDPACKS_NOT_ALLOWED_WITH_DOCKER; +import static org.cloudfoundry.multiapps.controller.core.Messages.BUILDPACKS_REQUIRED_FOR_CNB; +import static org.cloudfoundry.multiapps.controller.core.Messages.DOCKER_INFO_NOT_ALLOWED_WITH_LIFECYCLE; +import static org.cloudfoundry.multiapps.controller.core.Messages.DOCKER_INFO_REQUIRED; +import static org.cloudfoundry.multiapps.controller.core.Messages.UNSUPPORTED_LIFECYCLE_VALUE; +public class StagingParametersParser implements ParametersParser { private static final String DEFAULT_HEALTH_CHECK_HTTP_ENDPOINT = "/"; private static final String HTTP_HEALTH_CHECK_TYPE = "http"; @@ -59,24 +66,41 @@ private LifecycleType parseLifecycleType(List> parametersLis try { return LifecycleType.valueOf(lifecycleValue.toUpperCase()); } catch (IllegalArgumentException e) { - throw new SLException("Unsupported lifecycle value: " + lifecycleValue); + throw new ContentException(MessageFormat.format(UNSUPPORTED_LIFECYCLE_VALUE, lifecycleValue)); } } private void validateLifecycleType(LifecycleType lifecycleType, List buildpacks, DockerInfo dockerInfo) { - if (lifecycleType == LifecycleType.CNB && (buildpacks == null || buildpacks.isEmpty())) { - throw new SLException("Buildpacks must be provided when lifecycle is set to 'cnb'."); - } + validateBuildpacksWithCNB(lifecycleType, buildpacks); + // Validate Docker-specific conditions - if (lifecycleType == LifecycleType.DOCKER) { - if (dockerInfo == null) { - throw new SLException("Docker information must be provided when lifecycle is set to 'docker'."); - } - if (buildpacks != null && !buildpacks.isEmpty()) { - throw new SLException("Buildpacks must not be provided when lifecycle is set to 'docker'."); - } - } else if (dockerInfo != null && lifecycleType != null) { - throw new SLException("Docker information must not be provided when lifecycle is set to " + lifecycleType + "'."); + validateDockerInfoWithDocker(lifecycleType, dockerInfo); + validateBuildpacksWithDocker(lifecycleType, buildpacks); + validateDockerInfoWithNonDocker(lifecycleType, dockerInfo); + } + + private void validateBuildpacksWithCNB(LifecycleType lifecycleType, List buildpacks) { + if (lifecycleType == LifecycleType.CNB && CollectionUtils.isEmpty(buildpacks)) { + throw new ContentException(BUILDPACKS_REQUIRED_FOR_CNB); + } + } + + private void validateDockerInfoWithDocker(LifecycleType lifecycleType, DockerInfo dockerInfo) { + if (lifecycleType == LifecycleType.DOCKER && dockerInfo == null) { + throw new ContentException(DOCKER_INFO_REQUIRED); + } + } + + private void validateBuildpacksWithDocker(LifecycleType lifecycleType, List buildpacks) { + if (lifecycleType == LifecycleType.DOCKER && !CollectionUtils.isEmpty(buildpacks)) { + throw new ContentException(BUILDPACKS_NOT_ALLOWED_WITH_DOCKER); + } + } + + private void validateDockerInfoWithNonDocker(LifecycleType lifecycleType, DockerInfo dockerInfo) { + if (lifecycleType != LifecycleType.DOCKER && lifecycleType != null && dockerInfo != null) { + throw new ContentException( + MessageFormat.format(DOCKER_INFO_NOT_ALLOWED_WITH_LIFECYCLE, lifecycleType)); } } diff --git a/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java index a85e99aa67..aef7f3495e 100644 --- a/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java +++ b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParserTest.java @@ -1,21 +1,41 @@ package org.cloudfoundry.multiapps.controller.core.parser; +import java.text.MessageFormat; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; -import org.cloudfoundry.multiapps.common.SLException; -import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters; +import com.sap.cloudfoundry.client.facade.domain.LifecycleType; +import com.sap.cloudfoundry.client.facade.domain.Staging; +import org.cloudfoundry.multiapps.common.ContentException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.util.CollectionUtils; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.cloudfoundry.multiapps.controller.core.Messages.BUILDPACKS_NOT_ALLOWED_WITH_DOCKER; +import static org.cloudfoundry.multiapps.controller.core.Messages.BUILDPACKS_REQUIRED_FOR_CNB; +import static org.cloudfoundry.multiapps.controller.core.Messages.DOCKER_INFO_NOT_ALLOWED_WITH_LIFECYCLE; +import static org.cloudfoundry.multiapps.controller.core.Messages.DOCKER_INFO_REQUIRED; +import static org.cloudfoundry.multiapps.controller.core.Messages.UNSUPPORTED_LIFECYCLE_VALUE; +import static org.cloudfoundry.multiapps.controller.core.model.SupportedParameters.BUILDPACK; +import static org.cloudfoundry.multiapps.controller.core.model.SupportedParameters.BUILDPACKS; +import static org.cloudfoundry.multiapps.controller.core.model.SupportedParameters.DOCKER; +import static org.cloudfoundry.multiapps.controller.core.model.SupportedParameters.LIFECYCLE; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; class StagingParametersParserTest { + public static final String INVALID_VALUE = "invalid_value"; + public static final String CNB = "cnb"; + public static final String CUSTOM_BUILDPACK_URL = "custom-buildpack-url"; + public static final String IMAGE = "image"; + public static final String CLOUDFOUNDRY_TEST_APP = "cloudfoundry/test-app"; + public static final String SOME_BUILDPACK = "some-buildpack"; private StagingParametersParser parser; private List> parametersList; @@ -27,48 +47,103 @@ void setup() { @Test void testValidateLifecycleWithCnbAndValidBuildpacks() { - parametersList.add(Collections.singletonMap("lifecycle", "cnb")); - parametersList.add(Collections.singletonMap("buildpacks", List.of("custom-buildpack-url"))); - assertDoesNotThrow(() -> parser.parse(parametersList)); + parametersList.add(mapOf(LIFECYCLE, CNB)); + parametersList.add(mapOf(BUILDPACKS, List.of(CUSTOM_BUILDPACK_URL))); + + Staging staging = parser.parse(parametersList); + + assertNotNull(staging); + assertEquals(LifecycleType.CNB, staging.getLifecycleType()); + assertNotNull(staging.getBuildpacks()); + assertEquals(1, staging.getBuildpacks() + .size()); + assertEquals(CUSTOM_BUILDPACK_URL, staging.getBuildpacks() + .get(0)); } @Test void testValidateLifecycleWithCnbAndNoBuildpacks() { - parametersList.add(Collections.singletonMap(SupportedParameters.LIFECYCLE, "cnb")); - Exception exception = assertThrows(SLException.class, () -> parser.parse(parametersList)); - assertEquals("Buildpacks must be provided when lifecycle is set to 'cnb'.", exception.getMessage()); + parametersList.add(mapOf(LIFECYCLE, CNB)); + + ContentException exception = assertThrows(ContentException.class, () -> parser.parse(parametersList)); + assertEquals(BUILDPACKS_REQUIRED_FOR_CNB, exception.getMessage()); } @Test void testValidateLifecycleWithDockerAndValidDockerInfo() { parametersList.add(getDockerParams()); - assertDoesNotThrow(() -> parser.parse(parametersList)); + + Staging staging = parser.parse(parametersList); + + assertNotNull(staging); + assertEquals(LifecycleType.DOCKER, staging.getLifecycleType()); + assertNotNull(staging.getDockerInfo()); } @Test void testValidateLifecycleWithDockerAndBuildpacksProvided() { parametersList.add(getDockerParams()); - parametersList.add(Collections.singletonMap("buildpacks", List.of("some-buildpack"))); + parametersList.add(mapOf(BUILDPACKS, List.of(SOME_BUILDPACK))); - Exception exception = assertThrows(SLException.class, () -> parser.parse(parametersList)); - assertEquals("Buildpacks must not be provided when lifecycle is set to 'docker'.", exception.getMessage()); + ContentException exception = assertThrows(ContentException.class, () -> parser.parse(parametersList)); + assertEquals(BUILDPACKS_NOT_ALLOWED_WITH_DOCKER, exception.getMessage()); } @Test void testValidateLifecycleWithBuildpackAndNoBuildpacks() { - parametersList.add(Collections.singletonMap("lifecycle", "buildpack")); - assertDoesNotThrow(() -> parser.parse(parametersList)); + parametersList.add(mapOf(LIFECYCLE, BUILDPACK)); + + Staging staging = parser.parse(parametersList); + + assertNotNull(staging); + assertEquals(LifecycleType.BUILDPACK, staging.getLifecycleType()); + + List buildpacks = staging.getBuildpacks(); + assertTrue(CollectionUtils.isEmpty(buildpacks)); } @Test void testValidateLifecycleWithInvalidLifecycleValue() { - parametersList.add(Collections.singletonMap("lifecycle", "invalid_value")); - Exception exception = assertThrows(SLException.class, () -> parser.parse(parametersList)); - assertEquals("Unsupported lifecycle value: invalid_value", exception.getMessage()); + parametersList.add(mapOf(LIFECYCLE, INVALID_VALUE)); + + ContentException exception = assertThrows(ContentException.class, () -> parser.parse(parametersList)); + assertEquals(MessageFormat.format(UNSUPPORTED_LIFECYCLE_VALUE, INVALID_VALUE), exception.getMessage()); + } + + @Test + void testValidateLifecycleWithDockerAndNoDockerInfo() { + parametersList.add(mapOf(LIFECYCLE, DOCKER)); + + ContentException exception = assertThrows(ContentException.class, () -> parser.parse(parametersList)); + assertEquals(DOCKER_INFO_REQUIRED, exception.getMessage()); + } + + @Test + void testValidateDockerInfoWithNonDockerLifecycle() { + parametersList.add(mapOf(LIFECYCLE, CNB)); + parametersList.add(mapOf(BUILDPACKS, List.of(SOME_BUILDPACK))); + parametersList.add(getDockerParams()); + + ContentException exception = assertThrows(ContentException.class, () -> parser.parse(parametersList)); + assertEquals(MessageFormat.format(DOCKER_INFO_NOT_ALLOWED_WITH_LIFECYCLE, LifecycleType.CNB), exception.getMessage()); + } + + @Test + void testValidateWithAllParametersMissing() { + Staging staging = parser.parse(parametersList); + + assertNotNull(staging); + assertNull(staging.getLifecycleType()); + assertTrue(CollectionUtils.isEmpty(staging.getBuildpacks())); + assertNull(staging.getDockerInfo()); + } + + private static Map mapOf(String key, Object value) { + return Collections.singletonMap(key, value); } private static Map getDockerParams() { - return Map.of(SupportedParameters.LIFECYCLE, "docker", SupportedParameters.DOCKER, Map.of("image", "cloudfoundry/test-app")); + return Map.of(LIFECYCLE, DOCKER, DOCKER, Map.of(IMAGE, CLOUDFOUNDRY_TEST_APP)); } } From c978648495330bb8a373c0486245c0a4a10ffa6e Mon Sep 17 00:00:00 2001 From: Stefan Yonkov Date: Mon, 9 Jun 2025 15:43:46 +0300 Subject: [PATCH 5/5] Adopt released cf-java-client --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a8cde12d3e..f41a5235db 100644 --- a/pom.xml +++ b/pom.xml @@ -40,7 +40,7 @@ 2.19.0 4.32.0 5.1.0 - 2.56.0 + 2.57.0 1.6.16 2.7.0 33.4.8-jre