Skip to content

Conversation

@anmolnar
Copy link
Contributor

Line up Thrift server with HTTP and REST by adding HTTP security headers if SSL is enabled.
Includes a new unit test class specific for HTTP+SSL based Thrift server + some refactoring.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 25s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗 mvndep 0m 17s Maven dependency ordering for branch
+1 💚 mvninstall 2m 56s master passed
+1 💚 compile 0m 54s master passed
+1 💚 shadedjars 5m 27s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 53s master passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 11s Maven dependency ordering for patch
+1 💚 mvninstall 2m 49s the patch passed
+1 💚 compile 0m 58s the patch passed
+1 💚 javac 0m 58s the patch passed
+1 💚 shadedjars 5m 26s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 51s the patch passed
_ Other Tests _
+1 💚 unit 0m 49s hbase-http in the patch passed.
+1 💚 unit 6m 28s hbase-thrift in the patch passed.
+1 💚 unit 3m 27s hbase-rest in the patch passed.
33m 11s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR #5864
Optional Tests javac javadoc unit shadedjars compile
uname Linux 928347fcbaf6 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 / 8a2f3ef
Default Java Eclipse Adoptium-17.0.10+7
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/testReport/
Max. process+thread count 1640 (vs. ulimit of 30000)
modules C: hbase-http hbase-thrift hbase-rest U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/console
versions git=2.34.1 maven=3.8.6
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 36s Docker mode activated.
-0 ⚠️ yetus 0m 2s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗 mvndep 0m 10s Maven dependency ordering for branch
+1 💚 mvninstall 3m 1s master passed
+1 💚 compile 1m 2s master passed
+1 💚 shadedjars 5m 16s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 1m 2s master passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 14s Maven dependency ordering for patch
+1 💚 mvninstall 2m 50s the patch passed
+1 💚 compile 1m 1s the patch passed
+1 💚 javac 1m 1s the patch passed
+1 💚 shadedjars 5m 17s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 1m 1s the patch passed
_ Other Tests _
+1 💚 unit 0m 59s hbase-http in the patch passed.
+1 💚 unit 6m 44s hbase-thrift in the patch passed.
+1 💚 unit 3m 31s hbase-rest in the patch passed.
34m 10s
Subsystem Report/Notes
Docker ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #5864
Optional Tests javac javadoc unit shadedjars compile
uname Linux c47bae970cfa 5.4.0-174-generic #193-Ubuntu SMP Thu Mar 7 14:29:28 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 8a2f3ef
Default Java Eclipse Adoptium-11.0.17+8
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/testReport/
Max. process+thread count 1678 (vs. ulimit of 30000)
modules C: hbase-http hbase-thrift hbase-rest U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/console
versions git=2.34.1 maven=3.8.6
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 51s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗 mvndep 0m 16s Maven dependency ordering for branch
+1 💚 mvninstall 4m 31s master passed
+1 💚 compile 1m 57s master passed
+1 💚 checkstyle 0m 59s master passed
+1 💚 spotless 0m 52s branch has no errors when running spotless:check.
+1 💚 spotbugs 2m 15s master passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 12s Maven dependency ordering for patch
+1 💚 mvninstall 3m 59s the patch passed
+1 💚 compile 1m 55s the patch passed
+1 💚 javac 1m 55s the patch passed
+1 💚 checkstyle 1m 6s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 xml 0m 2s The patch has no ill-formed XML file.
+1 💚 hadoopcheck 6m 16s Patch does not cause any errors with Hadoop 3.3.6.
+1 💚 spotless 0m 51s patch has no errors when running spotless:check.
+1 💚 spotbugs 3m 1s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 44s The patch does not generate ASF License warnings.
38m 37s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #5864
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile xml
uname Linux 14a06661c5cd 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 / 8a2f3ef
Default Java Eclipse Adoptium-11.0.17+8
Max. process+thread count 81 (vs. ulimit of 30000)
modules C: hbase-http hbase-thrift hbase-rest U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/console
versions git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 56s Docker mode activated.
-0 ⚠️ yetus 0m 2s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗 mvndep 0m 14s Maven dependency ordering for branch
+1 💚 mvninstall 3m 57s master passed
+1 💚 compile 1m 8s master passed
+1 💚 shadedjars 6m 54s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 1m 15s master passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 15s Maven dependency ordering for patch
+1 💚 mvninstall 3m 19s the patch passed
+1 💚 compile 1m 6s the patch passed
+1 💚 javac 1m 6s the patch passed
+1 💚 shadedjars 6m 18s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 53s the patch passed
_ Other Tests _
+1 💚 unit 1m 8s hbase-http in the patch passed.
+1 💚 unit 6m 45s hbase-thrift in the patch passed.
+1 💚 unit 4m 56s hbase-rest in the patch passed.
40m 38s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #5864
Optional Tests javac javadoc unit shadedjars compile
uname Linux 5989d0f5beb1 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 / 8a2f3ef
Default Java Temurin-1.8.0_352-b08
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/testReport/
Max. process+thread count 1638 (vs. ulimit of 30000)
modules C: hbase-http hbase-thrift hbase-rest U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/console
versions git=2.34.1 maven=3.8.6
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Copy link
Contributor

@stoty stoty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM

ServletContextHandler ctxHandler =
new ServletContextHandler(httpServer, "/", ServletContextHandler.SESSIONS);
ctxHandler.addServlet(new ServletHolder(thriftHttpServlet), "/*");
HttpServerUtil.addClickjackingPreventionFilter(ctxHandler, conf, PATH_SPEC_ANY);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:
You mention in the ticket description that we add this when "HTTP support is enabled".

While I understand that this refers to the HTTP frontend as opposed to the binary, it can still cause misundestandings.

I suggest updating the ticket text to "HTTP/HTTPS support is enabled"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've updated the ticket.

@Apache9
Copy link
Contributor

Apache9 commented Jun 6, 2024

@anmolnar You can merge this by yourself since you are a committer now.

@anmolnar anmolnar merged commit 97de291 into apache:master Aug 13, 2024
anmolnar added a commit that referenced this pull request Aug 13, 2024
Signed-off-by: Duo Zhang <[email protected]>
Signed-off-by: Pankaj <[email protected]>
Signed-off-by: Istvan Toth <[email protected]>
anmolnar added a commit to anmolnar/hbase that referenced this pull request Aug 13, 2024
Signed-off-by: Duo Zhang <[email protected]>
Signed-off-by: Pankaj <[email protected]>
Signed-off-by: Istvan Toth <[email protected]>
@anmolnar anmolnar deleted the HBASE-27118 branch September 19, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants