From 2242c4df73e95791aaca1539fcbda76eaae86d6f Mon Sep 17 00:00:00 2001 From: Eric Wittmann Date: Wed, 29 Jan 2025 14:07:13 -0500 Subject: [PATCH] Apply PR feedback --- .../resource/ActivationConditions.java | 10 ++-- .../operator/it/NetworkPolicyITTest.java | 53 +++++++++++-------- operator/install/install.yaml | 49 +++++++++++------ .../operator/api/v1/spec/ComponentSpec.java | 25 ++++++--- .../api/v1/spec/NetworkPolicySpec.java | 46 ++++++++++++++++ 5 files changed, 135 insertions(+), 48 deletions(-) create mode 100644 operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/NetworkPolicySpec.java diff --git a/operator/controller/src/main/java/io/apicurio/registry/operator/resource/ActivationConditions.java b/operator/controller/src/main/java/io/apicurio/registry/operator/resource/ActivationConditions.java index bef9b18bd2..81469f2851 100644 --- a/operator/controller/src/main/java/io/apicurio/registry/operator/resource/ActivationConditions.java +++ b/operator/controller/src/main/java/io/apicurio/registry/operator/resource/ActivationConditions.java @@ -5,6 +5,7 @@ import io.apicurio.registry.operator.api.v1.spec.AppSpec; import io.apicurio.registry.operator.api.v1.spec.ComponentSpec; import io.apicurio.registry.operator.api.v1.spec.IngressSpec; +import io.apicurio.registry.operator.api.v1.spec.NetworkPolicySpec; import io.apicurio.registry.operator.api.v1.spec.StudioUiSpec; import io.apicurio.registry.operator.api.v1.spec.UiSpec; import io.apicurio.registry.operator.resource.app.AppIngressResource; @@ -53,7 +54,8 @@ public static class AppNetworkPolicyActivationCondition public boolean isMet(DependentResource resource, ApicurioRegistry3 primary, Context context) { Boolean isManaged = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getApp) - .map(ComponentSpec::getManageNetworkPolicy).orElse(Boolean.TRUE); + .map(ComponentSpec::getNetworkPolicy).map(NetworkPolicySpec::getEnabled) + .orElse(Boolean.TRUE); if (!isManaged) { ((AppNetworkPolicyResource) resource).delete(primary, context); } @@ -85,7 +87,8 @@ public static class UINetworkPolicyActivationCondition public boolean isMet(DependentResource resource, ApicurioRegistry3 primary, Context context) { Boolean isManaged = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getUi) - .map(ComponentSpec::getManageNetworkPolicy).orElse(Boolean.TRUE); + .map(ComponentSpec::getNetworkPolicy).map(NetworkPolicySpec::getEnabled) + .orElse(Boolean.TRUE); if (!isManaged) { ((UINetworkPolicyResource) resource).delete(primary, context); } @@ -133,7 +136,8 @@ public static class StudioUINetworkPolicyActivationCondition public boolean isMet(DependentResource resource, ApicurioRegistry3 primary, Context context) { Boolean isManaged = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getStudioUi) - .map(ComponentSpec::getManageNetworkPolicy).orElse(Boolean.TRUE); + .map(ComponentSpec::getNetworkPolicy).map(NetworkPolicySpec::getEnabled) + .orElse(Boolean.TRUE); if (!isManaged) { ((StudioUINetworkPolicyResource) resource).delete(primary, context); } diff --git a/operator/controller/src/test/java/io/apicurio/registry/operator/it/NetworkPolicyITTest.java b/operator/controller/src/test/java/io/apicurio/registry/operator/it/NetworkPolicyITTest.java index 435f856e97..8253b1a79b 100644 --- a/operator/controller/src/test/java/io/apicurio/registry/operator/it/NetworkPolicyITTest.java +++ b/operator/controller/src/test/java/io/apicurio/registry/operator/it/NetworkPolicyITTest.java @@ -9,6 +9,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Map; import java.util.stream.Collectors; @QuarkusTest @@ -18,8 +19,8 @@ public class NetworkPolicyITTest extends ITBase { @Test void testNetworkPolicy() { - ApicurioRegistry3 registry = ResourceFactory - .deserialize("/k8s/examples/simple.apicurioregistry3.yaml", ApicurioRegistry3.class); + ApicurioRegistry3 registry = ResourceFactory.deserialize( + "/k8s/examples/simple-with-studio.apicurioregistry3.yaml", ApicurioRegistry3.class); client.resource(registry).create(); // Wait for the deployment to exist @@ -28,29 +29,37 @@ void testNetworkPolicy() { // Check that the two expected NetworkPolicy resources were created NetworkPolicy appPolicy = checkNetworkPolicyExists(registry, ResourceFactory.COMPONENT_APP); NetworkPolicy uiNetworkPolicy = checkNetworkPolicyExists(registry, ResourceFactory.COMPONENT_UI); + NetworkPolicy studioNetworkPolicy = checkNetworkPolicyExists(registry, + ResourceFactory.COMPONENT_STUDIO_UI); // Verify the content of the app component's network policy - Assertions - .assertThat(appPolicy.getMetadata().getLabels().entrySet().stream() - .map(l -> l.getKey() + "=" + l.getValue()).collect(Collectors.toSet())) - .contains("app.kubernetes.io/component=app", - "app.kubernetes.io/managed-by=apicurio-registry-operator", - "app.kubernetes.io/name=apicurio-registry"); - Assertions - .assertThat(appPolicy.getSpec().getPodSelector().getMatchLabels().entrySet().stream() - .map(l -> l.getKey() + "=" + l.getValue()).collect(Collectors.toSet())) - .contains("app.kubernetes.io/component=app", "app.kubernetes.io/name=apicurio-registry"); + assertLabelsContains(appPolicy.getMetadata().getLabels(), "app.kubernetes.io/component=app", + "app.kubernetes.io/managed-by=apicurio-registry-operator", + "app.kubernetes.io/name=apicurio-registry"); + assertLabelsContains(appPolicy.getSpec().getPodSelector().getMatchLabels(), + "app.kubernetes.io/component=app", "app.kubernetes.io/name=apicurio-registry", + "app.kubernetes.io/instance=" + registry.getMetadata().getName()); // Verify the content of the ui component's network policy - Assertions - .assertThat(uiNetworkPolicy.getMetadata().getLabels().entrySet().stream() - .map(l -> l.getKey() + "=" + l.getValue()).collect(Collectors.toSet())) - .contains("app.kubernetes.io/component=ui", - "app.kubernetes.io/managed-by=apicurio-registry-operator", - "app.kubernetes.io/name=apicurio-registry"); - Assertions - .assertThat(uiNetworkPolicy.getSpec().getPodSelector().getMatchLabels().entrySet().stream() - .map(l -> l.getKey() + "=" + l.getValue()).collect(Collectors.toSet())) - .contains("app.kubernetes.io/component=ui", "app.kubernetes.io/name=apicurio-registry"); + assertLabelsContains(uiNetworkPolicy.getMetadata().getLabels(), "app.kubernetes.io/component=ui", + "app.kubernetes.io/managed-by=apicurio-registry-operator", + "app.kubernetes.io/name=apicurio-registry"); + assertLabelsContains(uiNetworkPolicy.getSpec().getPodSelector().getMatchLabels(), + "app.kubernetes.io/component=ui", "app.kubernetes.io/name=apicurio-registry", + "app.kubernetes.io/instance=" + registry.getMetadata().getName()); + + // Verify the content of the studio component's network policy + assertLabelsContains(studioNetworkPolicy.getMetadata().getLabels(), + "app.kubernetes.io/component=studio-ui", + "app.kubernetes.io/managed-by=apicurio-registry-operator", + "app.kubernetes.io/name=apicurio-registry"); + assertLabelsContains(studioNetworkPolicy.getSpec().getPodSelector().getMatchLabels(), + "app.kubernetes.io/component=studio-ui", "app.kubernetes.io/name=apicurio-registry", + "app.kubernetes.io/instance=" + registry.getMetadata().getName()); + } + + private void assertLabelsContains(Map labels, String... values) { + Assertions.assertThat(labels.entrySet().stream().map(l -> l.getKey() + "=" + l.getValue()) + .collect(Collectors.toSet())).contains(values); } } \ No newline at end of file diff --git a/operator/install/install.yaml b/operator/install/install.yaml index 5aa89e45ed..020b715941 100644 --- a/operator/install/install.yaml +++ b/operator/install/install.yaml @@ -117,11 +117,17 @@ spec: automatically.' type: string type: object - manageNetworkPolicy: - description: | - Whether a NetworkPolicy should be managed by the operator. Defaults to 'true'. - Set this to 'false' if you want to create your own custom NetworkPolicy. - type: boolean + networkPolicy: + description: |2 + Configuration of a NetworkPolicy for the component. + properties: + enabled: + description: | + Whether a NetworkPolicy should be managed by the operator. Defaults to 'true'. + + Set this to 'false' if you want to create your own custom NetworkPolicy. + type: boolean + type: object podTemplateSpec: description: |- `PodTemplateSpec` describes the data a pod should have when created from a template. @@ -3259,11 +3265,17 @@ spec: IMPORTANT: If the Ingress already exists and the value becomes empty, the Ingress will be deleted. type: string type: object - manageNetworkPolicy: - description: | - Whether a NetworkPolicy should be managed by the operator. Defaults to 'true'. - Set this to 'false' if you want to create your own custom NetworkPolicy. - type: boolean + networkPolicy: + description: |2 + Configuration of a NetworkPolicy for the component. + properties: + enabled: + description: | + Whether a NetworkPolicy should be managed by the operator. Defaults to 'true'. + + Set this to 'false' if you want to create your own custom NetworkPolicy. + type: boolean + type: object podTemplateSpec: description: |- `PodTemplateSpec` describes the data a pod should have when created from a template. @@ -6253,11 +6265,17 @@ spec: IMPORTANT: If the Ingress already exists and the value becomes empty, the Ingress will be deleted. type: string type: object - manageNetworkPolicy: - description: | - Whether a NetworkPolicy should be managed by the operator. Defaults to 'true'. - Set this to 'false' if you want to create your own custom NetworkPolicy. - type: boolean + networkPolicy: + description: |2 + Configuration of a NetworkPolicy for the component. + properties: + enabled: + description: | + Whether a NetworkPolicy should be managed by the operator. Defaults to 'true'. + + Set this to 'false' if you want to create your own custom NetworkPolicy. + type: boolean + type: object podTemplateSpec: description: |- `PodTemplateSpec` describes the data a pod should have when created from a template. @@ -9312,6 +9330,7 @@ rules: - networking.k8s.io resources: - ingresses + - networkpolicies verbs: - '*' --- diff --git a/operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/ComponentSpec.java b/operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/ComponentSpec.java index 4ce20c0fc4..53514581c8 100644 --- a/operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/ComponentSpec.java +++ b/operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/ComponentSpec.java @@ -1,12 +1,22 @@ package io.apicurio.registry.operator.api.v1.spec; -import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyDescription; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; import com.fasterxml.jackson.databind.JsonDeserializer.None; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import io.apicurio.registry.operator.api.v1.ContainerNames; import io.fabric8.kubernetes.api.model.EnvVar; import io.fabric8.kubernetes.api.model.PodTemplateSpec; -import lombok.*; +import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.ToString; import lombok.experimental.SuperBuilder; import java.util.ArrayList; @@ -19,7 +29,7 @@ @JsonDeserialize(using = None.class) @JsonInclude(NON_NULL) -@JsonPropertyOrder({ "env", "ingress", "host", "podTemplateSpec", "manageNetworkPolicy" }) +@JsonPropertyOrder({ "env", "ingress", "host", "podTemplateSpec", "networkPolicy" }) @NoArgsConstructor(access = PROTECTED) @AllArgsConstructor(access = PROTECTED) @SuperBuilder(toBuilder = true) @@ -93,15 +103,14 @@ public abstract class ComponentSpec { private Integer replicas; /** - * Indicates whether to create a pod disruption budget + * Network policy config */ - @JsonProperty("manageNetworkPolicy") + @JsonProperty("networkPolicy") @JsonPropertyDescription(""" - Whether a NetworkPolicy should be managed by the operator. Defaults to 'true'. - Set this to 'false' if you want to create your own custom NetworkPolicy. + Configuration of a NetworkPolicy for the component. """) @JsonSetter(nulls = Nulls.SKIP) - private Boolean manageNetworkPolicy; + private NetworkPolicySpec networkPolicy; public IngressSpec withIngress() { if (ingress == null) { diff --git a/operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/NetworkPolicySpec.java b/operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/NetworkPolicySpec.java new file mode 100644 index 0000000000..e2ac24f00f --- /dev/null +++ b/operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/NetworkPolicySpec.java @@ -0,0 +1,46 @@ +package io.apicurio.registry.operator.api.v1.spec; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyDescription; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; +import com.fasterxml.jackson.databind.JsonDeserializer.None; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.ToString; +import lombok.experimental.SuperBuilder; + +import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL; +import static lombok.AccessLevel.PRIVATE; + +@JsonDeserialize(using = None.class) +@JsonInclude(NON_NULL) +@JsonPropertyOrder({ "enabled" }) +@NoArgsConstructor +@AllArgsConstructor(access = PRIVATE) +@SuperBuilder(toBuilder = true) +@Getter +@Setter +@EqualsAndHashCode +@ToString +public class NetworkPolicySpec { + + /** + * Indicates whether to create and manage a network policy + */ + @JsonProperty("enabled") + @JsonPropertyDescription(""" + Whether a NetworkPolicy should be managed by the operator. Defaults to 'true'. + + Set this to 'false' if you want to create your own custom NetworkPolicy. + """) + @JsonSetter(nulls = Nulls.SKIP) + private Boolean enabled; + +} \ No newline at end of file