RANGER-632 Simplify policy and service-def validation related error messages that would be seen by the user.
Signed-off-by: Alok Lal <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/eda1dd7d Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/eda1dd7d Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/eda1dd7d Branch: refs/heads/HDP-2.3.2-groupid Commit: eda1dd7dfca72f9b224d760abb6e762e060b3bec Parents: 23d8bef Author: Alok Lal <[email protected]> Authored: Fri Sep 4 18:42:05 2015 -0700 Committer: Alok Lal <[email protected]> Committed: Tue Sep 15 09:34:23 2015 -0700 ---------------------------------------------------------------------- .../plugin/errors/ValidationErrorCode.java | 36 +++++++++--------- .../model/validation/RangerPolicyValidator.java | 39 ++++++++++++++------ .../validation/ValidationFailureDetails.java | 2 +- 3 files changed, 48 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/eda1dd7d/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java index b458394..72f7205 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java @@ -32,12 +32,12 @@ public enum ValidationErrorCode { SERVICE_VALIDATION_ERR_NULL_SERVICE_OBJECT(1003, "Internal error: service object passed in was null"), SERVICE_VALIDATION_ERR_EMPTY_SERVICE_ID(1004, "Internal error: service id was null/empty/blank"), SERVICE_VALIDATION_ERR_INVALID_SERVICE_ID(1005, "No service found for id [{0}]"), - SERVICE_VALIDATION_ERR_INVALID_SERVICE_NAME(1006, "Service name[{0}] was null/empty/blank"), - SERVICE_VALIDATION_ERR_SERVICE_NAME_CONFICT(1007, "service with the name[{0}] already exists"), - SERVICE_VALIDATION_ERR_ID_NAME_CONFLICT(1008, "id/name conflict: another service already exists with name[{0}], its id is [{1}]"), - SERVICE_VALIDATION_ERR_MISSING_SERVICE_DEF(1009, "service def [{0}] was null/empty/blank"), - SERVICE_VALIDATION_ERR_INVALID_SERVICE_DEF(1010, "service def named[{0}] not found"), - SERVICE_VALIDATION_ERR_REQUIRED_PARM_MISSING(1011, "required configuration parameter is missing; missing parameters: {0}"), + SERVICE_VALIDATION_ERR_INVALID_SERVICE_NAME(1006, "Missing service name"), + SERVICE_VALIDATION_ERR_SERVICE_NAME_CONFICT(1007, "Duplicate service name: name=[{0}]"), + SERVICE_VALIDATION_ERR_ID_NAME_CONFLICT(1008, "Duplicate service name: name=[{0}], id=[{1}]"), + SERVICE_VALIDATION_ERR_MISSING_SERVICE_DEF(1009, "Missing service def"), + SERVICE_VALIDATION_ERR_INVALID_SERVICE_DEF(1010, "Service def not found: service-def-name=[{0}]"), + SERVICE_VALIDATION_ERR_REQUIRED_PARM_MISSING(1011, "Missing required configuration parameter(s): missing parameters={0}"), // SERVICE-DEF VALIDATION SERVICE_DEF_VALIDATION_ERR_UNSUPPORTED_ACTION(2001, "Internal error: unsupported action[{0}]; isValid(Long) is only supported for DELETE"), @@ -65,26 +65,28 @@ public enum ValidationErrorCode { POLICY_VALIDATION_ERR_UNSUPPORTED_ACTION(3001, "Internal error: method signature isValid(Long) is only supported for DELETE"), POLICY_VALIDATION_ERR_MISSING_FIELD(3002, "Internal error: missing field[{0}]"), POLICY_VALIDATION_ERR_NULL_POLICY_OBJECT(3003, "Internal error: policy object passed in was null"), - POLICY_VALIDATION_ERR_INVALID_POLICY_ID(3004, "Invalid policy id provided for update: no policy found for id[{0}]"), + POLICY_VALIDATION_ERR_INVALID_POLICY_ID(3004, "No policy found for id[{0}]"), POLICY_VALIDATION_ERR_POLICY_NAME_MULTIPLE_POLICIES_WITH_SAME_NAME(3005, "Internal error: multiple policies found with the name[{0}]"), - POLICY_VALIDATION_ERR_POLICY_NAME_CONFLICT(3006, "id/name conflict: another policy already exists with name[{0}] for service[{1}, its id is[{2}]"), + POLICY_VALIDATION_ERR_POLICY_NAME_CONFLICT(3006, "Another policy already exists for this name: policy-id=[{0}], service=[{1}]"), POLICY_VALIDATION_ERR_INVALID_SERVICE_NAME(3007, "no service found with name[{0}]"), POLICY_VALIDATION_ERR_MISSING_POLICY_ITEMS(3008, "at least one policy item must be specified if audit isn't enabled"), POLICY_VALIDATION_ERR_MISSING_SERVICE_DEF(3009, "Internal error: Service def[{0}] of policy's service[{1}] does not exist!"), - POLICY_VALIDATION_ERR_DUPLICATE_POLICY_RESOURCE(3010, "another policy[{0}] with matching resources[{1}] exists for service[{2}]!"), - POLICY_VALIDATION_ERR_INVALID_RESOURCE_NO_COMPATIBLE_HIERARCHY(3011, "policy resources [{0}] are not compatible with any resource hierarchy for service def[{1}]! Valid hierarchies are: {2}"), - POLICY_VALIDATION_ERR_INVALID_RESOURCE_MISSING_MANDATORY(3012, "policy is missing required resources. Mandatory resources of potential hierarchies are: {0}"), + POLICY_VALIDATION_ERR_DUPLICATE_POLICY_RESOURCE(3010, "Another policy already exists for matching resource: policy-name=[{0}], service=[{1}]"), + POLICY_VALIDATION_ERR_INVALID_RESOURCE_NO_COMPATIBLE_HIERARCHY(3011, "Invalid resources specified. {0} policy can specify values for one of the following resource sets: {1}"), + POLICY_VALIDATION_ERR_INVALID_RESOURCE_MISSING_MANDATORY(3012, "Invalid resources specified. {0} policy must specify values for one of the following resource sets: {1}"), POLICY_VALIDATION_ERR_NULL_RESOURCE_DEF(3013, "Internal error: a resource-def on resource def collection of service-def[{0}] was null"), POLICY_VALIDATION_ERR_MISSING_RESOURCE_DEF_NAME(3014, "Internal error: name of a resource-def on resource def collection of service-def[{0}] was null"), - POLICY_VALIDATION_ERR_EXCLUDES_NOT_SUPPORTED(3015, "isExcludes specified as [{0}] for resource [{1}] which doesn't support isExcludes"), - POLICY_VALIDATION_ERR_EXCLUDES_REQUIRES_ADMIN(3016, "isExcludes specified as [{0}] for resource [{1}]. Insufficient permissions to create excludes policy."), - POLICY_VALIDATION_ERR_RECURSIVE_NOT_SUPPORTED(3017, "isRecursive specified as [{0}] for resource [{1}] which doesn't support isRecursive"), - POLICY_VALIDATION_ERR_INVALID_RESOURCE_VALUE_REGEX(3018, "Value[{0}] of resource[{1}] does not conform to the validation regex[{2}] defined on the service-def[{3}]"), + POLICY_VALIDATION_ERR_EXCLUDES_NOT_SUPPORTED(3015, "Excludes option not supported: resource-name=[{0}]"), + POLICY_VALIDATION_ERR_EXCLUDES_REQUIRES_ADMIN(3016, "Insufficient permissions to create excludes policy"), + POLICY_VALIDATION_ERR_RECURSIVE_NOT_SUPPORTED(3017, "Recursive option not supported: resource-name=[{0}]."), + POLICY_VALIDATION_ERR_INVALID_RESOURCE_VALUE_REGEX(3018, "Invalid resource specified. A value of [{0}] is not valid for resource [{1}]"), POLICY_VALIDATION_ERR_NULL_POLICY_ITEM(3019, "policy items object was null"), POLICY_VALIDATION_ERR_MISSING_USER_AND_GROUPS(3020, "both users and user-groups collections on the policy item were null/empty"), POLICY_VALIDATION_ERR_NULL_POLICY_ITEM_ACCESS(3021, "policy items access object was null"), - POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_INVALID(3022, "access type[{0}] not among valid types for service[{1}]"), - POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_DENY(3023, "access type is set to deny. Currently deny access types are not supported."), + POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_INVALID(3022, "Invalid access type: access type=[{0}], valid access types=[{1}]"), + POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_DENY(3023, "Currently deny access types are not supported. Access type is set to deny."), + POLICY_VALIDATION_ERR_INVALID_RESOURCE_NO_COMPATIBLE_HIERARCHY_SINGLE(3024, "Invalid resources specified. {0} policy can specify values for the following resources: {1}"), + POLICY_VALIDATION_ERR_INVALID_RESOURCE_MISSING_MANDATORY_SINGLE(3025, "Invalid resources specified. {0} policy must specify values for the following resources: {1}"), ; http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/eda1dd7d/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java index da817c6..66768c2 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java @@ -170,7 +170,7 @@ public class RangerPolicyValidator extends RangerValidator { failures.add(new ValidationFailureDetailsBuilder() .field("policy name") .isSemanticallyIncorrect() - .becauseOf(error.getMessage(policyName, serviceName, policies.iterator().next().getId())) + .becauseOf(error.getMessage(policies.iterator().next().getId(), serviceName)) .errorCode(error.getErrorCode()) .build()); valid = false; @@ -179,7 +179,7 @@ public class RangerPolicyValidator extends RangerValidator { failures.add(new ValidationFailureDetailsBuilder() .field("id/name") .isSemanticallyIncorrect() - .becauseOf(error.getMessage(policyName, serviceName, policies.iterator().next().getId())) + .becauseOf(error.getMessage(policies.iterator().next().getId(), serviceName)) .errorCode(error.getErrorCode()) .build()); valid = false; @@ -297,7 +297,7 @@ public class RangerPolicyValidator extends RangerValidator { failures.add(new ValidationFailureDetailsBuilder() .field("resources") .isSemanticallyIncorrect() - .becauseOf(error.getMessage(matchedPolicy.getName(), matchedPolicy.getResources(), policy.getService())) + .becauseOf(error.getMessage(matchedPolicy.getName(), policy.getService())) .errorCode(error.getErrorCode()) .build()); valid = false; @@ -333,12 +333,21 @@ public class RangerPolicyValidator extends RangerValidator { */ Set<List<RangerResourceDef>> candidateHierarchies = filterHierarchies_hierarchyHasAllPolicyResources(policyResources, hierarchies, defHelper); if (candidateHierarchies.isEmpty()) { - ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_NO_COMPATIBLE_HIERARCHY; + if (LOG.isDebugEnabled()) { + LOG.debug(String.format("No compatible resource hierarchies found: resource[%s], service-def[%s], valid-resource-hierarchies[%s]", + policyResources.toString(), serviceDef.getName(), toStringHierarchies_all(hierarchies, defHelper))); + } + ValidationErrorCode error; + if (hierarchies.size() == 1) { // we can give a simpler message for single hierarchy service-defs which is the majority of cases + error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_NO_COMPATIBLE_HIERARCHY_SINGLE; + } else { + error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_NO_COMPATIBLE_HIERARCHY; + } failures.add(new ValidationFailureDetailsBuilder() .field("policy resources") .subField("incompatible") .isSemanticallyIncorrect() - .becauseOf(error.getMessage(policyResources.toString(), serviceDef.getName(), toStringHierarchies_all(hierarchies, defHelper))) + .becauseOf(error.getMessage(serviceDef.getName(), toStringHierarchies_all(hierarchies, defHelper))) .errorCode(error.getErrorCode()) .build()); valid = false; @@ -353,12 +362,17 @@ public class RangerPolicyValidator extends RangerValidator { */ Set<List<RangerResourceDef>> validHierarchies = filterHierarchies_mandatoryResourcesSpecifiedInPolicy(policyResources, candidateHierarchies, defHelper); if (validHierarchies.isEmpty()) { - ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_MISSING_MANDATORY; + ValidationErrorCode error; + if (candidateHierarchies.size() == 1) { // we can provide better message if there is a single candidate hierarchy + error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_MISSING_MANDATORY_SINGLE; + } else { + error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_MISSING_MANDATORY; + } failures.add(new ValidationFailureDetailsBuilder() .field("policy resources") .subField("missing mandatory") .isSemanticallyIncorrect() - .becauseOf(error.getMessage(toStringHierarchies_mandatory(candidateHierarchies, defHelper))) + .becauseOf(error.getMessage(serviceDef.getName(), toStringHierarchies_mandatory(candidateHierarchies, defHelper))) .errorCode(error.getErrorCode()) .build()); valid = false; @@ -495,7 +509,7 @@ public class RangerPolicyValidator extends RangerValidator { .field("isExcludes") .subField(resourceName) .isSemanticallyIncorrect() - .becauseOf(error.getMessage(policyResourceIsExcludes, resourceName)) + .becauseOf(error.getMessage(resourceName)) .errorCode(error.getErrorCode()) .build()); valid = false; @@ -506,7 +520,7 @@ public class RangerPolicyValidator extends RangerValidator { .field("isExcludes") .subField("isAdmin") .isSemanticallyIncorrect() - .becauseOf(error.getMessage(policyResourceIsExcludes, resourceName)) + .becauseOf(error.getMessage()) .errorCode(error.getErrorCode()) .build()); valid = false; @@ -519,7 +533,7 @@ public class RangerPolicyValidator extends RangerValidator { .field("isRecursive") .subField(resourceName) .isSemanticallyIncorrect() - .becauseOf(error.getMessage(policyIsRecursive, resourceName)) + .becauseOf(error.getMessage(resourceName)) .errorCode(error.getErrorCode()) .build()); valid = false; @@ -551,12 +565,15 @@ public class RangerPolicyValidator extends RangerValidator { if (StringUtils.isBlank(aValue)) { LOG.debug("resource value was blank"); } else if (!aValue.matches(regEx)) { + if (LOG.isDebugEnabled()) { + LOG.debug(String.format("Resource failed regex check: value[%s], resource-name[%s], regEx[%s], service-def-name[%s]", aValue, name, regEx, serviceDef.getName())); + } ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_VALUE_REGEX; failures.add(new ValidationFailureDetailsBuilder() .field("resource-values") .subField(name) .isSemanticallyIncorrect() - .becauseOf(error.getMessage(aValue, name, regEx, serviceDef.getName())) + .becauseOf(error.getMessage(aValue, name)) .errorCode(error.getErrorCode()) .build()); valid = false; http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/eda1dd7d/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/ValidationFailureDetails.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/ValidationFailureDetails.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/ValidationFailureDetails.java index a0e8573..e9ad40a 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/ValidationFailureDetails.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/ValidationFailureDetails.java @@ -74,7 +74,7 @@ public class ValidationFailureDetails { @Override public String toString() { LOG.debug("ValidationFailureDetails.toString()"); - return String.format("%s: error code[%d], reason[%s], field[%s], subfield[%s], type[%s]", "Policy validation failure", + return String.format(" %s: error code[%d], reason[%s], field[%s], subfield[%s], type[%s]", "Validation failure", _errorCode, _reason, _fieldName, _subFieldName, getType()); }
