Octavi00 commented on PR #10975: URL: https://github.com/apache/gravitino/pull/10975#issuecomment-4426955733
Hello @roryqi, me and @Vonna9 spent some time implementing the TLS functionality on the client side of the code. At first, since the issue stated we needed a way to trust clients, our understanding was that we were from the perspective of the server, which is why we felt confused about the server functionality already being there, and just added a test to prove so. Now, what we've done is add in a TLSConfigurer Java file in the same fashion as the reference pr, and implemented it in a very similar method inside the HTTPClient.java file. We found quite a few bigger differences between the gravitino codebase and the iceberg codebase, which led to some changes I would really like to explain. The most important thing is that in order to allow testing the TLS integration from the TestHTTPClient file, I removed the private keyword from the configureConnectionManager() function. In doing so, the function remains package private so it's accessible within the same Java package that is used in the test file. I wanted to make sure you were aware of this to ensure it isn't sacrificing any security boundaries you intend to keep. I also added the properties variable as a parameter in the function, as previously the function would only rely on the GravitinoClientConfiguration object defined functions; however, properties isn't accessible from the object. I chose to keep that object separate from the TLS configuration instead of implementing it within the Gravitino client object, but it could also easily be moved into it for a cleaner HTTPClient file. Testing-wise, I implemented all of the tests in a similar fashion from the referenced pr, and added one more to ensure previous functionality without a TLS configuration. All tests currently pass, and the server compiles and runs. I feel that we are ready for a review now, and please let us know if there's a different direction you'd like us to take our implementation. Here are some photos of our testing test file passes <img width="1338" height="702" alt="image" src="https://github.com/user-attachments/assets/1d21d435-8cc8-493f-811b-d6ab6d38c84f"; /> server compiles <img width="1349" height="655" alt="image" src="https://github.com/user-attachments/assets/bc359db3-eb18-4a1f-9dd1-f92bcbd6f758"; /> server running terminal notification <img width="760" height="290" alt="image" src="https://github.com/user-attachments/assets/4fcfa4ec-5915-490d-b8ca-2f1e839586cc"; /> Thank you -- 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]
