fix: OpenShift route generated targetPort doesn't use the service "ta…

…rgetPort" #2847 (#2850)
eclipse-jkube · Mar 27, 2024 · 184feb6 · 184feb6
1 parent a3a9038
commit 184feb6
Show file tree

Hide file tree

Showing 10 changed files with 37 additions and 19 deletions.
diff --git a/gradle-plugin/it/src/it/multiple-services/expected/openshift.yml b/gradle-plugin/it/src/it/multiple-services/expected/openshift.yml
@@ -158,7 +158,7 @@ items:
     name: first-service
   spec:
     port:
-      targetPort: 80
+      targetPort: 9376
     to:
       kind: Service
       name: first-service
@@ -200,7 +200,7 @@ items:
     name: second-service
   spec:
     port:
-      targetPort: 80
+      targetPort: 9376
     to:
       kind: Service
       name: second-service
diff --git a/gradle-plugin/it/src/it/route-name-fragment/expected/openshift.yml b/gradle-plugin/it/src/it/route-name-fragment/expected/openshift.yml
@@ -110,7 +110,7 @@ items:
     name: customized-name
   spec:
     port:
-      targetPort: 9090
+      targetPort: 8080
     to:
       kind: Service
       name: customized-name
diff --git a/gradle-plugin/it/src/it/service-name-fragment/expected/openshift.yml b/gradle-plugin/it/src/it/service-name-fragment/expected/openshift.yml
@@ -106,7 +106,7 @@ items:
     name: customized-name
   spec:
     port:
-      targetPort: 9090
+      targetPort: 8080
     to:
       kind: Service
       name: customized-name
diff --git a/...-plugin/it/src/it/volume-permission/expected/custom-storageclass-annotation/openshift.yml b/...-plugin/it/src/it/volume-permission/expected/custom-storageclass-annotation/openshift.yml
@@ -145,7 +145,7 @@ items:
       name: jkube-docker-registry
     spec:
       port:
-        targetPort: 80
+        targetPort: 5000
       to:
         kind: Service
         name: jkube-docker-registry
diff --git a/gradle-plugin/it/src/it/volume-permission/expected/custom-storageclass/openshift.yml b/gradle-plugin/it/src/it/volume-permission/expected/custom-storageclass/openshift.yml
@@ -151,7 +151,7 @@ items:
     name: jkube-docker-registry
   spec:
     port:
-      targetPort: 80
+      targetPort: 5000
     to:
       kind: Service
       name: jkube-docker-registry
diff --git a/gradle-plugin/it/src/it/volume-permission/expected/default/openshift.yml b/gradle-plugin/it/src/it/volume-permission/expected/default/openshift.yml
@@ -150,7 +150,7 @@ items:
     name: jkube-docker-registry
   spec:
     port:
-      targetPort: 80
+      targetPort: 5000
     to:
       kind: Service
       name: jkube-docker-registry
diff --git a/...cher/generic/src/main/java/org/eclipse/jkube/enricher/generic/DefaultServiceEnricher.java b/...cher/generic/src/main/java/org/eclipse/jkube/enricher/generic/DefaultServiceEnricher.java
@@ -611,22 +611,38 @@ private String ensureProtocol(ServicePort port) {
         return protocol;
     }
 
