fix: properly calculate GCP storage bucket URIs for PRs and snapshots (#602) (#603)

* fix: search artifacts from the right folder

There is a bucket for snapshots and another for pull-requests

* fix: properly calculate GCP bucket URL for snapshots and PRs

* chore: include unit tests in the CI

* fix: wrong copy paste

Author: mdelapenya

.ci/Jenkinsfile

@@ -135,8 +135,8 @@ pipeline {
               }
               post {
                 always {
-                  junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/outputs/TEST-unit.xml")
-                  archiveArtifacts allowEmptyArchive: true, artifacts: "${BASE_DIR}/outputs/TEST-unit.xml"
+                  junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/outputs/TEST-unit-*.xml")
+                  archiveArtifacts allowEmptyArchive: true, artifacts: "${BASE_DIR}/outputs/TEST-unit-*.xml"
                 }
               }
             }

.ci/scripts/build-test.sh

@@ -15,4 +15,7 @@ mkdir -p $(pwd)/outputs
 go get -v -u gotest.tools/gotestsum
 
 # See https://pkg.go.dev/gotest.tools/gotestsum/#readme-junit-xml-output
-GOTESTSUM_JUNITFILE="$(pwd)/outputs/TEST-unit.xml" make -C cli install test
+GOTESTSUM_JUNITFILE="$(pwd)/outputs/TEST-unit-cli.xml" make -C cli install test
+
+# See https://pkg.go.dev/gotest.tools/gotestsum/#readme-junit-xml-output
+GOTESTSUM_JUNITFILE="$(pwd)/outputs/TEST-unit-e2e.xml" make -C e2e unit-test

e2e/Makefile

@@ -31,6 +31,8 @@ GO_IMAGE_TAG?='stretch'
 GOOS?='linux'
 GOARCH?='amd64'
 
+TEST_TIMEOUT?=5m
+
 .PHONT: build-docs
 build-docs:
 	rm -fr docs
@@ -84,3 +86,7 @@ notice:
 .PHONY: sync-integrations
 sync-integrations:
 	OP_LOG_LEVEL=${LOG_LEVEL} ./op sync integrations --delete
+
+.PHONY: unit-test
+unit-test:
+	gotestsum --format testname -- -count=1 -timeout=$(TEST_TIMEOUT) ./...

e2e/_suites/fleet/services.go

@@ -197,8 +197,13 @@ func downloadAgentBinary(artifact string, version string, OS string, arch string
 		log.Debug("Using CI snapshots for the Elastic Agent")
 
 		// We will use the snapshots produced by Beats CI
+		if extension == "tar.gz" {
+			fileName = fmt.Sprintf("%s-%s-%s-%s.%s", artifact, version, OS, arch, extension)
+		}
+
 		bucket := "beats-ci-artifacts"
-		object := fmt.Sprintf("snapshots/%s/%s", artifact, fileName)
+		prefix := fmt.Sprintf("snapshots/%s", artifact)
+		object := fileName
 
 		// we are setting a version from a pull request: the version of the artifact will be kept as the base one
 		// i.e. /pull-requests/pr-21100/elastic-agent/elastic-agent-7.x-SNAPSHOT-x86_64.rpm
@@ -213,12 +218,13 @@ func downloadAgentBinary(artifact string, version string, OS string, arch string
 				"agentVersion": agentVersionBase,
 				"PR":           version,
 			}).Debug("Using CI snapshots for a pull request")
-			object = fmt.Sprintf("pull-requests/%s/%s/%s", version, artifact, fileName)
+			prefix = fmt.Sprintf("pull-requests/%s", version)
+			object = fmt.Sprintf("%s/%s", artifact, fileName)
 		}
 
 		maxTimeout := time.Duration(timeoutFactor) * time.Minute
 
-		downloadURL, err = e2e.GetObjectURLFromBucket(bucket, object, maxTimeout)
+		downloadURL, err = e2e.GetObjectURLFromBucket(bucket, prefix, object, maxTimeout)
 		if err != nil {
 			return "", "", err
 		}

e2e/_testresources/gcp/nextPageParam.json

@@ -0,0 +1,4 @@
+{
+  "kind": "storage#objects",
+  "nextPageToken": "foo"
+}

e2e/_testresources/gcp/pull-requests.json

@@ -16,6 +16,7 @@ require (
 	github.com/pkg/errors v0.9.1
 	github.com/sirupsen/logrus v1.4.2
 	github.com/spf13/cobra v1.1.1 // indirect
+	github.com/stretchr/testify v1.6.1
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 // indirect
 )

e2e/_testresources/gcp/snapshots.json

@@ -211,7 +211,7 @@ func GetElasticArtifactURL(artifact string, version string, operativeSystem stri
 
 // GetObjectURLFromBucket extracts the media URL for the desired artifact from the
 // Google Cloud Storage bucket used by the CI to push snapshots
-func GetObjectURLFromBucket(bucket string, object string, maxtimeout time.Duration) (string, error) {
+func GetObjectURLFromBucket(bucket string, prefix string, object string, maxtimeout time.Duration) (string, error) {
 	exp := GetExponentialBackOff(maxtimeout)
 
 	retryCount := 1
@@ -222,14 +222,15 @@ func GetObjectURLFromBucket(bucket string, object string, maxtimeout time.Durati
 
 	storageAPI := func() error {
 		r := curl.HTTPRequest{
-			URL: fmt.Sprintf("https://storage.googleapis.com/storage/v1/b/%s/o?prefix=pull-requests%s", bucket, pageTokenQueryParam),
+			URL: fmt.Sprintf("https://storage.googleapis.com/storage/v1/b/%s/o?prefix=%s%s", bucket, prefix, pageTokenQueryParam),
 		}
 
 		response, err := curl.Get(r)
 		if err != nil {
 			log.WithFields(log.Fields{
 				"bucket":      bucket,
 				"elapsedTime": exp.GetElapsedTime(),
+				"prefix":      prefix,
 				"error":       err,
 				"object":      object,
 				"retry":       retryCount,
@@ -243,6 +244,7 @@ func GetObjectURLFromBucket(bucket string, object string, maxtimeout time.Durati
 		log.WithFields(log.Fields{
 			"bucket":      bucket,
 			"elapsedTime": exp.GetElapsedTime(),
+			"prefix":      prefix,
 			"object":      object,
 			"retries":     retryCount,
 			"url":         r.URL,
@@ -252,6 +254,7 @@ func GetObjectURLFromBucket(bucket string, object string, maxtimeout time.Durati
 		if err != nil {
 			log.WithFields(log.Fields{
 				"bucket": bucket,
+				"prefix": prefix,
 				"object": object,
 			}).Warn("Could not parse the response body for the object")
 
@@ -260,71 +263,58 @@ func GetObjectURLFromBucket(bucket string, object string, maxtimeout time.Durati
 			return err
 		}
 
-		items := jsonParsed.Path("items").Children()
-
-		log.WithFields(log.Fields{
-			"bucket":      bucket,
-			"elapsedTime": exp.GetElapsedTime(),
-			"objects":     len(items),
-			"retries":     retryCount,
-		}).Debug("Objects found")
-
-		for _, item := range items {
-			itemID := item.Path("id").Data().(string)
-			objectPath := bucket + "/" + object + "/"
-			if strings.HasPrefix(itemID, objectPath) {
-				mediaLink = item.Path("mediaLink").Data().(string)
-
-				log.WithFields(log.Fields{
-					"bucket":      bucket,
-					"elapsedTime": exp.GetElapsedTime(),
-					"medialink":   mediaLink,
-					"object":      object,
-					"retries":     retryCount,
-				}).Debug("Media link found for the object")
-				return nil
-			}
-
+		mediaLink, err = processBucketSearchPage(jsonParsed, currentPage, bucket, prefix, object)
+		if err != nil {
+			log.WithFields(log.Fields{
+				"currentPage": currentPage,
+				"bucket":      bucket,
+				"prefix":      prefix,
+				"object":      object,
+			}).Warn(err.Error())
+		} else if mediaLink != "" {
 			log.WithFields(log.Fields{
 				"bucket":      bucket,
 				"elapsedTime": exp.GetElapsedTime(),
+				"prefix":      prefix,
+				"medialink":   mediaLink,
 				"object":      object,
-				"itemID":      itemID,
 				"retries":     retryCount,
-			}).Trace("Media link not found")
+			}).Debug("Media link found for the object")
+			return nil
 		}
 
-		if jsonParsed.Path("nextPageToken") == nil {
+		pageTokenQueryParam = getBucketSearchNextPageParam(jsonParsed)
+		if pageTokenQueryParam == "" {
 			log.WithFields(log.Fields{
 				"currentPage": currentPage,
 				"bucket":      bucket,
+				"prefix":      prefix,
 				"object":      object,
 			}).Warn("Reached the end of the pages and the object was not found")
 
 			return nil
 		}
 
-		nextPageToken := jsonParsed.Path("nextPageToken").Data().(string)
-		pageTokenQueryParam = "&pageToken=" + nextPageToken
 		currentPage++
 
 		log.WithFields(log.Fields{
 			"currentPage": currentPage,
 			"bucket":      bucket,
 			"elapsedTime": exp.GetElapsedTime(),
+			"prefix":      prefix,
 			"object":      object,
 			"retries":     retryCount,
 		}).Warn("Object not found in current page. Continuing")
 
-		return fmt.Errorf("The %s object could not be found in the current page (%d) the %s bucket", object, currentPage, bucket)
+		return fmt.Errorf("The %s object could not be found in the current page (%d) the %s bucket and %s prefix", object, currentPage, bucket, prefix)
 	}
 
 	err := backoff.Retry(storageAPI, exp)
 	if err != nil {
 		return "", err
 	}
 	if mediaLink == "" {
-		return "", fmt.Errorf("Reached the end of the pages and the %s object was not found for the %s bucket", object, bucket)
+		return "", fmt.Errorf("Reached the end of the pages and the %s object was not found for the %s bucket and %s prefix", object, bucket, prefix)
 	}
 
 	return mediaLink, nil
@@ -406,6 +396,40 @@ func DownloadFile(url string) (string, error) {
 	return filepath, nil
 }
 
+func getBucketSearchNextPageParam(jsonParsed *gabs.Container) string {
+	token := jsonParsed.Path("nextPageToken")
+	if token == nil {
+		return ""
+	}
+
+	nextPageToken := token.Data().(string)
+	return "&pageToken=" + nextPageToken
+}
+
+func processBucketSearchPage(jsonParsed *gabs.Container, currentPage int, bucket string, prefix string, object string) (string, error) {
+	items := jsonParsed.Path("items").Children()
+
+	log.WithFields(log.Fields{
+		"bucket":  bucket,
+		"prefix":  prefix,
+		"objects": len(items),
+		"object":  object,
+	}).Debug("Objects found")
+
+	for _, item := range items {
+		itemID := item.Path("id").Data().(string)
+		objectPath := bucket + "/" + prefix + "/" + object + "/"
+		if strings.HasPrefix(itemID, objectPath) {
+			mediaLink := item.Path("mediaLink").Data().(string)
+
+			log.Infof("medialink: %s", mediaLink)
+			return mediaLink, nil
+		}
+	}
+
+	return "", fmt.Errorf("The %s object could not be found in the current page (%d) in the %s bucket and %s prefix", object, currentPage, bucket, prefix)
+}
+
 //nolint:unused
 func randomStringWithCharset(length int, charset string) string {
 	b := make([]byte, length)

e2e/go.mod

@@ -0,0 +1,86 @@
+package e2e
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	gabs "github.com/Jeffail/gabs/v2"
+)
+
+const bucket = "beats-ci-artifacts"
+const pullRequests = "pull-requests"
+const snapshots = "snapshots"
+
+var nextTokenParamJSON *gabs.Container
+var pullRequestsJSON *gabs.Container
+var snapshotsJSON *gabs.Container
+
+func init() {
+	nextTokenParamContent, err := ioutil.ReadFile("_testresources/gcp/nextPageParam.json")
+	if err != nil {
+		os.Exit(1)
+	}
+	nextTokenParamJSON, _ = gabs.ParseJSON([]byte(nextTokenParamContent))
+
+	pullRequestsContent, err := ioutil.ReadFile("_testresources/gcp/pull-requests.json")
+	if err != nil {
+		os.Exit(1)
+	}
+	pullRequestsJSON, _ = gabs.ParseJSON([]byte(pullRequestsContent))
+
+	snapshotsContent, err := ioutil.ReadFile("_testresources/gcp/snapshots.json")
+	if err != nil {
+		os.Exit(1)
+	}
+	snapshotsJSON, _ = gabs.ParseJSON([]byte(snapshotsContent))
+}
+
+func TestGetBucketSearchNextPageParam_HasMorePages(t *testing.T) {
+	expectedParam := "&pageToken=foo"
+
+	param := getBucketSearchNextPageParam(nextTokenParamJSON)
+	assert.True(t, param == expectedParam)
+}
+
+func TestGetBucketSearchNextPageParam_HasNoMorePages(t *testing.T) {
+	// this JSON file does not contain the tokken field
+	param := getBucketSearchNextPageParam(pullRequestsJSON)
+	assert.True(t, param == "")
+}
+
+func TestProcessBucketSearchPage_PullRequestsFound(t *testing.T) {
+	// retrieving last element in pull-requests.json
+	object := "pr-22495/filebeat/filebeat-8.0.0-SNAPSHOT-linux-amd64.docker.tar.gz"
+
+	mediaLink, err := processBucketSearchPage(pullRequestsJSON, 1, bucket, pullRequests, object)
+	assert.Nil(t, err)
+	assert.True(t, mediaLink == "https://storage.googleapis.com/download/storage/v1/b/beats-ci-artifacts/o/pull-requests%2Fpr-22495%2Ffilebeat%2Ffilebeat-8.0.0-SNAPSHOT-linux-amd64.docker.tar.gz?generation=1610634841693588&alt=media")
+}
+
+func TestProcessBucketSearchPage_PullRequestsNotFound(t *testing.T) {
+	object := "pr-fooo/filebeat/filebeat-8.0.0-SNAPSHOT-linux-amd64.docker.tar.gz"
+
+	mediaLink, err := processBucketSearchPage(pullRequestsJSON, 1, bucket, pullRequests, object)
+	assert.NotNil(t, err)
+	assert.True(t, mediaLink == "")
+}
+
+func TestProcessBucketSearchPage_SnapshotsFound(t *testing.T) {
+	// retrieving last element in snapshots.json
+	object := "filebeat/filebeat-oss-7.10.2-SNAPSHOT-arm64.deb"
+
+	mediaLink, err := processBucketSearchPage(snapshotsJSON, 1, bucket, snapshots, object)
+	assert.Nil(t, err)
+	assert.True(t, mediaLink == "https://storage.googleapis.com/download/storage/v1/b/beats-ci-artifacts/o/snapshots%2Ffilebeat%2Ffilebeat-oss-7.10.2-SNAPSHOT-arm64.deb?generation=1610629747796392&alt=media")
+}
+
+func TestProcessBucketSearchPage_SnapshotsNotFound(t *testing.T) {
+	object := "filebeat/filebeat-oss-7.12.2-SNAPSHOT-arm64.deb"
+
+	mediaLink, err := processBucketSearchPage(snapshotsJSON, 1, bucket, snapshots, object)
+	assert.NotNil(t, err)
+	assert.True(t, mediaLink == "")
+}