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.

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

service support https access #1036

Merged
merged 4 commits into from
Jun 18, 2020
Merged

service support https access #1036

merged 4 commits into from
Jun 18, 2020

Conversation

shzcore
Copy link
Contributor

@shzcore shzcore commented Jun 14, 2020

implement #445

@codecov
Copy link

codecov bot commented Jun 14, 2020

Codecov Report

Merging #1036 into master will decrease coverage by 0.12%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1036      +/-   ##
============================================
- Coverage     69.20%   69.08%   -0.13%     
- Complexity     5403     5422      +19     
============================================
  Files           328      329       +1     
  Lines         26334    26411      +77     
  Branches       3750     3762      +12     
============================================
+ Hits          18225    18246      +21     
- Misses         6331     6379      +48     
- Partials       1778     1786       +8     
Impacted Files Coverage Δ Complexity Δ
...in/java/com/baidu/hugegraph/server/RestServer.java 54.02% <30.76%> (-4.65%) 0.00 <0.00> (ø)
...java/com/baidu/hugegraph/config/ServerOptions.java 98.33% <100.00%> (+0.15%) 0.00 <0.00> (ø)
.../src/main/java/com/baidu/hugegraph/job/SysJob.java 71.42% <0.00%> (-28.58%) 3.00% <0.00%> (-1.00%)
.../java/com/baidu/hugegraph/auth/HugePermission.java 70.00% <0.00%> (-10.00%) 6.00% <0.00%> (-1.00%)
...ava/com/baidu/hugegraph/io/HugeGraphSONModule.java 77.66% <0.00%> (-4.16%) 11.00% <0.00%> (ø%)
...om/baidu/hugegraph/backend/store/BackendEntry.java 71.42% <0.00%> (-2.86%) 1.00% <0.00%> (ø%)
...om/baidu/hugegraph/api/filter/ExceptionFilter.java 68.36% <0.00%> (-2.61%) 0.00% <0.00%> (ø%)
...om/baidu/hugegraph/task/StandardTaskScheduler.java 85.26% <0.00%> (-1.27%) 67.00% <0.00%> (-2.00%)
...gegraph/backend/serializer/BinaryBackendEntry.java 82.27% <0.00%> (-0.44%) 28.00% <0.00%> (-1.00%)
...a/com/baidu/hugegraph/backend/query/Condition.java 77.94% <0.00%> (-0.37%) 32.00% <0.00%> (ø%)
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebe79aa...13074c4. Read the comment docs.

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

also update title "add sup https"

@@ -212,4 +212,27 @@ public static synchronized ServerOptions instance() {
disallowEmpty(),
"hugegraph:9fd95c9c-711b-415b-b85f-d4df46ba5c31"
);
public static final ConfigOption<String> KEYSTORE_SERVER_FILE =
new ConfigOption<>(
"server.keystore.file",
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer ssl.server_keystore_file

sslContext.setKeyStoreFile(keystoreServerFile);
sslContext.setKeyStorePass(keystoreServerPassword);
server = GrizzlyHttpServerFactory.createHttpServer(uri, rc, true,
new SSLEngineConfigurator(sslContext)
Copy link
Contributor

Choose a reason for hiding this comment

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

define a var for 'new SSLEngineConfigurator(sslContext)'


public static final ConfigOption<String> SECURITY_PROTOCOL =
new ConfigOption<>(
"security.protocol",
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer server.protocol

"server.keystore.password",
"The server keystore password.",
disallowEmpty(),
"abc123"
Copy link
Contributor

Choose a reason for hiding this comment

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

don't set by default, just keep empty

public static final ConfigOption<String> KEYSTORE_SERVER_FILE =
new ConfigOption<>(
"server.keystore.file",
"The server keystore file.",
Copy link
Contributor

Choose a reason for hiding this comment

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

improve description: The path of server keystore file used when https protocol is enabled

public static final ConfigOption<String> KEYSTORE_SERVER_PASSWORD =
new ConfigOption<>(
"server.keystore.password",
"The server keystore password.",
Copy link
Contributor

Choose a reason for hiding this comment

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

improve description


public static final ConfigOption<String> KEYSTORE_SERVER_PASSWORD =
new ConfigOption<>(
"server.keystore.password",
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer ssl.server_keystore_password

public static final ConfigOption<String> SECURITY_PROTOCOL =
new ConfigOption<>(
"security.protocol",
"security.protocol.",
Copy link
Contributor

Choose a reason for hiding this comment

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

improve description

"The server keystore file.",
"ssl.server_keystore_file",
"The path of server keystore file used when https " +
"protocol is enabled.",
Copy link
Contributor

Choose a reason for hiding this comment

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

align with "The path..."

"abc123"
"ssl.server_keystore_password",
"The password of the path of the server keystore file " +
"used when the https protocol is enabled.",
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

"security.protocol",
"security.protocol.",
"server.protocol",
"Fill in when https protocol is enabled.",
Copy link
Contributor

Choose a reason for hiding this comment

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

The protocol of rest-server, allowed values are: http or https.

new ConfigOption<>(
"server.protocol",
"Fill in when https protocol is enabled.",
disallowEmpty(),
Copy link
Contributor

Choose a reason for hiding this comment

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

allowValues("http", "https")

javeme
javeme previously approved these changes Jun 17, 2020
@shzcore shzcore changed the title add sup https service support https access Jun 17, 2020
houzhizhen
houzhizhen previously approved these changes Jun 18, 2020
"server.protocol",
"The protocol of rest-server, allowed values are: " +
"http or https.",
allowValues("http","https"),
Copy link
Contributor

Choose a reason for hiding this comment

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

add an space between "http" and "https"

@shzcore shzcore dismissed stale reviews from houzhizhen and javeme via 13074c4 June 18, 2020 02:23
@javeme javeme merged commit 0db1aba into apache:master Jun 18, 2020
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.

None yet

5 participants