From 8d28c6fc936eb4bd95ad1ebd013996cff4787e0e Mon Sep 17 00:00:00 2001 From: Ali Hockla Date: Fri, 8 Nov 2019 12:54:02 -0600 Subject: Added support to return status and error if pdp-x failed to load policy Issue-ID: POLICY-2175 Change-Id: I32d2fe78846f58d6e700100dd523732817f1f04d Signed-off-by: Ali Hockla --- .../common/XacmlApplicationServiceProvider.java | 2 +- .../std/StdXacmlApplicationServiceProvider.java | 6 ++-- .../StdXacmlApplicationServiceProviderTest.java | 7 ++-- .../java/org/onap/policy/pdpx/main/XacmlState.java | 19 +++++++---- .../pdpx/main/comm/XacmlPdpUpdatePublisher.java | 18 ++++++++-- .../pdpx/main/rest/XacmlPdpApplicationManager.java | 34 +++++++++---------- .../org/onap/policy/pdpx/main/XacmlStateTest.java | 8 ++++- .../main/comm/XacmlPdpUpdatePublisherTest.java | 39 ++++++++++++++++++++-- .../onap/policy/pdp/xacml/xacmltest/TestUtils.java | 8 ++--- 9 files changed, 98 insertions(+), 43 deletions(-) diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlApplicationServiceProvider.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlApplicationServiceProvider.java index 0b9975f7..edd33c0e 100644 --- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlApplicationServiceProvider.java +++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlApplicationServiceProvider.java @@ -89,7 +89,7 @@ public interface XacmlApplicationServiceProvider { * * @param toscaPolicy object */ - boolean loadPolicy(ToscaPolicy toscaPolicy) throws XacmlApplicationException; + void loadPolicy(ToscaPolicy toscaPolicy) throws XacmlApplicationException; /** * unloadPolicy a Tosca Policy. diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java index 12135f4a..5aea3457 100644 --- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java +++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java @@ -116,7 +116,7 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica } @Override - public synchronized boolean loadPolicy(ToscaPolicy toscaPolicy) { + public synchronized void loadPolicy(ToscaPolicy toscaPolicy) throws XacmlApplicationException { try { // // Convert the policies first @@ -165,10 +165,8 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica // this.mapLoadedPolicies.put(toscaPolicy, refPath); } catch (IOException | ToscaPolicyConversionException e) { - LOGGER.error("Failed to loadPolicies {}", e); - return false; + throw new XacmlApplicationException("loadPolicy failed", e); } - return true; } @Override diff --git a/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProviderTest.java b/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProviderTest.java index 6139a528..e7ea049a 100644 --- a/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProviderTest.java +++ b/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProviderTest.java @@ -190,10 +190,10 @@ public class StdXacmlApplicationServiceProviderTest { } @Test - public void testLoadPolicy_ConversionError() throws ToscaPolicyConversionException { + public void testLoadPolicy_ConversionError() throws XacmlApplicationException, ToscaPolicyConversionException { when(trans.convertPolicy(policy)).thenReturn(null); - assertFalse(prov.loadPolicy(policy)); + assertThatThrownBy(() -> prov.loadPolicy(policy)).isInstanceOf(XacmlApplicationException.class); } @Test @@ -203,7 +203,8 @@ public class StdXacmlApplicationServiceProviderTest { final Set set = XACMLProperties.getRootPolicyIDs(prov.getProperties()); - assertTrue(prov.loadPolicy(policy)); + // Load policy + prov.loadPolicy(policy); // policy file should have been created File policyFile = new File(TEMP_DIR, "my-name_1.2.3.xml"); diff --git a/main/src/main/java/org/onap/policy/pdpx/main/XacmlState.java b/main/src/main/java/org/onap/policy/pdpx/main/XacmlState.java index 3d96b4bd..03b0e9e3 100644 --- a/main/src/main/java/org/onap/policy/pdpx/main/XacmlState.java +++ b/main/src/main/java/org/onap/policy/pdpx/main/XacmlState.java @@ -21,6 +21,7 @@ package org.onap.policy.pdpx.main; import java.util.Collections; +import org.apache.commons.lang3.StringUtils; import org.onap.policy.common.utils.network.NetworkUtil; import org.onap.policy.models.pdp.concepts.PdpMessage; import org.onap.policy.models.pdp.concepts.PdpResponseDetails; @@ -52,7 +53,6 @@ public class XacmlState { */ private final PdpStatus status; - /** * Constructs the object, initializing the state. */ @@ -105,7 +105,7 @@ public class XacmlState { * within a group/subgroup. */ - PdpStatus status2 = makeResponse(message); + PdpStatus status2 = makeResponse(message, ""); // start/stop rest controller based on state change handleXacmlRestController(); @@ -123,12 +123,12 @@ public class XacmlState { * @param message message from which to update the internal state * @return a response to the message */ - public PdpStatus updateInternalState(PdpUpdate message) { + public PdpStatus updateInternalState(PdpUpdate message, String errMessage) { status.setPdpGroup(message.getPdpGroup()); status.setPdpSubgroup(message.getPdpSubgroup()); status.setPolicies(appManager.getToscaPolicyIdentifiers()); - return makeResponse(message); + return makeResponse(message, errMessage); } /** @@ -145,11 +145,18 @@ public class XacmlState { * Makes a response to the given message, based on the current state. * * @param message message for which the response should be made + * @param errMessage the error message to be sent to PAP * @return a new response */ - private PdpStatus makeResponse(PdpMessage message) { + private PdpStatus makeResponse(PdpMessage message, String errMessage) { PdpResponseDetails resp = new PdpResponseDetails(); - resp.setResponseStatus(PdpResponseStatus.SUCCESS); + + if (StringUtils.isBlank(errMessage)) { + resp.setResponseStatus(PdpResponseStatus.SUCCESS); + } else { + resp.setResponseStatus(PdpResponseStatus.FAIL); + resp.setResponseMessage(errMessage); + } resp.setResponseTo(message.getRequestId()); PdpStatus status2 = new PdpStatus(status); diff --git a/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisher.java b/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisher.java index 686a8edc..a2f139d6 100644 --- a/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisher.java +++ b/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisher.java @@ -20,6 +20,7 @@ package org.onap.policy.pdpx.main.comm; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.Set; @@ -27,6 +28,8 @@ import org.onap.policy.common.endpoints.event.comm.client.TopicSinkClient; import org.onap.policy.models.pdp.concepts.PdpStatus; import org.onap.policy.models.pdp.concepts.PdpUpdate; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; +import org.onap.policy.pdp.xacml.application.common.XacmlApplicationException; +import org.onap.policy.pdp.xacml.application.common.XacmlPolicyUtils; import org.onap.policy.pdpx.main.XacmlState; import org.onap.policy.pdpx.main.rest.XacmlPdpApplicationManager; import org.onap.policy.pdpx.main.rest.XacmlPdpStatisticsManager; @@ -73,13 +76,24 @@ public class XacmlPdpUpdatePublisher { } } + StringBuilder errorMessage = new StringBuilder(); // Deploy a policy // if deployed policies do not contain the incoming policy load it for (ToscaPolicy policy : incomingPolicies) { if (!deployedPolicies.contains(policy)) { - appManager.loadDeployedPolicy(policy); + try { + appManager.loadDeployedPolicy(policy); + } catch (XacmlApplicationException e) { + // Failed to load policy, return error(s) to PAP + LOGGER.error("Failed to load policy: {}", policy, e); + errorMessage.append("Failed to load policy: " + policy + ": " + + e.getMessage() + XacmlPolicyUtils.LINE_SEPARATOR); + } } } + // Return current deployed policies + message.setPolicies(new ArrayList(appManager.getToscaPolicies().keySet())); + LOGGER.debug("Returning current deployed policies: {} ", message.getPolicies()); // update the policy count statistic XacmlPdpStatisticsManager stats = XacmlPdpStatisticsManager.getCurrent(); @@ -87,7 +101,7 @@ public class XacmlPdpUpdatePublisher { stats.setTotalPolicyCount(appManager.getPolicyCount()); } - sendPdpUpdate(state.updateInternalState(message)); + sendPdpUpdate(state.updateInternalState(message, errorMessage.toString())); } private void sendPdpUpdate(PdpStatus status) { diff --git a/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpApplicationManager.java b/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpApplicationManager.java index 37132a8f..2f054bd1 100644 --- a/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpApplicationManager.java +++ b/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpApplicationManager.java @@ -169,31 +169,27 @@ public class XacmlPdpApplicationManager { } /** - * Finds the appropriate application and loads the policy. + * Finds the appropriate application and loads the policy, throws an exception if it fails. * * @param policy Incoming policy + * @throws XacmlApplicationException if loadPolicy fails */ - public void loadDeployedPolicy(ToscaPolicy policy) { + public void loadDeployedPolicy(ToscaPolicy policy) throws XacmlApplicationException { for (XacmlApplicationServiceProvider application : applicationLoader) { - try { - // - // There should be only one application per policytype. We can - // put more logic surrounding enforcement of that later. For now, - // just use the first one found. - // - if (application.canSupportPolicyType(policy.getTypeIdentifier())) { - if (application.loadPolicy(policy)) { - if (LOGGER.isInfoEnabled()) { - LOGGER.info("Loaded ToscaPolicy {} into application {}", policy.getMetadata(), - application.applicationName()); - } - mapLoadedPolicies.put(policy, application); - } - return; + // + // There should be only one application per policytype. We can + // put more logic surrounding enforcement of that later. For now, + // just use the first one found. + // + if (application.canSupportPolicyType(policy.getTypeIdentifier())) { + application.loadPolicy(policy); + mapLoadedPolicies.put(policy, application); + if (LOGGER.isInfoEnabled()) { + LOGGER.info("Loaded ToscaPolicy {} into application {}", policy.getMetadata(), + application.applicationName()); } - } catch (XacmlApplicationException e) { - LOGGER.error("Failed to load the Tosca Policy", e); + return; } } } diff --git a/main/src/test/java/org/onap/policy/pdpx/main/XacmlStateTest.java b/main/src/test/java/org/onap/policy/pdpx/main/XacmlStateTest.java index eef1f1be..12d832ac 100644 --- a/main/src/test/java/org/onap/policy/pdpx/main/XacmlStateTest.java +++ b/main/src/test/java/org/onap/policy/pdpx/main/XacmlStateTest.java @@ -23,6 +23,7 @@ package org.onap.policy.pdpx.main; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -154,17 +155,22 @@ public class XacmlStateTest { req.setPdpGroup(GROUP); req.setPdpSubgroup(SUBGROUP); - PdpStatus status = state.updateInternalState(req); + PdpStatus status = state.updateInternalState(req, ""); PdpResponseDetails resp = status.getResponse(); assertNotNull(resp); assertEquals(req.getRequestId(), resp.getResponseTo()); assertEquals(PdpResponseStatus.SUCCESS, resp.getResponseStatus()); + assertNull(resp.getResponseMessage()); // ensure info was saved status = state.genHeartbeat(); assertEquals(GROUP, status.getPdpGroup()); assertEquals(SUBGROUP, status.getPdpSubgroup()); + + status = state.updateInternalState(req, "Failed to load policy: failLoadPolicy1: null"); + assertEquals(status.getResponse().getResponseMessage(), "Failed to load policy: failLoadPolicy1: null"); + assertEquals(status.getResponse().getResponseStatus(), PdpResponseStatus.FAIL); } @Test diff --git a/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisherTest.java b/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisherTest.java index 31bec51e..c8c6a816 100644 --- a/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisherTest.java +++ b/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisherTest.java @@ -22,6 +22,8 @@ package org.onap.policy.pdpx.main.comm; import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.startsWith; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -38,6 +40,7 @@ import org.onap.policy.common.endpoints.event.comm.client.TopicSinkClient; import org.onap.policy.models.pdp.concepts.PdpStatus; import org.onap.policy.models.pdp.concepts.PdpUpdate; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; +import org.onap.policy.pdp.xacml.application.common.XacmlApplicationException; import org.onap.policy.pdp.xacml.application.common.XacmlApplicationServiceProvider; import org.onap.policy.pdpx.main.XacmlState; import org.onap.policy.pdpx.main.rest.XacmlPdpApplicationManager; @@ -81,9 +84,18 @@ public class XacmlPdpUpdatePublisherTest { @Mock private ToscaPolicy added2; + @Mock + private ToscaPolicy failPolicy1; + + @Mock + private ToscaPolicy failPolicy2; + @Mock private PdpUpdate update; + @Mock + private PdpUpdate failurePdpUpdate; + private XacmlPdpUpdatePublisher publisher; @@ -105,9 +117,12 @@ public class XacmlPdpUpdatePublisherTest { List updatePolicies = Arrays.asList(added1, deployed2, deployed3, added2); when(update.getPolicies()).thenReturn(updatePolicies); + List failureUpdatePolicies = Arrays.asList(added1, deployed2, deployed3, failPolicy1, failPolicy2); + when(failurePdpUpdate.getPolicies()).thenReturn(failureUpdatePolicies); + when(appmgr.getPolicyCount()).thenReturn(NEW_COUNT); - when(state.updateInternalState(update)).thenReturn(status); + when(state.updateInternalState(any(), any())).thenReturn(status); when(client.send(any())).thenReturn(true); @@ -115,7 +130,7 @@ public class XacmlPdpUpdatePublisherTest { } @Test - public void testHandlePdpUpdate() { + public void testHandlePdpUpdate() throws XacmlApplicationException { XacmlPdpStatisticsManager statmgr = new XacmlPdpStatisticsManager(); XacmlPdpStatisticsManager.setCurrent(statmgr); @@ -141,7 +156,25 @@ public class XacmlPdpUpdatePublisherTest { } @Test - public void testHandlePdpUpdate_NullPolicies() { + public void testHandlePdpUpdate_LoadPolicyFailed() throws XacmlApplicationException { + // Set loadPolicy to fail + doThrow(new XacmlApplicationException()).when(appmgr).loadDeployedPolicy(failPolicy1); + doThrow(new XacmlApplicationException()).when(appmgr).loadDeployedPolicy(failPolicy2); + + publisher.handlePdpUpdate(failurePdpUpdate); + + // two removed + verify(appmgr).removeUndeployedPolicy(deployed1); + verify(appmgr).removeUndeployedPolicy(deployed4); + + verify(failurePdpUpdate).setPolicies(any()); + + verify(state).updateInternalState(any(), startsWith("Failed to load policy")); + verify(client).send(status); + } + + @Test + public void testHandlePdpUpdate_NullPolicies() throws XacmlApplicationException { when(update.getPolicies()).thenReturn(null); publisher.handlePdpUpdate(update); diff --git a/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java b/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java index 70f9ebc0..cb12e0f6 100644 --- a/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java +++ b/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java @@ -25,7 +25,6 @@ package org.onap.policy.pdp.xacml.xacmltest; import java.util.ArrayList; import java.util.List; import java.util.Map; - import org.onap.policy.common.utils.coder.CoderException; import org.onap.policy.common.utils.coder.StandardYamlCoder; import org.onap.policy.common.utils.resources.ResourceUtils; @@ -82,10 +81,11 @@ public class TestUtils { // for (Map policies : completedJtst.getToscaTopologyTemplate().getPolicies()) { for (ToscaPolicy policy : policies.values()) { - if (service.loadPolicy(policy)) { + try { + service.loadPolicy(policy); loadedPolicies.add(policy); - } else { - LOGGER.error("Application failed to load policy"); + } catch (XacmlApplicationException e) { + LOGGER.error("Application failed to load policy", e); } } } -- cgit 1.2.3-korg