Make GuardedValue match repo name substring instead of exact name.
This allows guarded values to work with bzlmod, where a repo's name has
various bzlmod pieces surrounding the apparent repo name. For example,
under bzlmod:
* `@rules_python` -> `@rules_python~override` (module repo)
* `@rules_python_internal` -> `@rules_python~override~internal_deps~rules_python_internal` (repo created
as part of a module extension)
PiperOrigin-RevId: 568689554
Change-Id: I2e8cda4717bcd3dc1cd79d44dfd66e3d436aa8f8
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextAndFlagGuardedValue.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextAndFlagGuardedValue.java
index 91be2e9..82d4741 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextAndFlagGuardedValue.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextAndFlagGuardedValue.java
@@ -15,8 +15,6 @@
package com.google.devtools.build.lib.starlarkbuildapi.core;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.cmdline.BazelCompileContext;
-import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
@@ -32,41 +30,30 @@
public final class ContextAndFlagGuardedValue {
/**
* Creates a flag guard which only permits access of the given object when the given boolean flag
- * is false or the requesting .bzl file is in a specific patckage path. If the given flag is true
- * and the object would be accessed, an error is thrown describing the feature as deprecated, and
- * describing that the flag may be set to false to re-enable it.
+ * is false or the requesting .bzl file's (repo, path) matches one of the allowed entries (see
+ * ContextGuardedFlag for match criteria). If the given flag is true and the object would be
+ * accessed, an error is thrown describing the feature as deprecated, and describing that the flag
+ * may be set to false to re-enable it.
*
* <p>The flag identifier must have a + or - prefix; see StarlarkSemantics.
*/
public static GuardedValue onlyInAllowedReposOrWhenIncompatibleFlagIsFalse(
- String flag, Object obj, ImmutableSet<PackageIdentifier> allowedPrefixes) {
+ String flag, Object obj, ImmutableSet<PackageIdentifier> allowedEntries) {
GuardedValue flagGuard = FlagGuardedValue.onlyWhenIncompatibleFlagIsFalse(flag, obj);
+ GuardedValue contextGuard = ContextGuardedValue.onlyInAllowedRepos(obj, allowedEntries);
return new GuardedValue() {
@Override
public boolean isObjectAccessibleUsingSemantics(
StarlarkSemantics semantics, @Nullable Object clientData) {
- boolean accessible = flagGuard.isObjectAccessibleUsingSemantics(semantics, clientData);
- // Filtering of predeclareds is only done at compile time, when the client data is
- // BazelCompileContext and not BazelModuleContext.
- if (!accessible && clientData != null && clientData instanceof BazelCompileContext) {
- BazelCompileContext context = (BazelCompileContext) clientData;
- Label label = context.label();
-
- for (PackageIdentifier prefix : allowedPrefixes) {
- if (label.getRepository().equals(prefix.getRepository())
- && label.getPackageFragment().startsWith(prefix.getPackageFragment())) {
- return true;
- }
- }
- }
- return accessible;
+ return flagGuard.isObjectAccessibleUsingSemantics(semantics, clientData)
+ || contextGuard.isObjectAccessibleUsingSemantics(semantics, clientData);
}
@Override
public String getErrorFromAttemptingAccess(String name) {
return name
+ " may only be used from one of the following repositories or prefixes: "
- + allowedPrefixes.stream()
+ + allowedEntries.stream()
.map(PackageIdentifier::toString)
.collect(Collectors.joining(", "))
+ ". It may be temporarily re-enabled for general use by setting --"
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextGuardedValue.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextGuardedValue.java
index 7c670f6..5223e1b 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextGuardedValue.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextGuardedValue.java
@@ -18,6 +18,7 @@
import com.google.devtools.build.lib.cmdline.BazelCompileContext;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import net.starlark.java.eval.GuardedValue;
@@ -32,12 +33,13 @@
private ContextGuardedValue() {}
/**
- * Creates a guard which only permits access of the given object when the requesting .bzl file is
- * in a specific patckage path. An error is thrown if accessing it is done outside the allowed
- * package paths.
+ * Creates a guard which only permits access of the given object when the requesting .bzl file's
+ * repo name matches one of the (repoNameSubstr, pathPrefix) entries (note: under bzlmod, repos
+ * are prefixed with the bzlmod module name). An error is thrown if accessing it is done outside
+ * the allowed package paths.
*/
public static GuardedValue onlyInAllowedRepos(
- Object obj, ImmutableSet<PackageIdentifier> allowedPrefixes) {
+ Object obj, ImmutableSet<PackageIdentifier> allowedEntries) {
return new GuardedValue() {
@Override
public boolean isObjectAccessibleUsingSemantics(
@@ -48,9 +50,20 @@
BazelCompileContext context = (BazelCompileContext) clientData;
Label label = context.label();
- for (PackageIdentifier prefix : allowedPrefixes) {
- if (label.getRepository().equals(prefix.getRepository())
- && label.getPackageFragment().startsWith(prefix.getPackageFragment())) {
+ for (PackageIdentifier entry : allowedEntries) {
+ String pattern;
+ if (entry.getRepository().getName().isEmpty()) {
+ // String.matches has ^$ implicilty, so an empty pattern matches exactly the empty
+ // string.
+ pattern = "";
+ } else {
+ // Surrounding .* because String.matches has implicit "^$" anchor.
+ // Surrounding \b so it doesn't match a substring of the intended repo name.
+ // Quote the name so dots in the repo name don't get treated as part of the pattern
+ pattern = ".*\\b" + Pattern.quote(entry.getRepository().getName()) + "\\b.*";
+ }
+ if (label.getRepository().getName().matches(pattern)
+ && label.getPackageFragment().startsWith(entry.getPackageFragment())) {
return true;
}
}
@@ -62,7 +75,7 @@
public String getErrorFromAttemptingAccess(String name) {
return name
+ " may only be used from one of the following repositories or prefixes: "
- + allowedPrefixes.stream()
+ + allowedEntries.stream()
.map(PackageIdentifier::toString)
.collect(Collectors.joining(", "));
}
diff --git a/src/test/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextGuardedValueTest.java b/src/test/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextGuardedValueTest.java
new file mode 100644
index 0000000..5727b3d
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/starlarkbuildapi/core/ContextGuardedValueTest.java
@@ -0,0 +1,133 @@
+// Copyright 2022 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.starlarkbuildapi.core;
+
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.common.truth.Truth.assertThat;
+import static java.util.Arrays.stream;
+
+import com.google.devtools.build.lib.cmdline.BazelCompileContext;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import net.starlark.java.eval.GuardedValue;
+import net.starlark.java.eval.StarlarkSemantics;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class ContextGuardedValueTest {
+
+ /**
+ * We want to make sure the empty string doesn't result in "allow everything". That would be bad.
+ * Most allowlists have an entry like ("", "tools/build_defs/lang") for usage within Google.
+ */
+ @Test
+ public void emptyRepoAllowed_doesntMatchNonAllowed() throws Exception {
+ assertNotAllowed("@mylang//bar:baz", "@//tools/lang");
+ }
+
+ @Test
+ public void emptyRepoAllowed_matchesAllowed() throws Exception {
+ assertAllowed("@//tools/lang", "@//tools/lang");
+ }
+
+ @Test
+ public void workspaceRepo_matchesAllowedRepo() throws Exception {
+ assertAllowed("@rules_foo//tools/lang", "@rules_foo//");
+ }
+
+ @Test
+ public void workspaceRepo_doesntMatchCommonSubstr() throws Exception {
+ assertNotAllowed("@my_rules_foo_helper//tools/lang", "@rules_foo//");
+ }
+
+ @Test
+ public void bzlmodRepo_matchesStart() throws Exception {
+ assertAllowed("@rules_foo~override//tools/lang", "@rules_foo//");
+ assertAllowed("@rules_foo~1.2.3//tools/lang", "@rules_foo//");
+ }
+
+ @Test
+ public void bzlmodRepo_matchesWithin() throws Exception {
+ assertAllowed("@rules_lang~override~ext~foo_helper//tools/lang", "@foo_helper//");
+ }
+
+ @Test
+ public void bzlmodRepo_doesntMatchCommonSubstr() throws Exception {
+ assertNotAllowed("@rules_lang~override~ext~my_foo_helper_lib//tools/lang", "@foo_helper//");
+ }
+
+ @Test
+ public void reposWithDotsDontMatch() throws Exception {
+ assertNotAllowed("@my.lang//foo", "@my_lang//");
+ }
+
+ @Test
+ public void verifySomeRealisticCases() throws Exception {
+ // Python with workspace
+ assertAllowed("@//tools/build_defs/python/private", "@//tools/build_defs/python");
+ assertAllowed("@rules_python//python/private", "@rules_python//");
+
+ // Python with bzlmod
+ assertAllowed(
+ "@rules_python~override~internal_deps~rules_python_internal//private", "@rules_python//");
+
+ // CC with workspace
+ assertAllowed("@//tools/build_defs/cc", "@//tools/build_defs/cc");
+ assertNotAllowed("@rules_cc_helper//tools/build_defs/cc", "@rules_cc//");
+
+ // CC with Bzlmod
+ assertAllowed("@rules_cc~1.2.3~ext_name~local_cc_config//foo", "@local_cc_config//");
+ }
+
+ private Object createClientData(String callerLabelStr) {
+ return BazelCompileContext.create(
+ Label.parseCanonicalUnchecked(callerLabelStr), "unused_caller.bzl");
+ }
+
+ private GuardedValue createGuard(Object clientData, String... allowedLabelStrs) throws Exception {
+ var allowed =
+ stream(allowedLabelStrs)
+ .map(
+ labelStr -> {
+ try {
+ return PackageIdentifier.parse(labelStr);
+ } catch (LabelSyntaxException e) {
+ // We have to manually catch and re-throw this, otherwise Java is unhappy.
+ throw new RuntimeException(e);
+ }
+ })
+ .collect(toImmutableSet());
+
+ return ContextGuardedValue.onlyInAllowedRepos(clientData, allowed);
+ }
+
+ private void assertAllowed(String callerLabelStr, String... allowedLabelStrs) throws Exception {
+ var clientData = createClientData(callerLabelStr);
+ var guard = createGuard(clientData, allowedLabelStrs);
+ assertThat(guard.isObjectAccessibleUsingSemantics(StarlarkSemantics.DEFAULT, clientData))
+ .isTrue();
+ }
+
+ private void assertNotAllowed(String callerLabelStr, String... allowedLabelStrs)
+ throws Exception {
+ var clientData = createClientData(callerLabelStr);
+ var guard = createGuard(clientData, allowedLabelStrs);
+ assertThat(guard.isObjectAccessibleUsingSemantics(StarlarkSemantics.DEFAULT, clientData))
+ .isFalse();
+ }
+}
