Add Debug Mode configuration and move detailed logs to Debug log level (

#167) * Add Debug Mode configuration and move detailed logs to Debug log level - Add configuration to allow Debug mode that will show detailed logs. By default, they will not be written, but can be added for investigation. - Change log level of detailed logs from Info to Debug * Remove log level suffix from all log identifiers Log suffix is redundant, as it is written in the same line with the log. This allows the log level change from Info to Debug without impacting the log identifier. * Add Unit Tests for logger and DEBUG env var config * Update CHANGELOG.md Co-authored-by: Geri Jennings <[email protected]> Co-authored-by: Geri Jennings <[email protected]>
cyberark · Oct 1, 2020 · ac965f4 · ac965f4
1 parent 7674a43
commit ac965f4
Show file tree

Hide file tree

Showing 22 changed files with 259 additions and 140 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,17 +5,27 @@ 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]
+### Added
+- Users can set the `DEBUG` environment variable to run the client in debug mode and view more log messages.
+  [cyberark/conjur-authn-k8s-client#134](https://github.com/cyberark/conjur-authn-k8s-client/issues/134)
+
 ### Changed
+- Detailed logs moved from Info to Debug log level to decrease verbosity of log messages.
+  [cyberark/conjur-authn-k8s-client#134](https://github.com/cyberark/conjur-authn-k8s-client/issues/134)
+- Log level suffix was removed from log identifiers (e.g. `CAKC001**E**` -> `CAKC001`). To
+  avoid conflicts, some log identifiers had to be changed. See [log_messages.go](https://github.com/cyberark/conjur-authn-k8s-client/blob/master/pkg/log/log_messages.go)
+  for updated log identifiers.
+  [cyberark/conjur-authn-k8s-client#134](https://github.com/cyberark/conjur-authn-k8s-client/issues/134)
 - Log messages now show microseconds, for clarity and easier troubleshooting.
-  ([cyberark/conjur-authn-k8s-client#164](https://github.com/cyberark/conjur-authn-k8s-client/issues/164))
+  [cyberark/conjur-authn-k8s-client#164](https://github.com/cyberark/conjur-authn-k8s-client/issues/164)
 
 ## [0.18.1] - 2020-09-13
 ### Fixed
 - Logs now correctly print only the Conjur identity without the policy branch prefix.
-  ([cyberark/conjur-authn-k8s-client#126](https://github.com/cyberark/conjur-authn-k8s-client/issues/126))
+  [cyberark/conjur-authn-k8s-client#126](https://github.com/cyberark/conjur-authn-k8s-client/issues/126)
 - When authentication fails, the exponential backoff retry is correctly reset so 
   that it will continue to attempt to authenticate until backoff is exhausted.
-  ([cyberark/conjur-authn-k8s-client#158](https://github.com/cyberark/conjur-authn-k8s-client/issues/158))
+  [cyberark/conjur-authn-k8s-client#158](https://github.com/cyberark/conjur-authn-k8s-client/issues/158)
 
 ### Changed
 - Wait slightly for the client certificate file to exist after login before
@@ -33,16 +43,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - The authenticator-client now runs as a limited user in the Docker image
   instead of as root, which is best practice and better follows the principle of
   least privilege 
-  ([cyberark/conjur-authn-k8s-client#111](https://github.com/cyberark/conjur-authn-k8s-client/pull/111))
+  [cyberark/conjur-authn-k8s-client#111](https://github.com/cyberark/conjur-authn-k8s-client/pull/111)
 
 ## [0.17.0] - 2020-04-07
 ### Added
 - Authenticator client prints its version upon startup (#93)
 
 ## [0.16.1] - 2020-02-18
 ### Fixed
-- Only publish to DockerHub / RH registry when there is a new version (#72, #74,
-  #79, #83)
+- Only publish to DockerHub / RH registry when there is a new version
+  (#72, #74, #79, #83)
 
 ### Changed
 - Clean up implementation of default CONJUR_VERSION and add unit tests (#80)

diff --git a/cmd/authenticator/main.go b/cmd/authenticator/main.go
@@ -13,19 +13,19 @@ import (
 )
 
 func main() {
-	log.Info(log.CAKC014I, authenticator.FullVersionName)
+	log.Info(log.CAKC048, authenticator.FullVersionName)
 
 	var err error
 
 	config, err := authnConfig.NewFromEnv()
 	if err != nil {
-		printErrorAndExit(log.CAKC018E)
+		printErrorAndExit(log.CAKC018)
 	}
 
 	// Create new Authenticator
 	authn, err := authenticator.New(*config)
 	if err != nil {
-		printErrorAndExit(log.CAKC019E)
+		printErrorAndExit(log.CAKC019)
 	}
 
 	// Configure exponential backoff
@@ -38,22 +38,25 @@ func main() {
 
 	err = backoff.Retry(func() error {
 		for {
-			log.Info(log.CAKC006I, authn.Config.Username)
+			log.Info(log.CAKC040, authn.Config.Username)
+
 			resp, err := authn.Authenticate()
 			if err != nil {
-				return log.RecordedError(log.CAKC016E)
+				return log.RecordedError(log.CAKC016)
 			}
 
 			err = authn.ParseAuthenticationResponse(resp)
 			if err != nil {
-				return log.RecordedError(log.CAKC020E)
+				return log.RecordedError(log.CAKC020)
 			}
 
+			log.Info(log.CAKC035)
+
 			if authn.Config.ContainerMode == "init" {
 				os.Exit(0)
 			}
 
-			log.Info(log.CAKC013I, authn.Config.TokenRefreshTimeout)
+			log.Info(log.CAKC047, authn.Config.TokenRefreshTimeout)
 
 			fmt.Println()
 			time.Sleep(authn.Config.TokenRefreshTimeout)
@@ -64,7 +67,7 @@ func main() {
 	}, expBackoff)
 
 	if err != nil {
-		printErrorAndExit(log.CAKC031E)
+		printErrorAndExit(log.CAKC031)
 	}
 }
 

diff --git a/design/fips-compliance.md b/design/fips-compliance.md
@@ -215,7 +215,7 @@ We will not implement the tests in bash scripts like we do in the `secrets-provi
 Regardless of how we will run our tests, it is not optimal that we have only
  a vanilla flow. We should add another test where in case the authenticator-client 
  fails to authenticate with Conjur we don't provide an access token
-  and the log shows `CAKC015E Login failed`.
+  and the log shows `CAKC015 Login failed`.
 
 We do not need to test different permutations of error flows (e.g host does
 not exist, host is not permitted on the `authn-k8s/prod` authenticator) as
@@ -225,7 +225,7 @@ these test run in the `conjur` repository. As far as the authenticator-client
 | **Scenario**            | **Given**                                                    | **When**                                                 | **Then**                                                                                               |
 |-------------------------|--------------------------------------------------------------|----------------------------------------------------------|--------------------------------------------------------------------------------------------------------|
 | Authentication succeeds | A Running Conjur cluster with a configured K8s Authenticator | I run the authenticator client with a valid k8s host     | An access token is provided to the application container and it can retrieve a secret with it          |
-| Authentication fails    | A Running Conjur cluster with a configured K8s Authenticator | I run the authenticator client with a non-valid k8s host | An access token is not provided to the application container and the log shows `CAKC015E Login failed` |
+| Authentication fails    | A Running Conjur cluster with a configured K8s Authenticator | I run the authenticator client with a non-valid k8s host | An access token is not provided to the application container and the log shows `CAKC015 Login failed` |
 
 ## Docs
 

diff --git a/go.mod b/go.mod
@@ -5,5 +5,6 @@ go 1.12
 require (
 	github.com/cenkalti/backoff v2.2.1+incompatible
 	github.com/fullsailor/pkcs7 v0.0.0-20190404230743-d7302db945fa
+	github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d
 	github.com/smartystreets/goconvey v0.0.0-20190731233626-505e41936337
 )
diff --git a/pkg/access_token/file/access_token.go b/pkg/access_token/file/access_token.go
@@ -22,15 +22,15 @@ func NewAccessToken(filePath string) (*AccessToken, error) {
 
 func (token AccessToken) Read() ([]byte, error) {
 	if token.Data == nil {
-		return nil, log.RecordedError(log.CAKC006E)
+		return nil, log.RecordedError(log.CAKC006)
 	}
 
 	return token.Data, nil
 }
 
 func (token *AccessToken) Write(Data []byte) (err error) {
 	if Data == nil {
-		return log.RecordedError(log.CAKC005E)
+		return log.RecordedError(log.CAKC005)
 	}
 
 	token.Data = Data
@@ -41,14 +41,14 @@ func (token *AccessToken) Write(Data []byte) (err error) {
 		err = os.MkdirAll(tokenDir, 755)
 		if err != nil {
 			// Do not specify the directory in the error message for security reasons
-			return log.RecordedError(log.CAKC004E)
+			return log.RecordedError(log.CAKC004)
 		}
 	}
 
 	err = ioutil.WriteFile(token.FilePath, token.Data, 0644)
 	if err != nil {
 		// Do not specify the file path in the error message for security reasons
-		return log.RecordedError(log.CAKC003E)
+		return log.RecordedError(log.CAKC003)
 	}
 
 	return nil
@@ -58,7 +58,7 @@ func (token *AccessToken) Delete() (err error) {
 	err = os.Remove(token.FilePath)
 	if err != nil {
 		// Do not specify the file path in the error message for security reasons
-		return log.RecordedError(log.CAKC002E)
+		return log.RecordedError(log.CAKC002)
 	}
 
 	// Clear Data

diff --git a/pkg/access_token/file/access_token_test.go b/pkg/access_token/file/access_token_test.go
@@ -49,7 +49,7 @@ func TestAccessTokenFile(t *testing.T) {
 				_, err := accessToken.Read()
 
 				Convey("Raises an error that the data is empty", func() {
-					So(err.Error(), ShouldEqual, log.CAKC006E)
+					So(err.Error(), ShouldEqual, log.CAKC006)
 				})
 			})
 		})
@@ -108,7 +108,7 @@ func TestAccessTokenFile(t *testing.T) {
 				err := accessToken.Write(nil)
 
 				Convey("Raises an error that the access token data is empty", func() {
-					So(err.Error(), ShouldEqual, log.CAKC005E)
+					So(err.Error(), ShouldEqual, log.CAKC005)
 				})
 			})
 		})
@@ -177,7 +177,7 @@ func TestAccessTokenFile(t *testing.T) {
 					err = accessToken.Delete()
 
 					Convey("Finishes with proper error", func() {
-						So(err.Error(), ShouldContainSubstring, log.CAKC002E)
+						So(err.Error(), ShouldContainSubstring, log.CAKC002)
 					})
 				})
 			})
@@ -209,7 +209,7 @@ func TestAccessTokenFile(t *testing.T) {
 						})
 
 						Convey("Raises the proper error", func() {
-							So(err.Error(), ShouldEqual, log.CAKC006E)
+							So(err.Error(), ShouldEqual, log.CAKC006)
 						})
 					})
 				})

diff --git a/pkg/access_token/memory/access_token.go b/pkg/access_token/memory/access_token.go
@@ -16,15 +16,15 @@ func NewAccessToken() (token *AccessToken, err error) {
 
 func (token AccessToken) Read() ([]byte, error) {
 	if token.Data == nil {
-		return nil, log.RecordedError(log.CAKC006E)
+		return nil, log.RecordedError(log.CAKC006)
 	}
 
 	return token.Data, nil
 }
 
 func (token *AccessToken) Write(Data []byte) (err error) {
 	if Data == nil {
-		return log.RecordedError(log.CAKC005E)
+		return log.RecordedError(log.CAKC005)
 	}
 
 	token.Data = Data

diff --git a/pkg/access_token/memory/access_token_test.go b/pkg/access_token/memory/access_token_test.go
@@ -48,7 +48,7 @@ func TestAccessTokenMemory(t *testing.T) {
 				_, err := accessToken.Read()
 
 				Convey("Raises an error that the data is empty", func() {
-					So(err.Error(), ShouldEqual, log.CAKC006E)
+					So(err.Error(), ShouldEqual, log.CAKC006)
 				})
 			})
 		})
@@ -79,7 +79,7 @@ func TestAccessTokenMemory(t *testing.T) {
 			err := accessToken.Write(nil)
 
 			Convey("Raises an error that the data is empty", func() {
-				So(err.Error(), ShouldEqual, log.CAKC005E)
+				So(err.Error(), ShouldEqual, log.CAKC005)
 			})
 		})
 	})
@@ -153,7 +153,7 @@ func TestAccessTokenMemory(t *testing.T) {
 						})
 
 						Convey("Raises the proper error", func() {
-							So(err.Error(), ShouldEqual, log.CAKC006E)
+							So(err.Error(), ShouldEqual, log.CAKC006)
 						})
 					})
 				})