Skip to content

Commit

Permalink
Fixes bug in error handling within the VerifyFileExists method
Browse files Browse the repository at this point in the history
Fixes bug in error handling within the `VerifyFileExists` method that resulted in a panic when the error from os.Stat` was not `ErrNotExist`. The fix includes introducing the `CAKC058` error and log message for when the path to a file exists but is not a regular file.
  • Loading branch information
doodlesbykumbi committed Mar 25, 2021
1 parent 1e88260 commit cfb1e76
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 5 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
## Fixed
- Fixes bug in error handling within the `VerifyFileExists` method that resulted in a
panic when the error from `os.Stat` was not `ErrNotExist`. The fix includes introducing
the `CAKC058` error and log message for when the path to a file exists but is not a
regular file.
[cyberark/conjur-authn-k8s-client#252](https://github.com/cyberark/conjur-authn-k8s-client/issues/252)

## [0.19.1] - 2021-02-08
### Changed
Expand Down
1 change: 1 addition & 0 deletions pkg/log/log_messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,4 @@ const CAKC054 string = "CAKC054 Failed to delete file %s"
const CAKC055 string = "CAKC055 Cert placement failed with the following error:\n%s"
const CAKC056 string = "CAKC056 File %s does not exist"
const CAKC057 string = "CAKC057 Removing file %s"
const CAKC058 string = "CAKC058 Path exists but does not contain regular file: %s"
7 changes: 3 additions & 4 deletions pkg/utils/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,9 @@ func WaitForFile(

func VerifyFileExists(path string) error {
info, err := os.Stat(path)
if !os.IsNotExist(err) && info.Mode().IsRegular() {
// No error, the file exists
return nil
if err == nil && !info.Mode().IsRegular() {
// Path exists but is not a regular file
err = log.RecordedError(log.CAKC058, path)
}

return err
}
20 changes: 19 additions & 1 deletion pkg/utils/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,30 @@ func TestFile(t *testing.T) {
So(err, ShouldBeNil)
})

Convey("A non-existing file returns the error", func() {
Convey("A folder at the path returns an error", func() {
path := "/"
expectedOutput := fmt.Errorf(
"CAKC058 Path exists but does not contain regular file: %s",
path,
)

err := VerifyFileExists(path)

So(err, ShouldResemble, expectedOutput)
})

Convey("A non-existing file returns an error", func() {
path := "non/existing/path"

err := VerifyFileExists(path)

So(os.IsNotExist(err), ShouldBeTrue)
})

Convey("A non-ErrNotExist error is returned", func() {
err := VerifyFileExists("\000invalid")

So(err.Error(), ShouldResemble, "stat \x00invalid: invalid argument")
})
})
}

0 comments on commit cfb1e76

Please sign in to comment.