Skip to content

Commit ca3931f

Browse files
jsenkojsenko
committed
refactor(operator): code improvements from PR review
1 parent 81c2b3c commit ca3931f

File tree

5 files changed

+50
-51
lines changed

5 files changed

+50
-51
lines changed

‎operator/controller/src/main/java/io/apicurio/registry/operator/EnvironmentVariables.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,12 @@ public class EnvironmentVariables {
1212

1313
public static final String APICURIO_REST_MUTABILITY_ARTIFACT_VERSION_CONTENT_ENABLED = "APICURIO_REST_MUTABILITY_ARTIFACT-VERSION-CONTENT_ENABLED";
1414

15+
private static final String KAFKA_PREFIX = "APICURIO_KAFKA_COMMON_";
16+
public static final String KAFKASQL_SECURITY_PROTOCOL = KAFKA_PREFIX + "SECURITY_PROTOCOL";
17+
public static final String KAFKASQL_SSL_KEYSTORE_TYPE = KAFKA_PREFIX + "SSL_KEYSTORE_TYPE";
18+
public static final String KAFKASQL_SSL_KEYSTORE_LOCATION = KAFKA_PREFIX + "SSL_KEYSTORE_LOCATION";
19+
public static final String KAFKASQL_SSL_KEYSTORE_PASSWORD = KAFKA_PREFIX + "SSL_KEYSTORE_PASSWORD";
20+
public static final String KAFKASQL_SSL_TRUSTSTORE_TYPE = KAFKA_PREFIX + "SSL_TRUSTSTORE_TYPE";
21+
public static final String KAFKASQL_SSL_TRUSTSTORE_LOCATION = KAFKA_PREFIX + "SSL_TRUSTSTORE_LOCATION";
22+
public static final String KAFKASQL_SSL_TRUSTSTORE_PASSWORD = KAFKA_PREFIX + "SSL_TRUSTSTORE_PASSWORD";
1523
}

‎operator/controller/src/main/java/io/apicurio/registry/operator/feat/KafkaSqlTLS.java‎

Lines changed: 24 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -11,87 +11,70 @@
1111
import io.fabric8.kubernetes.api.model.apps.Deployment;
1212

1313
import java.util.Map;
14+
import java.util.Optional;
1415

16+
import static io.apicurio.registry.operator.EnvironmentVariables.*;
1517
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.addEnvVar;
1618
import static java.util.Optional.ofNullable;
1719

