comments applied

elastic · Sep 19, 2023 · 230126e · 230126e
1 parent a75f74a
commit 230126e
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 47 deletions.
diff --git a/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go
@@ -122,13 +122,14 @@ func (v *Verifier) verifyAsc(fullPath string, skipDefaultPgp bool, pgpSources ..
 			continue
 		}
 		raw, err := download.PgpBytesFromSource(check, v.client)
+		if errors.Is(err, download.ErrRemotePGPDownloadFailed) {
+			v.log.Warnf("Skipped remote PGP located at %q because it's unavailable: %v", strings.TrimPrefix(check, download.PgpSourceURIPrefix), err)
+			continue
+		}
 		if err != nil {
-			if errors.Is(err, download.ErrRemotePGPDownloadFailed) {
-				v.log.Warnf("Skipped remote PGP located at %q because it's unavailable: %v", strings.TrimPrefix(check, download.PgpSourceURIPrefix), err)
-				continue
-			}
 			return err
 		}
+
 		if len(raw) == 0 {
 			continue
 		}

diff --git a/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier_test.go b/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier_test.go
@@ -175,9 +175,7 @@ func prepareFetchVerifyTests(dropPath, targetDir, targetFilePath, hashTargetFile
 func TestVerify(t *testing.T) {
 	log, obs := logger.NewTesting("TestVerify")
 	targetDir, err := ioutil.TempDir(os.TempDir(), "")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	timeout := 30 * time.Second
 
@@ -191,44 +189,37 @@ func TestVerify(t *testing.T) {
 		},
 	}
 
-	if err := prepareTestCase(beatSpec, version, config); err != nil {
-		t.Fatal(err)
-	}
+	err = prepareTestCase(beatSpec, version, config)
+	require.NoError(t, err)
 
 	testClient := NewDownloader(config)
 	artifact, err := testClient.Download(context.Background(), beatSpec, version)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		os.Remove(artifact)
+		os.Remove(artifact + ".sha512")
+		os.RemoveAll(config.DropPath)
+	})
 
 	_, err = os.Stat(artifact)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	testVerifier, err := NewVerifier(log, config, true, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	err = testVerifier.Verify(beatSpec, version, false)
 	require.NoError(t, err)
 
 	// log message informing remote PGP was skipped
 	logs := obs.FilterMessageSnippet("Skipped remote PGP located at")
-	require.Equal(t, 0, logs.Len())
-
-	os.Remove(artifact)
-	os.Remove(artifact + ".sha512")
-	os.RemoveAll(config.DropPath)
+	require.Empty(t, logs)
 }
 
 func TestVerifyUnreachableUri(t *testing.T) {
 	log, obs := logger.NewTesting("TestVerifyUnreachableUri")
 	targetDir, err := ioutil.TempDir(os.TempDir(), "")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	timeout := 30 * time.Second
 
@@ -242,25 +233,24 @@ func TestVerifyUnreachableUri(t *testing.T) {
 		},
 	}
 
-	if err := prepareTestCase(beatSpec, version, config); err != nil {
-		t.Fatal(err)
-	}
+	err = prepareTestCase(beatSpec, version, config)
+	require.NoError(t, err)
 
 	testClient := NewDownloader(config)
 	artifact, err := testClient.Download(context.Background(), beatSpec, version)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		os.Remove(artifact)
+		os.Remove(artifact + ".sha512")
+		os.RemoveAll(config.DropPath)
+	})
 
 	_, err = os.Stat(artifact)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	testVerifier, err := NewVerifier(log, config, true, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	unreachableURI := "https://127.0.0.1:3452/path/does/not/exist"
 	unreachablePGP := download.PgpSourceURIPrefix + unreachableURI
@@ -271,10 +261,6 @@ func TestVerifyUnreachableUri(t *testing.T) {
 	// log message informing remote PGP was skipped
 	logs := obs.FilterMessageSnippet(fmt.Sprintf("Skipped remote PGP located at %q", unreachableURI))
 	require.Equal(t, 1, logs.Len())
-
-	os.Remove(artifact)
-	os.Remove(artifact + ".sha512")
-	os.RemoveAll(config.DropPath)
 }
 
 func prepareTestCase(a artifact.Artifact, version string, cfg *artifact.Config) error {

diff --git a/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go
@@ -122,13 +122,14 @@ func (v *Verifier) verifyAsc(a artifact.Artifact, version string, skipDefaultPgp
 			continue
 		}
 		raw, err := download.PgpBytesFromSource(check, v.client)
+		if errors.Is(err, download.ErrRemotePGPDownloadFailed) {
+			v.log.Warnf("Skipped remote PGP located at %q because it's unavailable: %v", strings.TrimPrefix(check, download.PgpSourceURIPrefix), err)
+			continue
+		}
 		if err != nil {
-			if errors.Is(err, download.ErrRemotePGPDownloadFailed) {
-				v.log.Warnf("Skipped remote PGP located at %q because it's unavailable: %v", strings.TrimPrefix(check, download.PgpSourceURIPrefix), err)
-				continue
-			}
 			return err
 		}
+
 		if len(raw) == 0 {
 			continue
 		}