pilot: gateway listeners should respect targetPort #20838
Conversation
googlebot
added
the
cla: yes
istio-testing
added
the
size/L
|
@costinm @howardjohn this is part of the multi-tenancy work: it enables non-privileged gateway pods |
|
/retest |
|
Interesting, seems like something we should have already done 🙂 would prefer if @rshriram also takes a look in case there is some reason this is a bad idea I am missing, but I assume you have done this on maistra for a while? |
dgn
force-pushed
the
dgn/gateway-on-target-port
branch
from
ce35292
to
97fcb5d
Compare
| }, | ||
| }, | ||
| }, | ||
| []string{"0.0.0.0_8080"}, |
There was a problem hiding this comment.
For ingress gateway, there is no inbound listener, so the target port does not take effect for it.
There was a problem hiding this comment.
Hi @hzxuzhonghu, is this a problem with the test I wrote or the implementation? I tested this with an ingress-gateway and it works, it creates a listener on the targetPort instead of the port
There was a problem hiding this comment.
The listener of gateway should have nothing to do with the targetport.
There was a problem hiding this comment.
I don't agree, that's the whole point of the change: to have an ingress-gateway service with port: 80 and targetPort: 8080, so that an Ingress resource can still point to port 80 of the ingress-gateway service, but the pod running the gateway listens on port 8080, which does not require elevated privileges.
There was a problem hiding this comment.
We currently assume in the code that port and targetPort for services targeting gateways are always the same, which they don't have to be.
|
/retest |
* Respect targetPort in gateway listener generation * Change listening ports to unprivileged ports (>1023) * Update operator's golden files * Fix linting issues
This makes sure that you can define non-privileged
targetPorts for your ingress-gateway services. It also changes the default listener ports to non-privileged ports.[x] Networking