From b05111fd063dc1d6d4ceaf70a30c883fa1089763 Mon Sep 17 00:00:00 2001 From: Googlom <36107508+Googlom@users.noreply.github.com> Date: Wed, 13 Aug 2025 14:40:12 +0300 Subject: [PATCH] chore(logging): clean/refactor up unnecessary warnings (#5448) Co-authored-by: Gulom Alimov --- cmd/awsS3Upload.go | 20 +++++++++----------- cmd/awsS3Upload_test.go | 8 ++++---- cmd/mavenBuild.go | 11 ++++------- cmd/vaultRotateSecretId.go | 15 +++++---------- cmd/vaultRotateSecretId_test.go | 8 +++++--- pkg/config/vault.go | 19 ++++--------------- pkg/docker/docker.go | 2 +- pkg/log/ansHook.go | 6 +++--- pkg/sonar/taskService.go | 3 --- pkg/vault/vault.go | 6 +++--- pkg/whitesource/scanUA.go | 1 - 11 files changed, 38 insertions(+), 61 deletions(-) diff --git a/cmd/awsS3Upload.go b/cmd/awsS3Upload.go index 679d2070a..a035bf687 100644 --- a/cmd/awsS3Upload.go +++ b/cmd/awsS3Upload.go @@ -3,6 +3,7 @@ package cmd import ( "context" "encoding/json" + "fmt" "os" "path/filepath" @@ -79,18 +80,16 @@ func runAwsS3Upload(configOptions *awsS3UploadOptions, client S3PutObjectAPI, bu err := filepath.Walk(configOptions.FilePath, func(currentFilePath string, f os.FileInfo, err error) error { // Handle Failure to prevent panic (e.g. in case of an invalid filepath) if err != nil { - log.Entry().WithError(err).Warnf("Failed to access path: '%v'", currentFilePath) - return err + return fmt.Errorf("failed to access path: '%v', error: %w", currentFilePath, err) } // Skip directories, only upload files if !f.IsDir() { log.Entry().Infof("Current target path is: '%v'", currentFilePath) // Open File - currentFile, e := os.Open(currentFilePath) - if e != nil { - log.Entry().WithError(e).Warnf("Could not open the file '%s'", currentFilePath) - return e + currentFile, err := os.Open(currentFilePath) + if err != nil { + return fmt.Errorf("failed to open file: '%v', error: %w", currentFilePath, err) } defer currentFile.Close() @@ -106,14 +105,13 @@ func runAwsS3Upload(configOptions *awsS3UploadOptions, client S3PutObjectAPI, bu // Upload File log.Entry().Infof("Start upload of file '%v'", currentFilePath) - _, e = PutFile(context.TODO(), client, inputObject) - if e != nil { - log.Entry().WithError(e).Warnf("There was an error during the upload of file '%v'", currentFilePath) - return e + _, err = PutFile(context.TODO(), client, inputObject) + if err != nil { + return fmt.Errorf("failed to upload file '%v', error: %w", currentFilePath, err) } log.Entry().Infof("Upload of file '%v' was successful!", currentFilePath) - return e + return nil } return nil }) diff --git a/cmd/awsS3Upload_test.go b/cmd/awsS3Upload_test.go index 4ded223b9..3f8901a55 100644 --- a/cmd/awsS3Upload_test.go +++ b/cmd/awsS3Upload_test.go @@ -6,7 +6,6 @@ package cmd import ( "context" "fmt" - "io/fs" "log" "os" "path/filepath" @@ -61,8 +60,8 @@ func TestRunAwsS3Upload(t *testing.T) { // test err := runAwsS3Upload(&config, client(t, config.FilePath), "fooBucket") // assert - _, ok := err.(*fs.PathError) - assert.True(t, ok) + assert.Error(t, err) + assert.Contains(t, err.Error(), "no such file or directory") }) t.Run("error bucket", func(t *testing.T) { @@ -87,7 +86,8 @@ func TestRunAwsS3Upload(t *testing.T) { // test err = runAwsS3Upload(&config, client(t, config.FilePath), "errorBucket") // assert - assert.EqualError(t, err, "expect fooBucket, got errorBucket") + assert.Error(t, err) + assert.Contains(t, err.Error(), "expect fooBucket, got errorBucket") }) } diff --git a/cmd/mavenBuild.go b/cmd/mavenBuild.go index 602496674..22fe1ac56 100644 --- a/cmd/mavenBuild.go +++ b/cmd/mavenBuild.go @@ -221,10 +221,7 @@ func runMavenBuild(config *mavenBuildOptions, _ *telemetry.CustomData, utils mav return err } if config.CreateBuildArtifactsMetadata { - err2, done := createBuildArtifactsMetadata(config, commonPipelineEnvironment) - if done { - return err2 - } + createBuildArtifactsMetadata(config, commonPipelineEnvironment) } return nil @@ -236,7 +233,7 @@ func runMavenBuild(config *mavenBuildOptions, _ *telemetry.CustomData, utils mav return err } -func createBuildArtifactsMetadata(config *mavenBuildOptions, commonPipelineEnvironment *mavenBuildCommonPipelineEnvironment) (error, bool) { +func createBuildArtifactsMetadata(config *mavenBuildOptions, commonPipelineEnvironment *mavenBuildCommonPipelineEnvironment) bool { fileUtils := &piperutils.Files{} buildCoordinates := []versioning.Coordinates{} options := versioning.Options{ @@ -267,7 +264,7 @@ func createBuildArtifactsMetadata(config *mavenBuildOptions, commonPipelineEnvir if len(buildCoordinates) == 0 { log.Entry().Warnf("unable to identify artifact coordinates for the maven packages published") - return nil, true + return true } var buildArtifacts build.BuildArtifacts @@ -275,7 +272,7 @@ func createBuildArtifactsMetadata(config *mavenBuildOptions, commonPipelineEnvir buildArtifacts.Coordinates = buildCoordinates jsonResult, _ := json.Marshal(buildArtifacts) commonPipelineEnvironment.custom.mavenBuildArtifacts = string(jsonResult) - return nil, false + return false } func createOrUpdateProjectSettingsXML(projectSettingsFile string, altDeploymentRepositoryID string, altDeploymentRepositoryUser string, altDeploymentRepositoryPassword string, utils maven.Utils) (string, error) { diff --git a/cmd/vaultRotateSecretId.go b/cmd/vaultRotateSecretId.go index 5f4a16496..ead60a02a 100644 --- a/cmd/vaultRotateSecretId.go +++ b/cmd/vaultRotateSecretId.go @@ -116,8 +116,7 @@ func runVaultRotateSecretID(utils vaultRotateSecretIDUtils) error { } if err = utils.UpdateSecretInStore(config, newSecretID); err != nil { - log.Entry().WithError(err).Warnf("Could not write secret back to secret store %s", config.SecretStore) - return err + return fmt.Errorf("could not write secret back to secret store %s: %w", config.SecretStore, err) } log.Entry().Infof("Secret has been successfully updated in secret store %s", config.SecretStore) @@ -160,26 +159,22 @@ func writeVaultSecretIDToStore(config *vaultRotateSecretIdOptions, secretID stri ctx, client, err := piperGithub.NewClientBuilder(config.GithubToken, config.GithubAPIURL).Build() if err != nil { - log.Entry().Warnf("Could not write secret ID back to GitHub Actions: GitHub client not created: %v", err) - return err + return fmt.Errorf("could not create GitHub client: %w", err) } publicKey, _, err := client.Actions.GetRepoPublicKey(ctx, config.Owner, config.Repository) if err != nil { - log.Entry().Warnf("Could not write secret ID back to GitHub Actions: repository's public key not retrieved: %v", err) - return err + return fmt.Errorf("could not retrieve repository's public key: %w", err) } encryptedSecret, err := piperGithub.CreateEncryptedSecret(config.VaultAppRoleSecretTokenCredentialsID, secretID, publicKey) if err != nil { - log.Entry().Warnf("Could not write secret ID back to GitHub Actions: secret encryption failed: %v", err) - return err + return fmt.Errorf("could not encrypt secret ID: %w", err) } _, err = client.Actions.CreateOrUpdateRepoSecret(ctx, config.Owner, config.Repository, encryptedSecret) if err != nil { - log.Entry().Warnf("Could not write secret ID back to GitHub Actions: submission to GitHub failed: %v", err) - return err + return fmt.Errorf("could not write secret ID back to GitHub Actions: submission to GitHub failed: %w", err) } default: return fmt.Errorf("error: invalid secret store: %s", config.SecretStore) diff --git a/cmd/vaultRotateSecretId_test.go b/cmd/vaultRotateSecretId_test.go index d346e73d1..0fd28b81d 100644 --- a/cmd/vaultRotateSecretId_test.go +++ b/cmd/vaultRotateSecretId_test.go @@ -5,9 +5,10 @@ package cmd import ( "fmt" - "github.com/stretchr/testify/assert" "testing" "time" + + "github.com/stretchr/testify/assert" ) type mockVaultRotateSecretIDUtilsBundle struct { @@ -100,6 +101,7 @@ func TestRunVaultRotateSecretID(t *testing.T) { }) t.Run("Error updating secret in store", func(t *testing.T) { + expectedErr := fmt.Errorf("failed to update secret in store") mock := &mockVaultRotateSecretIDUtilsBundle{ t: t, newSecret: "new-secret-id", @@ -110,13 +112,13 @@ func TestRunVaultRotateSecretID(t *testing.T) { }, updateFuncCalled: false, UpdateSecretFunc: func(config *vaultRotateSecretIdOptions, secretID string) error { - return fmt.Errorf("failed to update secret in store") + return expectedErr }, // Override the behavior } err := runVaultRotateSecretID(mock) assert.Error(t, err) - assert.EqualError(t, err, "failed to update secret in store") + assert.ErrorIs(t, err, expectedErr) assert.True(t, mock.updateFuncCalled) }) } diff --git a/pkg/config/vault.go b/pkg/config/vault.go index 8b299a24f..72cf7e2ac 100644 --- a/pkg/config/vault.go +++ b/pkg/config/vault.go @@ -177,9 +177,10 @@ func resolveVaultReference(ref *ResourceReference, config *StepConfig, client Va secretValue = lookupPath(client, vaultPath, ¶m) if secretValue != nil { log.Entry().WithField("vaultPath", vaultPath).Debug("Vault secret resolved successfully") - if ref.Type == "vaultSecret" { + switch ref.Type { + case "vaultSecret": config.Config[param.Name] = *secretValue - } else if ref.Type == "vaultSecretFile" { + case "vaultSecretFile": filePath, err := createTemporarySecretFile(param.Name, *secretValue) if err != nil { log.Entry().WithError(err).Warnf("Couldn't create temporary secret file for '%s'", param.Name) @@ -191,7 +192,7 @@ func resolveVaultReference(ref *ResourceReference, config *StepConfig, client Va } } if secretValue == nil { - log.Entry().Warn("Failed to resolve vault secret from all configured paths") + log.Entry().Info("The secret could not be resolved from Vault. Please check if the secret is available via configured paths.") } } @@ -513,15 +514,3 @@ func getSecretReferencePaths(reference *ResourceReference, config map[string]int } return retPaths } - -func toStringSlice(interfaceSlice []interface{}) []string { - retSlice := make([]string, 0, len(interfaceSlice)) - for _, vRaw := range interfaceSlice { - if v, ok := vRaw.(string); ok { - retSlice = append(retSlice, v) - continue - } - log.Entry().Warnf("'%s' needs to be of type string or an array of strings but got %T (%[2]v)", vaultPath, vRaw) - } - return retSlice -} diff --git a/pkg/docker/docker.go b/pkg/docker/docker.go index 30e287abf..4de096186 100644 --- a/pkg/docker/docker.go +++ b/pkg/docker/docker.go @@ -50,7 +50,7 @@ func MergeDockerConfigJSON(sourcePath, targetPath string, utils piperutils.FileU var targetConfig *configfile.ConfigFile if exists, _ := utils.FileExists(targetPath); !exists { - log.Entry().Warnf("target dockerConfigJSON file %q does not exist, creating a new one", sourcePath) + log.Entry().Infof("target dockerConfigJSON file %q does not exist, creating a new one", sourcePath) targetConfig = configfile.New(targetPath) } else { targetReader, err := utils.Open(targetPath) diff --git a/pkg/log/ansHook.go b/pkg/log/ansHook.go index d3d328d2c..7506fc498 100644 --- a/pkg/log/ansHook.go +++ b/pkg/log/ansHook.go @@ -2,11 +2,12 @@ package log import ( "fmt" + "os" + "strings" + "github.com/SAP/jenkins-library/pkg/ans" "github.com/pkg/errors" "github.com/sirupsen/logrus" - "os" - "strings" ) // ANSHook is used to set the hook features for the logrus hook @@ -122,7 +123,6 @@ func setupEventTemplate(customerEventTemplate, correlationID string) (ans.Event, if len(customerEventTemplate) > 0 { if err := event.MergeWithJSON([]byte(customerEventTemplate)); err != nil { - Entry().WithField("stepName", "ANS").Warnf("provided SAP Alert Notification Service event template '%s' could not be unmarshalled: %v", customerEventTemplate, err) return ans.Event{}, errors.Wrapf(err, "provided SAP Alert Notification Service event template '%s' could not be unmarshalled", customerEventTemplate) } } diff --git a/pkg/sonar/taskService.go b/pkg/sonar/taskService.go index b2ab4d737..5557a6181 100644 --- a/pkg/sonar/taskService.go +++ b/pkg/sonar/taskService.go @@ -65,9 +65,6 @@ func (service *TaskService) HasFinished() (bool, error) { if result.Task.Status == taskStatusPending || result.Task.Status == taskStatusProcessing { return false, nil } - // for _, warning := range result.Task.Warnings { - // log.Entry().Warnf("Warnings during analysis: %s", warning) - // } return true, nil } diff --git a/pkg/vault/vault.go b/pkg/vault/vault.go index c430109b5..9bcceddf6 100644 --- a/pkg/vault/vault.go +++ b/pkg/vault/vault.go @@ -191,18 +191,18 @@ func (c *Client) MustRevokeToken() { secret, err := c.GetSecret(lookupPath) if err != nil { - log.Entry().Warnf("Could not lookup token at %s, not continuing to revoke: %v", lookupPath, err) + log.Entry().Infof("Could not lookup token at %s, not continuing to revoke: %v", lookupPath, err) return } tokenID, ok := secret.Data["id"].(string) if !ok { - log.Entry().Warnf("Could not lookup token.Data.id at %s, not continuing to revoke", lookupPath) + log.Entry().Infof("Could not lookup token.Data.id at %s, not continuing to revoke", lookupPath) return } if !strings.HasPrefix(tokenID, serviceTokenPrefix) { - log.Entry().Warnf("Service token not identified at %s, not continuing to revoke", lookupPath) + log.Entry().Infof("Service token not identified at %s, not continuing to revoke", lookupPath) return } diff --git a/pkg/whitesource/scanUA.go b/pkg/whitesource/scanUA.go index 1e3e3fe01..4eb88e5df 100644 --- a/pkg/whitesource/scanUA.go +++ b/pkg/whitesource/scanUA.go @@ -226,7 +226,6 @@ func downloadAgent(config *ScanOptions, utils Utils) error { if strings.Contains(err.Error(), "unable to copy content from url to file") || strings.Contains(err.Error(), "returned with response 404 Not Found") || strings.Contains(err.Error(), "returned with response 403 Forbidden") { // retry the download once again log.Entry().Warnf("[Retry] Previous download failed due to %v", err) - err = nil // reset error to nil err = utils.DownloadFile(config.AgentDownloadURL, agentFile, nil, nil) } }