Make downloader.go testable

- Decouple `QuerySource` and `GetRegistryOptions` from the downloader
  struct.

- Create the `sourceQuerier` and `registryOptionsGetter` interfaces, so
  it would be possible to mock the real implementation in the unit
  tests.

- Add the `appRegistrySourceQuerier` struct which implements the `sourceQuerier`
  interface with the same functionality that exist before this change.

- Add the `secretRegistryOptionsGetter` struct which implements the `registryOptionsGetter`
  interface with the same functionality that exist before this change.

- Add a unit test for downloader.Prepare.

Signed-off-by: gbenhaim <galbh2@gmail.com>

Author: gbenhaim

go.mod

@@ -28,6 +28,7 @@ require (
 	k8s.io/apimachinery v0.0.0-20191004115801-a2eda9f80ab8
 	k8s.io/client-go v0.0.0-20191016111102-bec269661e48
 	k8s.io/klog v1.0.0
+	k8s.io/kubectl v0.0.0-20191016120415-2ed914427d51
 )
 
 replace github.com/docker/docker => github.com/moby/moby v0.7.3-0.20190826074503-38ab9da00309 // Required by Helm

go.sum

@@ -16,6 +16,7 @@ github.com/Azure/go-autorest/tracing v0.5.0/go.mod h1:r/s2XiOKccPW3HrqB+W0TQzfbt
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
 github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E=
+github.com/MakeNowJust/heredoc v0.0.0-20171113091838-e9091a26100e h1:eb0Pzkt15Bm7f2FFYv7sjY7NPFi3cPkS3tv1CcrFBWA=
 github.com/MakeNowJust/heredoc v0.0.0-20171113091838-e9091a26100e/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E=
 github.com/Masterminds/goutils v1.1.0/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU=
 github.com/Masterminds/semver/v3 v3.0.1 h1:2kKm5lb7dKVrt5TYUiAavE6oFc1cFT0057UVGT+JqLk=
@@ -126,6 +127,7 @@ github.com/docker/go-units v0.3.3/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDD
 github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
 github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7/go.mod h1:cyGadeNEkKy96OOhEzfZl+yxihPEzKnqJwvfuSUqbZE=
 github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM=
+github.com/docker/spdystream v0.0.0-20181023171402-6480d4af844c h1:ZfSZ3P3BedhKGUhzj7BQlPSU4OvT6tfOKe3DVHzOA7s=
 github.com/docker/spdystream v0.0.0-20181023171402-6480d4af844c/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM=
 github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=
 github.com/eapache/go-resiliency v1.1.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs=
@@ -140,6 +142,7 @@ github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT
 github.com/emicklei/go-restful v2.11.1+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
 github.com/evanphx/json-patch v4.2.0+incompatible h1:fUDGZCv/7iAN7u0puUVhvKCcsR6vRfwrJatElLBEf0I=
 github.com/evanphx/json-patch v4.2.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
+github.com/evanphx/json-patch v4.5.0+incompatible h1:ouOWdg56aJriqS0huScTkVXPC5IcNrDCXZ6OoTAWu7M=
 github.com/evanphx/json-patch v4.5.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
 github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d/go.mod h1:ZZMPRZwes7CROmyNKgQzC3XPs6L/G2EJLHddWejkmf4=
 github.com/fatih/camelcase v1.0.0/go.mod h1:yN2Sb0lFhZJUdVvtELVWefmrXpuZESvPmqwoZc+/fpc=
@@ -370,11 +373,13 @@ github.com/miekg/dns v0.0.0-20181005163659-0d29b283ac0f/go.mod h1:W1PPwlIAgtquWB
 github.com/mitchellh/copystructure v1.0.0 h1:Laisrj+bAB6b/yJwB5Bt3ITZhGJdqmxquMKeZ+mmkFQ=
 github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
+github.com/mitchellh/go-wordwrap v1.0.0 h1:6GlHJ/LTGMrIJbwgdqdl2eEH8o+Exx/0m8ir9Gns0u4=
 github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
 github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQzvN1EDeE=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
 github.com/mitchellh/reflectwalk v1.0.0 h1:9D+8oIskB4VJBN5SFlmc27fSlIBZaov1Wpk/IfikLNY=
 github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
+github.com/moby/moby v0.7.3-0.20190826074503-38ab9da00309 h1:cvy4lBOYN3gKfKj8Lzz5Q9TfviP+L7koMHY7SvkyTKs=
 github.com/moby/moby v0.7.3-0.20190826074503-38ab9da00309/go.mod h1:fDXVQ6+S340veQPv35CzDahGBmHsiclFwfEygB/TWMc=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=
@@ -732,7 +737,9 @@ k8s.io/klog v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8=
 k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I=
 k8s.io/kube-openapi v0.0.0-20190816220812-743ec37842bf h1:EYm5AW/UUDbnmnI+gK0TJDVK9qPLhM+sRHYanNKw0EQ=
 k8s.io/kube-openapi v0.0.0-20190816220812-743ec37842bf/go.mod h1:1TqjTSzOxsLGIKfj0lK8EeCP7K1iUG65v09OM0/WG5E=
