Skip to content

Commit

Permalink
update method name to be more generic
Browse files Browse the repository at this point in the history
  • Loading branch information
ThomasCAI-mlv committed Jul 17, 2024
1 parent 4bc35b3 commit 4a90976
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -359,17 +359,18 @@ public Optional<AccessControlEntry> findByName(String namespace, String name) {
return accessControlEntryRepository.findByName(namespace, name);
}


/**
* Check if there is any ACL concerning the given topic.
* Check if there is any ACL concerning the given resource.
*
* @param acls The OWNER ACL list on TOPIC resource
* @param topicName The topic name to check ACL against
* @return true if there is any OWNER ACL concerning the given topic, false otherwise
* @param acls The OWNER ACL list on resource
* @param resourceName The resource name to check ACL against
* @return true if there is any OWNER ACL concerning the given resource, false otherwise
*/
public boolean isAnyAclOfTopic(List<AccessControlEntry> acls, String topicName) {
public boolean isAnyAclOfResource(List<AccessControlEntry> acls, String resourceName) {
return acls.stream().anyMatch(acl -> switch (acl.getSpec().getResourcePatternType()) {
case PREFIXED -> topicName.startsWith(acl.getSpec().getResource());
case LITERAL -> topicName.equals(acl.getSpec().getResource());
case PREFIXED -> resourceName.startsWith(acl.getSpec().getResource());
case LITERAL -> resourceName.equals(acl.getSpec().getResource());
});
}
}
4 changes: 2 additions & 2 deletions src/main/java/com/michelin/ns4kafka/service/TopicService.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public List<Topic> findAllForNamespace(Namespace namespace) {
.findResourceOwnerGrantedToNamespace(namespace, AccessControlEntry.ResourceType.TOPIC);
return topicRepository.findAllForCluster(namespace.getMetadata().getCluster())
.stream()
.filter(topic -> accessControlEntryService.isAnyAclOfTopic(acls, topic.getMetadata().getName()))
.filter(topic -> accessControlEntryService.isAnyAclOfResource(acls, topic.getMetadata().getName()))
.toList();
}

Expand All @@ -87,7 +87,7 @@ public List<Topic> findAllForNamespace(Namespace namespace, TopicSearchParams pa
List<String> nameFilterPatterns = RegexUtils.wildcardStringsToRegexPatterns(params.name());
return topicRepository.findAllForCluster(namespace.getMetadata().getCluster())
.stream()
.filter(topic -> accessControlEntryService.isAnyAclOfTopic(acls, topic.getMetadata().getName())
.filter(topic -> accessControlEntryService.isAnyAclOfResource(acls, topic.getMetadata().getName())
&& nameFilterPatterns.stream()
.anyMatch(pattern -> Pattern.compile(pattern).matcher(topic.getMetadata().getName()).matches()))
.toList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -906,62 +906,62 @@ void shouldNotCollideIfDifferentResource() {
}

@Test
void isPrefixedAclOfTopic() {
void isPrefixedAclOfResource() {
AccessControlEntry ace1 = AccessControlEntry.builder()
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.resourceType(AccessControlEntry.ResourceType.TOPIC)
.resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED)
.permission(AccessControlEntry.Permission.OWNER)
.resource("abc.")
.grantedTo("namespace")
.build())
.build();
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.resourceType(AccessControlEntry.ResourceType.TOPIC)
.resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED)
.permission(AccessControlEntry.Permission.OWNER)
.resource("abc.")
.grantedTo("namespace")
.build())
.build();

AccessControlEntry ace2 = AccessControlEntry.builder()
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.resourceType(AccessControlEntry.ResourceType.CONNECT)
.resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED)
.permission(AccessControlEntry.Permission.OWNER)
.resource("abc_")
.grantedTo("namespace")
.build())
.build();
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.resourceType(AccessControlEntry.ResourceType.CONNECT)
.resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED)
.permission(AccessControlEntry.Permission.OWNER)
.resource("abc_")
.grantedTo("namespace")
.build())
.build();
List<AccessControlEntry> acls = List.of(ace1, ace2);

assertFalse(accessControlEntryService.isAnyAclOfTopic(acls, "xyz.topic1"));
assertFalse(accessControlEntryService.isAnyAclOfTopic(acls, "topic1-abc"));
assertFalse(accessControlEntryService.isAnyAclOfTopic(acls, "abc-topic1"));
assertTrue(accessControlEntryService.isAnyAclOfTopic(acls, "abc.topic1"));
assertTrue(accessControlEntryService.isAnyAclOfTopic(acls, "abc_topic1"));
assertFalse(accessControlEntryService.isAnyAclOfResource(acls, "xyz.topic1"));
assertFalse(accessControlEntryService.isAnyAclOfResource(acls, "topic1-abc"));
assertFalse(accessControlEntryService.isAnyAclOfResource(acls, "abc-topic1"));
assertTrue(accessControlEntryService.isAnyAclOfResource(acls, "abc.topic1"));
assertTrue(accessControlEntryService.isAnyAclOfResource(acls, "abc_topic1"));
}

