From b7f6c67411384f22615d762188e847535edd3781 Mon Sep 17 00:00:00 2001 From: Shawn Seymour Date: Wed, 17 Feb 2021 18:27:08 -0600 Subject: [PATCH] feat: fix skip-acls on apply and add tests (#57) --- docs/quick-start.md | 7 +- .../devshawn/kafka/gitops/MainCommand.java | 2 +- .../devshawn/kafka/gitops/StateManager.java | 8 +- .../kafka/gitops/cli/ApplyCommand.java | 2 +- .../kafka/gitops/cli/PlanCommand.java | 2 +- .../kafka/gitops/manager/PlanManager.java | 4 +- .../devshawn/kafka/gitops/util/LogUtil.java | 10 +- .../devshawn/kafka/gitops/util/PlanUtil.java | 6 +- .../gitops/ApplyCommandIntegrationSpec.groovy | 25 +++++ .../gitops/PlanCommandIntegrationSpec.groovy | 26 +++++ .../plans/skip-acls-apply-apply-output.txt | 24 +++++ .../resources/plans/skip-acls-apply-plan.json | 94 +++++++++++++++++++ src/test/resources/plans/skip-acls-plan.json | 28 ++++++ src/test/resources/plans/skip-acls.yaml | 26 +++++ 14 files changed, 248 insertions(+), 16 deletions(-) create mode 100644 src/test/resources/plans/skip-acls-apply-apply-output.txt create mode 100644 src/test/resources/plans/skip-acls-apply-plan.json create mode 100644 src/test/resources/plans/skip-acls-plan.json create mode 100644 src/test/resources/plans/skip-acls.yaml diff --git a/docs/quick-start.md b/docs/quick-start.md index c9e442a8..aecee291 100644 --- 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 index ef3b9a5c..3ed53480 100644 --- 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 { @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 index a10c8319..be0b797b 100644 --- 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 index af47826b..e5e062d2 100644 --- 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 index d6e3c119..5c848748 100644 --- 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 index d339342c..5ca0ee3f 100644 --- 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 index c6fbfae4..a4af6ff1 100644 --- 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 index 1f2153c3..ff86ca95 100644 --- 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 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 index b6c03b55..a2c3ca69 100644 --- 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 index 205022cb..85b8cf72 100644 --- 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 new file mode 100644 index 00000000..558c15b3 --- /dev/null +++ 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 new file mode 100644 index 00000000..b080f9e2 --- /dev/null +++ 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" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/plans/skip-acls-plan.json b/src/test/resources/plans/skip-acls-plan.json new file mode 100644 index 00000000..c054c079 --- /dev/null +++ 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": [] +} \ No newline at end of file diff --git a/src/test/resources/plans/skip-acls.yaml b/src/test/resources/plans/skip-acls.yaml new file mode 100644 index 00000000..d52c0a41 --- /dev/null +++ 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 \ No newline at end of file