From 593a0bb146ff49442c78e786ab969b28b599d81a Mon Sep 17 00:00:00 2001 From: Stanislav Trailov Date: Tue, 11 Jul 2023 08:59:12 +0300 Subject: [PATCH] Expose approval remark and decided by in rollout mgmt API (#1389) Signed-off-by: Stanislav Trailov --- .../util/RolloutTestApprovalStrategy.java | 8 ++++- .../rollout/MgmtRolloutResponseBody.java | 22 +++++++++++++ .../mgmt/rest/resource/MgmtRolloutMapper.java | 3 ++ .../resource/MgmtRolloutResourceTest.java | 32 +++++++++++++++++++ .../RolloutResourceDocumentationTest.java | 20 +++++++++--- 5 files changed, 79 insertions(+), 6 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/RolloutTestApprovalStrategy.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/RolloutTestApprovalStrategy.java index e065c4d43..7cc0581f1 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/RolloutTestApprovalStrategy.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/RolloutTestApprovalStrategy.java @@ -19,6 +19,8 @@ public class RolloutTestApprovalStrategy implements RolloutApprovalStrategy { private boolean approvalNeeded = false; + private String approvalDecidedBy; + @Override public boolean isApprovalNeeded(Rollout rollout) { return approvalNeeded; @@ -28,6 +30,10 @@ public class RolloutTestApprovalStrategy implements RolloutApprovalStrategy { this.approvalNeeded = approvalNeeded; } + public void setApproveDecidedBy(final String user) { + this.approvalDecidedBy = user; + } + @Override public void onApprovalRequired(Rollout rollout) { // do nothing, as no action is needed when testing @@ -35,6 +41,6 @@ public class RolloutTestApprovalStrategy implements RolloutApprovalStrategy { @Override public String getApprovalUser(Rollout rollout) { - return null; + return approvalDecidedBy; } } diff --git a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/rollout/MgmtRolloutResponseBody.java b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/rollout/MgmtRolloutResponseBody.java index b86871229..762fe4f87 100644 --- a/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/rollout/MgmtRolloutResponseBody.java +++ b/hawkbit-rest/hawkbit-mgmt-api/src/main/java/org/eclipse/hawkbit/mgmt/json/model/rollout/MgmtRolloutResponseBody.java @@ -59,6 +59,12 @@ public class MgmtRolloutResponseBody extends MgmtNamedEntity { @JsonProperty private Integer weight; + @JsonProperty + private String approvalRemark; + + @JsonProperty + private String approveDecidedBy; + public boolean isDeleted() { return deleted; } @@ -158,4 +164,20 @@ public class MgmtRolloutResponseBody extends MgmtNamedEntity { public Integer getTotalGroups() { return totalGroups; } + + public void setApprovalRemark(final String approvalRemark) { + this.approvalRemark = approvalRemark; + } + + public String getApprovalRemark() { + return approvalRemark; + } + + public void setApproveDecidedBy(final String approveDecidedBy) { + this.approveDecidedBy = approveDecidedBy; + } + + public String getApproveDecidedBy(final String approveDecidedBy) { + return approveDecidedBy; + } } diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutMapper.java b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutMapper.java index 1a2d7070e..74b20db56 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutMapper.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutMapper.java @@ -95,6 +95,9 @@ final class MgmtRolloutMapper { body.setTotalGroups(rollout.getRolloutGroupsCreated()); body.setStartAt(rollout.getStartAt()); + body.setApproveDecidedBy(rollout.getApprovalDecidedBy()); + body.setApprovalRemark(rollout.getApprovalRemark()); + body.add(linkTo(methodOn(MgmtRolloutRestApi.class).start(rollout.getId())).withRel("start").expand()); body.add(linkTo(methodOn(MgmtRolloutRestApi.class).pause(rollout.getId())).withRel("pause").expand()); body.add(linkTo(methodOn(MgmtRolloutRestApi.class).resume(rollout.getId())).withRel("resume").expand()); diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutResourceTest.java b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutResourceTest.java index 60b7990d9..bace8c0af 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutResourceTest.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutResourceTest.java @@ -48,6 +48,7 @@ import org.eclipse.hawkbit.repository.model.RolloutGroup.RolloutGroupSuccessCond import org.eclipse.hawkbit.repository.model.RolloutGroupConditionBuilder; import org.eclipse.hawkbit.repository.model.RolloutGroupConditions; import org.eclipse.hawkbit.repository.model.Target; +import org.eclipse.hawkbit.repository.test.util.RolloutTestApprovalStrategy; import org.eclipse.hawkbit.repository.test.util.WithSpringAuthorityRule; import org.eclipse.hawkbit.repository.test.util.WithUser; import org.eclipse.hawkbit.rest.util.JsonBuilder; @@ -83,6 +84,9 @@ class MgmtRolloutResourceTest extends AbstractManagementApiIntegrationTest { @Autowired private RolloutGroupManagement rolloutGroupManagement; + @Autowired + private RolloutTestApprovalStrategy approvalStrategy; + @Test @Description("Testing that creating rollout with wrong body returns bad request") void createRolloutWithInvalidBodyReturnsBadRequest() throws Exception { @@ -1286,6 +1290,34 @@ class MgmtRolloutResourceTest extends AbstractManagementApiIntegrationTest { assertThat(rollouts.get(0).getWeight()).get().isEqualTo(weight); } + @Test + @Description("Check if approvalDecidedBy and approvalRemark are present when rollout is approved") + public void validateIfApprovalFieldsArePresentAfterApproval() throws Exception { + approvalStrategy.setApprovalNeeded(true); + approvalStrategy.setApproveDecidedBy("testUser"); + final int amountTargets = 2; + final String remark = "Some remark"; + final List targets = testdataFactory.createTargets(amountTargets, "rollout"); + final DistributionSet dsA = testdataFactory.createDistributionSet(""); + final Rollout rollout = createRollout("rollout1", 3, dsA.getId(), "controllerId==rollout*", false); + + rolloutHandler.handleAll(); + + mvc.perform(get("/rest/v1/rollouts/{rolloutid}", rollout.getId())).andDo(MockMvcResultPrinter.print()) + .andExpect(status().isOk()).andExpect(jsonPath("$.status", equalTo("waiting_for_approval"))); + + rolloutManagement.approveOrDeny(rollout.getId(), Rollout.ApprovalDecision.APPROVED, remark); + + mvc.perform(get("/rest/v1/rollouts/{rolloutid}", rollout.getId())).andDo(MockMvcResultPrinter.print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.status", equalTo("ready"))) + .andExpect(jsonPath("$.approvalRemark", equalTo(remark))) + .andExpect(jsonPath("$.approveDecidedBy", equalTo("testUser"))); + + // revert + approvalStrategy.setApprovalNeeded(false); + } + private void postRollout(final String name, final int groupSize, final Long distributionSetId, final String targetFilterQuery, final int targets, final Action.ActionType type) throws Exception { postRollout(name, groupSize, distributionSetId, targetFilterQuery, targets, type, null, null); diff --git a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/mgmt/documentation/RolloutResourceDocumentationTest.java b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/mgmt/documentation/RolloutResourceDocumentationTest.java index 2fe15c6f9..ac912e2a2 100644 --- a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/mgmt/documentation/RolloutResourceDocumentationTest.java +++ b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/mgmt/documentation/RolloutResourceDocumentationTest.java @@ -87,7 +87,7 @@ public class RolloutResourceDocumentationTest extends AbstractApiRestDocumentati mockMvc.perform(get(MgmtRestConstants.ROLLOUT_V1_REQUEST_MAPPING).accept(MediaTypes.HAL_JSON_VALUE)) .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()) .andExpect(content().contentType(MediaTypes.HAL_JSON)) - .andDo(this.document.document(getRolloutResponseFields(true, false, + .andDo(this.document.document(getRolloutResponseFields(true, false, false, fieldWithPath("total").description(ApiModelPropertiesGeneric.TOTAL_ELEMENTS), fieldWithPath("size").type(JsonFieldType.NUMBER).description(ApiModelPropertiesGeneric.SIZE), fieldWithPath("content").description(MgmtApiModelProperties.ROLLOUT_LIST)))); @@ -106,7 +106,7 @@ public class RolloutResourceDocumentationTest extends AbstractApiRestDocumentati .andDo(this.document.document(getFilterRequestParamter())); } - private Snippet getRolloutResponseFields(final boolean isArray, final boolean withDetails, + private Snippet getRolloutResponseFields(final boolean isArray, final boolean withDetails, final boolean isApproveRequired, final FieldDescriptor... descriptors) { final String arrayPrefix = getArrayPrefix(isArray); final List allFieldDescriptor = new ArrayList<>(); @@ -162,6 +162,12 @@ public class RolloutResourceDocumentationTest extends AbstractApiRestDocumentati fieldWithPath(arrayPrefix + "_links.deny").description(MgmtApiModelProperties.ROLLOUT_LINKS_DENY)); allFieldDescriptor.add(fieldWithPath(arrayPrefix + "_links.distributionset") .description(MgmtApiModelProperties.LINK_TO_DS)); + if (isApproveRequired) { + allFieldDescriptor.add(fieldWithPath(arrayPrefix + "approveDecidedBy") + .description("Who Approved/Denied the rollout. Not present if the rollout is missing approval.")); + allFieldDescriptor.add(fieldWithPath(arrayPrefix + "approvalRemark") + .description("A user remark of the approve/denied decision. Not present if the rollout is missing approval.")); + } } return new DocumenationResponseFieldsSnippet(allFieldDescriptor); @@ -172,12 +178,16 @@ public class RolloutResourceDocumentationTest extends AbstractApiRestDocumentati + SpPermission.READ_ROLLOUT) public void getRollout() throws Exception { enableMultiAssignments(); + approvalStrategy.setApprovalNeeded(true); + approvalStrategy.setApproveDecidedBy("exampleUsername"); final Rollout rollout = createRolloutEntity(); + rolloutManagement.approveOrDeny(rollout.getId(), Rollout.ApprovalDecision.APPROVED, "Approved remark."); + mockMvc.perform(get(MgmtRestConstants.ROLLOUT_V1_REQUEST_MAPPING + "/{rolloutId}", rollout.getId()) .accept(MediaTypes.HAL_JSON_VALUE)).andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()) .andExpect(content().contentType(MediaTypes.HAL_JSON)) - .andDo(this.document.document(getRolloutResponseFields(false, true), + .andDo(this.document.document(getRolloutResponseFields(false, true, true), pathParameters(parameterWithName("rolloutId").description(ApiModelPropertiesGeneric.ITEM_ID)))); } @@ -256,7 +266,7 @@ public class RolloutResourceDocumentationTest extends AbstractApiRestDocumentati optionalRequestFieldWithPath("errorAction.expression") .description(MgmtApiModelProperties.ROLLOUT_ERROR_ACTION_EXP)), - getRolloutResponseFields(false, true))); + getRolloutResponseFields(false, true, false))); } @@ -386,7 +396,7 @@ public class RolloutResourceDocumentationTest extends AbstractApiRestDocumentati .attributes(key("value").value("['pause']")), optionalRequestFieldWithPath("groups[].errorAction.expression") .description(MgmtApiModelProperties.ROLLOUT_ERROR_ACTION_EXP)), - getRolloutResponseFields(false, true))); + getRolloutResponseFields(false, true, false))); } @Test