@Test
void isLiteralAclOfTopic() {
void isLiteralAclOfResource() {
AccessControlEntry ace1 = AccessControlEntry.builder()
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.resourceType(AccessControlEntry.ResourceType.TOPIC)
.resourcePatternType(AccessControlEntry.ResourcePatternType.LITERAL)
.permission(AccessControlEntry.Permission.OWNER)
.resource("abc.topic1")
.grantedTo("namespace")
.build())
.build();
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.resourceType(AccessControlEntry.ResourceType.TOPIC)
.resourcePatternType(AccessControlEntry.ResourcePatternType.LITERAL)
.permission(AccessControlEntry.Permission.OWNER)
.resource("abc.topic1")
.grantedTo("namespace")
.build())
.build();

AccessControlEntry ace2 = AccessControlEntry.builder()
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.resourceType(AccessControlEntry.ResourceType.CONNECT)
.resourcePatternType(AccessControlEntry.ResourcePatternType.LITERAL)
.permission(AccessControlEntry.Permission.OWNER)
.resource("abc-topic1")
.grantedTo("namespace")
.build())
.build();
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.resourceType(AccessControlEntry.ResourceType.CONNECT)
.resourcePatternType(AccessControlEntry.ResourcePatternType.LITERAL)
.permission(AccessControlEntry.Permission.OWNER)
.resource("abc-topic1")
.grantedTo("namespace")
.build())
.build();
List<AccessControlEntry> acls = List.of(ace1, ace2);

assertFalse(accessControlEntryService.isAnyAclOfTopic(acls, "xyz.topic1"));
assertFalse(accessControlEntryService.isAnyAclOfTopic(acls, "abc.topic12"));
assertFalse(accessControlEntryService.isAnyAclOfTopic(acls, "abc_topic1"));
assertTrue(accessControlEntryService.isAnyAclOfTopic(acls, "abc.topic1"));
assertTrue(accessControlEntryService.isAnyAclOfTopic(acls, "abc-topic1"));
assertFalse(accessControlEntryService.isAnyAclOfResource(acls, "xyz.topic1"));
assertFalse(accessControlEntryService.isAnyAclOfResource(acls, "abc.topic12"));
assertFalse(accessControlEntryService.isAnyAclOfResource(acls, "abc_topic1"));
assertTrue(accessControlEntryService.isAnyAclOfResource(acls, "abc.topic1"));
assertTrue(accessControlEntryService.isAnyAclOfResource(acls, "abc-topic1"));
}
}
78 changes: 40 additions & 38 deletions src/test/java/com/michelin/ns4kafka/service/TopicServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ void findByName() {
.build())
.build()
));
when(accessControlEntryService.isAnyAclOfTopic(any(), anyString())).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(any(), anyString())).thenReturn(true);

// search topic by name
Optional<Topic> actualTopicPrefixed = topicService.findByName(ns, "ns-topic1");
Expand Down Expand Up @@ -182,7 +182,7 @@ void findAllForNamespaceNoAcls() {
when(accessControlEntryService.findResourceOwnerGrantedToNamespace(ns, AccessControlEntry.ResourceType.TOPIC))
.thenReturn(List.of());

// list of topics is empty
// list of topics is empty
List<Topic> actual = topicService.findAllForNamespace(ns);
assertTrue(actual.isEmpty());
}
Expand Down Expand Up @@ -237,7 +237,7 @@ void findAllForNamespaceNoAclsOfTopic() {
.build())
.build()));

when(accessControlEntryService.isAnyAclOfTopic(any(), anyString())).thenReturn(false);
when(accessControlEntryService.isAnyAclOfResource(any(), anyString())).thenReturn(false);

// list of topics is empty
List<Topic> actual = topicService.findAllForNamespace(ns);
Expand Down Expand Up @@ -298,11 +298,11 @@ void findAllForNamespace() {
when(accessControlEntryService.findResourceOwnerGrantedToNamespace(ns, AccessControlEntry.ResourceType.TOPIC))
.thenReturn(acls);

when(accessControlEntryService.isAnyAclOfTopic(acls, "ns1-topic1")).thenReturn(false);
when(accessControlEntryService.isAnyAclOfTopic(acls, "ns2-topic1")).thenReturn(false);
when(accessControlEntryService.isAnyAclOfTopic(acls, "ns0-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfTopic(acls, "ns-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfTopic(acls, "ns-topic2")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns1-topic1")).thenReturn(false);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns2-topic1")).thenReturn(false);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns0-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns-topic2")).thenReturn(true);

// search for topics into namespace
List<Topic> actual = topicService.findAllForNamespace(ns);
Expand Down Expand Up @@ -467,10 +467,10 @@ void listUnsynchronizedAllExistingTopics() throws InterruptedException, Executio
when(topicRepository.findAllForCluster("local"))
.thenReturn(List.of(t1, t2, t3, t4));

when(accessControlEntryService.isAnyAclOfTopic(acls, "ns-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfTopic(acls, "ns-topic2")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfTopic(acls, "ns1-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfTopic(acls, "ns2-topic1")).thenReturn(false);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns-topic2")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns1-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns2-topic1")).thenReturn(false);

