Skip to content

Commit 24b8297

Browse files
scholzjscholzj
authored
Merge commit from fork
Signed-off-by: Jakub Scholz <www@scholzj.com>
1 parent aaa4b41 commit 24b8297

4 files changed

Lines changed: 145 additions & 34 deletions

File tree

‎cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaCluster.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1652,14 +1652,19 @@ public Role generateRole() {
16521652
}
16531653
}
16541654

1655-
List<PolicyRule> rules = List.of(new PolicyRuleBuilder()
1656-
.withApiGroups("")
1657-
.withResources("secrets")
1658-
.withVerbs("get")
1659-
.withResourceNames(certSecretNames.stream().toList())
1660-
.build());
1661-
1662-
return RbacUtils.createRole(componentName, namespace, rules, labels, ownerReference, null);
1655+
if (certSecretNames.isEmpty()) {
1656+
// This should never happen but just in case it does, we throw an error
1657+
throw new RuntimeException("No TLS certificate secrets found for the Kafka cluster.");
1658+
} else {
1659+
List<PolicyRule> rules = List.of(new PolicyRuleBuilder()
1660+
.withApiGroups("")
1661+
.withResources("secrets")
1662+
.withVerbs("get")
1663+
.withResourceNames(certSecretNames.stream().toList())
1664+
.build());
1665+
1666+
return RbacUtils.createRole(componentName, namespace, rules, labels, ownerReference, null);
1667+
}
16631668
}
16641669

