Skip to content

Commit

Permalink
Address pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@carlesarnal
carlesarnal committed Mar 6, 2025
1 parent 0cfa62a commit 26383ba
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 29 deletions.
13 changes: 4 additions & 9 deletions operator/controller/src/main/java/io/apicurio/registry/operator/Constants.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package io.apicurio.registry.operator;

import io.apicurio.registry.operator.api.v1.ApicurioRegistry3;
import io.fabric8.kubernetes.api.model.HTTPGetActionBuilder;
import io.fabric8.kubernetes.api.model.IntOrString;
import io.fabric8.kubernetes.api.model.Probe;
import io.fabric8.kubernetes.api.model.ProbeBuilder;
import io.fabric8.kubernetes.api.model.Quantity;
Expand All @@ -26,15 +28,8 @@ public class Constants {
public static final Map<String, Quantity> DEFAULT_LIMITS = Map.of("cpu",
new QuantityBuilder().withAmount("1").build(), "memory",
new QuantityBuilder().withAmount("1300").withFormat("Mi").build());
public static final Probe DEFAULT_READINESS_PROBE = new ProbeBuilder().withNewHttpGet()
.withPath("/health/ready").withNewPort().withValue(8080).endPort().endHttpGet()
.withInitialDelaySeconds(15).withTimeoutSeconds(5).withPeriodSeconds(10).withSuccessThreshold(1)
.withFailureThreshold(3).build();

public static final Probe DEFAULT_LIVENESS_PROBE = new ProbeBuilder().withNewHttpGet()
.withPath("/health/live").withNewPort().withValue(8080).endPort().endHttpGet()
.withInitialDelaySeconds(15).withTimeoutSeconds(5).withPeriodSeconds(10).withSuccessThreshold(1)
.withFailureThreshold(3).build();
public static final Probe DEFAULT_READINESS_PROBE = new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/health/ready").withPort(new IntOrString(8080)).withScheme("HTTP").build()).build();
public static final Probe DEFAULT_LIVENESS_PROBE = new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/health/live").withPort(new IntOrString(8080)).withScheme("HTTP").build()).build();

public static final Probe TLS_DEFAULT_READINESS_PROBE = new ProbeBuilder().withNewHttpGet()
.withScheme("HTTPS").withPath("/health/ready").withNewPort().withValue(8443).endPort().endHttpGet()
Expand Down
2 changes: 0 additions & 2 deletions operator/controller/src/main/java/io/apicurio/registry/operator/EnvironmentVariables.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ public class EnvironmentVariables {
public static final String QUARKUS_TLS_KEY_STORE_P12_PASSWORD = "QUARKUS_TLS_KEY_STORE_P12_PASSWORD";
public static final String QUARKUS_TLS_TRUST_STORE_P12_PATH = "QUARKUS_TLS_TRUST_STORE_P12_PATH";
public static final String QUARKUS_TLS_TRUST_STORE_P12_PASSWORD = "QUARKUS_TLS_TRUST_STORE_P12_PASSWORD";
public static final String QUARKUS_OIDC_TLS_TLS_CONFIGURATION_NAME = "QUARKUS_OIDC_TLS_TLS_CONFIGURATION_NAME";

public static final String APICURIO_REST_DELETION_ARTIFACT_VERSION_ENABLED = "APICURIO_REST_DELETION_ARTIFACT-VERSION_ENABLED";
public static final String APICURIO_REST_DELETION_ARTIFACT_ENABLED = "APICURIO_REST_DELETION_ARTIFACT_ENABLED";
public static final String APICURIO_REST_DELETION_GROUP_ENABLED = "APICURIO_REST_DELETION_GROUP_ENABLED";
Expand Down
4 changes: 4 additions & 0 deletions operator/controller/src/main/java/io/apicurio/registry/operator/feat/TLS.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,8 @@ private static Optional<TLSSpec> getTlsSpec(ApicurioRegistry3 primary) {
.map(ApicurioRegistry3Spec::getApp)
.map(AppSpec::getTls);
}

public static boolean insecureRequestsEnabled(TLSSpec tlsSpec) {
return "enabled".equals(tlsSpec.getInsecureRequests());
}
}
8 changes: 5 additions & 3 deletions ...ator/controller/src/main/java/io/apicurio/registry/operator/resource/ResourceFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ public Deployment getDefaultAppDeployment(ApicurioRegistry3 primary) {
ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getApp)
.map(AppSpec::getPodTemplateSpec).orElse(null)); // TODO:

