add validation for tls private key and cert file values #771
Conversation
devang-gaur
force-pushed
the
fix_769
branch
7 times, most recently
from
7a412d0
to
2ff24ca
Compare
Codecov Report
@@ Coverage Diff @@
## master #771 +/- ##
==========================================
+ Coverage 73.78% 74.75% +0.96%
==========================================
Files 110 111 +1
Lines 3285 3324 +39
==========================================
+ Hits 2424 2485 +61
+ Misses 677 654 -23
- Partials 184 185 +1
|
pkg/http-server/start.go
Outdated
| @@ -100,3 +116,27 @@ func (g *APIServer) start(routes []*Route, port, certFile, privateKeyFile string | |||
| } | |||
| logger.Info("server exiting gracefully") | |||
| } | |||
|
|
|||
| func validateFiles(privateKeyFile, certFile string) error { | |||
There was a problem hiding this comment.
we should add unit tests for the new functions added
pkg/http-server/start.go
Outdated
|
|
||
| if privateKeyFile != "" || certFile != "" { |
There was a problem hiding this comment.
we should add e2e server tests for this case
pkg/http-server/start.go
Outdated
| e2 := validateFileName(certFile) | ||
|
|
||
| if e1 != nil && e2 != nil { | ||
| return errors.Errorf("error with --key-path filename: %s, error with --cert-path flag value: %s", e1.Error(), e2.Error()) |
There was a problem hiding this comment.
is the go-errors required?
we could have wrapped errors with %w
pkg/http-server/start.go
Outdated
| if privateKeyFile != "" || certFile != "" { | ||
| logger.Debugf("certfile is %s, privateKeyFile is %s", certFile, privateKeyFile) | ||
|
|
||
| if err := validateFiles(privateKeyFile, certFile); err != nil { | ||
| logger.Fatal(err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Good change!
This piece of code can be moved into it's own function, this function can have more server related validations in the future.
Also, we can associate the newly created function with APIServer{}, since these are API server validations.
pkg/http-server/helpers_test.go
Outdated
| @@ -0,0 +1,58 @@ | |||
| package httpserver | |||
There was a problem hiding this comment.
Kindly add headers to all the newly added files!
pkg/utils/file.go
Outdated
| "strings" | ||
| ) | ||
|
|
||
| func ValidateFileName(file string) error { |
There was a problem hiding this comment.
I feel, we can get rid of this method!
devang-gaur
changed the title
|
Kudos, SonarCloud Quality Gate passed!
|
fixes #769