Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OpenShift route generator: prefer port name over port number for targetPort when it's possible #2857

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

OpenShift route generator: prefer port name over port number for targetPort when it's possible #2857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/autotls/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -142,7 +142,7 @@ items:
name: autotls
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: autotls
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/dockerfile-simple/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -89,7 +89,7 @@ items:
name: dockerfile-simple
spec:
port:
targetPort: 9080
targetPort: glrpc
to:
kind: Service
name: dockerfile-simple
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/expose/expected/ftp-port-expose-annotation-enricher/openshift.yml
View file
Open in desktop
Expand Up @@ -134,7 +134,7 @@ items:
name: expose
spec:
port:
targetPort: 21
targetPort: ftp
to:
kind: Service
name: expose
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/expose/expected/ftp-port-expose-annotation/openshift.yml
View file
Open in desktop
Expand Up @@ -134,7 +134,7 @@ items:
name: expose
spec:
port:
targetPort: 21
targetPort: ftp
to:
kind: Service
name: expose
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/expose/expected/ftp-port-external/openshift.yml
View file
Open in desktop
Expand Up @@ -132,7 +132,7 @@ items:
name: expose
spec:
port:
targetPort: 21
targetPort: ftp
to:
kind: Service
name: expose
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/expose/expected/http-port-external/openshift.yml
View file
Open in desktop
Expand Up @@ -132,7 +132,7 @@ items:
name: expose
spec:
port:
targetPort: 80
targetPort: http
to:
kind: Service
name: expose
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/expose/expected/http-port/openshift.yml
View file
Open in desktop
Expand Up @@ -132,7 +132,7 @@ items:
name: expose
spec:
port:
targetPort: 80
targetPort: http
to:
kind: Service
name: expose
4 changes: 2 additions & 2 deletions gradle-plugin/it/src/it/expose/expected/multiple-services/openshift.yml
View file
Open in desktop
Expand Up @@ -230,7 +230,7 @@ items:
name: expose
spec:
port:
targetPort: 443
targetPort: https
to:
kind: Service
name: expose
Expand All @@ -256,7 +256,7 @@ items:
name: ssh
spec:
port:
targetPort: 22
targetPort: ssh
to:
kind: Service
name: ssh
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/helidon/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -137,7 +137,7 @@ items:
name: helidon
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: helidon
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/initcontainers/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -126,7 +126,7 @@ items:
name: initcontainers
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: initcontainers
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/metadata/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -156,7 +156,7 @@ items:
name: metadata
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: metadata
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/multiple-services/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -179,7 +179,7 @@ items:
name: multiple-services
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: multiple-services
Expand Down
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/openliberty/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -137,7 +137,7 @@ items:
name: openliberty
spec:
port:
targetPort: 9080
targetPort: glrpc
to:
kind: Service
name: openliberty
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/probes-groovy-dsl-config/expected/liveness-readiness/openshift.yml
View file
Open in desktop
Expand Up @@ -151,7 +151,7 @@ items:
name: probes-groovy-dsl-config
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: probes-groovy-dsl-config
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/probes-groovy-dsl-config/expected/none/openshift.yml
View file
Open in desktop
Expand Up @@ -119,7 +119,7 @@ items:
name: probes-groovy-dsl-config
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: probes-groovy-dsl-config
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/probes-groovy-dsl-config/expected/startup/openshift.yml
View file
Open in desktop
Expand Up @@ -127,7 +127,7 @@ items:
name: probes-groovy-dsl-config
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: probes-groovy-dsl-config
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/project-label/expected/app/openshift.yml
View file
Open in desktop
Expand Up @@ -110,7 +110,7 @@ items:
name: project-label
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: project-label
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/project-label/expected/custom/openshift.yml
View file
Open in desktop
Expand Up @@ -109,7 +109,7 @@ items:
name: project-label
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: project-label
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/project-label/expected/default/openshift.yml
View file
Open in desktop
Expand Up @@ -109,7 +109,7 @@ items:
name: project-label
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: project-label
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/project-label/expected/group/openshift.yml
View file
Open in desktop
Expand Up @@ -109,7 +109,7 @@ items:
name: project-label
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: project-label
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/project-label/expected/version/openshift.yml
View file
Open in desktop
Expand Up @@ -109,7 +109,7 @@ items:
name: project-label
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: project-label
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/route-name-fragment/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -110,7 +110,7 @@ items:
name: customized-name
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: customized-name
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/route/expected/routeEnabled/openshift.yml
View file
Open in desktop
Expand Up @@ -110,7 +110,7 @@ items:
name: route
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: route
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/service-name-fragment/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -106,7 +106,7 @@ items:
name: customized-name
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: customized-name
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/service-name/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -106,7 +106,7 @@ items:
name: customized-name
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: customized-name
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/service/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -133,7 +133,7 @@ items:
name: svc1
spec:
port:
targetPort: 8080
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is value different in this case? I see in this case it's port1 instead of http

