[7.1.0] Make it possible to toggle cache key scrubbing by rule kind (#…

…21276)

Add the target kind (e.g. java_library) to the scrubbing matcher
configuration. This is useful because different kinds may use the same
mnemonic, and specifying an explicit set of labels is labor-intensive
and error-prone.

Closes #21151.

Commit
4bcf855

PiperOrigin-RevId: 605624773
Change-Id: Ib5748ca2dbb3fd6d30b27fc1b292d7cd826ced62

Co-authored-by: Artem V. Navrotskiy <[email protected]>

src/main/java/com/google/devtools/build/lib/remote/Scrubber.java

@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
 import com.google.devtools.build.lib.remote.RemoteScrubbing.Config;
@@ -104,6 +105,7 @@ public static class SpawnScrubber {
 
     private final Pattern mnemonicPattern;
     private final Pattern labelPattern;
+    private final Pattern kindPattern;
     private final boolean matchTools;
 
     private final ImmutableList<Pattern> omittedInputPatterns;
@@ -114,6 +116,7 @@ private SpawnScrubber(Config.Rule ruleProto) {
       Config.Matcher matcherProto = ruleProto.getMatcher();
       this.mnemonicPattern = Pattern.compile(emptyToAll(matcherProto.getMnemonic()));
       this.labelPattern = Pattern.compile(emptyToAll(matcherProto.getLabel()));
+      this.kindPattern = Pattern.compile(emptyToAll(matcherProto.getKind()));
       this.matchTools = matcherProto.getMatchTools();
 
       Config.Transform transformProto = ruleProto.getTransform();
@@ -134,12 +137,15 @@ private String emptyToAll(String s) {
     /** Whether this scrubber applies to the given {@link Spawn}. */
     private boolean matches(Spawn spawn) {
       String mnemonic = spawn.getMnemonic();
-      String label = spawn.getResourceOwner().getOwner().getLabel().getCanonicalForm();
-      boolean isForTool = spawn.getResourceOwner().getOwner().isBuildConfigurationForTool();
+      ActionOwner actionOwner = spawn.getResourceOwner().getOwner();
+      String label = actionOwner.getLabel().getCanonicalForm();
+      String kind = actionOwner.getTargetKind();
+      boolean isForTool = actionOwner.isBuildConfigurationForTool();
 
       return (!isForTool || matchTools)
           && mnemonicPattern.matcher(mnemonic).matches()
-          && labelPattern.matcher(label).matches();
+          && labelPattern.matcher(label).matches()
+          && kindPattern.matcher(kind).matches();
     }
 
     /** Whether the given input should be omitted from the cache key. */

src/main/protobuf/remote_scrubbing.proto

@@ -23,6 +23,10 @@ option java_package = "com.google.devtools.build.lib.remote";
 message Config {
   // Describes a set of actions. An action must pass all criteria to match.
   message Matcher {
+    // A regex matching the rule kind of the action owner.
+    // Use .* if a partial match is desired.
+    // If empty, matches every kind.
+    string kind = 4;
     // A regex matching the canonical label of the action owner.
     // Use .* if a partial match is desired.
     // If empty, matches every label.

src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java

@@ -43,7 +43,8 @@
 public class FakeOwner implements ActionExecutionMetadata {
   private final String mnemonic;
   private final String progressMessage;
-  @Nullable private final String ownerLabel;
+  private final String ownerLabel;
+  private final String ownerRuleKind;
   @Nullable private final Artifact primaryOutput;
   @Nullable private final PlatformInfo platform;
   private final ImmutableMap<String, String> execProperties;
@@ -53,13 +54,15 @@ public class FakeOwner implements ActionExecutionMetadata {
       String mnemonic,
       String progressMessage,
       String ownerLabel,
+      String ownerRuleKind,
       @Nullable Artifact primaryOutput,
       @Nullable PlatformInfo platform,
       ImmutableMap<String, String> execProperties,
       boolean isBuiltForToolConfiguration) {
     this.mnemonic = mnemonic;
     this.progressMessage = progressMessage;
     this.ownerLabel = checkNotNull(ownerLabel);
+    this.ownerRuleKind = checkNotNull(ownerRuleKind);
     this.primaryOutput = primaryOutput;
     this.platform = platform;
     this.execProperties = execProperties;
@@ -72,6 +75,7 @@ private FakeOwner(
         mnemonic,
         progressMessage,
         ownerLabel,
+        /* ownerRuleKind= */ "dummy-target-kind",
         /* primaryOutput= */ null,
         platform,
         ImmutableMap.of(),
@@ -87,7 +91,7 @@ public ActionOwner getOwner() {
     return ActionOwner.createDummy(
         Label.parseCanonicalUnchecked(ownerLabel),
         new Location("dummy-file", 0, 0),
-        /* targetKind= */ "dummy-target-kind",
+        ownerRuleKind,
         mnemonic,
         /* configurationChecksum= */ "configurationChecksum",
         new BuildConfigurationEvent(

src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java

@@ -47,6 +47,7 @@ public final class SpawnBuilder {
   private String mnemonic = "Mnemonic";
   private String progressMessage = "progress message";
   private String ownerLabel = "//dummy:label";
+  private String ownerRuleKind = "dummy-target-kind";
   @Nullable private Artifact ownerPrimaryOutput;
   @Nullable private PlatformInfo platform;
   private final List<String> args;
@@ -96,6 +97,7 @@ public Spawn build() {
             mnemonic,
             progressMessage,
             ownerLabel,
+            ownerRuleKind,
             ownerPrimaryOutput,
             platform,
             execProperties,
@@ -140,6 +142,12 @@ public SpawnBuilder withOwnerLabel(String ownerLabel) {
     return this;
   }
 
+  @CanIgnoreReturnValue
+  public SpawnBuilder withOwnerRuleKind(String ownerRuleKind) {
+    this.ownerRuleKind = checkNotNull(ownerRuleKind);
+    return this;
+  }
+
   @CanIgnoreReturnValue
   public SpawnBuilder withOwnerPrimaryOutput(Artifact output) {
     ownerPrimaryOutput = checkNotNull(output);

src/test/java/com/google/devtools/build/lib/remote/ScrubberTest.java

@@ -125,6 +125,54 @@ public void matchWildcardLabel() {
     assertThat(scrubber.forSpawn(createSpawn("//spam:eggs", "Foo"))).isNull();
   }
 
+  @Test
+  public void matchExactKind() {
+    var scrubber =
+        new Scrubber(
+            Config.newBuilder()
+                .addRules(
+                    Config.Rule.newBuilder()
+                        .setMatcher(Config.Matcher.newBuilder().setKind("java_library")))
+                .build());
+
+    assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", false)))
+        .isNotNull();
+    assertThat(scrubber.forSpawn(createSpawn("//foo:barbaz", "Foo", "java_test", false))).isNull();
+  }
+
+  @Test
+  public void matchUnionKind() {
+    var scrubber =
+        new Scrubber(
+            Config.newBuilder()
+                .addRules(
+                    Config.Rule.newBuilder()
+                        .setMatcher(Config.Matcher.newBuilder().setKind("java_library|java_test")))
+                .build());
+
+    assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", false)))
+        .isNotNull();
+    assertThat(scrubber.forSpawn(createSpawn("//spam:eggs", "Foo", "java_test", false)))
+        .isNotNull();
+    assertThat(scrubber.forSpawn(createSpawn("//quux:xyzzy", "Foo", "go_library", false))).isNull();
+  }
+
+  @Test
+  public void matchWildcardKind() {
+    var scrubber =
+        new Scrubber(
+            Config.newBuilder()
+                .addRules(
+                    Config.Rule.newBuilder()
+                        .setMatcher(Config.Matcher.newBuilder().setKind("java_.*")))
+                .build());
+
+    assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", false)))
+        .isNotNull();
+    assertThat(scrubber.forSpawn(createSpawn("//foo:baz", "Foo", "java_test", false))).isNotNull();
+    assertThat(scrubber.forSpawn(createSpawn("//spam:eggs", "Foo", "go_library", false))).isNull();
+  }
+
   @Test
   public void rejectToolAction() {
     var scrubber =
@@ -137,7 +185,9 @@ public void rejectToolAction() {
                 .build());
 
     assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull();
-    assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", /* forTool= */ true))).isNull();
+    assertThat(
+            scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", /* forTool= */ true)))
+        .isNull();
   }
 
   @Test
@@ -155,7 +205,9 @@ public void acceptToolAction() {
                 .build());
 
     assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull();
-    assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", /* forTool= */ true))).isNotNull();
+    assertThat(
+            scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", /* forTool= */ true)))
+        .isNotNull();
   }
 
   @Test
@@ -420,13 +472,15 @@ private static Spawn createSpawn() {
   }
 
   private static Spawn createSpawn(String label, String mnemonic) {
-    return createSpawn(label, mnemonic, /* forTool= */ false);
+    return createSpawn(label, mnemonic, /* ruleKind= */ "dummy-target-kind", /* forTool= */ false);
   }
 
-  private static Spawn createSpawn(String label, String mnemonic, boolean forTool) {
+  private static Spawn createSpawn(
+      String label, String mnemonic, String ruleKind, boolean forTool) {
     return new SpawnBuilder("cmd")
         .withOwnerLabel(label)
         .withMnemonic(mnemonic)
+        .withOwnerRuleKind(ruleKind)
         .setBuiltForToolConfiguration(forTool)
         .build();
   }