From 92f3af452bd5ee0d0e62daa0e0a5190bfa134eea Mon Sep 17 00:00:00 2001
From: robnester-rh <rnester@redhat.com>
Date: Thu, 5 Sep 2024 09:18:02 -0400
Subject: [PATCH] EC-806 Ensure ec uses go-gather by default

This commit removes the check for the env var `USEGOGATHER` and instead
has ec use the go-gather downloader. As we've previously verified that
the data downloaded by go-gather matches the information used via the
prior downloader implementation, the corresponding downloader code has
been removed in `internal/downloader/downloader.go`. The associated
tests in `internal/downloader/downloader_test.go` has been updated.
Additionally, since we're no longer using branching logic to determine
if we should use go-gather for downloading, the `UseGoGather` function
in `internal/utils/helpers.go` has been removed.

Signed-off-by: robnester-rh <rnester@redhat.com>
---
 internal/downloader/downloader.go      | 41 +-------------------------
 internal/downloader/downloader_test.go | 26 ++--------------
 internal/utils/helpers.go              |  4 ---
 3 files changed, 3 insertions(+), 68 deletions(-)

diff --git a/internal/downloader/downloader.go b/internal/downloader/downloader.go
index d0bd0f267..645d9026d 100644
--- a/internal/downloader/downloader.go
+++ b/internal/downloader/downloader.go
@@ -23,14 +23,10 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
-	"sync"
 
 	"github.com/enterprise-contract/go-gather/gather"
 	"github.com/enterprise-contract/go-gather/metadata"
-	"github.com/open-policy-agent/conftest/downloader"
 	log "github.com/sirupsen/logrus"
-
-	"github.com/enterprise-contract/ec-cli/internal/utils"
 )
 
 type key int
@@ -43,17 +39,12 @@ type downloadImpl interface {
 
 var gatherFunc = gather.Gather
 
-var dlMutex sync.Mutex
-
 // WithDownloadImpl replaces the downloadImpl implementation used
 func WithDownloadImpl(ctx context.Context, d downloadImpl) context.Context {
 	return context.WithValue(ctx, downloadImplKey, d)
 }
 
 // Download is used to download files from various sources.
-//
-// Note that it handles just one url at a time even though the equivalent
-// Conftest function can take a list of source urls.
 func Download(ctx context.Context, destDir string, sourceUrl string, showMsg bool) (metadata.Metadata, error) {
 	if !isSecure(sourceUrl) {
 		return nil, fmt.Errorf("attempting to download from insecure source: %s", sourceUrl)
@@ -65,40 +56,10 @@ func Download(ctx context.Context, destDir string, sourceUrl string, showMsg boo
 		fmt.Println(msg)
 	}
 
-	dl := func(ctx context.Context, sourceUrl, destDir string) (metadata.Metadata, error) {
-		// conftest's Download function leverages oras under the hood to fetch from OCI. It uses the
-		// global oras client and sets the user agent to "conftest". This is not a thread safe
-		// operation. Here we get around this limitation by ensuring a single download happens at a
-		// time.
-		dlMutex.Lock()
-		defer dlMutex.Unlock()
-		return nil, downloader.Download(ctx, destDir, []string{sourceUrl})
-	}
-
-	if utils.UseGoGather() {
-		dl = func(ctx context.Context, sourceUrl, destDir string) (metadata.Metadata, error) {
-			m, err := gatherFunc(ctx, sourceUrl, destDir)
-			if err != nil {
-				log.Debug("Download failed!")
-			}
-			return m, err
-		}
-	} else if d, ok := ctx.Value(downloadImplKey).(downloadImpl); ok {
-		dl = func(ctx context.Context, sourceUrl, destDir string) (metadata.Metadata, error) {
-			err := d.Download(ctx, destDir, []string{sourceUrl})
-			if err != nil {
-				log.Debug("Download failed!")
-			}
-			return nil, err
-		}
-	}
-
-	m, err := dl(ctx, sourceUrl, destDir)
-
+	m, err := gatherFunc(ctx, sourceUrl, destDir)
 	if err != nil {
 		log.Debug("Download failed!")
 	}
-
 	return m, err
 }
 
diff --git a/internal/downloader/downloader_test.go b/internal/downloader/downloader_test.go
index 85958eda1..4ea579cd1 100644
--- a/internal/downloader/downloader_test.go
+++ b/internal/downloader/downloader_test.go
@@ -21,7 +21,6 @@ package downloader
 import (
 	"context"
 	"errors"
-	"os"
 	"testing"
 
 	"github.com/enterprise-contract/go-gather/metadata"
@@ -46,27 +45,12 @@ func TestDownloader_Download(t *testing.T) {
 		source      string
 		errExpected bool
 		err         error
-		useGoGather bool
 	}{
 		{
 			name:   "Downloads",
 			dest:   "dir",
 			source: "https://example.com/org/repo.git",
 		},
-		{
-			name:        "Downloads with go-gather",
-			dest:        "dir",
-			source:      "https://example.com/org/repo.git",
-			useGoGather: true,
-		},
-		{
-			name:        "Fails to download with go-gather",
-			dest:        "dir",
-			source:      "https://example.com/org/repo.git",
-			errExpected: true,
-			err:         errors.New("expected error with go-gather"),
-			useGoGather: true,
-		},
 		{
 			name:        "Fails to download",
 			dest:        "dir",
@@ -93,14 +77,8 @@ func TestDownloader_Download(t *testing.T) {
 				gatherFunc = originalGatherFunction
 			}()
 
-			if tt.useGoGather {
-				t.Setenv("USEGOGATHER", "1")
-				gatherFunc = func(_ context.Context, _ string, _ string) (metadata.Metadata, error) {
-					return nil, tt.err
-				}
-			} else {
-				os.Unsetenv("USEGOGATHER")
-				d.On("Download", ctx, tt.dest, []string{tt.source}).Return(tt.err)
+			gatherFunc = func(_ context.Context, _ string, _ string) (metadata.Metadata, error) {
+				return nil, tt.err
 			}
 
 			_, err := Download(ctx, tt.dest, tt.source, false)
diff --git a/internal/utils/helpers.go b/internal/utils/helpers.go
index cf0284397..7235a6094 100644
--- a/internal/utils/helpers.go
+++ b/internal/utils/helpers.go
@@ -124,10 +124,6 @@ func Experimental() bool {
 	return os.Getenv("EC_EXPERIMENTAL") == "1"
 }
 
-func UseGoGather() bool {
-	return os.Getenv("USEGOGATHER") == "1"
-}
-
 // detect if the string is json
 func IsJson(data string) bool {
 	var jsMsg json.RawMessage