var readinessProbe = new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/health/ready").withPort(new IntOrString(8080)).withScheme("HTTP").build()).build();
var livenessProbe = new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/health/live").withPort(new IntOrString(8080)).withScheme("HTTP").build()).build();
var readinessProbe = DEFAULT_READINESS_PROBE;
var livenessProbe = DEFAULT_LIVENESS_PROBE;
var containerPort = List.of(new ContainerPortBuilder().withName("http").withProtocol("TCP").withContainerPort(8080).build());

Optional<TLSSpec> tlsSpec = ofNullable(primary.getSpec())
.map(ApicurioRegistry3Spec::getApp)
Expand All @@ -68,6 +69,7 @@ public Deployment getDefaultAppDeployment(ApicurioRegistry3 primary) {
if (tlsSpec.isPresent()) {
readinessProbe = TLS_DEFAULT_READINESS_PROBE;
livenessProbe = TLS_DEFAULT_LIVENESS_PROBE;
containerPort = List.of(new ContainerPortBuilder().withName("https").withProtocol("TCP").withContainerPort(8443).build());
}

// Replicas
Expand All @@ -77,7 +79,7 @@ public Deployment getDefaultAppDeployment(ApicurioRegistry3 primary) {
r.getSpec().getTemplate(),
REGISTRY_APP_CONTAINER_NAME,
Configuration.getAppImage(),
List.of(new ContainerPortBuilder().withName("http").withProtocol("TCP").withContainerPort(8080).build()),
containerPort,
readinessProbe,
livenessProbe,
Map.of("cpu", new Quantity("500m"), "memory", new Quantity("512Mi")),
Expand Down
3 changes: 2 additions & 1 deletion ...er/src/main/java/io/apicurio/registry/operator/resource/app/AppNetworkPolicyResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import io.apicurio.registry.operator.api.v1.ApicurioRegistry3;
import io.apicurio.registry.operator.api.v1.ApicurioRegistry3Spec;
import io.apicurio.registry.operator.api.v1.spec.AppSpec;
import io.apicurio.registry.operator.feat.TLS;
import io.apicurio.registry.operator.resource.LabelDiscriminators.AppNetworkPolicyDiscriminator;
import io.fabric8.kubernetes.api.model.IntOrStringBuilder;
import io.fabric8.kubernetes.api.model.networking.v1.NetworkPolicy;
Expand Down Expand Up @@ -50,7 +51,7 @@ protected NetworkPolicy desired(ApicurioRegistry3 primary, Context<ApicurioRegis
var httpPolicy = new io.fabric8.kubernetes.api.model.networking.v1.NetworkPolicyPortBuilder()
.withPort(new IntOrStringBuilder().withValue(8080).build()).build();

if (tls.getInsecureRequests() != null && !tls.getInsecureRequests().equals("enabled")) {
if (!TLS.insecureRequestsEnabled(tls)) {
networkPolicy.getSpec().setIngress(List.of(new NetworkPolicyIngressRuleBuilder()
.withPorts(httpsPolicy)
.build()));
Expand Down
3 changes: 2 additions & 1 deletion ...ntroller/src/main/java/io/apicurio/registry/operator/resource/app/AppServiceResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import io.apicurio.registry.operator.api.v1.ApicurioRegistry3;
import io.apicurio.registry.operator.api.v1.ApicurioRegistry3Spec;
import io.apicurio.registry.operator.api.v1.spec.AppSpec;
import io.apicurio.registry.operator.feat.TLS;
import io.fabric8.kubernetes.api.model.IntOrStringBuilder;
import io.fabric8.kubernetes.api.model.Service;
import io.fabric8.kubernetes.api.model.ServicePortBuilder;
Expand Down Expand Up @@ -48,7 +49,7 @@ protected Service desired(ApicurioRegistry3 primary, Context<ApicurioRegistry3>
.withTargetPort(new IntOrStringBuilder().withValue(8443).build())
.build();

if (tls.getInsecureRequests() != null && tls.getInsecureRequests().equals("enabled")) {
if (!TLS.insecureRequestsEnabled(tls)) {
s.getSpec().setPorts(List.of(httpsPort, httpPort));
}
else {
Expand Down
25 changes: 13 additions & 12 deletions operator/controller/src/test/java/io/apicurio/registry/operator/it/TlsITTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import io.fabric8.kubernetes.api.model.networking.v1.NetworkPolicyIngressRule;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.quarkus.test.junit.QuarkusTest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -68,6 +69,7 @@ void testTLS() {
.withName(registry.getMetadata().getName() + "-app-service").get().getSpec();

assertThat(service.getClusterIP()).isNotBlank();
Assertions.assertEquals(1, service.getPorts().size());
assertThat(service.getPorts().get(0).getPort()).isEqualTo(443);
assertThat(service.getClusterIP()).isNotBlank();
return true;
Expand All @@ -86,9 +88,12 @@ void testTLS() {

// Network Policy
await().ignoreExceptions().until(() -> {
assertThat(client.network().v1().networkPolicies().inNamespace(namespace)
NetworkPolicyIngressRule networkPolicyIngressRule = client.network().v1().networkPolicies().inNamespace(namespace)
.withName("simple-app-networkpolicy").get().getSpec().getIngress()
.get(0).getPorts().get(0).getPort().getIntVal()).isEqualTo(8443);
.get(0);
Assertions.assertEquals(1, networkPolicyIngressRule.getPorts().size());

assertThat(networkPolicyIngressRule.getPorts().get(0).getPort().getIntVal()).isEqualTo(8443);
return true;
});
}
Expand Down Expand Up @@ -134,26 +139,22 @@ void testTLSInsecureTrafficEnabled() {
assertThat(service.getClusterIP()).isNotBlank();
assertThat(service.getPorts().get(0).getPort()).isEqualTo(443);
assertThat(service.getPorts().get(1).getPort()).isEqualTo(8080);

Assertions.assertEquals(2, service.getPorts().size());

assertThat(service.getClusterIP()).isNotBlank();
return true;
});

// Ingresses
await().ignoreExceptions().until(() -> {
assertThat(client.network().v1().ingresses().inNamespace(namespace)
.withName(registry.getMetadata().getName() + "-app-ingress").get().getSpec().getRules()
.get(0).getHost()).isEqualTo(registry.getSpec().getApp().getIngress().getHost());
assertThat(client.network().v1().ingresses().inNamespace(namespace)
.withName(registry.getMetadata().getName() + "-ui-ingress").get().getSpec().getRules()
.get(0).getHost()).isEqualTo(registry.getSpec().getUi().getIngress().getHost());
return true;
});

// Network Policy
await().ignoreExceptions().until(() -> {
NetworkPolicyIngressRule networkPolicyIngressRule = client.network().v1().networkPolicies().inNamespace(namespace)
.withName("simple-app-networkpolicy").get().getSpec().getIngress()
.get(0);

Assertions.assertEquals(2, networkPolicyIngressRule.getPorts().size());

assertThat(networkPolicyIngressRule.getPorts().get(0).getPort().getIntVal()).isEqualTo(8443);
assertThat(networkPolicyIngressRule.getPorts().get(1).getPort().getIntVal()).isEqualTo(8080);
return true;
Expand Down
2 changes: 1 addition & 1 deletion ...tor/controller/src/test/resources/k8s/examples/tls/simple-with_tls.apicurioregistry3.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ spec:
host: simple-ui.apps.cluster.example
env:
- name: REGISTRY_API_URL
value: https://simple-app.apps.cluster.example/apis/registry/v3
value: https://simple-app.apps.cluster.example/apis/registry/v3 # This example is only suitable for in-cluster connections. An external https ingress would need to be created manually for external access.

0 comments on commit 26383ba

Please sign in to comment.