diff options
author | Alex Humesky <ahumesky@google.com> | 2016-02-03 00:52:04 +0000 |
---|---|---|
committer | David Chen <dzc@google.com> | 2016-02-03 03:06:15 +0000 |
commit | c0d27699b3ecc347b968e1a7eea33a6d3257acc7 (patch) | |
tree | 14762aca42124ce579a46c0e2bdbc3cd76f0fc3c /src/main/java/com | |
parent | fd5f390d3669423bb3c74f86a5499e3c4129098c (diff) |
Code cleanup.
--
MOS_MIGRATED_REVID=113692613
Diffstat (limited to 'src/main/java/com')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java | 324 |
1 files changed, 174 insertions, 150 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java b/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java index edd367830e..7426272281 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java @@ -16,7 +16,7 @@ package com.google.devtools.build.lib.runtime; import com.google.common.base.CharMatcher; import com.google.common.base.Function; import com.google.common.base.Functions; -import com.google.common.base.Joiner; +import com.google.common.base.Strings; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; @@ -45,8 +45,9 @@ import java.util.logging.Logger; import javax.annotation.Nullable; /** - * Given an OptionsParser and a InvocationPolicy proto, enforces the FlagPolicies on an - * OptionsParser. + * Enforces the {@link FlagPolicy}s (from an {@link InvocationPolicy} proto) on an + * {@link OptionsParser} by validating and changing the flag values in the given + * {@link OptionsParser}. * * <p>"Flag" and "Option" are used interchangeably in this file. */ @@ -58,9 +59,8 @@ public final class InvocationPolicyEnforcer { * --invocation_policy flag and does not enforce any policy on the flags in the provider. * * @param startupOptionsProvider an options provider which provides a BlazeServerStartupOptions - * options class - * - * @throws OptionsParsingException if the value of --invocation_policy is invalid + * options class. + * @throws OptionsParsingException if the value of --invocation_policy is invalid. */ public static InvocationPolicyEnforcer create(OptionsProvider startupOptionsProvider) throws OptionsParsingException { @@ -79,13 +79,12 @@ public final class InvocationPolicyEnforcer { /** * Parses the given InvocationPolicy string, which may be a base64-encoded binary-serialized * InvocationPolicy message, or a text formatted InvocationPolicy message. Note that the - * text format is not backwards compatible as the binary format is, and the option to - * provide a text formatted proto is provided only for debugging. + * text format is not backwards compatible as the binary format is. * - * @throws OptionsParsingException if the value of --invocation_policy is invalid + * @throws OptionsParsingException if the value of --invocation_policy is invalid. */ private static InvocationPolicy parsePolicy(String policy) throws OptionsParsingException { - if (policy == null || policy.isEmpty()) { + if (Strings.isNullOrEmpty(policy)) { return null; } @@ -96,7 +95,7 @@ public final class InvocationPolicyEnforcer { BaseEncoding.base64().decode(CharMatcher.WHITESPACE.removeFrom(policy))); } catch (IllegalArgumentException e) { // If the flag value can't be decoded from base64, try decoding the policy as a text - // formated proto. + // formatted proto. InvocationPolicy.Builder builder = InvocationPolicy.newBuilder(); TextFormat.merge(policy, builder); return builder.build(); @@ -106,11 +105,20 @@ public final class InvocationPolicyEnforcer { } } - private static final Logger LOG = Logger.getLogger(InvocationPolicyEnforcer.class.getName()); + private static final Logger log = Logger.getLogger(InvocationPolicyEnforcer.class.getName()); + + private static final Function<Object, String> INVOCATION_POLICY_SOURCE = + Functions.constant("Invocation policy"); @Nullable private final InvocationPolicy invocationPolicy; + /** + * Creates an InvocationPolicyEnforcer that enforces the given policy. + * + * @param invocationPolicy the policy to enforce. A null policy means this enforcer will do + * nothing in calls to enforce(). + */ public InvocationPolicyEnforcer(@Nullable InvocationPolicy invocationPolicy) { this.invocationPolicy = invocationPolicy; } @@ -119,8 +127,9 @@ public final class InvocationPolicyEnforcer { * Applies this OptionsPolicyEnforcer's policy to the given OptionsParser. * * @param parser The OptionsParser to enforce policy on. - * @param command The command to which the options in the OptionsParser apply. - * @throws OptionsParsingException + * @param command The current blaze command, for flag policies that apply to only specific + * commands. + * @throws OptionsParsingException if any flag policy is invalid. */ public void enforce(OptionsParser parser, String command) throws OptionsParsingException { if (invocationPolicy == null) { @@ -128,19 +137,19 @@ public final class InvocationPolicyEnforcer { } if (invocationPolicy.getFlagPoliciesCount() == 0) { - LOG.warning("InvocationPolicy contains no flag policies."); + log.warning("InvocationPolicy contains no flag policies."); + return; } - Function<Object, String> sourceFunction = Functions.constant("Invocation policy"); - for (FlagPolicy flagPolicy : invocationPolicy.getFlagPoliciesList()) { String flagName = flagPolicy.getFlagName(); - // Skip the flag policy if it doesn't apply to this command. + // Skip the flag policy if it doesn't apply to this command. If the commands list is empty, + // then the policy applies to all commands. if (!flagPolicy.getCommandsList().isEmpty() && !flagPolicy.getCommandsList().contains(command)) { - LOG.info(String.format("Skipping flag policy for flag '%s' because it " - + "applies only to commands %s and the current command is '%s'", + log.info(String.format("Skipping flag policy for flag '%s' because it " + + "applies only to commands %s and the current command is '%s'", flagName, flagPolicy.getCommandsList(), command)); continue; } @@ -152,7 +161,7 @@ public final class InvocationPolicyEnforcer { // This flag doesn't exist. We are deliberately lenient if the flag policy has a flag // we don't know about. This is for better future proofing so that as new flags are added, // new policies can use the new flags without worrying about older versions of Bazel. - LOG.info(String.format( + log.info(String.format( "Flag '%s' specified by invocation policy does not exist", flagName)); continue; } @@ -165,7 +174,7 @@ public final class InvocationPolicyEnforcer { switch (flagPolicy.getOperationCase()) { case SET_VALUE: - applySetValueOperation(parser, sourceFunction, flagPolicy, flagName, + applySetValueOperation(parser, flagPolicy, flagName, valueDescription, optionDescription); break; @@ -174,13 +183,25 @@ public final class InvocationPolicyEnforcer { break; case ALLOW_VALUES: - applyAllowValuesOperation(parser, sourceFunction, flagPolicy, - flagName, valueDescription, optionDescription); + AllowValues allowValues = flagPolicy.getAllowValues(); + FilterValueOperation.ALLOW_VALUE_OPERATION.apply( + parser, + allowValues.getAllowedValuesList(), + allowValues.hasNewDefaultValue() ? allowValues.getNewDefaultValue() : null, + flagName, + valueDescription, + optionDescription); break; case DISALLOW_VALUES: - applyDisallowValuesOperation(parser, sourceFunction, flagPolicy, - flagName, valueDescription, optionDescription); + DisallowValues disallowValues = flagPolicy.getDisallowValues(); + FilterValueOperation.DISALLOW_VALUE_OPERATION.apply( + parser, + disallowValues.getDisallowedValuesList(), + disallowValues.hasNewDefaultValue() ? disallowValues.getNewDefaultValue() : null, + flagName, + valueDescription, + optionDescription); break; case OPERATION_NOT_SET: @@ -188,7 +209,7 @@ public final class InvocationPolicyEnforcer { + "have an operation", flagName)); default: - LOG.warning(String.format("Unknown operation '%s' from invocation policy for flag '%s'", + log.warning(String.format("Unknown operation '%s' from invocation policy for flag '%s'", flagPolicy.getOperationCase(), flagName)); break; } @@ -197,7 +218,6 @@ public final class InvocationPolicyEnforcer { private static void applySetValueOperation( OptionsParser parser, - Function<Object, String> sourceFunction, FlagPolicy flagPolicy, String flagName, OptionValueDescription valueDescription, @@ -216,14 +236,14 @@ public final class InvocationPolicyEnforcer { if (setValue.getFlagValueCount() > 1 && !optionDescription.getAllowMultiple()) { throw new OptionsParsingException(String.format( "SetValue operation from invocation policy sets multiple values for flag '%s' which " - + "does not allow multiple values", flagName)); + + "does not allow multiple values", flagName)); } if (setValue.getOverridable() && valueDescription != null) { // The user set the value for the flag but the flag policy is overridable, so keep the user's // value. - LOG.info(String.format("Keeping value '%s' from source '%s' for flag '%s' " - + "because the invocation policy specifying the value(s) '%s' is overridable", + log.info(String.format("Keeping value '%s' from source '%s' for flag '%s' " + + "because the invocation policy specifying the value(s) '%s' is overridable", valueDescription.getValue(), valueDescription.getSource(), flagName, setValue.getFlagValueList())); } else { @@ -236,15 +256,15 @@ public final class InvocationPolicyEnforcer { // Set all the flag values from the policy. for (String flagValue : setValue.getFlagValueList()) { if (valueDescription == null) { - LOG.info(String.format("Setting value for flag '%s' from invocation " - + "policy to '%s', overriding the default value '%s'", flagName, flagValue, + log.info(String.format("Setting value for flag '%s' from invocation " + + "policy to '%s', overriding the default value '%s'", flagName, flagValue, optionDescription.getDefaultValue())); } else { - LOG.info(String.format("Setting value for flag '%s' from invocation " - + "policy to '%s', overriding value '%s' from '%s'", flagName, flagValue, + log.info(String.format("Setting value for flag '%s' from invocation " + + "policy to '%s', overriding value '%s' from '%s'", flagName, flagValue, valueDescription.getValue(), valueDescription.getSource())); } - setFlagValue(parser, flagName, flagValue, sourceFunction); + setFlagValue(parser, flagName, flagValue); } } } @@ -254,145 +274,150 @@ public final class InvocationPolicyEnforcer { Map<String, OptionValueDescription> clearedValues = parser.clearValue(flagName); for (Entry<String, OptionValueDescription> clearedValue : clearedValues.entrySet()) { - OptionValueDescription clearedValueDesc = clearedValue.getValue(); + OptionValueDescription clearedValueDescription = clearedValue.getValue(); String clearedFlagName = clearedValue.getKey(); - String originalValue = clearedValueDesc.getValue().toString(); - String source = clearedValueDesc.getSource(); + String originalValue = clearedValueDescription.getValue().toString(); + String source = clearedValueDescription.getSource(); - OptionDescription clearedFlagDesc = parser.getOptionDescription(clearedFlagName); - Object clearedFlagdefaultValue = clearedFlagDesc.getDefaultValue(); + Object clearedFlagDefaultValue = parser.getOptionDescription(clearedFlagName) + .getDefaultValue(); - LOG.info(String.format("Using default value '%s' for flag '%s' as " - + "specified by invocation policy, overriding original value '%s' from '%s'", - clearedFlagdefaultValue, clearedFlagName, originalValue, source)); + log.info(String.format("Using default value '%s' for flag '%s' as " + + "specified by invocation policy, overriding original value '%s' from '%s'", + clearedFlagDefaultValue, clearedFlagName, originalValue, source)); } } - private static void applyAllowValuesOperation( - OptionsParser parser, - Function<Object, String> sourceFunction, - FlagPolicy flagPolicy, - String flagName, - OptionValueDescription valueDescription, - OptionDescription optionDescription) throws OptionsParsingException { - - AllowValues allowValues = flagPolicy.getAllowValues(); - applyAllowDisallowValueOperation( - parser, - sourceFunction, - /*allowValues=*/ true, - allowValues.getAllowedValuesList(), - allowValues.hasNewDefaultValue() ? allowValues.getNewDefaultValue() : null, - flagName, - valueDescription, - optionDescription); - } - - private static void applyDisallowValuesOperation( - OptionsParser parser, - Function<Object, String> sourceFunction, - FlagPolicy flagPolicy, - String flagName, - OptionValueDescription valueDescription, - OptionDescription optionDescription) throws OptionsParsingException { - - DisallowValues disallowValues = flagPolicy.getDisallowValues(); - applyAllowDisallowValueOperation( - parser, - sourceFunction, - /*allowValues=*/ false, - disallowValues.getDisallowedValuesList(), - disallowValues.hasNewDefaultValue() ? disallowValues.getNewDefaultValue() : null, - flagName, - valueDescription, - optionDescription); - } - /** - * Shared logic between AllowValues and DisallowValues operations. - * - * @param parser - * @param sourceFunction - * @param allowValues True if this is an AllowValues operation, false if DisallowValues - * @param policyValues The list of allowed or disallowed values - * @param newDefaultValue The new default to use if the default value for the flag is now allowed - * (i.e. not in the list of allowed values or in the list of disallowed values). - * @param flagName - * @param valueDescription - * @param optionDescription - * - * @throws OptionsParsingException + * Checks the user's flag values against a filtering function. */ - private static void applyAllowDisallowValueOperation( - OptionsParser parser, - Function<Object, String> sourceFunction, - boolean allowValues, - List<String> policyValues, - String newDefaultValue, - String flagName, - OptionValueDescription valueDescription, - OptionDescription optionDescription) throws OptionsParsingException { + private abstract static class FilterValueOperation { - // For error reporting. - String policyType = allowValues ? "Allow" : "Disallow"; + private static final FilterValueOperation ALLOW_VALUE_OPERATION = + new FilterValueOperation("Allow") { + @Override + boolean filter(Set<Object> convertedPolicyValues, Object value) { + return convertedPolicyValues.contains(value); + } + }; + + private static final FilterValueOperation DISALLOW_VALUE_OPERATION = + new FilterValueOperation("Disallow") { + @Override + boolean filter(Set<Object> convertedPolicyValues, Object value) { + // In a disallow operation, the values that the flag policy specifies are not allowed, so + // the value is allowed if the set of policy values does not contain the current flag value. + return !convertedPolicyValues.contains(value); + } + }; - // Convert all the allowed values from strings to real object using the option's - // converter so that they can be checked for equality using real .equals() instead - // of string comparison. For example, "--foo=0", "--foo=false", "--nofoo", and "-f-" - // (if the option has an abbreviation) are all equal for boolean flags. Plus converters - // can be arbitrarily complex. - Set<Object> convertedPolicyValues = Sets.newHashSet(); - for (String value : policyValues) { - convertedPolicyValues.add(optionDescription.getConverter().convert(value)); + private final String policyType; + + FilterValueOperation(String policyType) { + this.policyType = policyType; } - if (valueDescription == null) { - // Nothing has set the value yet, so check that the default value from the flag's - // definition is allowed. The else case below (i.e. valueDescription is not null) checks for - // the flag allowing multiple values, however, flags that allow multiple values cannot have - // default values, and their value is always the empty list if they haven't been specified, - // which is why new_default_value is not a repeated field. - // - // This is xor'ed with allowValues because if the policy is to allow these values, - // then we want to apply the new default (or throw an error) if the default value of the flag - // is not in the set of allowed values. If the policy is to disallow these values - // (allowValues is false), then we want to apply the new default (or throw an error) if - // the default value of the flag is in the set of disallowed values. This works out to xor. - if (allowValues ^ convertedPolicyValues.contains(optionDescription.getDefaultValue())) { + /** + * Determines if the given value is allowed. + * + * @param convertedPolicyValues The values given from the FlagPolicy, converted to real objects. + * @param value The user value of the flag. + * @return True if the value should be allowed, false if it should not. + */ + abstract boolean filter(Set<Object> convertedPolicyValues, Object value); + + void apply( + OptionsParser parser, + List<String> policyValues, + String newDefaultValue, + String flagName, + OptionValueDescription valueDescription, + OptionDescription optionDescription) throws OptionsParsingException { + + // Convert all the allowed values from strings to real objects using the options' + // converters so that they can be checked for equality using real .equals() instead + // of string comparison. For example, "--foo=0", "--foo=false", "--nofoo", and "-f-" + // (if the option has an abbreviation) are all equal for boolean flags. Plus converters + // can be arbitrarily complex. + Set<Object> convertedPolicyValues = Sets.newHashSet(); + for (String value : policyValues) { + convertedPolicyValues.add(optionDescription.getConverter().convert(value)); + } + + if (valueDescription == null) { + // Nothing has set the value yet, so check that the default value from the flag's + // definition is allowed. The else case below (i.e. valueDescription is not null) checks for + // the flag allowing multiple values, however, flags that allow multiple values cannot have + // default values, and their value is always the empty list if they haven't been specified, + // which is why new_default_value is not a repeated field. + checkDefaultValue( + parser, + policyValues, + newDefaultValue, + flagName, + optionDescription, + convertedPolicyValues); + } else { + checkUserValue( + policyValues, + flagName, + valueDescription, + optionDescription, + convertedPolicyValues); + } + } + + void checkDefaultValue( + OptionsParser parser, + List<String> policyValues, + String newDefaultValue, + String flagName, + OptionDescription optionDescription, + Set<Object> convertedPolicyValues) throws OptionsParsingException { + + if (!filter(convertedPolicyValues, optionDescription.getDefaultValue())) { if (newDefaultValue != null) { // Use the default value from the policy. - LOG.info(String.format("Overriding default value '%s' for flag '%s' with " - + "new default value '%s' specified by invocation policy. %sed values are: %s", + log.info(String.format("Overriding default value '%s' for flag '%s' with " + + "new default value '%s' specified by invocation policy. %sed values are: %s", optionDescription.getDefaultValue(), flagName, newDefaultValue, - policyType, Joiner.on(", ").join(policyValues))); + policyType, policyValues)); parser.clearValue(flagName); - setFlagValue(parser, flagName, newDefaultValue, sourceFunction); + setFlagValue(parser, flagName, newDefaultValue); } else { // The operation disallows the default value, but doesn't supply its own default. throw new OptionsParsingException(String.format( "Default flag value '%s' for flag '%s' is not allowed by invocation policy, but " - + "the policy does not provide a new default value. " - + "%sed values are: %s", optionDescription.getDefaultValue(), flagName, - policyType, Joiner.on(", ").join(policyValues))); + + "the policy does not provide a new default value. " + + "%sed values are: %s", optionDescription.getDefaultValue(), flagName, + policyType, policyValues)); } } - } else { - // Check that the flag's value is allowed. - List<?> values; + } + + void checkUserValue( + List<String> policyValues, + String flagName, + OptionValueDescription valueDescription, + OptionDescription optionDescription, + Set<Object> convertedPolicyValues) throws OptionsParsingException { + + // Get the option values: there might be one of them or a list of them, so convert everything + // to a list (possibly of just the one value). + List<?> optionValues; if (optionDescription.getAllowMultiple()) { - // allowMultiple requires that the type of the option be List<T>. - values = (List<?>) valueDescription.getValue(); + // allowMultiple requires that the type of the option be List<T>, so cast from Object + // to List<?>. + optionValues = (List<?>) valueDescription.getValue(); } else { - values = ImmutableList.of(valueDescription.getValue()); + optionValues = ImmutableList.of(valueDescription.getValue()); } - for (Object value : values) { - // See above about the xor. - if (allowValues ^ convertedPolicyValues.contains(value)) { + for (Object value : optionValues) { + if (!filter(convertedPolicyValues, value)) { throw new OptionsParsingException(String.format( "Flag value '%s' for flag '%s' is not allowed by invocation policy. " - + "%sed values are: %s", value, flagName, policyType, - Joiner.on(", ").join(policyValues))); + + "%sed values are: %s", value, flagName, policyType, policyValues)); } } } @@ -401,10 +426,9 @@ public final class InvocationPolicyEnforcer { private static void setFlagValue( OptionsParser parser, String flagName, - String flagValue, - Function<? super String, String> sourceFunction) throws OptionsParsingException { + String flagValue) throws OptionsParsingException { - parser.parseWithSourceFunction(OptionPriority.INVOCATION_POLICY, sourceFunction, + parser.parseWithSourceFunction(OptionPriority.INVOCATION_POLICY, INVOCATION_POLICY_SOURCE, Arrays.asList(String.format("--%s=%s", flagName, flagValue))); } } |