Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor(client/v2)!: remove client.Context #22493
base: main
Are you sure you want to change the base?
refactor(client/v2)!: remove client.Context #22493
Changes from 35 commits
962ed4b
528af7c
73d8d85
3ad45d6
004d071
a404604
30f57d3
9509d1d
06ffcbe
7e0583f
76556fc
e2af16d
3b2c8d5
f67063c
7482bef
f3320d6
5198686
1a8f7b5
68b5fed
fe28cec
4d47bc4
879db32
ed9d006
045e717
c6a699c
488c781
4401ad8
f81c59c
4727ae4
958c106
54254d4
79adc32
8c25235
ae69fec
dc85d30
2f4029a
6722779
f9b7320
6e4a04b
2be9f51
d0938d3
e0abf39
dab815d
2ff6fab
7421b85
2949a28
11ebebf
9c7eb26
e63aa2b
ff44a71
e289671
6cafaa8
6e1f112
a509f03
d8909ed
ae00cea
d4b36f9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
🛠️ Refactor suggestion
Set explicit server name in TLS configuration
When creating TLS credentials with
credentials.NewTLS
, theServerName
field intls.Config
is not set, which might cause hostname verification to be skipped. This can reduce the security of the TLS connection.Modify the TLS configuration to include the server name derived from the address:
Implement a helper function
extractHostname
to parse and return the hostname from theaddr
variable.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.
Correct the use of 'grpc.NewClient' to 'grpc.Dial'
The function
grpc.NewClient
does not exist in thegoogle.golang.org/grpc
package. The correct function to establish a gRPC connection isgrpc.Dial
.Apply this diff to fix the issue:
return grpc.NewClient(addr, []grpc.DialOption{grpc.WithTransportCredentials(creds)}...) +return grpc.Dial(addr, grpc.WithTransportCredentials(creds))
Ensure you handle the returned
*grpc.ClientConn
and associated errors appropriately.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.
🛠️ Refactor suggestion
Defaulting to insecure gRPC connections may pose security risks
In the
getQueryClientConn
function, theinsecure
variable is initialized totrue
, which could default to establishing insecure gRPC connections if thegrpc-insecure
flag is not explicitly set by the user. This may inadvertently expose the connection to man-in-the-middle attacks or other vulnerabilities.Recommend changing the default value of
insecure
tofalse
to prioritize secure connections by default. Apply this diff to implement the change:This ensures that the application defaults to secure gRPC connections, enhancing security. Users can still opt-in to insecure connections by explicitly setting the
--grpc-insecure
flag.📝 Committable suggestion