Fixed the default port for https and http in admin client

[merlimat]

Motivation

After #3249, the client is not able to use the correct default port for https, switching instead to the non-default of 8443.

This makes it very difficult and confusing to use standard ports for the service.

[rdhabalia]

@merlimat can you describe the exact issue?
and is it a good idea to do port hardcoding into the src: https://github.com/apache/pulsar/blob/master/pulsar-common/src/main/java/org/apache/pulsar/common/net/ServiceURI.java#L53

[merlimat]

@rdhabalia For admin client it results that https://broker.example.com will be interpreted as https://broker.example.com:8443, instead of using the default port for https.
Same for http, we must default to 80, not to 8080.

[sijie]

@merlimat Is this a breaking change? For example, if a user has set up a pulsar cluster and use https://example.pulsar.com. Right now it will be connecting to port 8443, because the default http and https ports for Pulsar are 8080 and 8443. With your change, the client will be connect to 80 and 443, but fail.

If Pulsar's default http and https ports are 8080 and 8443, we should try to stick to Pulsar's default value, not the general http and https default values. Otherwise it might be break Pulsar 2.3.x clients, right?

[merlimat]

@merlimat Is this a breaking change? For example, if a user has set up a pulsar cluster and use https://example.pulsar.com. Right now it will be connecting to port 8443, because the default http and https ports for Pulsar are 8080 and 8443. With your change, the client will be connect to 80 and 443, but fail.

I think the breaking change was in #3249. Before that, https was correctly resolving to 443 and http to 80.

If Pulsar's default http and https ports are 8080 and 8443, we should try to stick to Pulsar's default value, not the general http and https default values. Otherwise it might be break Pulsar 2.3.x clients, right?

The fact that on server side we use 8080 and 8443 by default, shouldn't mean that a client should hide the port for that.

This is seriously confusing. https://my-broker.com must mean port 443. If you use a different client, other than pulsar admin, you would have to specify :443.

Now, imagine debugging why the same url works in pulsar-admin but it doesn't if you try with http directly.

[sijie]

I think the breaking change was in #3249. Before that, https was correctly resolving to 443 and http to 80.

I don't think #3249 is a breaking change. If I remembered correctly, prior to #3249, you had to specify port in service url in order to make it work. it doesn't resolve directly to 443 for https or 80 for http. #3249 introduced the capability for resolving server url to default ports without specifying the port. I used 8080 for http and 8443 for https because those were the default ports when you setup a pulsar cluster.

This is seriously confusing. https://my-broker.com must mean port 443. If you use a different client, other than pulsar admin, you would have to specify :443.
Now, imagine debugging why the same url works in pulsar-admin but it doesn't if you try with http directly.

agreed. If so, I would rather suggesting adding constraints that port is required. This will makes the behavior same as prior to #3249.

[merlimat]

I don't think #3249 is a breaking change. If I remembered correctly, prior to #3249, you had to specify port in service url in order to make it work.

I just double checked on version 2.2.1 and the pulsar-admin client correctly uses 80 and 443 for http and https ports.

[sijie]

I just double checked on version 2.2.1 and the pulsar-admin client correctly uses 80 and 443 for http and https ports.

OK. Then I am fine with this change.

[merlimat]

retest this please