16651670
/**

‎cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaConnectCluster.java‎

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,6 @@ private List<VolumeMount> getExternalConfigurationVolumeMounts() {
595595

596596
/**
597597
* Generates the StrimziPodSet for the Kafka cluster.
598-
* enabled.
599598
*
600599
* @param replicas Number of replicas the StrimziPodSet should have. During scale-ups or scale-downs, node
601600
* sets with different numbers of pods are generated.
@@ -909,13 +908,11 @@ public ClusterRoleBinding generateClusterRoleBinding() {
909908
}
910909

911910
/**
912-
* Creates a Role for reading TLS certificate secrets in the same namespace as the resource.
913-
* This is used for loading certificates from secrets directly.
914-
**
915-
* @return role for the Kafka Connect
911+
* @return The list of Secrets the Pods will need access to through Kubernetes API to load their values using
912+
* configuration providers.
916913
*/
917914
@SuppressWarnings("deprecation") // OAuth authentication is deprecated
918-
public Role generateRole() {
915+
private List<String> secretsToAllowAccessTo() {
919916
List<String> certSecretNames = new ArrayList<>();
920917
if (tls != null && tls.getTrustedCertificates() != null && !tls.getTrustedCertificates().isEmpty()) {
921918
certSecretNames.add(KafkaConnectResources.internalTlsTrustedCertsSecretName(cluster));
@@ -930,14 +927,30 @@ public Role generateRole() {
930927
}
931928
}
932929

933-
List<PolicyRule> rules = List.of(new PolicyRuleBuilder()
934-
.withApiGroups("")
935-
.withResources("secrets")
936-
.withVerbs("get")
937-
.withResourceNames(certSecretNames)
938-
.build());
930+
return certSecretNames;
931+
}
932+
933+
/**
934+
* Creates a Role for reading TLS certificate secrets in the same namespace as the resource.
935+
* This is used for loading certificates from secrets directly.
936+
**
937+
* @return role for the Kafka Connect
938+
*/
939+
public Role generateRole() {
940+
List<String> certSecretNames = secretsToAllowAccessTo();
939941

940-
return RbacUtils.createRole(componentName, namespace, rules, labels, ownerReference, null);
942+
if (certSecretNames.isEmpty()) {
943+
return null;
944+
} else {
945+
List<PolicyRule> rules = List.of(new PolicyRuleBuilder()
946+
.withApiGroups("")
947+
.withResources("secrets")
948+
.withVerbs("get")
949+
.withResourceNames(certSecretNames)
950+
.build());
951+
952+
return RbacUtils.createRole(componentName, namespace, rules, labels, ownerReference, null);
953+
}
941954
}
942955

943956
/**
@@ -946,19 +959,23 @@ public Role generateRole() {
946959
* @return Role Binding for the Kafka Connect
947960
*/
948961
public RoleBinding generateRoleBindingForRole() {
949-
Subject subject = new SubjectBuilder()
950-
.withKind("ServiceAccount")
951-
.withName(componentName)
952-
.withNamespace(namespace)
953-
.build();
954-
955-
RoleRef roleRef = new RoleRefBuilder()
956-
.withName(componentName)
957-
.withApiGroup("rbac.authorization.k8s.io")
958-
.withKind("Role")
959-
.build();
960-
961-
return RbacUtils.createRoleBinding(getRoleBindingName(), namespace, roleRef, List.of(subject), labels, ownerReference, null);
962+
if (secretsToAllowAccessTo().isEmpty()) {
963+
return null;
964+
} else {
965+
Subject subject = new SubjectBuilder()
966+
.withKind("ServiceAccount")
967+
.withName(componentName)
968+
.withNamespace(namespace)
969+
.build();
970+
971+
RoleRef roleRef = new RoleRefBuilder()
972+
.withName(componentName)
973+
.withApiGroup("rbac.authorization.k8s.io")
974+
.withKind("Role")
975+
.build();
976+
977+
return RbacUtils.createRoleBinding(getRoleBindingName(), namespace, roleRef, List.of(subject), labels, ownerReference, null);
978+
}
962979
}
963980

964981
/**

‎cluster-operator/src/test/java/io/strimzi/operator/cluster/model/KafkaConnectClusterTest.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
import io.fabric8.kubernetes.api.model.networking.v1.NetworkPolicyIngressRule;
4747
import io.fabric8.kubernetes.api.model.policy.v1.PodDisruptionBudget;
4848
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;
49+
import io.fabric8.kubernetes.api.model.rbac.Role;
50+
import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
4951
import io.strimzi.api.kafka.model.common.CertSecretSource;
5052
import io.strimzi.api.kafka.model.common.CertSecretSourceBuilder;
5153
import io.strimzi.api.kafka.model.common.JvmOptions;
@@ -2378,4 +2380,42 @@ public void testOciConnectorPlugins() {
23782380
assertThat(containers.get(0).getVolumeMounts().get(4).getMountPath(), is("/opt/kafka/plugins/second-connector/695ab9d6"));
23792381
});
23802382
}
2383+
2384+
@Test
2385+
public void testRoleAbdRoleBindingNoSecrets() {
2386+
assertThat(KC.generateRole(), is(nullValue()));
2387+
assertThat(KC.generateRoleBindingForRole(), is(nullValue()));
2388+
}
2389+
2390+
@Test
2391+
public void testRoleAbdRoleBindingWithSecrets() {
2392+
KafkaConnect resource = new KafkaConnectBuilder(RESOURCE)
2393+
.editSpec()
2394+
.withNewTls()
2395+
.withTrustedCertificates(new CertSecretSourceBuilder().withSecretName("my-secret").withCertificate("ca.crt").build())
2396+
.endTls()
2397+
.endSpec()
2398+
.build();
2399+
2400+
KafkaConnectCluster kc = KafkaConnectCluster.fromCrd(Reconciliation.DUMMY_RECONCILIATION, resource, VERSIONS, SHARED_ENV_PROVIDER);
2401+
2402+
Role role = kc.generateRole();
2403+
assertThat(role.getMetadata().getName(), is(kc.componentName));
2404+
assertThat(role.getMetadata().getNamespace(), is(NAMESPACE));
2405+
assertThat(role.getRules().size(), is(1));
2406+
assertThat(role.getRules().get(0).getApiGroups(), is(List.of("")));
2407+
assertThat(role.getRules().get(0).getResources(), is(List.of("secrets")));
2408+
assertThat(role.getRules().get(0).getVerbs(), is(List.of("get")));
2409+
assertThat(role.getRules().get(0).getResourceNames(), is(List.of(kc.componentName + "-tls-trusted-certs")));
2410+
2411+
RoleBinding rb = kc.generateRoleBindingForRole();
2412+
assertThat(rb.getMetadata().getName(), is(KafkaConnectResources.connectRoleBindingName(NAME)));
2413+
assertThat(rb.getMetadata().getNamespace(), is(NAMESPACE));
2414+
assertThat(rb.getSubjects().size(), is(1));
2415+
assertThat(rb.getSubjects().get(0).getKind(), is("ServiceAccount"));
2416+
assertThat(rb.getSubjects().get(0).getNamespace(), is(NAMESPACE));
2417+
assertThat(rb.getSubjects().get(0).getName(), is(kc.componentName));
2418+
assertThat(rb.getRoleRef().getKind(), is("Role"));
2419+
assertThat(rb.getRoleRef().getName(), is(kc.componentName));
2420+
}
23812421
}

‎cluster-operator/src/test/java/io/strimzi/operator/cluster/model/KafkaMirrorMaker2ClusterTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
import io.fabric8.kubernetes.api.model.networking.v1.NetworkPolicyIngressRule;
4343
import io.fabric8.kubernetes.api.model.policy.v1.PodDisruptionBudget;
4444
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;
45+
import io.fabric8.kubernetes.api.model.rbac.Role;
46+
import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
4547
import io.strimzi.api.kafka.model.common.CertSecretSource;
4648
import io.strimzi.api.kafka.model.common.CertSecretSourceBuilder;
4749
import io.strimzi.api.kafka.model.common.JvmOptions;
@@ -2498,4 +2500,51 @@ public void testLoggingWithLog4j2() {
24982500
ConfigMap cm = KMM2.generateConnectConfigMap(new MetricsAndLogging(METRICS_CONFIG, null));
24992501
assertThat(cm.getData().get(LoggingModel.LOG4J2_CONFIG_MAP_KEY), is(notNullValue()));
25002502
}
2503+
2504+
@Test
2505+
public void testRoleAbdRoleBindingNoSecrets() {
2506+
assertThat(KMM2.generateRole(), is(nullValue()));
2507+
assertThat(KMM2.generateRoleBindingForRole(), is(nullValue()));
2508+
}
2509+
2510+
@Test
2511+
public void testRoleAbdRoleBindingWithSecrets() {
2512+
KafkaMirrorMaker2 resource = new KafkaMirrorMaker2Builder(RESOURCE)
2513+
.editSpec()
2514+
.editTarget()
2515+
.withNewTls()
2516+
.withTrustedCertificates(new CertSecretSourceBuilder().withSecretName("my-secret").withCertificate("ca.crt").build())
2517+
.endTls()
2518+
.endTarget()
2519+
.editFirstMirror()
2520+
.editSource()
2521+
.withNewTls()
2522+
.withTrustedCertificates(new CertSecretSourceBuilder().withSecretName("my-source-secret").withCertificate("ca.crt").build())
2523+
.endTls()
2524+
.endSource()
2525+
.endMirror()
2526+
.endSpec()
2527+
.build();
2528+
2529+
KafkaMirrorMaker2Cluster kmm2 = KafkaMirrorMaker2Cluster.fromCrd(Reconciliation.DUMMY_RECONCILIATION, resource, VERSIONS, SHARED_ENV_PROVIDER);
2530+
2531+
Role role = kmm2.generateRole();
2532+
assertThat(role.getMetadata().getName(), is(kmm2.componentName));
2533+
assertThat(role.getMetadata().getNamespace(), is(NAMESPACE));
2534+
assertThat(role.getRules().size(), is(1));
2535+
assertThat(role.getRules().get(0).getApiGroups(), is(List.of("")));
2536+
assertThat(role.getRules().get(0).getResources(), is(List.of("secrets")));
2537+
assertThat(role.getRules().get(0).getVerbs(), is(List.of("get")));
2538+
assertThat(role.getRules().get(0).getResourceNames(), is(List.of(KafkaConnectResources.componentName(NAME) + "-tls-trusted-certs")));
2539+
2540+
RoleBinding rb = kmm2.generateRoleBindingForRole();
2541+
assertThat(rb.getMetadata().getName(), is(KafkaMirrorMaker2Resources.mm2RoleBindingName(NAME)));
2542+
assertThat(rb.getMetadata().getNamespace(), is(NAMESPACE));
2543+
assertThat(rb.getSubjects().size(), is(1));
2544+
assertThat(rb.getSubjects().get(0).getKind(), is("ServiceAccount"));
2545+
assertThat(rb.getSubjects().get(0).getNamespace(), is(NAMESPACE));
2546+
assertThat(rb.getSubjects().get(0).getName(), is(kmm2.componentName));
2547+
assertThat(rb.getRoleRef().getKind(), is("Role"));
2548+
assertThat(rb.getRoleRef().getName(), is(kmm2.componentName));
2549+
}
25012550
}

0 commit comments

Comments
 (0)