From 1bedb591cc68c10c7db916fda8a3f02d67c2314d Mon Sep 17 00:00:00 2001 From: Pamela Dragosh Date: Sun, 3 Nov 2019 17:42:09 -0500 Subject: [PATCH] More examples of optimization policies and cleanup Fixed a couple of sonar issues - log exception and do not nest more 3 if-else-try. Cleaned up the JUnit test to make debugging a bit easier. Added more examples for testing optimization tests. Moved the target back into the Policy and kept the Condition on the Rule. Works exactly the same, just a bit cleaner and one less rule to deal with. Issue-ID: POLICY-2066 Change-Id: Ife28dc2ce959dcf1fb8ca72061ebc5dca862a7f4 Signed-off-by: Pamela Dragosh --- .../common/std/StdMatchableTranslator.java | 61 ++++--- .../OptimizationPdpApplicationTest.java | 188 ++++++++++----------- .../vCPE.policies.optimization.input.tosca.yaml | 94 ++++++++++- .../onap/policy/pdp/xacml/xacmltest/TestUtils.java | 3 +- 4 files changed, 215 insertions(+), 131 deletions(-) diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java index 7b47ad14..66770e91 100644 --- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java +++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java @@ -140,9 +140,10 @@ public class StdMatchableTranslator extends StdBaseTranslator { // newPolicyType.setRuleCombiningAlgId(XACML3.ID_RULE_FIRST_APPLICABLE.stringValue()); // - // Generate the TargetType + // Generate the TargetType - the policy should not be evaluated + // unless all the matchable properties it cares about are matched. // - newPolicyType.setTarget(new TargetType()); + newPolicyType.setTarget(generateTargetType(toscaPolicy.getProperties(), toscaPolicyTypes)); // // Now represent the policy as Json // @@ -158,30 +159,18 @@ public class StdMatchableTranslator extends StdBaseTranslator { // addObligation(newPolicyType, jsonPolicy); // - // Now create the Permit Rule for all the - // matchable properties. + // Now create the Permit Rule. // RuleType rule = new RuleType(); rule.setDescription("Default is to PERMIT if the policy matches."); rule.setRuleId(policyName + ":rule"); rule.setEffect(EffectType.PERMIT); - rule.setTarget(generateTargetType(toscaPolicy.getProperties(), toscaPolicyTypes)); - rule.setCondition(generateConditionForPolicyType(toscaPolicy.getType())); - // - // Add the rule to the policy - // - newPolicyType.getCombinerParametersOrRuleCombinerParametersOrVariableDefinition().add(rule); + rule.setTarget(new TargetType()); // - // If a Decision is for a specific policy-type, make sure - // there is a rule for it. + // The rule contains the Condition which adds logic for + // optional policy-type filtering. // - rule = new RuleType(); - rule.setDescription("Match on policy-type " + toscaPolicy.getType()); - rule.setRuleId(policyName + ":rule:policy-type"); - rule.setEffect(EffectType.PERMIT); - TargetType target = new TargetType(); - target.getAnyOf().add(this.generateAnyOfForPolicyType(toscaPolicy.getType())); - rule.setTarget(target); + rule.setCondition(generateConditionForPolicyType(toscaPolicy.getType())); // // Add the rule to the policy // @@ -219,7 +208,7 @@ public class StdMatchableTranslator extends StdBaseTranslator { // Find matchable properties // if (isMatchable(entrySet.getKey(), policyTypes)) { - LOGGER.info("Found matchable property {}", entrySet.getValue()); + LOGGER.info("Found matchable property {}", entrySet.getKey()); generateMatchable(targetType, entrySet.getKey(), entrySet.getValue()); } } @@ -238,20 +227,36 @@ public class StdMatchableTranslator extends StdBaseTranslator { protected boolean isMatchable(String propertyName, Collection policyTypes) { for (ToscaPolicyType policyType : policyTypes) { for (Entry propertiesEntry : policyType.getProperties().entrySet()) { - if (! propertiesEntry.getKey().equals(propertyName) - || propertiesEntry.getValue().getMetadata() == null) { - continue; - } - for (Entry entrySet : propertiesEntry.getValue().getMetadata().entrySet()) { - if ("matchable".equals(entrySet.getKey()) && "true".equals(entrySet.getValue())) { - return true; - } + if (checkIsMatchableProperty(propertyName, propertiesEntry)) { + return true; } } } return false; } + /** + * checkIsMatchableProperty - checks the policy contents for matchable field. If the metadata doesn't exist, + * then definitely not. If the property doesn't exist, then definitely not. Otherwise need to have a metadata + * section with the matchable property set to true. + * + * @param propertyName String value of property + * @param propertiesEntry Section of the TOSCA Policy Type where properties and metadata sections are held + * @return true if matchable + */ + protected boolean checkIsMatchableProperty(String propertyName, Entry propertiesEntry) { + if (! propertiesEntry.getKey().equals(propertyName) + || propertiesEntry.getValue().getMetadata() == null) { + return false; + } + for (Entry entrySet : propertiesEntry.getValue().getMetadata().entrySet()) { + if ("matchable".equals(entrySet.getKey()) && "true".equals(entrySet.getValue())) { + return true; + } + } + return false; + } + /** * generateMatchable - Given the object, generates list of MatchType objects and add them * to the TargetType object. diff --git a/applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java b/applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java index d7e85c47..1bcb5222 100644 --- a/applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java +++ b/applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java @@ -29,7 +29,6 @@ import static org.mockito.Mockito.when; import com.att.research.xacml.api.Response; import com.google.common.collect.Lists; - import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -183,10 +182,11 @@ public class OptimizationPdpApplicationTest { } @Test - public void test02NoPolicies() { + public void test02NoPolicies() throws CoderException { // // Ask for a decision when there are no policies loaded // + LOGGER.info("Request {}", gson.encode(baseRequest)); Pair decision = service.makeDecision(baseRequest, null); LOGGER.info("Decision {}", decision.getKey()); @@ -202,26 +202,50 @@ public class OptimizationPdpApplicationTest { // TestUtils.loadPolicies("src/test/resources/vCPE.policies.optimization.input.tosca.yaml", service); // - // Ask for a decision for default + // Ask for a decision for available default policies // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); + DecisionResponse response = makeDecision(); - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(1); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(2); + // + // Validate it + // + validateDecision(response, baseRequest); + } + + @SuppressWarnings("unchecked") + @Test + public void test04OptimizationDefaultHpa() throws CoderException, FileNotFoundException, IOException, + XacmlApplicationException { + // + // Add in policy type // - // Double check that the contents are what we expect + List policyTypes = Lists.newArrayList("onap.policies.optimization.HpaPolicy"); + baseRequest.getResource().put("policy-type", policyTypes); // - LOGGER.info(gson.encode(decision.getKey())); + // Ask for a decision for default HPA policy + // + DecisionResponse response = makeDecision(); + + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(1); + response.getPolicies().forEach((key, value) -> { + assertThat(((Map) value).get("type")).isEqualTo(("onap.policies.optimization.HpaPolicy")); + }); // // Validate it // - validateDecision(decision.getKey(), baseRequest); + validateDecision(response, baseRequest); } @SuppressWarnings("unchecked") @Test - public void test04OptimizationDefaultGeography() throws CoderException { + public void test05OptimizationDefaultGeography() throws CoderException { + // + // Remove all the policy-type resources from the request + // + cleanOutResources(); // // Add US to the geography list // @@ -229,24 +253,18 @@ public class OptimizationPdpApplicationTest { // // Ask for a decision for default US Policy // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); - - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(2); - // - // Double check that the contents are what we expect - // - LOGGER.info(gson.encode(decision.getKey())); + DecisionResponse response = makeDecision(); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(3); // Should be 1 // // Validate it // - validateDecision(decision.getKey(), baseRequest); + validateDecision(response, baseRequest); } @SuppressWarnings("unchecked") @Test - public void test05OptimizationDefaultGeographyAndService() throws CoderException { + public void test06OptimizationDefaultGeographyAndService() throws CoderException { // // Add vCPE to the service list // @@ -254,24 +272,19 @@ public class OptimizationPdpApplicationTest { // // Ask for a decision for default US policy for vCPE service // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); + DecisionResponse response = makeDecision(); - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(5); - // - // Double check that the contents are what we expect - // - LOGGER.info(gson.encode(decision.getKey())); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(6); // should be 1 // // Validate it // - validateDecision(decision.getKey(), baseRequest); + validateDecision(response, baseRequest); } @SuppressWarnings("unchecked") @Test - public void test06OptimizationDefaultGeographyAndServiceAndResource() throws CoderException { + public void test07OptimizationDefaultGeographyAndServiceAndResource() throws CoderException { // // Add vCPE to the service list // @@ -279,24 +292,19 @@ public class OptimizationPdpApplicationTest { // // Ask for a decision for default US service vCPE resource vG policy // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); + DecisionResponse response = makeDecision(); - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(9); - // - // Double check that the contents are what we expect - // - LOGGER.info(gson.encode(decision.getKey())); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(11); // should be 4 // // Validate it // - validateDecision(decision.getKey(), baseRequest); + validateDecision(response, baseRequest); } @SuppressWarnings("unchecked") @Test - public void test07OptimizationGeographyAndServiceAndResourceAndScope() throws CoderException { + public void test08OptimizationGeographyAndServiceAndResourceAndScope() throws CoderException { // // Add gold as a scope // @@ -304,24 +312,19 @@ public class OptimizationPdpApplicationTest { // // Ask for a decision for specific US vCPE vG gold // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); + DecisionResponse response = makeDecision(); - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(10); - // - // Double check that the contents are what we expect - // - LOGGER.info(gson.encode(decision.getKey())); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(12); // should be 1 // // Validate it // - validateDecision(decision.getKey(), baseRequest); + validateDecision(response, baseRequest); } @SuppressWarnings("unchecked") @Test - public void test08OptimizationGeographyAndServiceAndResourceAndScopeIsGoldOrPlatinum() throws CoderException { + public void test09OptimizationGeographyAndServiceAndResourceAndScopeIsGoldOrPlatinum() throws CoderException { // // Add platinum to the scope list: this is now gold OR platinum // @@ -329,24 +332,19 @@ public class OptimizationPdpApplicationTest { // // Ask for a decision for specific US vCPE vG (gold or platinum) // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); + DecisionResponse response = makeDecision(); - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(11); - // - // Double check that the contents are what we expect - // - LOGGER.info(gson.encode(decision.getKey())); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(14); // should be 3 // // Validate it // - validateDecision(decision.getKey(), baseRequest); + validateDecision(response, baseRequest); } @SuppressWarnings("unchecked") @Test - public void test09OptimizationGeographyAndServiceAndResourceAndScopeNotGold() throws CoderException { + public void test10OptimizationGeographyAndServiceAndResourceAndScopeNotGold() throws CoderException { // // Add gold as a scope // @@ -354,27 +352,18 @@ public class OptimizationPdpApplicationTest { // // Ask for a decision for specific US vCPE vG gold // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); + DecisionResponse response = makeDecision(); - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(11); - // - // Double check that the contents are what we expect - // - LOGGER.info(gson.encode(decision.getKey())); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(13); // should be 2 // // Validate it // - validateDecision(decision.getKey(), baseRequest); + validateDecision(response, baseRequest); } @Test - public void test10OptimizationPolicyTypeDefault() throws CoderException { - // - // Remove all the other resources from the request - // - cleanOutResources(); + public void test11OptimizationPolicyTypeDefault() throws CoderException { // // Add in policy type // @@ -383,40 +372,44 @@ public class OptimizationPdpApplicationTest { // // Ask for a decision for default // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); + DecisionResponse response = makeDecision(); - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(4); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(4); // should be 1 // - // Double check that the contents are what we expect + // Validate it // - LOGGER.info(gson.encode(decision.getKey())); + validateDecision(response, baseRequest); } + @SuppressWarnings("unchecked") @Test - public void test20OptimizationPolicyTypeDefault() throws CoderException { - // - // Remove all the other resources from the request - // - cleanOutResources(); + public void test12OptimizationPolicyTypeDefault() throws CoderException { // - // Add in policy type + // Add in another policy type // - List policyTypes = Lists.newArrayList("onap.policies.optimization.HpaPolicy"); - baseRequest.getResource().put("policy-type", policyTypes); + ((List) baseRequest.getResource().get("policy-type")).add("onap.policies.optimization.HpaPolicy"); // // Ask for a decision for default // - Pair decision = service.makeDecision(baseRequest, null); - LOGGER.info("Decision {}", decision.getKey()); + DecisionResponse response = makeDecision(); - assertThat(decision.getKey()).isNotNull(); - assertThat(decision.getKey().getPolicies().size()).isEqualTo(1); + assertThat(response).isNotNull(); + assertThat(response.getPolicies().size()).isEqualTo(6); // should be 2 // - // Double check that the contents are what we expect + // Validate it // - LOGGER.info(gson.encode(decision.getKey())); + validateDecision(response, baseRequest); + } + + private DecisionResponse makeDecision() { + Pair decision = service.makeDecision(baseRequest, null); + LOGGER.info("Request Resources {}", baseRequest.getResource()); + LOGGER.info("Decision {}", decision.getKey()); + for (Entry entrySet : decision.getKey().getPolicies().entrySet()) { + LOGGER.info("Policy {}", entrySet.getKey()); + } + return decision.getKey(); } @SuppressWarnings("unchecked") @@ -465,5 +458,8 @@ public class OptimizationPdpApplicationTest { ((List)baseRequest.getResource().get("services")).clear(); ((List)baseRequest.getResource().get("resources")).clear(); ((List)baseRequest.getResource().get("geography")).clear(); + if (((List)baseRequest.getResource().get("policy-type")) != null) { + baseRequest.getResource().remove("policy-type"); + } } } diff --git a/applications/optimization/src/test/resources/vCPE.policies.optimization.input.tosca.yaml b/applications/optimization/src/test/resources/vCPE.policies.optimization.input.tosca.yaml index 919a2f8b..093156c6 100644 --- a/applications/optimization/src/test/resources/vCPE.policies.optimization.input.tosca.yaml +++ b/applications/optimization/src/test/resources/vCPE.policies.optimization.input.tosca.yaml @@ -38,12 +38,12 @@ topology_template: qualifier: same category: complex - - OSDF_CASABLANCA.Affinity_Default_vCPE_0: + OSDF_CASABLANCA.Affinity_Default_vCPE_US_0: type: onap.policies.optimization.AffinityPolicy version: 1.0.0 type_version: 1.0.0 metadata: - policy-id: OSDF_CASABLANCA.Affinity_Default_vCPE_0 + policy-id: OSDF_CASABLANCA.Affinity_Default_vCPE_US_0 policy-version: 1 properties: scope: [] @@ -56,7 +56,7 @@ topology_template: qualifier: different category: complex - - OSDF_CASABLANCA.Affinity_vCPE_1: + OSDF_CASABLANCA.Affinity_vCPE_US_Gold_1: type: onap.policies.optimization.AffinityPolicy version: 1.0.0 type_version: 1.0.0 @@ -64,7 +64,7 @@ topology_template: policy-id: OSDF_CASABLANCA.Affinity_vCPE_1 policy-version: 1 properties: - scope: [gold, platinum] + scope: [gold] services: [vCPE] resources: [vGMuxInfra, vG] geography: [US, INTERNATIONAL] @@ -73,6 +73,24 @@ topology_template: affinityProperties: qualifier: same category: availabilityZone + - + OSDF_CASABLANCA.Affinity_vCPE_US_Platinum_1: + type: onap.policies.optimization.AffinityPolicy + version: 1.0.0 + type_version: 1.0.0 + metadata: + policy-id: OSDF_CASABLANCA.Affinity_vCPE_1 + policy-version: 1 + properties: + scope: [platinum] + services: [vCPE] + resources: [vGMuxInfra, vG] + geography: [US, INTERNATIONAL] + identity: affinity_vCPE + applicableResources: any + affinityProperties: + qualifier: different + category: availabilityZone - OSDF_CASABLANCA.Capacity_vG_1: type: onap.policies.optimization.Vim_fit @@ -91,6 +109,24 @@ topology_template: capacityProperty: controller: multicloud request: "{\"vCPU\": 10, \"Memory\": {\"quantity\": {\"get_param\": \"REQUIRED_MEM\"}, \"unit\": \"GB\"}, \"Storage\": {\"quantity\": {\"get_param\": \"REQUIRED_DISK\"}, \"unit\": \"GB\"}}" + - + OSDF_CASABLANCA.Capacity_vG_2: + type: onap.policies.optimization.Vim_fit + version: 1.0.0 + type_version: 1.0.0 + metadata: + policy-id: OSDF_CASABLANCA.Capacity_vG_2 + policy-version: 1 + properties: + scope: [] + services: [vCPE] + resources: [vG] + geography: [US, INTERNATIONAL] + identity: capacity_vG + applicableResources: any + capacityProperty: + controller: multicloud + request: "{\"vCPU\": 15, \"Memory\": {\"quantity\": {\"get_param\": \"REQUIRED_MEM\"}, \"unit\": \"MB\"}, \"Storage\": {\"quantity\": {\"get_param\": \"REQUIRED_DISK\"}, \"unit\": \"GB\"}}" - OSDF_CASABLANCA.Distance_vG_1: type: onap.policies.optimization.DistancePolicy @@ -112,6 +148,54 @@ topology_template: value: 1500 operator: "<" unit: km + - + OSDF_CASABLANCA.hpa_policy_Default: + type: onap.policies.optimization.HpaPolicy + version: 1.0.0 + type_version: 1.0.0 + metadata: + policy-id: OSDF_CASABLANCA.hpa_policy_Default + policy-version: 1 + properties: + scope: [] + services: [] + resources: [] + geography: [] + identity: hpa-vG + flavorFeatures: + - + id: vg_1 + type: vnfc + directives: + - type: flavor_directives + attributes: + - attribute_name: flavor_label_vm_01 + attribute_value: "" + flavorProperties: + - + hpa-feature: basicCapabilities + mandatory: True + architecture: generic + directives: [] + hpa-feature-attributes: + - hpa-attribute-key: numVirtualCpu + hpa-attribute-value: 8 + operator: ['>='] + unit: "" + - hpa-attribute-key: virtualMemSize + hpa-attribute-value: 6 + operator: ['<='] + unit: "" + - + hpa-feature: ovsDpdk + mandatory: False + architecture: generic + directives: [] + hpa-feature-attributes: + - hpa-attribute-key: dataProcessingAccelerationLibrary + hpa-attribute-value: ovsDpdk_version + operator: [=] + unit: "" - OSDF_CASABLANCA.hpa_policy_vG_1: type: onap.policies.optimization.HpaPolicy @@ -122,7 +206,7 @@ topology_template: policy-version: 1 properties: scope: [] - services: [vCPE] + services: [vCPE, vOtherService] resources: [vG] geography: [] identity: hpa-vG 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 16bea1d1..70f9ebc0 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 @@ -69,8 +69,7 @@ public class TestUtils { try { serviceTemplate = yamlCoder.decode(policyYaml, ToscaServiceTemplate.class); } catch (CoderException e) { - throw new XacmlApplicationException("Failed to decode policy from resource file " - + e.getLocalizedMessage()); + throw new XacmlApplicationException("Failed to decode policy from resource file", e); } // // Make sure all the fields are setup properly -- 2.16.6