Apply PR feedback

Apicurio · Jan 29, 2025 · 2242c4d · 2242c4d
1 parent d52e4c2
commit 2242c4d
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 48 deletions.
diff --git a/...controller/src/main/java/io/apicurio/registry/operator/resource/ActivationConditions.java b/...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<NetworkPolicy, ApicurioRegistry3> resource,
                 ApicurioRegistry3 primary, Context<ApicurioRegistry3> 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<NetworkPolicy, ApicurioRegistry3> resource,
                 ApicurioRegistry3 primary, Context<ApicurioRegistry3> 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<NetworkPolicy, ApicurioRegistry3> resource,
                 ApicurioRegistry3 primary, Context<ApicurioRegistry3> 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
@@ -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<String, String> labels, String... values) {
+        Assertions.assertThat(labels.entrySet().stream().map(l -> l.getKey() + "=" + l.getValue())
+                .collect(Collectors.toSet())).contains(values);
     }
 }
diff --git 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
@@ -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/...ator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/NetworkPolicySpec.java b/...ator/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;
+
+}