CheckPoint: cluster member policy inheritance (#7653)

projects/batfish/src/main/java/org/batfish/vendor/check_point_gateway/representation/CheckPointGatewayConfiguration.java

@@ -164,6 +164,7 @@ public List<Configuration> toVendorIndependentConfigurations() throws VendorConv
             .map(cmc -> (CheckpointManagementConfiguration) cmc);
     Optional<Entry<ManagementDomain, GatewayOrServer>> domainAndGateway =
         mgmtConfig.flatMap(this::findGatewayAndDomain);
+    Optional<Cluster> cluster = domainAndGateway.flatMap(e -> getCluster(e.getValue(), e.getKey()));
 
     if (!mgmtConfig.isPresent()) {
       _w.redFlag(
@@ -176,14 +177,14 @@ public List<Configuration> toVendorIndependentConfigurations() throws VendorConv
     }
 
     Optional<ManagementPackage> mgmtPackage =
-        domainAndGateway.flatMap(e -> findAccessPackage(e.getKey(), e.getValue()));
+        domainAndGateway.flatMap(e -> findAccessPackage(e.getKey(), e.getValue(), cluster));
     Map<Uid, NamedManagementObject> mgmtObjects =
         mgmtPackage
             .map(pakij -> getAllObjects(pakij, domainAndGateway.get().getKey()))
             .orElse(ImmutableMap.of());
 
     // Initial management data conversion
-    domainAndGateway.ifPresent(e -> convertCluster(e.getValue(), e.getKey()));
+    cluster.ifPresent(this::convertCluster);
     mgmtPackage.ifPresent(pakij -> convertPackage(pakij, mgmtObjects, domainAndGateway.get()));
 
     // Gateways don't have VRFs, so put everything in a generated default VRF
@@ -206,10 +207,10 @@ public List<Configuration> toVendorIndependentConfigurations() throws VendorConv
     return ImmutableList.of(_c);
   }
 
-  /** Populates cluster virtual IP metadata if this gateway is a member of a cluster. */
-  private void convertCluster(GatewayOrServer gateway, ManagementDomain domain) {
+  /** Gets the {@link Cluster} for the specified gateway. */
+  private Optional<Cluster> getCluster(GatewayOrServer gateway, ManagementDomain domain) {
     if (!(gateway instanceof ClusterMember)) {
-      return;
+      return Optional.empty();
     }
     Class<? extends Cluster> clusterClass = ((ClusterMember) gateway).getClusterClass();
     String gatewayName = gateway.getName();
@@ -233,8 +234,13 @@ private void convertCluster(GatewayOrServer gateway, ManagementDomain domain) {
           String.format(
               "Could not find matching cluster of type %s for this gateway of type %s",
               clusterClass.getSimpleName(), gateway.getClass().getSimpleName()));
-      return;
+      return Optional.empty();
     }
+    return Optional.of(cluster);
+  }
+
+  /** Populates cluster virtual IP metadata for this gateway. */
+  private void convertCluster(Cluster cluster) {
     _clusterInterfaces =
         cluster.getInterfaces().stream()
             .collect(
@@ -622,16 +628,25 @@ private static String packetPolicyName(@Nullable String incomingFilter) {
     return statements.build();
   }
 
+  /**
+   * Get the {@link ManagementPackage} for the specified gateway. If the gateway has a cluster, use
+   * the package from the cluster.
+   */
   private @Nonnull Optional<ManagementPackage> findAccessPackage(
-      ManagementDomain domain, GatewayOrServer gateway) {
-    String accessPackageName = gateway.getPolicy().getAccessPolicyName();
+      ManagementDomain domain, GatewayOrServer gateway, Optional<Cluster> cluster) {
+    // Use the cluster's access package, if the gateway is a cluster member
+    String accessPackageName =
+        cluster.isPresent()
+            ? cluster.get().getPolicy().getAccessPolicyName()
+            : gateway.getPolicy().getAccessPolicyName();
     if (accessPackageName == null) {
       _w.redFlag(
           String.format(
               "No access package found for gateway '%s', so no access rules will be added",
               gateway.getName()));
       return Optional.empty();
     }
+
     // TODO: can be more efficient if we also store map: packageName -> package in ManagementDomain
     Optional<ManagementPackage> maybePackage =
         domain.getPackages().values().stream()

projects/batfish/src/test/java/org/batfish/vendor/check_point_gateway/grammar/CheckPointGatewayGrammarTest.java

@@ -1210,6 +1210,77 @@ public void testAccessRulesConversion() throws IOException {
     assertThat(c, hasInterface("eth3", hasIncomingFilter(rejects(denied, "eth1", c))));
   }
 
+  /**
+   * Test that access rules are correctly associated with cluster members when attached *only* to
+   * their cluster.
+   */
+  @Test
+  public void testAccessRulesForClusterMember() throws IOException {
+    Uid cpmiAnyUid = Uid.of("99999");
+    CpmiAnyObject any = new CpmiAnyObject(cpmiAnyUid);
+    Uid acceptUid = Uid.of("10");
+    Uid policyTargetsUid = Uid.of("11");
+    Uid packageUid = Uid.of("12");
+    Uid clusterMemberUid = Uid.of("13");
+    Uid clusterUid = Uid.of("14");
+
+    ImmutableMap<Uid, NamedManagementObject> objs =
+        ImmutableMap.<Uid, NamedManagementObject>builder()
+            .put(cpmiAnyUid, any)
+            .put(acceptUid, new RulebaseAction("Accept", acceptUid, "Accept"))
+            .put(policyTargetsUid, new PolicyTargets(policyTargetsUid))
+            .build();
+    ImmutableList<AccessRuleOrSection> rulebase =
+        ImmutableList.of(
+            AccessRule.testBuilder(cpmiAnyUid)
+                .setAction(acceptUid)
+                .setInstallOn(ImmutableList.of(policyTargetsUid))
+                .setUid(Uid.of("100"))
+                .setName("rule1")
+                .build());
+
+    AccessLayer accessLayer = new AccessLayer(objs, rulebase, Uid.of("uid-al"), "accessLayerFoo");
+    ImmutableMap<Uid, ManagementPackage> packages =
+        ImmutableMap.of(
+            packageUid,
+            new ManagementPackage(
+                ImmutableList.of(accessLayer),
+                null,
+                new Package(
+                    new Domain("d", Uid.of("0")),
+                    AllInstallationTargets.instance(),
+                    "p1",
+                    true,
+                    false,
+                    packageUid)));
+
+    ImmutableMap<Uid, GatewayOrServer> gateways =
+        ImmutableMap.of(
+            clusterMemberUid,
+            new CpmiClusterMember(
+                Ip.parse("10.0.0.1"),
+                "access_rules_cluster_member",
+                ImmutableList.of(),
+                new GatewayOrServerPolicy(null, null),
+                clusterMemberUid),
+            clusterUid,
+            new CpmiGatewayCluster(
+                ImmutableList.of("access_rules_cluster_member"),
+                Ip.parse("10.0.2.1"),
+                "access_rules_cluster",
+                ImmutableList.of(),
+                new GatewayOrServerPolicy("p1", null),
+                clusterUid));
+
+    CheckpointManagementConfiguration mgmt =
+        toCheckpointMgmtConfig(gateways, packages, ImmutableList.of());
+    Map<String, Configuration> configs = parseTextConfigs(mgmt, "access_rules");
+    Configuration c = configs.get("access_rules");
+
+    // Just check for existence, extraction/conversion is tested elsewhere
+    assertThat(c.getIpAccessLists(), hasKey(aclName(accessLayer)));
+  }
+
   @Test
   public void testObjectConversionWarnings() throws IOException {
     Uid unknownUid = Uid.of("10");