Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Documentation on Enabling SSL/TLS support for hana. #21994

Merged
merged 1 commit into from
Feb 28, 2024
Merged

Documentation on Enabling SSL/TLS support for hana. #21994

merged 1 commit into from
Feb 28, 2024

Conversation

sabbasani
Copy link
Contributor

@sabbasani sabbasani commented Feb 23, 2024 •
edited
Loading

Description

Enabling SSL support for Hana connector.

Motivation and Context

Added security feature SSL/TLS where users can connect to SSL/TLS enabled Datasource.

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==
Hana Docs
* Add Connection security documentation for the Hana Connector.

@github-actions github-actions bot added the docs label Feb 23, 2024
steveburnett
steveburnett requested changes Feb 23, 2024
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Thanks for the doc! Two nits to consider.

presto-docs/src/main/sphinx/connector/hana.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/hana.rst Outdated Show resolved Hide resolved
steveburnett
steveburnett previously approved these changes Feb 23, 2024
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull updated branch, new local build, everything looks good. Thanks!

@sabbasani sabbasani marked this pull request as ready for review February 23, 2024 17:59
@sabbasani sabbasani requested a review from a team as a code owner February 23, 2024 17:59
@sabbasani sabbasani requested a review from presto-oss February 23, 2024 17:59
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 23, 2024 •
edited
Loading

Codenotify: Notifying subscribers in CODENOTIFY files for diff d4e9213...579f6b1.

Notify File(s)
@steveburnett presto-docs/src/main/sphinx/connector/hana.rst

steveburnett
steveburnett previously approved these changes Feb 23, 2024
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

@skairali skairali self-assigned this Feb 26, 2024
skairali
skairali previously approved these changes Feb 26, 2024
View reviewed changes
Copy link
Member

@skairali skairali left a comment

Choose a reason for hiding this comment

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

This will enable Presto consumers to have more secure configuration in this use case . Looks good to me

tdcmeehan
tdcmeehan requested changes Feb 26, 2024
View reviewed changes
presto-docs/src/main/sphinx/connector/hana.rst Outdated

The JDBC driver and connector automatically use Transport Layer Security (TLS) encryption and certificate validation. This requires a suitable TLS certificate configured on your Hana database host.

To disable encryption in the connection string, use the ``encrypt`` property:
Copy link
Contributor

@tdcmeehan tdcmeehan Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It appears actually the default is false? https://help.sap.com/docs/SAP_HANA_PLATFORM/b3ee5778bc2e4a089d3299b82ec762a7/2624cb2191be4f68897b91023ab41d0c.html

@steveburnett
Copy link
Contributor

Suggest revising the release note entry following the Documentation-only Pull Requests topic of the Release Notes Guidelines.

Something like:

== RELEASE NOTES ==
General Changes
* Add Connection security documentation for the Hana Connector

@sabbasani sabbasani dismissed stale reviews from skairali and steveburnett via bbd35c7 February 28, 2024 13:58
@sabbasani @steveburnett
Add TLS documentation for hana
579f6b1 
Co-authored-by: Steve Burnett <burnett@pobox.com>
@sabbasani
Copy link
Contributor Author

Suggest revising the release note entry following the Documentation-only Pull Requests topic of the Release Notes Guidelines.

Something like:

== RELEASE NOTES ==
General Changes
* Add Connection security documentation for the Hana Connector

@steveburnett I have updated RELEASE NOTES.

@tdcmeehan tdcmeehan merged commit eeda9c4 into prestodb:master Feb 28, 2024
57 checks passed
@wanglinsong wanglinsong mentioned this pull request May 1, 2024
Merged
48 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdcmeehan tdcmeehan tdcmeehan approved these changes

@presto-oss presto-oss Awaiting requested review from presto-oss presto-oss is a code owner automatically assigned from prestodb/committers

@skairali skairali Awaiting requested review from skairali

@steveburnett steveburnett Awaiting requested review from steveburnett steveburnett is a code owner

Assignees

@skairali skairali

Labels
docs Security
Projects
Presto Documentation
Status: ✅ Done
Security
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sabbasani @steveburnett @tdcmeehan @skairali