feat: fix skip-acls on apply and add tests (#57)

devshawn · Feb 18, 2021 · b7f6c67 · b7f6c67
1 parent 1e47feb
commit b7f6c67
Show file tree

Hide file tree

Showing 14 changed files with 248 additions and 16 deletions.
diff --git a/docs/quick-start.md b/docs/quick-start.md
@@ -89,11 +89,16 @@ Plan: 1 to create, 0 to update, 0 to delete.
 
 In most cases, you will want to output the plan to a file which can then be passed to the apply command. Plan files are `JSON` files. This can be done by running:
 
-
 ```bash
 kafka-gitops plan -o plan.json
 ```
 
+If running against a Kafka cluster with no authorizer configured or if you simply want to only manage topics, you can ignore ACLs completely. This can be done by running:
+
+```bash
+kafka-gitops --skip-acls plan
+```
+
 ## Apply
 
 To execute a plan against the cluster, we use the apply command.

diff --git a/src/main/java/com/devshawn/kafka/gitops/MainCommand.java b/src/main/java/com/devshawn/kafka/gitops/MainCommand.java
@@ -30,7 +30,7 @@ public class MainCommand implements Callable<Integer> {
     @Option(names = {"--no-delete"}, description = "Disable the ability to delete resources.")
     private boolean deleteDisabled = false;
 
-    @Option(names = {"--skip-acls"}, description = "Do not take ACL into account in the plan file.")
+    @Option(names = {"--skip-acls"}, description = "Do not take ACLs into account during plans or applies.")
     private boolean skipAcls = false;
 
     @Option(names = {"-h", "--help"}, usageHelp = true, description = "Display this help message.")

diff --git a/src/main/java/com/devshawn/kafka/gitops/StateManager.java b/src/main/java/com/devshawn/kafka/gitops/StateManager.java
@@ -79,7 +79,7 @@ public DesiredStateFile getAndValidateStateFile() {
     public DesiredPlan plan() {
         DesiredPlan desiredPlan = generatePlan();
         planManager.writePlanToFile(desiredPlan);
-        planManager.validatePlanHasChanges(desiredPlan, managerConfig.isDeleteDisabled());
+        planManager.validatePlanHasChanges(desiredPlan, managerConfig.isDeleteDisabled(), managerConfig.isSkipAclsDisabled());
         return desiredPlan;
     }
 
@@ -99,10 +99,12 @@ public DesiredPlan apply() {
             desiredPlan = generatePlan();
         }
 
-        planManager.validatePlanHasChanges(desiredPlan, managerConfig.isDeleteDisabled());
+        planManager.validatePlanHasChanges(desiredPlan, managerConfig.isDeleteDisabled(), managerConfig.isSkipAclsDisabled());
 
         applyManager.applyTopics(desiredPlan);
-        applyManager.applyAcls(desiredPlan);
+        if (!managerConfig.isSkipAclsDisabled()) {
+            applyManager.applyAcls(desiredPlan);
+        }
 
         return desiredPlan;
     }

diff --git a/src/main/java/com/devshawn/kafka/gitops/cli/ApplyCommand.java b/src/main/java/com/devshawn/kafka/gitops/cli/ApplyCommand.java
@@ -34,7 +34,7 @@ public Integer call() {
             ParserService parserService = new ParserService(parent.getFile());
             StateManager stateManager = new StateManager(generateStateManagerConfig(), parserService);
             DesiredPlan desiredPlan = stateManager.apply();
-            LogUtil.printApplyOverview(PlanUtil.getOverview(desiredPlan, parent.isDeleteDisabled()));
+            LogUtil.printApplyOverview(PlanUtil.getOverview(desiredPlan, parent.isDeleteDisabled(), parent.areAclsDisabled()));
             return 0;
         } catch (PlanIsUpToDateException ex) {
             LogUtil.printNoChangesMessage();

diff --git a/src/main/java/com/devshawn/kafka/gitops/cli/PlanCommand.java b/src/main/java/com/devshawn/kafka/gitops/cli/PlanCommand.java
@@ -36,7 +36,7 @@ public Integer call() {
             ParserService parserService = new ParserService(parent.getFile());
             StateManager stateManager = new StateManager(generateStateManagerConfig(), parserService);
             DesiredPlan desiredPlan = stateManager.plan();
-            LogUtil.printPlan(desiredPlan, parent.isDeleteDisabled());
+            LogUtil.printPlan(desiredPlan, parent.isDeleteDisabled(), parent.areAclsDisabled());
             return 0;
         } catch (PlanIsUpToDateException ex) {
             LogUtil.printNoChangesMessage();

diff --git a/src/main/java/com/devshawn/kafka/gitops/manager/PlanManager.java b/src/main/java/com/devshawn/kafka/gitops/manager/PlanManager.java
@@ -173,8 +173,8 @@ public void planAcls(DesiredState desiredState, DesiredPlan.Builder desiredPlan)
         });
     }
 
-    public void validatePlanHasChanges(DesiredPlan desiredPlan, boolean deleteDisabled) {
-        PlanOverview planOverview = PlanUtil.getOverview(desiredPlan, deleteDisabled);
+    public void validatePlanHasChanges(DesiredPlan desiredPlan, boolean deleteDisabled, boolean skipAclsDisabled) {
+        PlanOverview planOverview = PlanUtil.getOverview(desiredPlan, deleteDisabled, skipAclsDisabled);
         if (planOverview.getAdd() == 0 && planOverview.getUpdate() == 0 && planOverview.getRemove() == 0) {
             throw new PlanIsUpToDateException();
         }

diff --git a/src/main/java/com/devshawn/kafka/gitops/util/LogUtil.java b/src/main/java/com/devshawn/kafka/gitops/util/LogUtil.java
@@ -14,8 +14,8 @@
 
 public class LogUtil {
 
-    public static void printPlan(DesiredPlan desiredPlan, boolean deleteDisabled) {
-        PlanOverview planOverview = PlanUtil.getOverview(desiredPlan, deleteDisabled);
+    public static void printPlan(DesiredPlan desiredPlan, boolean deleteDisabled, boolean skipAclsDisabled) {
+        PlanOverview planOverview = PlanUtil.getOverview(desiredPlan, deleteDisabled, skipAclsDisabled);
 
         printLegend(planOverview);
 
@@ -25,7 +25,7 @@ public static void printPlan(DesiredPlan desiredPlan, boolean deleteDisabled) {
         printAclOverview(desiredPlan, deleteDisabled);
         desiredPlan.getAclPlans().forEach(LogUtil::printAclPlan);
 
-        printOverview(desiredPlan, deleteDisabled);
+        printOverview(desiredPlan, deleteDisabled, skipAclsDisabled);
     }
 
     public static void printValidationResult(String message, boolean success) {
@@ -131,8 +131,8 @@ public static void printPostApply() {
      * Helpers
      */
 
-    private static void printOverview(DesiredPlan desiredPlan, boolean deleteDisabled) {
-        PlanOverview planOverview = PlanUtil.getOverview(desiredPlan, deleteDisabled);
+    private static void printOverview(DesiredPlan desiredPlan, boolean deleteDisabled, boolean skipAclsDisabled) {
+        PlanOverview planOverview = PlanUtil.getOverview(desiredPlan, deleteDisabled, skipAclsDisabled);
         System.out.println(String.format("%s: %s, %s, %s.", bold("Plan"), toCreate(planOverview.getAdd()),
                 toUpdate(planOverview.getUpdate()), toDelete(planOverview.getRemove())));
     }

diff --git a/src/main/java/com/devshawn/kafka/gitops/util/PlanUtil.java b/src/main/java/com/devshawn/kafka/gitops/util/PlanUtil.java
@@ -9,10 +9,12 @@
 
 public class PlanUtil {
 
-    public static PlanOverview getOverview(DesiredPlan desiredPlan, boolean deleteDisabled) {
+    public static PlanOverview getOverview(DesiredPlan desiredPlan, boolean deleteDisabled, boolean skipAclsDisabled) {
         EnumMap<PlanAction, Long> map = getPlanActionMap();
         desiredPlan.getTopicPlans().forEach(it -> addToMap(map, it.getAction(), deleteDisabled));
-        desiredPlan.getAclPlans().forEach(it -> addToMap(map, it.getAction(), deleteDisabled));
+        if(!skipAclsDisabled) {
+            desiredPlan.getAclPlans().forEach(it -> addToMap(map, it.getAction(), deleteDisabled));
+        }
         return buildPlanOverview(map);
     }
 

diff --git a/src/test/groovy/com/devshawn/kafka/gitops/ApplyCommandIntegrationSpec.groovy b/src/test/groovy/com/devshawn/kafka/gitops/ApplyCommandIntegrationSpec.groovy
@@ -62,6 +62,31 @@ class ApplyCommandIntegrationSpec extends Specification {
         ]
     }
 
+    void 'test skip-acls flag'() {
+        setup:
+        ByteArrayOutputStream out = new ByteArrayOutputStream()
+        PrintStream oldOut = System.out
+        System.setOut(new PrintStream(out))
+        String file = TestUtils.getResourceFilePath("plans/${planFile}-plan.json")
+        MainCommand mainCommand = new MainCommand()
+        CommandLine cmd = new CommandLine(mainCommand)
+
+        when:
+        int exitCode = cmd.execute("-f", file, "--skip-acls", "apply", "-p", file)
+
+        then:
+        exitCode == 0
+        out.toString() == TestUtils.getResourceFileContent("plans/${planFile}-apply-output.txt")
+
+        cleanup:
+        System.setOut(oldOut)
+
+        where:
+        planFile << [
+                "skip-acls-apply"
+        ]
+    }
+
     void 'test various valid applies with seed - #planFile #deleteDisabled'() {
         setup:
         TestUtils.seedCluster()

diff --git a/src/test/groovy/com/devshawn/kafka/gitops/PlanCommandIntegrationSpec.groovy b/src/test/groovy/com/devshawn/kafka/gitops/PlanCommandIntegrationSpec.groovy
@@ -71,6 +71,32 @@ class PlanCommandIntegrationSpec extends Specification {
         ]
     }
 
+    void 'test skip-acls flag'() {
+        setup:
+        String planOutputFile = "/tmp/plan.json"
+        String file = TestUtils.getResourceFilePath("plans/${planName}.yaml")
+        MainCommand mainCommand = new MainCommand()
+        CommandLine cmd = new CommandLine(mainCommand)
+
+        when:
+        int exitCode = cmd.execute("-f", file, "--skip-acls", "plan", "-o", planOutputFile)
+
+        then:
+        exitCode == 0
+
+        when:
+        String actualPlan = TestUtils.getFileContent(planOutputFile)
+        String expectedPlan = TestUtils.getResourceFileContent("plans/${planName}-plan.json")
+
+        then:
+        JSONAssert.assertEquals(expectedPlan, actualPlan, true)
+
+        where:
+        planName << [
+                "skip-acls"
+        ]
+    }
+
     void 'test various valid plans with seed - #planName'() {
         setup:
         TestUtils.cleanUpCluster()

diff --git a/src/test/resources/plans/skip-acls-apply-apply-output.txt b/src/test/resources/plans/skip-acls-apply-apply-output.txt
@@ -0,0 +1,24 @@
+Executing apply...
+
+Applying: [CREATE]
+
++ [TOPIC] MY_TOPIC
+	+ partitions: 6
+	+ replication: 1
+
+
+Successfully applied.
+
+Applying: [CREATE]
+
++ [TOPIC] another.topic.0
+	+ partitions: 1
+	+ replication: 1
+	+ configs:
+		+ cleanup.policy: compact
+		+ segment.bytes: 100000
+
+
+Successfully applied.
+
+[SUCCESS] Apply complete! Resources: 2 created, 0 updated, 0 deleted.
diff --git a/src/test/resources/plans/skip-acls-apply-plan.json b/src/test/resources/plans/skip-acls-apply-plan.json
@@ -0,0 +1,94 @@
+{
+    "topicPlans": [
+        {
+            "name": "MY_TOPIC",
+            "action": "ADD",
+            "topicDetails": {
+                "partitions": 6,
+                "replication": 1,
+                "configs": {}
+            },
+            "topicConfigPlans": []
+        },
+        {
+            "name": "another.topic.0",
+            "action": "ADD",
+            "topicDetails": {
+                "partitions": 1,
+                "replication": 1,
+                "configs": {
+                    "cleanup.policy": "compact",
+                    "segment.bytes": "100000"
+                }
+            },
+            "topicConfigPlans": []
+        }
+    ],
+    "aclPlans": [
+        {
+            "name": "test-service-0",
+            "aclDetails": {
+                "name": "another.topic.0",
+                "type": "TOPIC",
+                "pattern": "LITERAL",
+                "principal": "User:test",
+                "host": "*",
+                "operation": "WRITE",
+                "permission": "ALLOW"
+            },
+            "action": "ADD"
+        },
+        {
+            "name": "test-service-1",
+            "aclDetails": {
+                "name": "MY_TOPIC",
+                "type": "TOPIC",
+                "pattern": "LITERAL",
+                "principal": "User:test",
+                "host": "*",
+                "operation": "READ",
+                "permission": "ALLOW"
+            },
+            "action": "ADD"
+        },
+        {
+            "name": "test-service-2",
+            "aclDetails": {
+                "name": "test-service",
+                "type": "GROUP",
+                "pattern": "LITERAL",
+                "principal": "User:test",
+                "host": "*",
+                "operation": "READ",
+                "permission": "ALLOW"
+            },
+            "action": "ADD"
+        },
+        {
+            "name": "my-other-service-0",
+            "aclDetails": {
+                "name": "another.topic.0",
+                "type": "TOPIC",
+                "pattern": "LITERAL",
+                "principal": "User:test",
+                "host": "*",
+                "operation": "READ",
+                "permission": "ALLOW"
+            },
+            "action": "ADD"
+        },
+        {
+            "name": "my-other-service-1",
+            "aclDetails": {
+                "name": "my-other-service",
+                "type": "GROUP",
+                "pattern": "LITERAL",
+                "principal": "User:test",
+                "host": "*",
+                "operation": "READ",
+                "permission": "ALLOW"
+            },
+            "action": "ADD"
+        }
+    ]
+}
diff --git a/src/test/resources/plans/skip-acls-plan.json b/src/test/resources/plans/skip-acls-plan.json
@@ -0,0 +1,28 @@
+{
+    "topicPlans": [
+        {
+            "name": "MY_TOPIC",
+            "action": "ADD",
+            "topicDetails": {
+                "partitions": 6,
+                "replication": 1,
+                "configs": {}
+            },
+            "topicConfigPlans": []
+        },
+        {
+            "name": "another.topic.0",
+            "action": "ADD",
+            "topicDetails": {
+                "partitions": 1,
+                "replication": 1,
+                "configs": {
+                    "cleanup.policy": "compact",
+                    "segment.bytes": "100000"
+                }
+            },
+            "topicConfigPlans": []
+        }
+    ],
+    "aclPlans": []
+}
diff --git a/src/test/resources/plans/skip-acls.yaml b/src/test/resources/plans/skip-acls.yaml
@@ -0,0 +1,26 @@
+topics:
+  MY_TOPIC:
+    partitions: 6
+    replication: 1
+
+  another.topic.0:
+    partitions: 1
+    replication: 1
+    configs:
+      cleanup.policy: compact
+      segment.bytes: 100000
+
+services:
+  test-service:
+    type: application
+    principal: User:test
+    produces:
+      - another.topic.0
+    consumes:
+      - MY_TOPIC
+
+  my-other-service:
+    type: application
+    principal: User:test
+    consumes:
+      - another.topic.0