-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[DSIP-76] Support ssl in netty #16673
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to discuss this first in #16672
I think you miss the code of master-server .The task of other system sholud call other system in master-server . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requires adding at least one of the following tests to ensure stability.
- cluster-test
- api-test
- e2e-test
I will complete it later |
Is it adding UT, testing functions in the class? |
UT and these tests are different types of tests. You can find the corresponding examples in the following links. |
Thank you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Please add e2e test later
okay, I will do it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still many unaddressed issues in this PR. Such as e2e, K8S, docs, etc...So I don't think we can meet the requirements of merger at present.
Quality Gate passedIssues Measures |
@@ -200,4 +200,4 @@ jobs: | |||
if [[ ${{ needs.e2e.result }} != 'success' ]]; then | |||
echo "E2E Failed!" | |||
exit -1 | |||
fi | |||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert these unrelated changes, this can help the reviewer have a better review.
To enable SLL authentication, you have two things to do. Firstly, you need to generate `cert.crt` and `private.pem` files. | ||
|
||
Step 1: Generate private key (private.pem) | ||
|
||
Open the terminal and run the following command to generate a private key: | ||
|
||
```bash | ||
openssl genpkey -algorithm RSA -out private.pem -pkeyopt rsa_keygen_bits:2048 | ||
``` | ||
|
||
This command will generate a 2048 bit RSA private key and save it as a private.pem file. | ||
|
||
Step 2: Generate Certificate Signing Request (CSR) | ||
|
||
Before generating a certificate, you need to generate a Certificate Signing Request (CSR). Run the following command: | ||
|
||
```bash | ||
openssl req -new -key private.pem -out request.csr | ||
``` | ||
|
||
This command will prompt you to enter some information, such as country, state/province, organization name, etc. The information you input will be embedded into the generated certificate. | ||
|
||
Step 3: Generate a self signed certificate (cert.crt) | ||
|
||
Use CSR to generate self signed certificates. Run the following command: | ||
|
||
```bash | ||
openssl x509 -req -days 365 -in request.csr -signkey private.pem -out cert.crt | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to address how to generate cert.crt
and private.pem
.
RegistryConfiguration.class, | ||
NettySslConfig.class}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why mark these resolved?
@Data | ||
public class NettyRpcConfig { | ||
|
||
private NettySslConfig nettySslConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private NettySslConfig nettySslConfig; | |
private NettySslConfig nettySslConfig = new NettySslConfig(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add default value?
@Data | ||
public class NettySslConfig { | ||
|
||
public boolean enabled; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public boolean enabled; | |
public boolean enabled = false; |
close #16688