Fix regression #354

CF CC aays provides previous_value field. BoshServiceProvisionningTest was not providing default_value for maintenance_info, hence not detecting the issue. Fixes #354
orange-cloudfoundry · Jan 25, 2021 · 137c39b · 137c39b
1 parent 50313ce
commit 137c39b
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 2 deletions.
diff --git a/...cloudfoundry/broker/opsautomation/ondemandbroker/sample/BoshServiceProvisionningTest.java b/...cloudfoundry/broker/opsautomation/ondemandbroker/sample/BoshServiceProvisionningTest.java
@@ -420,7 +420,8 @@ public void supports_crud_lifecycle() throws IOException {
         assertThatThrownBy(() -> {update_service_plan(aRejectedPlanUpdateServiceInstanceRequest());})
             .isInstanceOf(FeignException.class)
             .hasMessageContaining(("422"))
-            .hasMessageContaining(("Service instance update not supported"));
+            .hasMessageContaining(("Service instance update not supported"))
+            .hasMessageContaining(("List of supported plan upgrades from"));
 
         operation = upgrade_service();
         polls_last_operation(operation, HttpStatus.SC_OK, "in progress", "");
@@ -614,7 +615,7 @@ private UpdateServiceInstanceRequest anUpgradeServiceInstanceRequest() {
                 .build())
             .parameters(OsbBuilderHelper.osbCmdbCustomParam(BROKERED_SERVICE_INSTANCE_ID))
             .previousValues(new UpdateServiceInstanceRequest.PreviousValues(
-                null,
+                UPGRADED_SERVICE_PLAN_ID,
                 aCreateServiceInstanceRequest().getMaintenanceInfo()))
             .context(aCfUserContext())
             .originatingIdentity(aCfUserContext())

diff --git a/...range/oss/cloudfoundry/broker/opsautomation/ondemandbroker/pipeline/OsbBuilderHelper.java b/...range/oss/cloudfoundry/broker/opsautomation/ondemandbroker/pipeline/OsbBuilderHelper.java
@@ -198,6 +198,22 @@ public static UpdateServiceInstanceRequest anUpdateServiceInstanceRequest() {
 
                 .build();
     }
+    /**
+     * Constructs a "cf update-service -c "{}" request
+     */
+    public static UpdateServiceInstanceRequest anUpdateServiceInstanceRequestWithoutPreviousValue() {
+        // Given an incoming delete request
+        return UpdateServiceInstanceRequest.builder()
+                .serviceDefinitionId("service_id")
+                .serviceDefinition(aCatalog().getServiceDefinitions().get(0))
+                .planId(SERVICE_PLAN_ID)
+                .plan(aSmallPlan())
+                .parameters(new HashMap<>())
+                .serviceInstanceId("instance_id")
+                .maintenanceInfo( anInitialMaintenanceInfo())
+
+                .build();
+    }
 
     /**
      * Constructs a "cf update-service -p plan2" request from plan 1

diff --git a/...udfoundry/broker/opsautomation/ondemandbroker/pipeline/PlanUpgradeValidatorProcessor.java b/...udfoundry/broker/opsautomation/ondemandbroker/pipeline/PlanUpgradeValidatorProcessor.java
@@ -51,6 +51,11 @@ public void preUpdate(Context ctx) {
 			logger.debug("Receiving update request without previous plan_id, assuming params-only update");
 			return;
 		}
+		if (previousValuesPlanId.equals(toPlan.getId())) {
+			logger.debug("Receiving update request without plan change, assuming params-only update, or " +
+				"maintenance_info upgrade. Accepting the request");
+			return;
+		}
 		ServiceDefinition serviceDefinition = updateRequest.getServiceDefinition();
 		Plan fromPlan = serviceDefinition.getPlans().stream()
 			.filter((s) -> s.getId().equals(previousValuesPlanId))

diff --git a/...undry/broker/opsautomation/ondemandbroker/pipeline/PlanUpgradeValidatorProcessorTest.java b/...undry/broker/opsautomation/ondemandbroker/pipeline/PlanUpgradeValidatorProcessorTest.java
@@ -25,7 +25,51 @@
 class PlanUpgradeValidatorProcessorTest {
 
 
+	/**
+	 * a `cf update-service --upgrade` is accepted
+	 */
+	@Test
+	void accepts_any_maintenance_info_upgrade() {
+		//Given a validator
+		PlanUpgradeValidatorProperties planUpgradeCheckerProperties = aPlanUpgradeCheckerProperties();
+		PlanUpgradeValidatorProcessor planUpgradeValidatorProcessor = new PlanUpgradeValidatorProcessor(planUpgradeCheckerProperties);
+
+		//Given a noop upgrade update request
+		UpdateServiceInstanceRequest request = OsbBuilderHelper.anUpgradeServiceInstanceRequest();
+		assertThat(request.getPreviousValues().getPlanId()).isEqualTo(request.getPlan().getId());
+
+		//Given a populated context
+		Context context = new Context();
+		context.contextKeys.put(ProcessorChainServiceInstanceService.UPDATE_SERVICE_INSTANCE_REQUEST, request);
+
+		//When
+		planUpgradeValidatorProcessor.preUpdate(context);
+
+		//Then no exception is thrown
+	}
 
+	/**
+	 * e.g. a non compliant OSB client which does not provide the previous_value field
+	 */
+	@Test
+	void accepts_any_update_without_previous_value() {
+		//Given a validator
+		PlanUpgradeValidatorProperties planUpgradeCheckerProperties = aPlanUpgradeCheckerProperties();
+		PlanUpgradeValidatorProcessor planUpgradeValidatorProcessor = new PlanUpgradeValidatorProcessor(planUpgradeCheckerProperties);
+
+		//Given a noop upgrade update request
+		UpdateServiceInstanceRequest request = OsbBuilderHelper.anUpdateServiceInstanceRequestWithoutPreviousValue();
+		assertThat(request.getPreviousValues()).isNull();
+
+		//Given a populated context
+		Context context = new Context();
+		context.contextKeys.put(ProcessorChainServiceInstanceService.UPDATE_SERVICE_INSTANCE_REQUEST, request);
+
+		//When
+		planUpgradeValidatorProcessor.preUpdate(context);
+
+		//Then no exception is thrown
+	}
 	@Test
 	void accepts_supported_plan_upgrade() {
 		//Given a validator