Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2267689459
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -70,19 +73,16 @@ public InfoServer(String name, String bindAddress, int
port, boolean findPort,
// We are using the Hadoop HTTP server config properties.
// This makes it easy to keep in sync with Hadoop's UI servers, but hard
to set this
// separately for HBase.
- builder
-.keyPassword(HBaseConfiguration.getPassword(c,
"ssl.server.keystore.keypassword", null))
-.keyStore(c.get("ssl.server.keystore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.keystore.password",
null),
- c.get("ssl.server.keystore.type", "jks"))
-.trustStore(c.get("ssl.server.truststore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.truststore.password",
null),
- c.get("ssl.server.truststore.type", "jks"))
+ builder.keyPassword(getTLSPassword(c, "keystore.keypassword"))
+.keyStore(getTLSProperty(c, "keystore.location"), getTLSPassword(c,
"keystore.password"),
Review Comment:
Agreed!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
stoty merged PR #7204: URL: https://github.com/apache/hbase/pull/7204 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
stoty commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2266535875
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -70,19 +73,16 @@ public InfoServer(String name, String bindAddress, int
port, boolean findPort,
// We are using the Hadoop HTTP server config properties.
// This makes it easy to keep in sync with Hadoop's UI servers, but hard
to set this
// separately for HBase.
- builder
-.keyPassword(HBaseConfiguration.getPassword(c,
"ssl.server.keystore.keypassword", null))
-.keyStore(c.get("ssl.server.keystore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.keystore.password",
null),
- c.get("ssl.server.keystore.type", "jks"))
-.trustStore(c.get("ssl.server.truststore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.truststore.password",
null),
- c.get("ssl.server.truststore.type", "jks"))
+ builder.keyPassword(getTLSPassword(c, "keystore.keypassword"))
+.keyStore(getTLSProperty(c, "keystore.location"), getTLSPassword(c,
"keystore.password"),
Review Comment:
Those are existing Hadoop properties.
The TLS property names are all over the place (RPC, Thrift, REST), IMO being
consistent with the Hadoop UI server properties is preferable in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
stoty commented on PR #7204: URL: https://github.com/apache/hbase/pull/7204#issuecomment-3167373390 Thanks @NihalJain. I will wait a day or two in case someone else decides to comment. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on PR #7204: URL: https://github.com/apache/hbase/pull/7204#issuecomment-3167352537 Sure, i think we are good then. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
stoty commented on PR #7204: URL: https://github.com/apache/hbase/pull/7204#issuecomment-3167343086 Thank you. That test doesn't use InfoServer unfortunately. The only easily testable parts are the fallback-enabled getters, which are trivial. We could factor out the TLS settings in InfoServer into a separate method and test the config logic with a mocked HttpServer.Builder , but I don't think that's worth the effort. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on PR #7204: URL: https://github.com/apache/hbase/pull/7204#issuecomment-3166882774 Sorry added more comments after approval. But new comments are good to have, not required. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on PR #7204: URL: https://github.com/apache/hbase/pull/7204#issuecomment-3166880830 Also please consider writing tests for new keys, refer https://github.com/apache/hbase/blob/master/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSSLHttpServer.java We could do so by extedning existing one and overriding the setup() -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2262177677
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -70,19 +73,16 @@ public InfoServer(String name, String bindAddress, int
port, boolean findPort,
// We are using the Hadoop HTTP server config properties.
// This makes it easy to keep in sync with Hadoop's UI servers, but hard
to set this
// separately for HBase.
- builder
-.keyPassword(HBaseConfiguration.getPassword(c,
"ssl.server.keystore.keypassword", null))
-.keyStore(c.get("ssl.server.keystore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.keystore.password",
null),
- c.get("ssl.server.keystore.type", "jks"))
-.trustStore(c.get("ssl.server.truststore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.truststore.password",
null),
- c.get("ssl.server.truststore.type", "jks"))
+ builder.keyPassword(getTLSPassword(c, "keystore.keypassword"))
+.keyStore(getTLSProperty(c, "keystore.location"), getTLSPassword(c,
"keystore.password"),
Review Comment:
ideally location is correct usage
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
Apache-HBase commented on PR #7204:
URL: https://github.com/apache/hbase/pull/7204#issuecomment-3166849801
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 29s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s):
--brief-report-file --spotbugs-strict-precheck --author-ignore-list
--blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
_ Prechecks _ |
_ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 22s | | master passed |
| +1 :green_heart: | compile | 0m 18s | | master passed |
| +1 :green_heart: | javadoc | 0m 14s | | master passed |
| +1 :green_heart: | shadedjars | 6m 9s | | branch has no errors when
building our shaded downstream artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 5s | | the patch passed |
| +1 :green_heart: | compile | 0m 17s | | the patch passed |
| +1 :green_heart: | javac | 0m 17s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 13s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 4s | | patch has no errors when
building our shaded downstream artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 0m 57s | | hbase-http in the patch passed.
|
| | | 22m 8s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base:
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
|
| GITHUB PR | https://github.com/apache/hbase/pull/7204 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 87674fe7cfee 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May
23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6562ee5cd9610e291b17c827d627b712801a0c31 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/2/testReport/
|
| Max. process+thread count | 291 (vs. ulimit of 3) |
| modules | C: hbase-http U: hbase-http |
| Console output |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/2/console
|
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2262168435
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -70,19 +73,16 @@ public InfoServer(String name, String bindAddress, int
port, boolean findPort,
// We are using the Hadoop HTTP server config properties.
// This makes it easy to keep in sync with Hadoop's UI servers, but hard
to set this
// separately for HBase.
- builder
-.keyPassword(HBaseConfiguration.getPassword(c,
"ssl.server.keystore.keypassword", null))
-.keyStore(c.get("ssl.server.keystore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.keystore.password",
null),
- c.get("ssl.server.keystore.type", "jks"))
-.trustStore(c.get("ssl.server.truststore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.truststore.password",
null),
- c.get("ssl.server.truststore.type", "jks"))
+ builder.keyPassword(getTLSPassword(c, "keystore.keypassword"))
+.keyStore(getTLSProperty(c, "keystore.location"), getTLSPassword(c,
"keystore.password"),
Review Comment:
We could also consider aligning these names based on other impls within
hbase for eg: rest and thrift uses `keystore.store`
>
https://github.com/apache/hbase/blob/master/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/Constants.java#L51
>
https://github.com/apache/hbase/blob/master/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/Constants.java#L58
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -70,19 +73,16 @@ public InfoServer(String name, String bindAddress, int
port, boolean findPort,
// We are using the Hadoop HTTP server config properties.
// This makes it easy to keep in sync with Hadoop's UI servers, but hard
to set this
// separately for HBase.
- builder
-.keyPassword(HBaseConfiguration.getPassword(c,
"ssl.server.keystore.keypassword", null))
-.keyStore(c.get("ssl.server.keystore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.keystore.password",
null),
- c.get("ssl.server.keystore.type", "jks"))
-.trustStore(c.get("ssl.server.truststore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.truststore.password",
null),
- c.get("ssl.server.truststore.type", "jks"))
+ builder.keyPassword(getTLSPassword(c, "keystore.keypassword"))
+.keyStore(getTLSProperty(c, "keystore.location"), getTLSPassword(c,
"keystore.password"),
Review Comment:
We could also consider aligning these names based on other impls within
hbase for eg: rest and thrift uses `keystore.store`
-
https://github.com/apache/hbase/blob/master/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/Constants.java#L51
-
https://github.com/apache/hbase/blob/master/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/Constants.java#L58
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
Apache-HBase commented on PR #7204:
URL: https://github.com/apache/hbase/pull/7204#issuecomment-3166864509
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 29s | | Docker mode activated. |
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files
found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain
any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any
anti-patterns. |
_ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 55s | | master passed |
| +1 :green_heart: | compile | 0m 25s | | master passed |
| +1 :green_heart: | checkstyle | 0m 9s | | master passed |
| +1 :green_heart: | spotbugs | 0m 24s | | master passed |
| +1 :green_heart: | spotless | 0m 45s | | branch has no errors when
running spotless:check. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 54s | | the patch passed |
| +1 :green_heart: | compile | 0m 24s | | the patch passed |
| +1 :green_heart: | javac | 0m 24s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| +1 :green_heart: | checkstyle | 0m 10s | | the patch passed |
| +1 :green_heart: | spotbugs | 0m 32s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 11m 14s | | Patch does not cause any
errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 43s | | patch has no errors when
running spotless:check. |
_ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | | The patch does not
generate ASF License warnings. |
| | | 28m 34s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base:
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/2/artifact/yetus-general-check/output/Dockerfile
|
| GITHUB PR | https://github.com/apache/hbase/pull/7204 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell
detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux cafaef5df332 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May
23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6562ee5cd9610e291b17c827d627b712801a0c31 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 83 (vs. ulimit of 3) |
| modules | C: hbase-http U: hbase-http |
| Console output |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/2/console
|
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2262168435
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -70,19 +73,16 @@ public InfoServer(String name, String bindAddress, int
port, boolean findPort,
// We are using the Hadoop HTTP server config properties.
// This makes it easy to keep in sync with Hadoop's UI servers, but hard
to set this
// separately for HBase.
- builder
-.keyPassword(HBaseConfiguration.getPassword(c,
"ssl.server.keystore.keypassword", null))
-.keyStore(c.get("ssl.server.keystore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.keystore.password",
null),
- c.get("ssl.server.keystore.type", "jks"))
-.trustStore(c.get("ssl.server.truststore.location"),
- HBaseConfiguration.getPassword(c, "ssl.server.truststore.password",
null),
- c.get("ssl.server.truststore.type", "jks"))
+ builder.keyPassword(getTLSPassword(c, "keystore.keypassword"))
+.keyStore(getTLSProperty(c, "keystore.location"), getTLSPassword(c,
"keystore.password"),
Review Comment:
We could also con sider aligning these names based on other impls within
hbase for eg: rest and thrift uses `keystore.store`
https://github.com/apache/hbase/blob/master/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/Constants.java#L51
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2262161108
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -42,6 +42,9 @@ public class InfoServer {
private static final String HBASE_APP_DIR = "hbase-webapps";
private final org.apache.hadoop.hbase.http.HttpServer httpServer;
+ private static final String HADOOP_WEB_TLS_CONFIG_PREFIX = "ssl.server.";
+ private static final String HBASE_WEB_TLS_CONFIG_PREFIX =
"hbase.infoserver.";
Review Comment:
ah right!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
Apache-HBase commented on PR #7204:
URL: https://github.com/apache/hbase/pull/7204#issuecomment-3166797854
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 30s | | Docker mode activated. |
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files
found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain
any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any
anti-patterns. |
_ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 9s | | master passed |
| +1 :green_heart: | compile | 0m 25s | | master passed |
| +1 :green_heart: | checkstyle | 0m 10s | | master passed |
| +1 :green_heart: | spotbugs | 0m 27s | | master passed |
| +1 :green_heart: | spotless | 0m 49s | | branch has no errors when
running spotless:check. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 55s | | the patch passed |
| +1 :green_heart: | compile | 0m 24s | | the patch passed |
| +1 :green_heart: | javac | 0m 24s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| +1 :green_heart: | checkstyle | 0m 10s | | the patch passed |
| +1 :green_heart: | spotbugs | 0m 32s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 11m 23s | | Patch does not cause any
errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 44s | | patch has no errors when
running spotless:check. |
_ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | | The patch does not
generate ASF License warnings. |
| | | 29m 15s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base:
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/1/artifact/yetus-general-check/output/Dockerfile
|
| GITHUB PR | https://github.com/apache/hbase/pull/7204 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell
detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux f57217643099 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May
23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f653239f874b95d4680402dad17e1f27ecfaf180 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 83 (vs. ulimit of 3) |
| modules | C: hbase-http U: hbase-http |
| Console output |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/1/console
|
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
Apache-HBase commented on PR #7204:
URL: https://github.com/apache/hbase/pull/7204#issuecomment-3166781904
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | | Unprocessed flag(s):
--brief-report-file --spotbugs-strict-precheck --author-ignore-list
--blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
_ Prechecks _ |
_ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 21s | | master passed |
| +1 :green_heart: | compile | 0m 17s | | master passed |
| +1 :green_heart: | javadoc | 0m 14s | | master passed |
| +1 :green_heart: | shadedjars | 6m 7s | | branch has no errors when
building our shaded downstream artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 10s | | the patch passed |
| +1 :green_heart: | compile | 0m 17s | | the patch passed |
| +1 :green_heart: | javac | 0m 17s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 13s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 5s | | patch has no errors when
building our shaded downstream artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 0m 56s | | hbase-http in the patch passed.
|
| | | 22m 7s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base:
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
|
| GITHUB PR | https://github.com/apache/hbase/pull/7204 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 4330df639ad6 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May
23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f653239f874b95d4680402dad17e1f27ecfaf180 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/1/testReport/
|
| Max. process+thread count | 299 (vs. ulimit of 3) |
| modules | C: hbase-http U: hbase-http |
| Console output |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7204/1/console
|
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
stoty commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2262073912
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -42,6 +42,9 @@ public class InfoServer {
private static final String HBASE_APP_DIR = "hbase-webapps";
private final org.apache.hadoop.hbase.http.HttpServer httpServer;
+ private static final String HADOOP_WEB_TLS_CONFIG_PREFIX = "ssl.server.";
+ private static final String HBASE_WEB_TLS_CONFIG_PREFIX =
"hbase.infoserver.";
Review Comment:
Sure.
I also realize that I didn't have the "ssl" string in the HBase prefix, I
also fixed that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2262057529
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -42,6 +42,9 @@ public class InfoServer {
private static final String HBASE_APP_DIR = "hbase-webapps";
private final org.apache.hadoop.hbase.http.HttpServer httpServer;
+ private static final String HADOOP_WEB_TLS_CONFIG_PREFIX = "ssl.server.";
+ private static final String HBASE_WEB_TLS_CONFIG_PREFIX =
"hbase.infoserver.";
Review Comment:
nit: we could re-use prefix 'hbase.ui.' as in
`https://github.com/apache/hbase/blob/master/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java#L170`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29508 Define HBase specific TLS config properties for InfoServer [hbase]
NihalJain commented on code in PR #7204:
URL: https://github.com/apache/hbase/pull/7204#discussion_r2262052603
##
hbase-http/src/main/java/org/apache/hadoop/hbase/http/InfoServer.java:
##
@@ -104,6 +104,20 @@ public InfoServer(String name, String bindAddress, int
port, boolean findPort,
this.httpServer = builder.build();
}
+ private String getTLSPassword(Configuration c, String postfix) throws
IOException {
+return HBaseConfiguration.getPassword(c, HBASE_WEB_TLS_CONFIG_PREFIX +
postfix,
+ HBaseConfiguration.getPassword(c, HADOOP_WEB_TLS_CONFIG_PREFIX +
postfix, null));
+ }
+
+ private String getTLSProperty(Configuration c, String postfix) {
+return getTLSProperty(c, postfix, null);
+ }
+
+ private String getTLSProperty(Configuration c, String postfix, String
defaultValue) {
Review Comment:
ok so we are wrapping these into hbase specific confs with hadoop fallback
nice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
