feat: add TLS for gRPC server endpoint #477
Conversation
cmd/server.go
Outdated
| grpcOpts = append(grpcOpts, oauth.NewAuthInterceptor(o.oauthGroup)) | ||
| } | ||
|
|
||
| if o.tls=="tlsup"{ |
There was a problem hiding this comment.
I prefer to use bool instead of string here if you just want to have an option to determine if you enable the TLS.
cmd/server.go
Outdated
| dir,_:=os.Getwd() | ||
| cwd:=filepath.Dir(dir) | ||
| flags.StringVarP(&opt.tls,"tls-grpc","",os.Getenv("TLS_MODE"),"The tls mode, supported: tlsup. Keep it empty to disable tls") | ||
| flags.StringVarP(&opt.tlsCert, "cert-file", "", filepath.Join(cwd, "certs",""), "The path to the certificate file") |
There was a problem hiding this comment.
IMO, the default value should be empty.
There was a problem hiding this comment.
I hope users can use relative paths to read the certificates, so I did it this way
There was a problem hiding this comment.
Actually, the default value is incorrect because it's a directory instead of a file path. Assume user A uses the CLI like the below:
atest server --cert-file my-certIt will read the file from the current directory.
certs/README.md
Outdated
There was a problem hiding this comment.
Please move it to https://github.com/LinuxSuRen/api-testing/tree/dd90d1e90ef00db1af0657782004c934ea5e16b9/docs.
You can rename the filename to tls.md or certificate.md as well.
LinuxSuRen
added
enhancement
LinuxSuRen
left a comment
LinuxSuRen
left a comment
There was a problem hiding this comment.
I cannot visit the web UI when setting the tls. See the errors below:
connection error: desc = \"transport: authentication handshake failed: tls: failed to verify certificate: x509: certificate is not valid for any names, but wanted to match localhost\"
This is my step to start server:
cd console/atest-ui
npm i && npm run build-only
cd -
openssl genrsa -out server.key 2048
openssl req -new -x509 -key server.key -out server.crt -days 36500 -subj "/C=US/ST=Denial/L=Springfield/O=Dis/CN=www.example.com"
go run . server --tls-grpc --cert-file server.crt --key-file server.key --console-path console/atest-ui/dist --local-storage 'bin/*.yaml'
You need to use SAN certificates, as only SAN certificates are currently supported. I understand that CN certificates have been deprecated |
|
Have you tested it from the web page?
|
|
LinuxSuRen
left a comment
LinuxSuRen
left a comment
There was a problem hiding this comment.
LGTM
I have tested it manually. It works well. And thanks for your effort.
| openssl genrsa -out server.key 2048 | ||
| # Generate self-signed certificate | ||
| openssl req -new -x509 -key server.key -out server.crt -days 36500 \ | ||
| -subj "/C=US/ST=Denial/L=Springfield/O=Dis/CN=www.example.com" |
There was a problem hiding this comment.
Usually the new line will start with a tab for a single command.
* chore(deps): update louislam/uptime-kuma docker tag to v1.23.3 * Update app version [skip ci] --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: github-action update-app-version <githubaction@githubaction.com>
2 participants
What type of PR is this?
Which issue(s) this PR fixes:
Fixes #