Remove "legacyPortMapping" from DefaultServiceEnricher

[rhuss]

Description

At the moment the DefaultServiceEnricher maps the first exposed port from the image which is either 8080 or 9090 to the port 80 of the generated service, whereas all other exposed ports are mapped to the same service port. This is dangerous e.g. when in the image configuration the order of the ports changes (the order actually doesn't matter for the docker image), then a different port can be mapped to 80.

Also the selection of 8080 and especially 9090 is a bit random (why not also 8181, 8282, 8000, 8001, ....?

I suggest to get rid of this implicit mapping and allow a mapping in the enricher configuration. This could be

<enricher>
  <config>
    <fmp-service>
       <ports>80=8080,81=9090</ports>
     ....

This could be shortened to <port>80</port> if only one port is exposed.

If there are no objections, I would implement it that way, and would add this mapping to all quickstarts / devops / .. images which currently have used this implicit service mapping (I would scrap in the generated yaml files).

[davsclaus]

I suggest to touch with the @jstrachan @rawlingsj @jimmidyson and others. I personally love that fmp can be used OOTB with no configuration with basic microservices that has a HTTP service exposed.

Having to add boilerplate XML configuration to the is a bit downer. Its fine you can override and configure this. But OOTB we should have conventions for 8080 and 8181 for 80/81 IMHO.

Just look at your XML sample above, you need 4 xml tags just to get down to where are defined. Nobody can really remember that.

[jimmidyson]

Why do we need to map the ports at all by default? Mapping arbitrary ports probably fails the principle of least surprise. Using the same port for service & pod seems most consistent. The port is only important inside the cluster, from outside either ingress, route or node port will be used which are unaffected by the service port.

[rhuss]

That's also my point. Why is a service port of 80 so much better than a 8080 ? If you don't map it to a different number its so much easier to understand that an application which exports at 8080 is also reachable at 8080 within the cluster.

[rhuss]

@davsclaus Its not the purpose to add the sample configuration above by default, only that you are able to provide a mapping in 1% of the use cases.

[davsclaus]

Ah yeah it makes okay sense to use the same ports, eg 8080->8080 etc by default. The expose controller will expose it as a nice external url with port 80, wont it @rawlingsj ?

[rawlingsj]

@davsclaus @rhuss the exposecontroller for Ingress creates a rule that maps a hostname to a service name + port so what you propose should work.

I'm not 100% sure that's how it is for OpenShift Routes though. I have something in the back of my mind that says we had to use a Service port of 80 for Routes to work. Easily testable to be sure but there was probably a reason the automatic mapping was added in the first place.

[jimmidyson]

No: services for OpenShift routes can be any port. If a service has multiple ports, then you can specify the service port to use, but if it's a single port service then it maps to that port without cofig required.

[kameshsampath]

+1 @jimmidyson - sometimes automatic port addition to my resources kind of confuses the users. I had a scenario where I need to expose Eureka default port of 8761 but i was not able to enable it via my k8s manifests as the f8-m-p adds 8080 and maps to name http.. though i can do workaround of it .. but i don't want container ports for the ones which I dont define.

[rhuss]

Update now to the DefaultServiceEnricher to allow to remove the legacy mapping 8080 / 9090 --> 80.

It's still enabled because I don't know what apps it is going to affect as it is not backwards compatible.

Two new config options has been added:

• legacyPortMapping : Enable the old mapping 8080 / 9090 --> 80 (switched on by default but would like to turn if off soon)
• port : Service port to use for the first pod port to map, that's the first exposed port from the first configured image for a pod.

If a build relies on the old, implicit mapping, the fix is to add:

<configuration>
   <enricher>
      <config>
          <fmp-service>
              <legacyPortMapping>true</true>
               <!-- Alternatively: -->
               <port>80</port>
          </fmp-service>
      </config>
   </enricher>
</configuration>

as alternative, the property fabric8.enricher.fmp-service.legacyPortMapping=true can be set (that's true btw for every generator / enricher config, and I plan to make it even more flexible to allow to use enricher.fmp-service.legacyPortMapping, fmp-service.legacyPortMapping and legacyPortMapping, too. at least as sytem properties when given from the outside.

The question is, when should we switch over the mapping ? I suggest to make at least a 3.3.0 release to indicate this change.

@davsclaus @jstrachan @rawlingsj @jimmidyson @kameshsampath Any opinions on this ?

[rhuss]

Disabled now the legacy mapping so that the next release should be 3.3.0. That's a good fit as we change the default base image, too (from alpine to centos).

In cases where a mapping would apply the following warning is printed:

 log.warn("Implicit service port mapping to port 80 has been disabled for the used port %d. " +
    "To get back the old behaviour either use set the config port = 80 or use legacyPortMapping = true. " +
    "See https://maven.fabric8.io/#fmp-service** for details.", podPort);

I'm pretty sure that there will be some issue with downstream builds. @rawlingsj if there is anything breaking downstream when 3.3.0 is released (would grep for 'legacyPortMapping'), I'm more than happy to help.

[stale]

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

[rhuss]

For 4.0 we should remove the legacyPortMapping completely.

[stale]

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!