ZOOKEEPER-5029: Port unification for PrometheusMetricsProvider#2362
Open
PDavid wants to merge 1 commit intoapache:masterfrom
Open
ZOOKEEPER-5029: Port unification for PrometheusMetricsProvider#2362PDavid wants to merge 1 commit intoapache:masterfrom
PDavid wants to merge 1 commit intoapache:masterfrom
Conversation
Contributor
Author
TestingTested this locally as follows: Created keystore: Created truststore: Added these to Started ZooKeeper: ZooKeeper log: Call PrometheusMetricsProvider using http: Call PrometheusMetricsProvider using https: |
PDavid
force-pushed
the
ZOOKEEPER-5029-PrometheusMetricsProvider-portUnification
branch
from
9dcea19 to
a6c4741
Compare
PDavid
force-pushed
the
ZOOKEEPER-5029-PrometheusMetricsProvider-portUnification
branch
from
a6c4741 to
1c3e4dd
Compare
Contributor
Author
|
Rebased the branch on latest master to resolve conflicts. |
anmolnar
requested review from
anmolnar and
kezhuw
and removed request for
anmolnar
meszibalu
approved these changes
Apr 14, 2026
anmolnar
requested changes
Apr 14, 2026
|
|
||
| if (this.httpPort != -1 && this.httpsPort != -1 && this.httpPort == this.httpsPort) { | ||
| SecureRequestCustomizer customizer = new SecureRequestCustomizer(); | ||
| customizer.setStsMaxAge(DEFAULT_STS_MAX_AGE); |
Contributor
Author
There was a problem hiding this comment.
STS means the Strict-Transport-Security HTTP response header.
The HTTP Strict-Transport-Security response header (often abbreviated as HSTS) informs browsers that the host should only be accessed using HTTPS, and that any future attempts to access it using HTTP should automatically be upgraded to HTTPS. Additionally, on future connections to the host, the browser will not allow the user to bypass secure connection errors, such as an invalid certificate. HSTS identifies a host by its domain name only.
...
max-age=<expire-time>
The time, in seconds, that the browser should remember that a host is only to be accessed using HTTPS.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Strict-Transport-Security
This is implemented here the same way as we have it in JettyAdminServer:
https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/JettyAdminServer.java#L119
| if (this.httpPort != -1 && this.httpsPort != -1 && this.httpPort == this.httpsPort) { | ||
| SecureRequestCustomizer customizer = new SecureRequestCustomizer(); | ||
| customizer.setStsMaxAge(DEFAULT_STS_MAX_AGE); | ||
| customizer.setStsIncludeSubDomains(true); |
Contributor
There was a problem hiding this comment.
We might need to add some comments to briefly explain what is this about.
Contributor
Author
There was a problem hiding this comment.
This sets a directive of the Strict-Transport-Security HTTP response header.
includeSubDomains Optional
If this directive is specified, the HSTS policy applies to all subdomains of the host's domain as well.
Same as we have in JettyAdminServer here:
| new HttpConnectionFactory(config)); | ||
| connector.setPort(this.httpPort); | ||
| connector.setHost(this.host); | ||
| LOG.debug("Created unified ServerConnector for host: {}, httpPort: {}", host, httpPort); |
Contributor
There was a problem hiding this comment.
You can move these messages to INFO level. We should see clearly which type of server has been initialized.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JettyAdminServerhas a port unification feature, meaning both secure and insecure connections can be established on the same port. It can be enabled with theadmin.portUnification=trueconfiguration setting.In order to make it more consistent,
PrometheusMetricsProvidercan also provide such feature whenmetricsProvider.httpPortandmetricsProvider.httpsPortare configured to the same value.