-    public static Integer getPortToExpose(ServiceBuilder serviceBuilder) {
+    private static ServicePort getServicePortToExpose(ServiceBuilder serviceBuilder){
         ServiceSpec spec = serviceBuilder.buildSpec();
-        if (spec != null) {
+            if (spec != null) {
             final List<ServicePort> ports = spec.getPorts();
             if (ports != null && !ports.isEmpty()) {
                 for (ServicePort port : ports) {
                     if (Objects.equals(port.getName(), "http") || Objects.equals(port.getProtocol(), "http") ) {
-                        return port.getPort();
+                        return port;
                     }
                 }
                 ServicePort servicePort = ports.iterator().next();
                 if (servicePort.getPort() != null) {
-                    return servicePort.getPort();
+                    return servicePort;
                 }
             }
         }
         return null;
     }
+
+    public static Integer getPortToExpose(ServiceBuilder serviceBuilder) {
+       ServicePort servicePort = getServicePortToExpose(serviceBuilder);
+       if (servicePort == null){
+           return  null;
+       }
+       return servicePort.getPort();
+    }
+
+    public static Integer getTargetPortToExpose(ServiceBuilder serviceBuilder) {
+        ServicePort servicePort = getServicePortToExpose(serviceBuilder);
+        if (servicePort == null || servicePort.getTargetPort() == null){
+            return  null;
+        }
+        return servicePort.getTargetPort().getIntVal();
+    }
 }
diff --git a/...her/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java b/...her/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java
@@ -38,6 +38,7 @@
 import java.util.Objects;
 
 import static org.eclipse.jkube.enricher.generic.DefaultServiceEnricher.getPortToExpose;
+import static org.eclipse.jkube.enricher.generic.DefaultServiceEnricher.getTargetPortToExpose;
 import static org.eclipse.jkube.kit.enricher.api.util.KubernetesResourceUtil.mergeMetadata;
 import static org.eclipse.jkube.kit.enricher.api.util.KubernetesResourceUtil.mergeSimpleFields;
 import static org.eclipse.jkube.kit.enricher.api.util.KubernetesResourceUtil.removeItemFromKubernetesBuilder;
@@ -122,7 +123,8 @@ private void addRoute(KubernetesListBuilder listBuilder, ServiceBuilder serviceB
 
     private static RoutePort createRoutePort(ServiceBuilder serviceBuilder) {
         RoutePort routePort = null;
-        final Integer servicePort = getPortToExpose(serviceBuilder);
+        final Integer serviceTargetPort = getTargetPortToExpose(serviceBuilder);
+        final Integer servicePort = serviceTargetPort != null ? serviceTargetPort : getPortToExpose(serviceBuilder);
         if (servicePort != null) {
             routePort = new RoutePort();
             routePort.setTargetPort(new IntOrString(servicePort));

diff --git a/...c/test/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricherBehavioralTest.java b/...c/test/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricherBehavioralTest.java
@@ -86,7 +86,7 @@ void create_withServiceWithNoWebPortAndCreateExternalUrls_shouldCreateRoute() {
     // Given
     context.getProject().getProperties().put("jkube.createExternalUrls", "true");
     klb.addNewServiceItem().withNewMetadata().withName("http").endMetadata()
-      .withNewSpec().addNewPort().withPort(21).endPort().endSpec().endServiceItem();
+      .withNewSpec().addNewPort().withPort(21).withNewTargetPort(21).endPort().endSpec().endServiceItem();
     // When
     new RouteEnricher(context).create(PlatformMode.openshift, klb);
     // Then
@@ -106,7 +106,7 @@ class ServiceWithWebPort {
     @BeforeEach
     void setUp() {
       klb.addNewServiceItem().withNewMetadata().withName("http").endMetadata()
-        .withNewSpec().addNewPort().withPort(80).endPort().endSpec().endServiceItem();
+        .withNewSpec().addNewPort().withPort(80).withNewTargetPort(8080).endPort().endSpec().endServiceItem();
     }
 
     @Test
@@ -129,7 +129,7 @@ void inOpenShift_shouldCreateRoute() {
         .last()
         .hasFieldOrPropertyWithValue("apiVersion", "route.openshift.io/v1")
         .hasFieldOrPropertyWithValue("metadata.name", "http")
-        .hasFieldOrPropertyWithValue("spec.port.targetPort.value", 80)
+        .hasFieldOrPropertyWithValue("spec.port.targetPort.value", 8080)
         .hasFieldOrPropertyWithValue("spec.to.kind", "Service")
         .hasFieldOrPropertyWithValue("spec.to.name", "http");
     }
@@ -147,7 +147,7 @@ void withFragmentWithNoName_shouldMergeRoute() {
         .last()
         .hasFieldOrPropertyWithValue("apiVersion", "route.openshift.io/v1")
         .hasFieldOrPropertyWithValue("metadata.name", "http")
-        .hasFieldOrPropertyWithValue("spec.port.targetPort.value", 80)
+        .hasFieldOrPropertyWithValue("spec.port.targetPort.value", 8080)
         .hasFieldOrPropertyWithValue("spec.to.kind", "Service")
         .hasFieldOrPropertyWithValue("spec.to.name", "http")
         .extracting("metadata.annotations")
@@ -170,7 +170,7 @@ void withFragmentWithMatchingName_shouldMergeRoute() {
         .last()
         .hasFieldOrPropertyWithValue("apiVersion", "route.openshift.io/v1")
         .hasFieldOrPropertyWithValue("metadata.name", "http")
-        .hasFieldOrPropertyWithValue("spec.port.targetPort.value", 80)
+        .hasFieldOrPropertyWithValue("spec.port.targetPort.value", 8080)
         .hasFieldOrPropertyWithValue("spec.to.kind", "Service")
         .hasFieldOrPropertyWithValue("spec.to.name", "http")
         .extracting("metadata.annotations")
@@ -193,7 +193,7 @@ void withFragmentWithDifferentName_shouldCreateNewRoute() {
         .last()
         .hasFieldOrPropertyWithValue("apiVersion", "route.openshift.io/v1")
         .hasFieldOrPropertyWithValue("metadata.name", "http")
-        .hasFieldOrPropertyWithValue("spec.port.targetPort.value", 80)
+        .hasFieldOrPropertyWithValue("spec.port.targetPort.value", 8080)
         .hasFieldOrPropertyWithValue("spec.to.kind", "Service")
         .hasFieldOrPropertyWithValue("spec.to.name", "http")
         .extracting("metadata.annotations")

diff --git a/...generic/src/test/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricherTest.java b/...generic/src/test/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricherTest.java
@@ -292,7 +292,7 @@ void routeTargetPortFromServicePort() {
         // Then
         assertThat(route).isNotNull()
                 .extracting("metadata.name", "spec.host", "spec.to.kind", "spec.to.name", "spec.port.targetPort.intVal")
-                .contains("test-svc", "example.com", "Service", "test-svc", 80);
+                .contains("test-svc", "example.com", "Service", "test-svc", 8080);
     }
 
     @Test