Got rid of ExposeEnricher #1673
Conversation
devang-gaur
added
pr/wip
CHANGELOG.md
Outdated
| @@ -9,6 +9,9 @@ We use semantic versioning in some slight variation until our feature set has st | |||
| * The `PATCH_LEVEL` is used for regular CD releases which add new features and bug fixes. | |||
|
|
|||
| After this we will switch probably to real [Semantic Versioning 2.0.0](http://semver.org/) | |||
| ### 4.2-SNAPSHOT | |||
There was a problem hiding this comment.
Rename 4.1-SNAPSHOT and move items there
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.*; |
There was a problem hiding this comment.
nit, You introduced wildcard imports here
| String name = metadata.getName(); | ||
| if (!hasRoute(listBuilder, name)) { | ||
| //if (!hasRoute(listBuilder, name)) { |
| @@ -105,6 +100,18 @@ private RoutePort createRoutePort(ServiceBuilder serviceBuilder) { | |||
| return routePort; | |||
| } | |||
|
|
|||
| private boolean hasWebPort(List<ServicePort> ports) { | |||
There was a problem hiding this comment.
Why are we checking only a specific set of ports of web ports? What if user has supplied some other port rather than these ports? if he has, can we add that check here as well along with standard ports?
There was a problem hiding this comment.
I have followed what the expose enricher used to do.
Also, the purpose of a Route is basically to service incoming traffic..which comes through http and https only. So this set of ports looks legitimate to me.
@lordofthejars thoughts ?
There was a problem hiding this comment.
I'm okay with these ports, but I'm just concerned about the case when the port is not among one of these ports
There was a problem hiding this comment.
Then that simply violates http and https and it should not be accepted, imo
rohanKanojia
removed
the
jkube/pending
fixes #1374