From 9f41ede37c78dd5dcea1851e114b7bdf9a5df7a5 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Wed, 5 Jun 2019 11:00:21 -0400 Subject: Reject policy deployment with version mismatch PAP should not allow multiple versions of a policy to be deployed to the same PDP. Modified the code to reject deployment requests where a different version of the policy is already deployed. This impacts both the PDP-Group-Deploy and the Simple-Deploy REST APIs. Change-Id: I586b764951c20228d0d80ec8326869215e970fdf Issue-ID: POLICY-1785 Signed-off-by: Jim Hahn --- .../main/rest/depundep/PdpGroupDeployProvider.java | 71 ++++++++++++++++++--- .../rest/depundep/TestPdpGroupDeployProvider.java | 33 ++++++++++ .../simpleDeploy/upgradeGroupDao_DiffVers.json | 72 ++++++++++++++++++++++ 3 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 main/src/test/resources/simpleDeploy/upgradeGroupDao_DiffVers.json diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java index 01b41613..e30bf63d 100644 --- a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java +++ b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java @@ -65,6 +65,8 @@ import org.slf4j.LoggerFactory; public class PdpGroupDeployProvider extends ProviderBase { private static final Logger logger = LoggerFactory.getLogger(PdpGroupDeployProvider.class); + private static final String POLICY_RESULT_NAME = "policy"; + /** * Constructs the object. @@ -326,7 +328,7 @@ public class PdpGroupDeployProvider extends ProviderBase { BeanValidationResult result = new BeanValidationResult(subgrp.getPdpType(), subgrp); result.addResult(validateSupportedTypes(data, subgrp)); - result.addResult(validatePolicies(data, subgrp)); + result.addResult(validatePolicies(data, null, subgrp)); return result; } @@ -391,7 +393,7 @@ public class PdpGroupDeployProvider extends ProviderBase { "cannot change properties")); } - result.addResult(validatePolicies(data, subgrp)); + result.addResult(validatePolicies(data, dbsub, subgrp)); container.addResult(result); return result.isValid(); @@ -444,22 +446,39 @@ public class PdpGroupDeployProvider extends ProviderBase { * Performs additional validations of the policies within a subgroup. * * @param data session data + * @param dbsub subgroup from the DB, or {@code null} if this is a new subgroup * @param subgrp the subgroup to be validated * @param result the validation result * @throws PfModelException if an error occurred */ - private ValidationResult validatePolicies(SessionData data, PdpSubGroup subgrp) throws PfModelException { + private ValidationResult validatePolicies(SessionData data, PdpSubGroup dbsub, PdpSubGroup subgrp) + throws PfModelException { + + // build a map of the DB data, from policy name to policy version + Map dbname2vers = new HashMap<>(); + if (dbsub != null) { + dbsub.getPolicies().forEach(ident -> dbname2vers.put(ident.getName(), ident.getVersion())); + } + BeanValidationResult result = new BeanValidationResult(subgrp.getPdpType(), subgrp); for (ToscaPolicyIdentifier ident : subgrp.getPolicies()) { + String actualVersion; + ToscaPolicy policy = data.getPolicy(new ToscaPolicyIdentifierOptVersion(ident)); if (policy == null) { - result.addResult(new ObjectValidationResult("policy", ident, ValidationStatus.INVALID, + result.addResult(new ObjectValidationResult(POLICY_RESULT_NAME, ident, ValidationStatus.INVALID, "unknown policy")); } else if (!subgrp.getSupportedPolicyTypes().contains(policy.getTypeIdentifier())) { - result.addResult(new ObjectValidationResult("policy", ident, ValidationStatus.INVALID, + result.addResult(new ObjectValidationResult(POLICY_RESULT_NAME, ident, ValidationStatus.INVALID, "not a supported policy for the subgroup")); + + } else if ((actualVersion = dbname2vers.get(ident.getName())) != null + && !actualVersion.equals(ident.getVersion())) { + // policy exists in the DB subgroup, but has the wrong version + result.addResult(new ObjectValidationResult(POLICY_RESULT_NAME, ident, ValidationStatus.INVALID, + "different version already deployed: " + actualVersion)); } } @@ -515,10 +534,7 @@ public class PdpGroupDeployProvider extends ProviderBase { return false; } - if (subgroup.getPolicies().contains(desiredIdent)) { - // already has the desired policy - logger.info("subgroup {} {} already contains policy {} {}", group.getName(), subgroup.getPdpType(), - desiredIdent.getName(), desiredIdent.getVersion()); + if (containsPolicy(group, subgroup, desiredIdent)) { return false; } @@ -537,4 +553,41 @@ public class PdpGroupDeployProvider extends ProviderBase { return true; }; } + + /** + * Determines if a subgroup already contains the desired policy. + * + * @param group group that contains the subgroup + * @param subgroup subgroup of interest + * @param desiredIdent identifier of the desired policy + * @return {@code true} if the subgroup contains the desired policy, {@code false} + * otherwise + * @throws PfModelRuntimeException if the subgroup contains a different version of the + * desired policy + */ + private boolean containsPolicy(PdpGroup group, PdpSubGroup subgroup, ToscaPolicyIdentifier desiredIdent) { + String desnm = desiredIdent.getName(); + String desvers = desiredIdent.getVersion(); + + for (ToscaPolicyIdentifier actualIdent : subgroup.getPolicies()) { + if (!actualIdent.getName().equals(desnm)) { + continue; + } + + // found the policy - ensure the version matches + if (!actualIdent.getVersion().equals(desvers)) { + throw new PfModelRuntimeException(Status.BAD_REQUEST, + "group " + group.getName() + " subgroup " + subgroup.getPdpType() + " policy " + desnm + + " " + desvers + " different version already deployed: " + + actualIdent.getVersion()); + } + + // already has the desired policy & version + logger.info("subgroup {} {} already contains policy {} {}", group.getName(), subgroup.getPdpType(), desnm, + desvers); + return true; + } + + return false; + } } diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java index 6d193fef..7590c645 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java @@ -521,6 +521,25 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { assertGroupUpdate(group, subgrp); } + @Test + public void testUpdateSubGroup_PolicyVersionMismatch() throws Exception { + PdpGroups groups = loadPdpGroups("createGroups.json"); + PdpGroup newgrp = groups.getGroups().get(0); + PdpGroup dbgroup = new PdpGroup(newgrp); + when(dao.getPdpGroups(dbgroup.getName())).thenReturn(Arrays.asList(dbgroup)); + + // arrange for DB policy version to be different + PdpSubGroup dbsubgrp = dbgroup.getPdpSubgroups().get(0); + dbsubgrp.getPolicies().get(0).setVersion("9.9.9"); + + when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("daoPolicyList.json")); + + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("different version already deployed"); + + assertNoGroupAction(); + } + @Test public void testValidateSubGroup_PropertiesMismatch() throws Exception { PdpGroups groups = loadPdpGroups("createGroups.json"); @@ -603,6 +622,20 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { assertUpdate(requests, GROUP1_NAME, PDP4_TYPE, PDP4); } + @Test + public void testMakeUpdater_PolicyVersionMismatch() throws Exception { + + // subgroup has a different version of the Policy + when(dao.getFilteredPdpGroups(any())).thenReturn(loadGroups("upgradeGroupDao_DiffVers.json")); + + assertThatThrownBy(() -> prov.deployPolicies(loadRequest())).isInstanceOf(PfModelRuntimeException.class) + .hasMessageContaining("pdpTypeC").hasMessageContaining("different version already deployed"); + + verify(dao, never()).createPdpGroups(any()); + verify(dao, never()).updatePdpGroups(any()); + verify(reqmap, never()).addRequest(any(PdpUpdate.class)); + } + @Test public void testMakeUpdater_NoPdps() throws Exception { diff --git a/main/src/test/resources/simpleDeploy/upgradeGroupDao_DiffVers.json b/main/src/test/resources/simpleDeploy/upgradeGroupDao_DiffVers.json new file mode 100644 index 00000000..3697389e --- /dev/null +++ b/main/src/test/resources/simpleDeploy/upgradeGroupDao_DiffVers.json @@ -0,0 +1,72 @@ +{ + "groups": [ + { + "name": "groupA", + "version": "200.2.3", + "pdpSubgroups": [ + { + "pdpType": "pdpTypeA", + "supportedPolicyTypes": [], + "pdpInstances": [ + { + "instanceId": "pdpA" + } + ], + "policies": [] + }, + { + "pdpType": "pdpTypeB", + "supportedPolicyTypes": [ + { + "name": "typeA", + "version": "100.2.3" + } + ], + "pdpInstances": [ + { + "instanceId": "pdpB" + } + ], + "policies": [] + }, + { + "pdpType": "pdpTypeC", + "supportedPolicyTypes": [ + { + "name": "typeA", + "version": "100.2.3" + } + ], + "pdpInstances": [ + { + "instanceId": "pdpC" + } + ], + "policies": [ + { + "name": "policyA", + "version": "9.9.9", + "type": "typeA", + "type_version": "100.2.3" + } + ] + }, + { + "pdpType": "pdpTypeD", + "supportedPolicyTypes": [ + { + "name": "typeA", + "version": "100.2.3" + } + ], + "pdpInstances": [ + { + "instanceId": "pdpD" + } + ], + "policies": [] + } + ] + } + ] +} -- cgit 1.2.3-korg