targetPort: port1
to:
kind: Service
name: svc1
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/smallrye-health/expected/openshift.yml
View file
Open in desktop
Expand Up @@ -137,7 +137,7 @@ items:
name: smallrye-health
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: smallrye-health
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/well-known-labels/expected/custom/openshift.yml
View file
Open in desktop
Expand Up @@ -137,7 +137,7 @@ items:
name: well-known-labels
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: well-known-labels
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/well-known-labels/expected/default/openshift.yml
View file
Open in desktop
Expand Up @@ -131,7 +131,7 @@ items:
name: well-known-labels
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: well-known-labels
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/well-known-labels/expected/disabled/openshift.yml
View file
Open in desktop
Expand Up @@ -109,7 +109,7 @@ items:
name: well-known-labels
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: well-known-labels
2 changes: 1 addition & 1 deletion gradle-plugin/it/src/it/well-known-labels/expected/labelsViaResourceConfig/openshift.yml
View file
Open in desktop
Expand Up @@ -138,7 +138,7 @@ items:
name: well-known-labels
spec:
port:
targetPort: 8080
targetPort: http
to:
kind: Service
name: well-known-labels
10 changes: 1 addition & 9 deletions ...cher/generic/src/main/java/org/eclipse/jkube/enricher/generic/DefaultServiceEnricher.java
View file
Open in desktop
Expand Up @@ -611,7 +611,7 @@ private String ensureProtocol(ServicePort port) {
return protocol;
}

private static ServicePort getServicePortToExpose(ServiceBuilder serviceBuilder){
public static ServicePort getServicePortToExpose(ServiceBuilder serviceBuilder){
ServiceSpec spec = serviceBuilder.buildSpec();
if (spec != null) {
final List<ServicePort> ports = spec.getPorts();
Expand All @@ -637,12 +637,4 @@ public static Integer getPortToExpose(ServiceBuilder serviceBuilder) {
}
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();
}
}
33 changes: 26 additions & 7 deletions ...her/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@
package org.eclipse.jkube.enricher.generic.openshift;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.ServicePort;
import io.fabric8.openshift.api.model.RouteSpec;
import org.eclipse.jkube.kit.common.Configs;
import org.eclipse.jkube.kit.common.util.FileUtil;
Expand All @@ -37,8 +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.enricher.generic.DefaultServiceEnricher.getServicePortToExpose;
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;
Expand Down Expand Up @@ -122,14 +122,33 @@
}

private static RoutePort createRoutePort(ServiceBuilder serviceBuilder) {

RoutePort routePort = null;
final Integer serviceTargetPort = getTargetPortToExpose(serviceBuilder);
final Integer servicePort = serviceTargetPort != null ? serviceTargetPort : getPortToExpose(serviceBuilder);
if (servicePort != null) {
routePort = new RoutePort();
final ServicePort servicePortToExpose = getServicePortToExpose(serviceBuilder);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add some unit tests verifying this precedence order?


if(servicePortToExpose == null){
return null;

Check warning on line 130 in jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java

View check run for this annotation

Codecov / codecov/patch

jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java#L130 

Added line #L130 was not covered by tests
}

routePort = new RoutePort();
final String sevicePortName = servicePortToExpose.getName();

if(sevicePortName != null){
routePort.setTargetPort(new IntOrString(sevicePortName));
return routePort;
}
final IntOrString serviceTargetPort = servicePortToExpose.getTargetPort();
if(serviceTargetPort.getValue() != null){
routePort.setTargetPort(serviceTargetPort);
return routePort;
}

final Integer servicePort = servicePortToExpose.getPort();

Check warning on line 146 in jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java

View check run for this annotation

Codecov / codecov/patch

jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java#L146 

Added line #L146 was not covered by tests
if(servicePort != null){
routePort.setTargetPort(new IntOrString(servicePort));
return routePort;

Check warning on line 149 in jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java

View check run for this annotation

Codecov / codecov/patch

jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java#L149 

Added line #L149 was not covered by tests
}
return routePort;
return null;

Check warning on line 151 in jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java

View check run for this annotation

Codecov / codecov/patch

jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/RouteEnricher.java#L151 

Added line #L151 was not covered by tests
}

private String prepareHostForRoute(String routeDomainPostfix, String name) {
Expand Down