List<String> actual = topicService.listUnsynchronizedTopicNames(ns);

Expand Down Expand Up @@ -546,7 +546,7 @@ void listUnsynchronizedPartialExistingTopics() throws InterruptedException, Exec
when(topicRepository.findAllForCluster("local"))
.thenReturn(List.of(t1));

when(accessControlEntryService.isAnyAclOfTopic(acls, "ns-topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "ns-topic1")).thenReturn(true);

List<String> actual = topicService.listUnsynchronizedTopicNames(ns);

Expand Down Expand Up @@ -689,37 +689,39 @@ void findTopicWithNameParameter() {
Topic topic1 = Topic.builder().metadata(Metadata.builder().name("prefix.topic1").build()).build();
Topic topic2 = Topic.builder().metadata(Metadata.builder().name("prefix.topic2").build()).build();
Topic topic3 = Topic.builder().metadata(Metadata.builder().name("prefix.topic3").build()).build();
Topic topic4 = Topic.builder().metadata(Metadata.builder().name("prefix2.topic").build()).build();

List<AccessControlEntry> acls = List.of(
AccessControlEntry.builder()
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.permission(AccessControlEntry.Permission.OWNER)
.grantedTo("namespace")
.resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED)
.resourceType(AccessControlEntry.ResourceType.TOPIC)
.resource("prefix.")
.build())
.build()
);

when(accessControlEntryService.findResourceOwnerGrantedToNamespace(ns, AccessControlEntry.ResourceType.TOPIC))
.thenReturn(List.of(
AccessControlEntry.builder()
.spec(AccessControlEntry.AccessControlEntrySpec.builder()
.permission(AccessControlEntry.Permission.OWNER)
.grantedTo("namespace")
.resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED)
.resourceType(AccessControlEntry.ResourceType.TOPIC)
.resource("prefix.")
.build())
.build()
));
when(topicRepository.findAllForCluster("local")).thenReturn(List.of(topic1, topic2, topic3));
when(accessControlEntryService.isAnyAclOfTopic(any(), any())).thenReturn(true);
.thenReturn(acls);
when(topicRepository.findAllForCluster("local")).thenReturn(List.of(topic1, topic2, topic3, topic4));
when(accessControlEntryService.isAnyAclOfResource(acls, "prefix.topic1")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "prefix.topic2")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "prefix.topic3")).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(acls, "prefix2.topic")).thenReturn(false);

TopicSearchParams params1 = TopicSearchParams.builder().name(List.of("prefix.topic2")).build();
List<Topic> list1 = topicService.findAllForNamespace(ns, params1);
assertEquals(List.of(topic2), list1);
assertEquals(List.of(topic2), topicService.findAllForNamespace(ns, params1));

TopicSearchParams params2 = TopicSearchParams.builder().name(List.of("topic2.suffix")).build(); // doesn't exist
List<Topic> list2 = topicService.findAllForNamespace(ns, params2);
assertTrue(list2.isEmpty());
assertTrue(topicService.findAllForNamespace(ns, params2).isEmpty());

TopicSearchParams params3 = TopicSearchParams.builder().name(List.of("prefix2.topic1")).build(); // no acl
List<Topic> list3 = topicService.findAllForNamespace(ns, params3);
assertTrue(list3.isEmpty());
TopicSearchParams params3 = TopicSearchParams.builder().name(List.of("prefix2.topic")).build(); // no acl
assertTrue(topicService.findAllForNamespace(ns, params3).isEmpty());

TopicSearchParams params4 = TopicSearchParams.builder().name(List.of("")).build(); // no acl
List<Topic> list4 = topicService.findAllForNamespace(ns, params4);
assertEquals(List.of(topic1, topic2, topic3), list4);
TopicSearchParams params4 = TopicSearchParams.builder().name(List.of("")).build();
assertEquals(List.of(topic1, topic2, topic3), topicService.findAllForNamespace(ns, params4));
}

@Test
Expand Down Expand Up @@ -770,7 +772,7 @@ void findTopicsWithWildcardNameParameter() {
));
when(topicRepository.findAllForCluster("local"))
.thenReturn(List.of(topic1, topic2, topic3, topic4, topic5, topic6));
when(accessControlEntryService.isAnyAclOfTopic(any(), any())).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(any(), any())).thenReturn(true);

// find one or multiple topics with wildcard
TopicSearchParams params1 = TopicSearchParams.builder().name(List.of("prefix1.*")).build();
Expand Down Expand Up @@ -863,7 +865,7 @@ void findTopicWithMultipleNameParameters() {
));
when(topicRepository.findAllForCluster("local"))
.thenReturn(List.of(topic1, topic2, topic3, topic4, topic5));
when(accessControlEntryService.isAnyAclOfTopic(any(), any())).thenReturn(true);
when(accessControlEntryService.isAnyAclOfResource(any(), any())).thenReturn(true);

TopicSearchParams params1 = TopicSearchParams.builder() // multiple simple parameters - found all topics
.name(List.of("prefix1.topic1", "prefix1.topic2")).build();
Expand Down

0 comments on commit 4a90976

Please sign in to comment.