feat(dgraph): enabling TLS config in http zero (#6691) #6867
Conversation
* enabling TLS config in http zero * making zero https configured * changing behaviour of cmux + adding test cases * fixing zero address in test * fixing docker files * adding alpha in docker compose * fixing test generate cert pool * renaming functions based on review * making zero https more vigilant with more checks * changing the enabled to disabled flag * fixing test case * fixing zero cmd flag desc and refactoring test cases
aman-bansal
requested review from
manishrjain and
vvbalaji-dgraph
as code owners
![codelingo[bot]](https://avatars.githubusercontent.com/in/14339?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| var testCasesHttps = []testCase{ |
There was a problem hiding this comment.
Avoid global variables to improve readability and reduce complexity
View Rule (ignored by CLAIR)
There was a problem hiding this comment.
@codelingo ignore
There was a problem hiding this comment.
This specific issue is being ignored for future reviews of this PR.
| response string | ||
| } | ||
|
|
||
| var testCasesHttp = []testCase{ |
There was a problem hiding this comment.
Avoid global variables to improve readability and reduce complexity
View Rule (ignored by CLAIR)
There was a problem hiding this comment.
@codelingo ignore
There was a problem hiding this comment.
This specific issue is being ignored for future reviews of this PR.
| response string | ||
| } | ||
|
|
||
| var testCasesHttp = []testCase{ |
There was a problem hiding this comment.
Avoid global variables to improve readability and reduce complexity
View Rule (ignored by CLAIR)
There was a problem hiding this comment.
@codelingo ignore
There was a problem hiding this comment.
This specific issue is being ignored for future reviews of this PR.
| return body | ||
| } | ||
|
|
||
| func generateCertPool(certPath string, useSystemCA bool) (*x509.CertPool, error) { |
There was a problem hiding this comment.
Boolean arguments can indicate low cohesion. Consider refactoring generateCertPool by using a separate function for each case and helper functions for repeated code. This will make each function clearer and more modular, leading to easier maintainability.
View Rule (ignored by CLAIR)
There was a problem hiding this comment.
@codelingo ignore
There was a problem hiding this comment.
This specific issue is being ignored for future reviews of this PR.
There was a problem hiding this comment.
this is test file
| pool, err := generateCertPool("../../tls/ca.crt", true) | ||
| require.NoError(t, err) | ||
|
|
||
| tlsCfg := &tls.Config{RootCAs: pool, ServerName: "localhost", InsecureSkipVerify: true} |
There was a problem hiding this comment.
Skipping verification exposes the connection to man-in-the-middle attacks. This should only be used for testing.
View Rule (ignored by CLAIR)
There was a problem hiding this comment.
@codelingo ignore
There was a problem hiding this comment.
This specific issue is being ignored for future reviews of this PR.
There was a problem hiding this comment.
this is test file
| defer cancel() | ||
| err = srv.Shutdown(ctx) | ||
| glog.Infoln("All http(s) requests finished.") | ||
| if err != nil { |
There was a problem hiding this comment.
Error check should immediately follow err assignment.
View Rule (ignored by CLAIR)
There was a problem hiding this comment.
@codelingo ignore
There was a problem hiding this comment.
This specific issue is being ignored for future reviews of this PR.
enabling TLS encryption in zero
This change is
Docs Preview: