Enable --feature_control_policy policy for feature flag usage.

To limit rollout of this potentially risky feature, a new policy
is added to the --feature_control_policy feature list. The contents
of the package_group pointed to by that label are then used to control
which targets are allowed to use feature flags and related features.

RELNOTES: None.
PiperOrigin-RevId: 160686186

11 files changed, 303 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 354442c871..aa4fb60214 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -96,6 +96,7 @@ import com.google.devtools.build.lib.rules.apple.cpp.AppleCcToolchainRule;
 import com.google.devtools.build.lib.rules.apple.swift.SwiftCommandLineOptions;
 import com.google.devtools.build.lib.rules.apple.swift.SwiftConfiguration;
 import com.google.devtools.build.lib.rules.config.ConfigFeatureFlagConfiguration;
+import com.google.devtools.build.lib.rules.config.ConfigFeatureFlagFeatureVisibility;
 import com.google.devtools.build.lib.rules.config.ConfigRuleClasses;
 import com.google.devtools.build.lib.rules.config.ConfigSkylarkCommon;
 import com.google.devtools.build.lib.rules.cpp.CcIncLibraryRule;
@@ -224,7 +225,9 @@ public class BazelRuleClassProvider {
         }
       };
 
-  public static final ImmutableSet<String> FEATURE_POLICY_FEATURES = ImmutableSet.<String>of();
+  public static final ImmutableSet<String> FEATURE_POLICY_FEATURES = ImmutableSet.<String>of(
+    ConfigFeatureFlagFeatureVisibility.POLICY_NAME
+  );
 
   public static final RuleSet CORE_RULES =
       new RuleSet() {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidBinaryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidBinaryRule.java
index c11917551f..1e50255f5b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidBinaryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidBinaryRule.java
@@ -18,6 +18,7 @@ import static com.google.devtools.build.lib.packages.Attribute.attr;
 
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
+import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyConfiguration;
 import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses;
 import com.google.devtools.build.lib.bazel.rules.java.BazelJavaRuleClasses;
 import com.google.devtools.build.lib.packages.BuildType;
@@ -42,7 +43,10 @@ public class BazelAndroidBinaryRule implements RuleDefinition {
   public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
     return builder
         .requiresConfigurationFragments(
-            AndroidConfiguration.class, JavaConfiguration.class, CppConfiguration.class)
+            AndroidConfiguration.class,
+            JavaConfiguration.class,
+            FeaturePolicyConfiguration.class,
+            CppConfiguration.class)
         .override(
             attr("manifest", BuildType.LABEL).mandatory().allowedFileTypes(FileType.of(".xml")))
         .add(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
index 29375a9ead..fe92de0d46 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
@@ -26,6 +26,7 @@ import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.AliasProvider;
+import com.google.devtools.build.lib.rules.config.ConfigFeatureFlagFeatureVisibility;
 import com.google.devtools.build.lib.rules.config.ConfigFeatureFlagProvider;
 import java.util.Map;
 
@@ -61,6 +62,14 @@ public abstract class AndroidFeatureFlagSetProvider implements TransitiveInfoPro
     Map<Label, String> expectedValues =
         NonconfigurableAttributeMapper.of(ruleContext.getRule())
             .get(FEATURE_FLAG_ATTR, BuildType.LABEL_KEYED_STRING_DICT);
+
+    if (expectedValues.isEmpty()) {
+      return ImmutableMap.of();
+    }
+
+    ConfigFeatureFlagFeatureVisibility.checkAvailable(
+        ruleContext, "the " + FEATURE_FLAG_ATTR + " attribute");
+
     Iterable<? extends TransitiveInfoCollection> actualTargets =
         ruleContext.getPrerequisites(FEATURE_FLAG_ATTR, Mode.TARGET);
     boolean aliasFound = false;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/BUILD b/src/main/java/com/google/devtools/build/lib/rules/config/BUILD
index eed0601673..54e5eb31d4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/BUILD
@@ -19,6 +19,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:preconditions",
         "//src/main/java/com/google/devtools/build/lib:skylarkinterface",
         "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/protobuf:option_filters_java_proto",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java
index 79d595baf7..2791a32edb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java
@@ -43,6 +43,9 @@ public class ConfigFeatureFlag implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException {
+    ConfigFeatureFlagFeatureVisibility.checkAvailable(
+        ruleContext, "the " + ruleContext.getRuleClassNameForLogging() + " rule");
+
     List<String> specifiedValues = ruleContext.attributes().get("allowed_values", STRING_LIST);
     ImmutableSet<String> values = ImmutableSet.copyOf(specifiedValues);
     Predicate<String> isValidValue = Predicates.in(values);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagFeatureVisibility.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagFeatureVisibility.java
new file mode 100644
index 0000000000..b0909b4fef
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagFeatureVisibility.java
@@ -0,0 +1,53 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License
+
+package com.google.devtools.build.lib.rules.config;
+
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyConfiguration;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
+
+/**
+ * A validator utility class which confirms that config_feature_flag-related features can be used by
+ * the current rule.
+ */
+public class ConfigFeatureFlagFeatureVisibility {
+
+  /** The name of the policy that is used to restrict access to the config_feature_flag rule. */
+  public static final String POLICY_NAME = "config_feature_flag";
+
+  private ConfigFeatureFlagFeatureVisibility() {}
+
+  /**
+   * Checks whether the rule in the given RuleContext has access to config_feature_flag and related
+   * features, and reports an error if not.
+   *
+   * @param ruleContext The context in which this check is being executed.
+   * @param feature The name of the config_feature_flag-related feature being used, to be printed in
+   *     the error if the policy forbids access to this rule.
+   */
+  public static void checkAvailable(RuleContext ruleContext, String feature)
+      throws RuleErrorException {
+    FeaturePolicyConfiguration policy = ruleContext.getFragment(FeaturePolicyConfiguration.class);
+    Label label = ruleContext.getLabel();
+    if (!policy.isFeatureEnabledForRule(POLICY_NAME, label)) {
+      ruleContext.ruleError(
+          String.format(
+              "%s is not available in package '%s' according to policy '%s'",
+              feature, label.getPackageIdentifier(), policy.getPolicyForFeature(POLICY_NAME)));
+      throw new RuleErrorException();
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
index a454719044..dcfe505a61 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
+import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyConfiguration;
 import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.syntax.Type;
@@ -155,6 +156,7 @@ public class ConfigRuleClasses {
                   .mandatoryProviders(
                       ImmutableList.of(ConfigFeatureFlagProvider.SKYLARK_IDENTIFIER))
                   .nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON))
+          .requiresConfigurationFragments(FeaturePolicyConfiguration.class)
           .setIsConfigMatcherForConfigSettingOnly()
           .setOptionReferenceFunctionForConfigSettingOnly(
               rule ->
@@ -244,7 +246,9 @@ config_setting(
     public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
       return builder
           .setUndocumented(/* It's unusable as yet, as there are no ways to interact with it. */)
-          .requiresConfigurationFragments(ConfigFeatureFlagConfiguration.class)
+          .requiresConfigurationFragments(
+              ConfigFeatureFlagConfiguration.class,
+              FeaturePolicyConfiguration.class)
           .add(
               attr("allowed_values", STRING_LIST)
                   .mandatory()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
index 82bc85c750..48977eb458 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
@@ -69,6 +69,11 @@ public class ConfigSetting implements RuleConfiguredTargetFactory {
             .get(
                 ConfigRuleClasses.ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE,
                 BuildType.LABEL_KEYED_STRING_DICT);
+    if (!flagSettings.isEmpty()) {
+      ConfigFeatureFlagFeatureVisibility.checkAvailable(
+          ruleContext,
+          "the " + ConfigRuleClasses.ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE + " attribute");
+    }
 
     List<? extends TransitiveInfoCollection> flagValues =
         ruleContext.getPrerequisites(
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index a52a1a60fb..9be5e5ca7f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -2934,4 +2934,94 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
 
     assertThat(javacAction.buildCommandLine()).doesNotContain("--testonly");
   }
+
+  @Test
+  public void testFeatureFlagPolicyMustContainRuleToUseFeatureFlags() throws Exception {
+    reporter.removeHandler(failFastHandler); // expecting an error
+    scratch.file(
+        "policy/BUILD",
+        "package_group(",
+        "    name = 'feature_flag_users',",
+        "    packages = ['//flag'])");
+    scratch.file(
+        "flag/BUILD",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['right', 'wrong'],",
+        "    default_value = 'right',",
+        "    visibility = ['//java/com/google/android/foo:__pkg__'],",
+        ")");
+    scratch.file(
+        "java/com/google/android/foo/BUILD",
+        "android_binary(",
+        "  name = 'foo',",
+        "  manifest = 'AndroidManifest.xml',",
+        "  srcs = [':FooFlags.java'],",
+        "  feature_flags = {",
+        "    '//flag:flag': 'right',",
+        "  }",
+        ")");
+    useConfiguration(
+        "--experimental_dynamic_configs=notrim",
+        "--feature_control_policy=config_feature_flag=//policy:feature_flag_users");
+    assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNull();
+    assertContainsEvent(
+        "in android_binary rule //java/com/google/android/foo:foo: the feature_flags attribute is "
+            + "not available in package 'java/com/google/android/foo' according to policy "
+            + "'//policy:feature_flag_users'");
+  }
+
+  @Test
+  public void testFeatureFlagPolicyDoesNotBlockRuleIfInPolicy() throws Exception {
+    scratch.file(
+        "policy/BUILD",
+        "package_group(",
+        "    name = 'feature_flag_users',",
+        "    packages = ['//flag', '//java/com/google/android/foo'])");
+    scratch.file(
+        "flag/BUILD",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['right', 'wrong'],",
+        "    default_value = 'right',",
+        "    visibility = ['//java/com/google/android/foo:__pkg__'],",
+        ")");
+    scratch.file(
+        "java/com/google/android/foo/BUILD",
+        "android_binary(",
+        "  name = 'foo',",
+        "  manifest = 'AndroidManifest.xml',",
+        "  srcs = [':FooFlags.java'],",
+        "  feature_flags = {",
+        "    '//flag:flag': 'right',",
+        "  }",
+        ")");
+    useConfiguration(
+        "--experimental_dynamic_configs=notrim",
+        "--feature_control_policy=config_feature_flag=//policy:feature_flag_users");
+    assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull();
+    assertNoEvents();
+  }
+
+  @Test
+  public void testFeatureFlagPolicyDoesNotBlockRuleIfFlagValuesNotUsed() throws Exception {
+    scratch.file(
+        "policy/BUILD",
+        "package_group(",
+        "    name = 'feature_flag_users',",
+        "    packages = ['//flag'])");
+    scratch.file("flag/BUILD");
+    scratch.file(
+        "java/com/google/android/foo/BUILD",
+        "android_binary(",
+        "  name = 'foo',",
+        "  manifest = 'AndroidManifest.xml',",
+        "  srcs = [':FooFlags.java'],",
+        ")");
+    useConfiguration(
+        "--experimental_dynamic_configs=notrim",
+        "--feature_control_policy=config_feature_flag=//policy:feature_flag_users");
+    assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull();
+    assertNoEvents();
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
index 0d87489daf..b8fd500689 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
@@ -288,6 +288,47 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
   }
 
   @Test
+  public void policy_mustContainRulesPackage() throws Exception {
+    reporter.removeHandler(failFastHandler); // expecting an error
+    scratch.file(
+        "policy/BUILD",
+        "package_group(name = 'feature_flag_users', packages = ['//some/other'])");
+    scratch.file(
+        "test/BUILD",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['default', 'configured', 'other'],",
+        "    default_value = 'default',",
+        ")");
+    useConfiguration(
+        "--experimental_dynamic_configs=on",
+        "--feature_control_policy=config_feature_flag=//policy:feature_flag_users");
+    assertThat(getConfiguredTarget("//test:flag")).isNull();
+    assertContainsEvent(
+        "in config_feature_flag rule //test:flag: the config_feature_flag rule is not available in "
+        + "package 'test' according to policy '//policy:feature_flag_users'");
+  }
+
+  @Test
+  public void policy_doesNotBlockRuleIfInPackageGroup() throws Exception {
+    scratch.file(
+        "policy/BUILD",
+        "package_group(name = 'feature_flag_users', packages = ['//test'])");
+    scratch.file(
+        "test/BUILD",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['default', 'configured', 'other'],",
+        "    default_value = 'default',",
+        ")");
+    useConfiguration(
+        "--experimental_dynamic_configs=on",
+        "--feature_control_policy=config_feature_flag=//policy:feature_flag_users");
+    assertThat(getConfiguredTarget("//test:flag")).isNotNull();
+    assertNoEvents();
+  }
+
+  @Test
   public void equalsTester() {
     new EqualsTester()
         .addEqualityGroup(
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
index 066c1be941..0ab3a66b70 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
@@ -1080,4 +1080,91 @@ public class ConfigSettingTest extends BuildViewTestCase {
         "    default_value = 'valid',",
         ")");
   }
+
+  @Test
+  public void policyMustContainRuleToUseFlagValues() throws Exception {
+    reporter.removeHandler(failFastHandler); // expecting an error
+    scratch.file(
+        "policy/BUILD",
+        "package_group(",
+        "    name = 'feature_flag_users',",
+        "    packages = ['//flag'])");
+    scratch.file(
+        "flag/BUILD",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['right', 'wrong'],",
+        "    default_value = 'right',",
+        "    visibility = ['//test:__pkg__'],",
+        ")");
+    scratch.file(
+        "test/BUILD",
+        "config_setting(",
+        "    name = 'flag_values_user',",
+        "    flag_values = {",
+        "        '//flag:flag': 'right',",
+        "    },",
+        ")");
+    useConfiguration(
+        "--experimental_dynamic_configs=on",
+        "--feature_control_policy=config_feature_flag=//policy:feature_flag_users");
+    assertThat(getConfiguredTarget("//test:flag_values_user")).isNull();
+    assertContainsEvent(
+        "in config_setting rule //test:flag_values_user: the flag_values attribute is not "
+            + "available in package 'test' according to policy "
+            + "'//policy:feature_flag_users'");
+  }
+
+  @Test
+  public void policyDoesNotBlockRuleIfInPolicy() throws Exception {
+    scratch.file(
+        "policy/BUILD",
+        "package_group(",
+        "    name = 'feature_flag_users',",
+        "    packages = ['//flag', '//test'])");
+    scratch.file(
+        "flag/BUILD",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['right', 'wrong'],",
+        "    default_value = 'right',",
+        "    visibility = ['//test:__pkg__'],",
+        ")");
+    scratch.file(
+        "test/BUILD",
+        "config_setting(",
+        "    name = 'flag_values_user',",
+        "    flag_values = {",
+        "        '//flag:flag': 'right',",
+        "    },",
+        ")");
+    useConfiguration(
+        "--experimental_dynamic_configs=on",
+        "--feature_control_policy=config_feature_flag=//policy:feature_flag_users");
+    assertThat(getConfiguredTarget("//test:flag_values_user")).isNotNull();
+    assertNoEvents();
+  }
+
+  @Test
+  public void policyDoesNotBlockRuleIfFlagValuesNotUsed() throws Exception {
+    scratch.file(
+        "policy/BUILD",
+        "package_group(",
+        "    name = 'feature_flag_users',",
+        "    packages = ['//flag'])");
+    scratch.file("flag/BUILD");
+    scratch.file(
+        "test/BUILD",
+        "config_setting(",
+        "    name = 'flag_values_user',",
+        "    values = {",
+        "        'cpu': 'k8',",
+        "    },",
+        ")");
+    useConfiguration(
+        "--experimental_dynamic_configs=on",
+        "--feature_control_policy=config_feature_flag=//policy:feature_flag_users");
+    assertThat(getConfiguredTarget("//test:flag_values_user")).isNotNull();
+    assertNoEvents();
+  }
 }