1820
public class KafkaSqlTLS {
1921

20-
public static final String ENV_KAFKASQL_SECURITY_PROTOCOL = "APICURIO_KAFKA_COMMON_SECURITY_PROTOCOL";
21-
22-
public static final String ENV_KAFKASQL_SSL_KEYSTORE_TYPE = "APICURIO_KAFKA_COMMON_SSL_KEYSTORE_TYPE";
23-
public static final String ENV_KAFKASQL_SSL_KEYSTORE_LOCATION = "APICURIO_KAFKA_COMMON_SSL_KEYSTORE_LOCATION";
24-
public static final String ENV_KAFKASQL_SSL_KEYSTORE_PASSWORD = "APICURIO_KAFKA_COMMON_SSL_KEYSTORE_PASSWORD";
25-
26-
public static final String ENV_KAFKASQL_SSL_TRUSTSTORE_TYPE = "APICURIO_KAFKA_COMMON_SSL_TRUSTSTORE_TYPE";
27-
public static final String ENV_KAFKASQL_SSL_TRUSTSTORE_LOCATION = "APICURIO_KAFKA_COMMON_SSL_TRUSTSTORE_LOCATION";
28-
public static final String ENV_KAFKASQL_SSL_TRUSTSTORE_PASSWORD = "APICURIO_KAFKA_COMMON_SSL_TRUSTSTORE_PASSWORD";
29-
3022
/**
3123
* Plain KafkaSQL must be already configured.
3224
*/
3325
public static boolean configureKafkaSQLTLS(ApicurioRegistry3 primary, Deployment deployment,
3426
String containerName, Map<String, EnvVar> env) {
3527

3628
// spotless:off
37-
var keystore = new SecretKeyRefTool(ofNullable(primary)
38-
.map(ApicurioRegistry3::getSpec)
39-
.map(ApicurioRegistry3Spec::getApp)
40-
.map(AppSpec::getStorage)
41-
.map(StorageSpec::getKafkasql)
42-
.map(KafkaSqlSpec::getTls)
29+
var keystore = new SecretKeyRefTool(getKafkaSqlTLSSpec(primary)
4330
.map(KafkaSqlTLSSpec::getKeystoreSecretRef)
4431
.orElse(null), "user.p12");
4532

46-
var keystorePassword = new SecretKeyRefTool(ofNullable(primary)
47-
.map(ApicurioRegistry3::getSpec)
48-
.map(ApicurioRegistry3Spec::getApp)
49-
.map(AppSpec::getStorage)
50-
.map(StorageSpec::getKafkasql)
51-
.map(KafkaSqlSpec::getTls)
33+
var keystorePassword = new SecretKeyRefTool(getKafkaSqlTLSSpec(primary)
5234
.map(KafkaSqlTLSSpec::getKeystorePasswordSecretRef)
5335
.orElse(null), "user.password");
5436

55-
var truststore = new SecretKeyRefTool(ofNullable(primary)
56-
.map(ApicurioRegistry3::getSpec)
57-
.map(ApicurioRegistry3Spec::getApp)
58-
.map(AppSpec::getStorage)
59-
.map(StorageSpec::getKafkasql)
60-
.map(KafkaSqlSpec::getTls)
37+
var truststore = new SecretKeyRefTool(getKafkaSqlTLSSpec(primary)
6138
.map(KafkaSqlTLSSpec::getTruststoreSecretRef)
6239
.orElse(null), "ca.p12");
6340

64-
var truststorePassword = new SecretKeyRefTool(ofNullable(primary)
65-
.map(ApicurioRegistry3::getSpec)
66-
.map(ApicurioRegistry3Spec::getApp)
67-
.map(AppSpec::getStorage)
68-
.map(StorageSpec::getKafkasql)
69-
.map(KafkaSqlSpec::getTls)
41+
var truststorePassword = new SecretKeyRefTool(getKafkaSqlTLSSpec(primary)
7042
.map(KafkaSqlTLSSpec::getTruststorePasswordSecretRef)
7143
.orElse(null), "ca.password");
7244
// spotless:on
7345

7446
if (truststore.isValid() && truststorePassword.isValid() && keystore.isValid()
7547
&& keystorePassword.isValid()) {
7648

77-
addEnvVar(env, ENV_KAFKASQL_SECURITY_PROTOCOL, "SSL");
49+
addEnvVar(env, KAFKASQL_SECURITY_PROTOCOL, "SSL");
7850

7951
// ===== Keystore
8052

81-
addEnvVar(env, ENV_KAFKASQL_SSL_KEYSTORE_TYPE, "PKCS12");
53+
addEnvVar(env, KAFKASQL_SSL_KEYSTORE_TYPE, "PKCS12");
8254
keystore.applySecretVolume(deployment, containerName);
83-
addEnvVar(env, ENV_KAFKASQL_SSL_KEYSTORE_LOCATION, keystore.getSecretVolumeKeyPath());
84-
keystorePassword.applySecretEnvVar(env, ENV_KAFKASQL_SSL_KEYSTORE_PASSWORD);
55+
addEnvVar(env, KAFKASQL_SSL_KEYSTORE_LOCATION, keystore.getSecretVolumeKeyPath());
56+
keystorePassword.applySecretEnvVar(env, KAFKASQL_SSL_KEYSTORE_PASSWORD);
8557

8658
// ===== Truststore
8759

88-
addEnvVar(env, ENV_KAFKASQL_SSL_TRUSTSTORE_TYPE, "PKCS12");
60+
addEnvVar(env, KAFKASQL_SSL_TRUSTSTORE_TYPE, "PKCS12");
8961
truststore.applySecretVolume(deployment, containerName);
90-
addEnvVar(env, ENV_KAFKASQL_SSL_TRUSTSTORE_LOCATION, truststore.getSecretVolumeKeyPath());
91-
truststorePassword.applySecretEnvVar(env, ENV_KAFKASQL_SSL_TRUSTSTORE_PASSWORD);
62+
addEnvVar(env, KAFKASQL_SSL_TRUSTSTORE_LOCATION, truststore.getSecretVolumeKeyPath());
63+
truststorePassword.applySecretEnvVar(env, KAFKASQL_SSL_TRUSTSTORE_PASSWORD);
9264

9365
return true;
9466
}
9567
return false;
9668
}
69+
70+
private static Optional<KafkaSqlTLSSpec> getKafkaSqlTLSSpec(ApicurioRegistry3 primary) {
71+
// spotless:off
72+
return ofNullable(primary)
73+
.map(ApicurioRegistry3::getSpec)
74+
.map(ApicurioRegistry3Spec::getApp)
75+
.map(AppSpec::getStorage)
76+
.map(StorageSpec::getKafkasql)
77+
.map(KafkaSqlSpec::getTls);
78+
// spotless:on
79+
}
9780
}

‎operator/controller/src/main/java/io/apicurio/registry/operator/resource/app/AppDeploymentResource.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public AppDeploymentResource() {
5252
@Override
5353
protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry3> context) {
5454

55-
var d = APP_DEPLOYMENT_KEY.getFactory().apply(primary);
55+
var deployment = APP_DEPLOYMENT_KEY.getFactory().apply(primary);
5656

5757
var envVars = new LinkedHashMap<String, EnvVar>();
5858
ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getApp).map(AppSpec::getEnv)
@@ -93,16 +93,16 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry
9393
.map(StorageSpec::getType).ifPresent(storageType -> {
9494
switch (storageType) {
9595
case POSTGRESQL -> PostgresSql.configureDatasource(primary, envVars);
96-
case KAFKASQL -> KafkaSql.configureKafkaSQL(primary, d, envVars);
96+
case KAFKASQL -> KafkaSql.configureKafkaSQL(primary, deployment, envVars);
9797
}
9898
});
9999

100100
// Set the ENV VARs on the deployment's container spec.
101-
var container = getContainerFromDeployment(d, REGISTRY_APP_CONTAINER_NAME);
101+
var container = getContainerFromDeployment(deployment, REGISTRY_APP_CONTAINER_NAME);
102102
container.setEnv(envVars.values().stream().toList());
103103

104-
log.debug("Desired {} is {}", APP_DEPLOYMENT_KEY.getId(), toYAML(d));
105-
return d;
104+
log.debug("Desired {} is {}", APP_DEPLOYMENT_KEY.getId(), toYAML(deployment));
105+
return deployment;
106106
}
107107

108108
public static void addEnvVar(Map<String, EnvVar> map, EnvVar envVar) {

‎operator/controller/src/main/java/io/apicurio/registry/operator/utils/SecretKeyRefTool.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
1313
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.getContainerFromDeployment;
1414
import static io.apicurio.registry.operator.utils.Utils.isBlank;
1515

16+
/**
17+
* This is a utility wrapper around {@link SecretKeyRef}, that helps with using the Secret reference in the
18+
* target Deployment. Usually, a secret value is either provided in an env. variable or accessed as a file.
19+
* This class helps with both use cases.
20+
*/
1621
public class SecretKeyRefTool {
1722

1823
private SecretKeyRef secretKeyRef;
@@ -54,11 +59,17 @@ private String getSecretVolumeMountPath() {
5459
return "/etc/" + getSecretVolumeName();
5560
}
5661

62+
/**
63+
* @return a path to a file mounted within the container that contains the value from the
64+
* {@link SecretKeyRef}
65+
*/
5766
public String getSecretVolumeKeyPath() {
5867
return getSecretVolumeMountPath() + "/" + secretKeyRef.getKey();
5968
}
6069

6170
/**
71+
* Mount the Secret as a volume and configure its mount inside the container.
72+
* <p>
6273
* Use this method in case the {@link SecretKeyRef} references a file to be mounted into the pod. You can
6374
* then use {@link #getSecretVolumeKeyPath()} to get the path to the file within the pod.
6475
*/
@@ -69,6 +80,8 @@ public void applySecretVolume(Deployment deployment, String containerName) {
6980
}
7081

7182
/**
83+
* Add an env. variable that references a value from the Secret.
84+
* <p>
7285
* Use this method in case the {@link SecretKeyRef} references data to be provided as an env. variable.
7386
*/
7487
public void applySecretEnvVar(Map<String, EnvVar> env, String envVarName) {

‎operator/controller/src/test/java/io/apicurio/registry/operator/it/KafkaSqlITTest.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ void testKafkaSQLPlain() {
3636
client.load(KafkaSqlITTest.class
3737
.getResourceAsStream("/k8s/examples/kafkasql/plain/example-cluster.kafka.yaml")).create();
3838
final var clusterName = "example-cluster";
39-
// client.load(getClass().getResourceAsStream("/k8s/examples/kafkasql/plain/example-cluster.kafka.yaml"))
40-
// .createOrReplace();
41-
// final var clusterName = "example-cluster";
4239

4340
await().ignoreExceptions().untilAsserted(() ->
4441
// Strimzi uses StrimziPodSet instead of ReplicaSet, so we have to check pods
@@ -52,8 +49,6 @@ void testKafkaSQLPlain() {
5249
var registry = deserialize(
5350
"k8s/examples/kafkasql/plain/example-kafkasql-plain.apicurioregistry3.yaml",
5451
ApicurioRegistry3.class);
55-
// var registry = deserialize("k8s/examples/kafkasql/plain/kafkasql-plain.apicurioregistry3.yaml",
56-
// ApicurioRegistry3.class);
5752
registry.getMetadata().setNamespace(namespace);
5853
registry.getSpec().getApp().getStorage().getKafkasql().setBootstrapServers(bootstrapServers);
5954

0 commit comments

Comments
 (0)