+k8s.io/kube-openapi v0.0.0-20190918143330-0270cf2f1c1d h1:Xpe6sK+RY4ZgCTyZ3y273UmFmURhjtoJiwOMbQsXitY=
 k8s.io/kube-openapi v0.0.0-20190918143330-0270cf2f1c1d/go.mod h1:1TqjTSzOxsLGIKfj0lK8EeCP7K1iUG65v09OM0/WG5E=
+k8s.io/kubectl v0.0.0-20191016120415-2ed914427d51 h1:RBkTKVMF+xsNsSOVc0+HdC0B5gD1sr6s6Cu5w9qNbuQ=
 k8s.io/kubectl v0.0.0-20191016120415-2ed914427d51/go.mod h1:gL826ZTIfD4vXTGlmzgTbliCAT9NGiqpCqK2aNYv5MQ=
 k8s.io/metrics v0.0.0-20191016113814-3b1a734dba6e/go.mod h1:ve7/vMWeY5lEBkZf6Bt5TTbGS3b8wAxwGbdXAsufjRs=
 k8s.io/utils v0.0.0-20190801114015-581e00157fb1 h1:+ySTxfHnfzZb9ys375PXNlLhkJPLKgHajBU0N62BDvE=

pkg/appregistry/appregistry.go

@@ -38,10 +38,7 @@ func NewLoader(kubeconfig string, dbName string, downloadPath string, logger *lo
 		input: &inputParser{
 			sourceSpecifier: specifier,
 		},
-		downloader: &downloader{
-			logger:     logger,
-			kubeClient: *kubeClient,
-		},
+		downloader:   newDownloader(logger, kubeClient),
 		downloadPath: downloadPath,
 		decoder:      decoder,
 		loader:       loader,

pkg/appregistry/downloader.go

@@ -26,9 +26,39 @@ func (d *downloadItem) String() string {
 	return fmt.Sprintf("%s", d.RepositoryMetadata)
 }
 
+type registryOptionsGetter interface {
+	GetRegistryOptions(source *Source) (*apprclient.Options, error)
+}
+
+type secretRegistryOptionsGetter struct {
+	kubeClient kubernetes.Interface
+}
+
+type sourceQuerier interface {
+	QuerySource(source *Source) ([]*apprclient.RegistryMetadata, error)
+}
+
+type appRegistrySourceQuerier struct {
+	kubeClient      kubernetes.Interface
+	regOptionGetter registryOptionsGetter
+}
+
 type downloader struct {
-	logger     *logrus.Entry
-	kubeClient kubernetes.Clientset
+	logger          *logrus.Entry
+	kubeClient      kubernetes.Interface
+	querier         sourceQuerier
+	regOptionGetter registryOptionsGetter
+}
+
+// NewDownloader returns a new instance of downloader
+func newDownloader(logger *logrus.Entry, kubeClient kubernetes.Interface) *downloader {
+	regOptionGetter := &secretRegistryOptionsGetter{kubeClient}
+	return &downloader{
+		logger,
+		kubeClient,
+		&appRegistrySourceQuerier{kubeClient, regOptionGetter},
+		regOptionGetter,
+	}
 }
 
 // Download downloads manifest(s) associated with the specified package(s) from
@@ -74,7 +104,7 @@ func (d *downloader) Prepare(input *Input) (items []*downloadItem, err error) {
 			break
 		}
 
-		repositoryList, err := d.QuerySource(source)
+		repositoryList, err := d.querier.QuerySource(source)
 		if err != nil {
 			allErrors = append(allErrors, err)
 			d.logger.Infof("skipping operator source due to error: %s", source)
@@ -126,7 +156,7 @@ func (d *downloader) DownloadRepositories(items []*downloadItem) (manifests []*a
 
 		d.logger.Infof("downloading repository: %s from %s", item.RepositoryMetadata, endpoint)
 
-		options, err := d.SetupRegistryOptions(item.Source)
+		options, err := d.regOptionGetter.GetRegistryOptions(item.Source)
 		if err != nil {
 			allErrors = append(allErrors, err)
 			d.logger.Infof("skipping repository: %s", item.RepositoryMetadata)
@@ -164,12 +194,12 @@ func (d *downloader) DownloadRepositories(items []*downloadItem) (manifests []*a
 // The function returns the spec ( associated with the OperatorSource object )
 // in the cluster and the list of repositories in remote registry associated
 // with it.
-func (d *downloader) QuerySource(source *Source) (repositories []*apprclient.RegistryMetadata, err error) {
+func (a *appRegistrySourceQuerier) QuerySource(source *Source) (repositories []*apprclient.RegistryMetadata, err error) {
 	if source == nil {
 		return nil, errors.New("specified source is <nil>")
 	}
 
-	options, err := d.SetupRegistryOptions(source)
+	options, err := a.regOptionGetter.GetRegistryOptions(source)
 	if err != nil {
 		return
 	}
@@ -187,17 +217,17 @@ func (d *downloader) QuerySource(source *Source) (repositories []*apprclient.Reg
 	return
 }
 
-// SetupRegistryOptions generates an Options object based on the OperatorSource spec. It passes along
+// GetRegistryOptions generates an Options object based on the OperatorSource spec. It passes along
 // the opsrc endpoint and, if defined, retrieves the authorization token from the specified Secret
 // object.
-func (d *downloader) SetupRegistryOptions(source *Source) (*apprclient.Options, error) {
+func (s *secretRegistryOptionsGetter) GetRegistryOptions(source *Source) (*apprclient.Options, error) {
 	if source == nil {
 		return nil, errors.New("specified source is <nil>")
 	}
 
 	token := ""
 	if source.IsSecretSpecified() {
-		secret, err := d.kubeClient.CoreV1().Secrets(source.Secret.Namespace).Get(source.Secret.Name, metav1.GetOptions{})
+		secret, err := s.kubeClient.CoreV1().Secrets(source.Secret.Namespace).Get(source.Secret.Name, metav1.GetOptions{})
 		if err != nil {
 			return nil, err
 		}

pkg/appregistry/downloader_test.go

@@ -0,0 +1,132 @@
+package appregistry
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/operator-framework/operator-registry/pkg/apprclient"
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+var testPrepare = []struct {
+	input                 *Input
+	sourceQuerier         sourceQuerier
+	expectedDownloadItems []*downloadItem
+	expectedError         utilerrors.Aggregate
+}{
+	{
+		input: &Input{
+			Sources: []*Source{
+				{
+					Endpoint:          "quay.io",
+					RegistryNamespace: "",
+				},
+			},
+			Packages: []string{
+				"Kubevirt",
+				"etcd",
+			},
+		},
+		sourceQuerier: &fakeSourceQuerier{
+			map[Source][]*apprclient.RegistryMetadata{
+				Source{
+					Endpoint:          "quay.io",
+					RegistryNamespace: "",
+				}: {
+					&apprclient.RegistryMetadata{Name: "Kubevirt"},
+				},
+			},
+			map[Source]error{},
+		},
+		expectedDownloadItems: []*downloadItem{
+			&downloadItem{
+				&apprclient.RegistryMetadata{Name: "Kubevirt"},
+				&Source{Endpoint: "quay.io", RegistryNamespace: ""},
+			},
+		},
+		expectedError: nil,
+	},
+	{
+		input: &Input{
+			Sources: []*Source{
+				{
+					Endpoint:          "quay.io",
+					RegistryNamespace: "",
+				},
+				{
+					Endpoint:          "other-endpoint.io",
+					RegistryNamespace: "",
+				},
+			},
+			Packages: []string{
+				"Kubevirt",
+				"etcd",
+			},
+		},
+		sourceQuerier: &fakeSourceQuerier{
+			map[Source][]*apprclient.RegistryMetadata{
+				Source{
+					Endpoint:          "quay.io",
+					RegistryNamespace: "",
+				}: {
+					&apprclient.RegistryMetadata{Name: "Kubevirt"},
+				},
+			},
+			map[Source]error{
+				Source{
+					Endpoint:          "other-endpoint.io",
+					RegistryNamespace: "",
+				}: errors.New("Failed to fetch sources from other-endpoint.io"),
+			},
+		},
+		expectedDownloadItems: []*downloadItem{
+			&downloadItem{
+				&apprclient.RegistryMetadata{Name: "Kubevirt"},
+				&Source{Endpoint: "quay.io", RegistryNamespace: ""},
+			},
+		},
+		expectedError: utilerrors.NewAggregate(
+			[]error{
+				errors.New("Failed to fetch sources from other-endpoint.io"),
+			},
+		),
+	},
+}
+
+type fakeSourceQuerier struct {
+	mapping    map[Source][]*apprclient.RegistryMetadata
+	errMapping map[Source]error
+}
+
+func (f *fakeSourceQuerier) QuerySource(source *Source) (repositories []*apprclient.RegistryMetadata, err error) {
+	err, ok := f.errMapping[*source]
+	if ok {
+		return nil, err
+	}
+	return f.mapping[*source], nil
+}
+
+func TestPrepare(t *testing.T) {
+	logger := logrus.WithField("test", "prepare")
+	clientset := fake.NewSimpleClientset()
+
+	for _, testItem := range testPrepare {
+		d := downloader{
+			logger,
+			clientset,
+			testItem.sourceQuerier,
+			nil,
+		}
+
+		downloadItems, err := d.Prepare(testItem.input)
+		assert.Equal(t, testItem.expectedDownloadItems, downloadItems)
+		if testItem.expectedError != nil {
+			assert.Equal(t, testItem.expectedError, err)
+		} else {
+			assert.NoError(t, err)
+		}
+	}
+}