Extend the project name format to include by default the separator, t…

…his is useful to narrow down optimised acls to be more secure (#515)
kafka-ops · Aug 3, 2022 · cc8f25c · cc8f25c
1 parent 3765517
commit cc8f25c
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 21 deletions.
diff --git a/src/main/java/com/purbon/kafka/topology/model/Impl/ProjectImpl.java b/src/main/java/com/purbon/kafka/topology/model/Impl/ProjectImpl.java
@@ -212,7 +212,10 @@ private String patternBasedProjectPrefix() {
 
   private String namePrefix(String topologyPrefix) {
     StringBuilder sb = new StringBuilder();
-    sb.append(topologyPrefix).append(config.getTopicPrefixSeparator()).append(name);
+    sb.append(topologyPrefix)
+        .append(config.getTopicPrefixSeparator())
+        .append(name)
+        .append(config.getTopicPrefixSeparator());
     return sb.toString();
   }
 

diff --git a/src/main/java/com/purbon/kafka/topology/model/Topic.java b/src/main/java/com/purbon/kafka/topology/model/Topic.java
@@ -161,7 +161,7 @@ private String patternBasedTopicNameStructureString() {
   private String defaultTopicStructureString(String projectPrefix) {
     StringBuilder sb = new StringBuilder();
     if (projectPrefix != null && !projectPrefix.isBlank()) {
-      sb.append(projectPrefix).append(appConfig.getTopicPrefixSeparator());
+      sb.append(projectPrefix);
     }
     sb.append(getName());
 

diff --git a/src/test/java/com/purbon/kafka/topology/AccessControlManagerTest.java b/src/test/java/com/purbon/kafka/topology/AccessControlManagerTest.java
@@ -101,7 +101,7 @@ public void newConsumerOptimisedACLsCreation() throws IOException {
         .buildBindingsForConsumers(users, builder.getProject().namePrefix(), true);
     accessControlManager.updatePlan(builder.buildTopology(), plan);
     verify(aclsBuilder, times(1))
-        .buildBindingsForConsumers(eq(users), eq(builder.getProject().namePrefix()), eq(true));
+        .buildBindingsForConsumers(eq(users), eq("ctx.project."), eq(true));
   }
 
   @Test
@@ -177,7 +177,7 @@ public void newProducerOptimizedACLsCreation() throws IOException {
         .buildBindingsForProducers(producers, builder.getProject().namePrefix(), true);
     accessControlManager.updatePlan(builder.buildTopology(), plan);
     verify(aclsBuilder, times(1))
-        .buildBindingsForProducers(eq(producers), eq(builder.getProject().namePrefix()), eq(true));
+        .buildBindingsForProducers(eq(producers), eq("ctx.project."), eq(true));
   }
 
   @Test
@@ -273,7 +273,7 @@ public void newKSqlApplicationCreation() throws IOException {
         .when(aclsBuilder)
         .buildBindingsForKSqlApp(any(KSqlApp.class), anyString());
 
-    verify(aclsBuilder, times(1)).buildBindingsForKSqlApp(app, "default.default");
+    verify(aclsBuilder, times(1)).buildBindingsForKSqlApp(app, "default.default.");
   }
 
   @Test(expected = IOException.class)

diff --git a/src/test/java/com/purbon/kafka/topology/ConfigurationTest.java b/src/test/java/com/purbon/kafka/topology/ConfigurationTest.java
@@ -288,7 +288,7 @@ public void shouldAddStreamsProjectPrefixAsInternalTopics() {
     var topology = TestTopologyBuilder.createProject().addKStream("foo").buildTopology();
 
     var internals = config.getKafkaInternalTopicPrefixes(Collections.singletonList(topology));
-    assertThat(internals).contains("ctx.project");
+    assertThat(internals).contains("ctx.project.");
     assertThat(internals).contains("_");
   }
 

diff --git a/src/test/java/com/purbon/kafka/topology/TopologyObjectBuilderTest.java b/src/test/java/com/purbon/kafka/topology/TopologyObjectBuilderTest.java
@@ -61,10 +61,10 @@ public void buildOutOfMultipleTopos() throws IOException {
 
     var projects =
         Arrays.asList(
-            "context2.source2.foo.bar.projectFoo",
-            "context2.source2.foo.bar.projectBar",
-            "context2.source2.foo.bar.projectZet",
-            "context2.source2.foo.bar.projectBear");
+            "context2.source2.foo.bar.projectFoo.",
+            "context2.source2.foo.bar.projectBar.",
+            "context2.source2.foo.bar.projectZet.",
+            "context2.source2.foo.bar.projectBear.");
     assertThat(context.getProjects()).hasSize(4);
     for (Project proj : context.getProjects()) {
       assertThat(projects).contains(proj.namePrefix());

diff --git a/src/test/java/com/purbon/kafka/topology/TopologySerdesTest.java b/src/test/java/com/purbon/kafka/topology/TopologySerdesTest.java
@@ -101,7 +101,7 @@ public void testDynamicFirstLevelAttributes() {
     Topology anotherTopology = parser.deserialise(TestUtils.getResourceFile("/descriptor.yaml"));
     Project anotherProject = anotherTopology.getProjects().get(0);
 
-    assertEquals("contextOrg.source.foo", anotherProject.namePrefix());
+    assertEquals("contextOrg.source.foo.", anotherProject.namePrefix());
   }
 
   @Test

diff --git a/src/test/java/com/purbon/kafka/topology/actions/topics/CreateTopicActionTest.java b/src/test/java/com/purbon/kafka/topology/actions/topics/CreateTopicActionTest.java
@@ -2,8 +2,10 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import com.purbon.kafka.topology.TestTopologyBuilder;
 import com.purbon.kafka.topology.api.adminclient.TopologyBuilderAdminClient;
 import com.purbon.kafka.topology.model.Topic;
+import com.purbon.kafka.topology.model.Topology;
 import java.util.Collections;
 import org.junit.Rule;
 import org.junit.Test;
@@ -19,17 +21,22 @@ public class CreateTopicActionTest {
 
   @Test
   public void shouldComposeDetailedViewOfProperties() {
-    Topic topic = new Topic("foo");
-    topic.setProjectPrefix("bar");
-    topic.setConfig(Collections.singletonMap("foo", "bar"));
+    Topic t = new Topic("foo");
+    t.setConfig(Collections.singletonMap("foo", "bar"));
+
+    TestTopologyBuilder builder = TestTopologyBuilder.createProject().addTopic(t);
+
+    Topology topology = builder.buildTopology();
+    var topic = topology.getProjects().get(0).getTopics().get(0);
+
     var action = new CreateTopicAction(adminClient, topic, topic.toString());
     var refs = action.refs();
     assertThat(refs).hasSize(1);
     var ref = refs.get(0);
     assertThat(ref)
         .contains(
-            "\"resource_name\" : \"rn://create.topic/com.purbon.kafka.topology.actions.topics.CreateTopicAction/bar.foo\"");
+            "\"resource_name\" : \"rn://create.topic/com.purbon.kafka.topology.actions.topics.CreateTopicAction/ctx.project.foo\"");
     assertThat(ref).contains("\"foo\" : \"bar\"");
-    assertThat(ref).contains("\"topic\" : \"bar.foo\",");
+    assertThat(ref).contains("\"topic\" : \"ctx.project.foo\",");
   }
 }
diff --git a/src/test/java/com/purbon/kafka/topology/actions/topics/DeleteTopicsActionTest.java b/src/test/java/com/purbon/kafka/topology/actions/topics/DeleteTopicsActionTest.java
@@ -2,8 +2,10 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import com.purbon.kafka.topology.TestTopologyBuilder;
 import com.purbon.kafka.topology.api.adminclient.TopologyBuilderAdminClient;
 import com.purbon.kafka.topology.model.Topic;
+import com.purbon.kafka.topology.model.Topology;
 import java.util.Collections;
 import org.junit.Rule;
 import org.junit.Test;
@@ -19,17 +21,22 @@ public class DeleteTopicsActionTest {
 
   @Test
   public void shouldComposeDetailedViewOfProperties() {
-    Topic topic = new Topic("foo");
-    topic.setProjectPrefix("bar");
-    topic.setConfig(Collections.singletonMap("foo", "bar"));
+    Topic t = new Topic("foo");
+    t.setConfig(Collections.singletonMap("foo", "bar"));
+
+    TestTopologyBuilder builder = TestTopologyBuilder.createProject().addTopic(t);
+
+    Topology topology = builder.buildTopology();
+    var topic = topology.getProjects().get(0).getTopics().get(0);
+
     var action = new DeleteTopics(adminClient, Collections.singletonList(topic.toString()));
 
     var refs = action.refs();
     assertThat(refs).hasSize(1);
     var ref = refs.get(0);
     assertThat(ref)
         .contains(
-            "\"resource_name\" : \"rn://delete.topic/com.purbon.kafka.topology.actions.topics.DeleteTopics$1/bar.foo\"");
-    assertThat(ref).contains("\"topic\" : \"bar.foo\",");
+            "\"resource_name\" : \"rn://delete.topic/com.purbon.kafka.topology.actions.topics.DeleteTopics$1/ctx.project.foo\"");
+    assertThat(ref).contains("\"topic\" : \"ctx.project.foo\",");
   }
 }