refactor: Move update logic to argocd package and provide more tests (#45)

* refactor: Move update logic to argocd package and provide more tests

* Fix typos

7 files changed, 329 insertions, 147 deletions

diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt
index 19ec2cf..19ebbad 100644
--- a/.github/actions/spelling/allow.txt
+++ b/.github/actions/spelling/allow.txt
@@ -6,9 +6,9 @@ apimachinery
 apps
 argocd
 argocdclient
+argomock
 argoproj
 argoprojlabs
-argproj
 args
 auths
 babayaga
@@ -141,6 +141,7 @@ readthedocs
 refactor
 refactoring
 regexp
+regmock
 RLock
 roadmap
 RUnlock
diff --git a/cmd/main.go b/cmd/main.go
index 5ece20f..70064f0 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -13,7 +13,6 @@ import (
 	"github.com/argoproj-labs/argocd-image-updater/pkg/client"
 	"github.com/argoproj-labs/argocd-image-updater/pkg/env"
 	"github.com/argoproj-labs/argocd-image-updater/pkg/health"
-	"github.com/argoproj-labs/argocd-image-updater/pkg/image"
 	"github.com/argoproj-labs/argocd-image-updater/pkg/log"
 	"github.com/argoproj-labs/argocd-image-updater/pkg/registry"
 	"github.com/argoproj-labs/argocd-image-updater/pkg/version"
@@ -41,150 +40,9 @@ type ImageUpdaterConfig struct {
 	RegistriesConf  string
 }
 
-// Stores some statistics about the results of a run
-type ImageUpdaterResult struct {
-	NumApplicationsProcessed int
-	NumImagesUpdated         int
-	NumImagesConsidered      int
-	NumSkipped               int
-	NumErrors                int
-}
-
-// Update all images of a single application. Will run in a goroutine.
-func updateApplication(argoClient argocd.ArgoCD, kubeClient *client.KubernetesClient, curApplication *argocd.ApplicationImages, dryRun bool) ImageUpdaterResult {
-	result := ImageUpdaterResult{}
-	app := curApplication.Application.GetName()
-
-	// Get all images that are deployed with the current application
-	applicationImages := argocd.GetImagesFromApplication(&curApplication.Application)
-
-	result.NumApplicationsProcessed += 1
-
-	// Loop through all images of current application, and check whether one of
-	// its images is eligible for updating.
-	//
-	// Whether an image qualifies for update is dependent on semantic version
-	// constraints which are part of the application's annotation values.
-	//
-	for _, applicationImage := range applicationImages {
-		updateableImage := curApplication.Images.ContainsImage(applicationImage, false)
-		if updateableImage == nil {
-			log.WithContext().AddField("application", app).Debugf("Image %s not in list of allowed images, skipping", applicationImage.ImageName)
-			result.NumSkipped += 1
-			continue
-		}
-
-		result.NumImagesConsidered += 1
-
-		imgCtx := log.WithContext().
-			AddField("application", app).
-			AddField("registry", applicationImage.RegistryURL).
-			AddField("image_name", applicationImage.ImageName).
-			AddField("image_tag", applicationImage.ImageTag)
-
-		imgCtx.Debugf("Considering this image for update")
-
-		rep, err := registry.GetRegistryEndpoint(applicationImage.RegistryURL)
-		if err != nil {
-			imgCtx.Errorf("Could not get registry endpoint from configuration: %v", err)
-			result.NumErrors += 1
-			continue
-		}
-
-		var vc image.VersionConstraint
-		if updateableImage.ImageTag != nil {
-			vc.Constraint = updateableImage.ImageTag.TagName
-			imgCtx.Debugf("Using version constraint '%s' when looking for a new tag", vc.Constraint)
-		} else {
-			imgCtx.Debugf("Using no version constraint when looking for a new tag")
-		}
-
-		vc.SortMode = updateableImage.GetParameterUpdateStrategy(curApplication.Application.Annotations)
-
-		ep, err := registry.GetRegistryEndpoint(updateableImage.RegistryURL)
-		if err != nil {
-			imgCtx.Errorf("Could not get registry endpoint: %v", err)
-			continue
-		}
-
-		err = ep.SetEndpointCredentials(kubeClient)
-		if err != nil {
-			imgCtx.Errorf("Could not set registry endpoint credentials: %v", err)
-			continue
-		}
-
-		regClient, err := registry.NewClient(ep)
-		if err != nil {
-			imgCtx.Errorf("Could not create registry client: %v", err)
-			continue
-		}
-
-		// Get list of available image tags from the repository
-		tags, err := rep.GetTags(applicationImage, regClient, &vc)
-		if err != nil {
-			imgCtx.Errorf("Could not get tags from registry: %v", err)
-			result.NumErrors += 1
-			continue
-		}
-
-		imgCtx.Tracef("List of available tags found: %v", tags.Tags())
-
-		// Get the latest available tag matching any constraint that might be set
-		// for allowed updates.
-		latest, err := applicationImage.GetNewestVersionFromTags(&vc, tags)
-		if err != nil {
-			imgCtx.Errorf("Unable to find newest version from available tags: %v", err)
-			result.NumErrors += 1
-			continue
-		}
-
-		// If we have no latest tag information, it means there was no tag which
-		// has met our version constraint (or there was no semantic versioned tag
-		// at all in the repository)
-		if latest == nil {
-			imgCtx.Debugf("No suitable image tag for upgrade found in list of available tags.")
-			result.NumSkipped += 1
-			continue
-		}
-
-		// If the latest tag does not match image's current tag, it means we have
-		// an update candidate.
-		if applicationImage.ImageTag.TagName != latest.TagName {
-			if dryRun {
-				imgCtx.Infof("Would upgrade image to %s, but this is a dry run. Skipping.", applicationImage.WithTag(latest).String())
-				continue
-			}
-
-			imgCtx.Infof("Upgrading image to %s", applicationImage.WithTag(latest).String())
-
-			if appType := argocd.GetApplicationType(&curApplication.Application); appType == argocd.ApplicationTypeKustomize {
-				err = argocd.SetKustomizeImage(argoClient, &curApplication.Application, updateableImage.WithTag(latest))
-			} else if appType == argocd.ApplicationTypeHelm {
-				err = argocd.SetHelmImage(argoClient, &curApplication.Application, updateableImage.WithTag(latest))
-			} else {
-				result.NumErrors += 1
-				err = fmt.Errorf("Could not update application %s - neither Helm nor Kustomize application", app)
-			}
-
-			if err != nil {
-				imgCtx.Errorf("Error while trying to update image: %v", err)
-				result.NumErrors += 1
-				continue
-			} else {
-				imgCtx.Infof("Successfully updated image '%s' to '%s'", applicationImage.GetFullNameWithTag(), applicationImage.WithTag(latest).GetFullNameWithTag())
-				result.NumImagesUpdated += 1
-			}
-		} else {
-			imgCtx.Debugf("Image '%s' already on latest allowed version", applicationImage.GetFullNameWithTag())
-		}
-	}
-
-	return result
-}
-
 // Main loop for argocd-image-controller
-func runImageUpdater(cfg *ImageUpdaterConfig) (ImageUpdaterResult, error) {
-	result := ImageUpdaterResult{}
+func runImageUpdater(cfg *ImageUpdaterConfig) (argocd.ImageUpdaterResult, error) {
+	result := argocd.ImageUpdaterResult{}
 	argoClient, err := argocd.NewClient(&cfg.ClientOpts)
 	if err != nil {
 		return result, err
@@ -231,7 +89,7 @@ func runImageUpdater(cfg *ImageUpdaterConfig) (ImageUpdaterResult, error) {
 		go func(app string, curApplication argocd.ApplicationImages) {
 			defer sem.Release(1)
 			log.Debugf("Processing application %s", app)
-			res := updateApplication(cfg.ArgoClient, cfg.KubeClient, &curApplication, cfg.DryRun)
+			res := argocd.UpdateApplication(registry.NewClient, cfg.ArgoClient, cfg.KubeClient, &curApplication, cfg.DryRun)
 			result.NumApplicationsProcessed += 1
 			result.NumErrors += res.NumErrors
 			result.NumImagesConsidered += res.NumImagesConsidered
diff --git a/docs/configuration/images.md b/docs/configuration/images.md
index 7cdda49..e869494 100644
--- a/docs/configuration/images.md
+++ b/docs/configuration/images.md
@@ -64,7 +64,7 @@ annotation, for example the following would assign the alias `myalias` to the
 image `some/image`:
 
 ```yaml
-argocd-image-updater.argproj.io/image-list: myalias=some/image
+argocd-image-updater.argoproj.io/image-list: myalias=some/image
 ```
 
 Assigning an alias name to an image is necessary in these scenarios:
diff --git a/pkg/argocd/argocd.go b/pkg/argocd/argocd.go
index e9f856c..159c7ca 100644
--- a/pkg/argocd/argocd.go
+++ b/pkg/argocd/argocd.go
@@ -113,6 +113,7 @@ func FilterApplicationsForUpdate(apps []v1alpha1.Application) (map[string]Applic
 	return appsForUpdate, nil
 }
 
+// GetApplication gets the application named appName from Argo CD API
 func (client *argoCD) GetApplication(ctx context.Context, appName string) (*v1alpha1.Application, error) {
 	conn, appClient, err := client.Client.NewApplicationClient()
 	if err != nil {
@@ -145,6 +146,7 @@ func (client *argoCD) ListApplications() ([]v1alpha1.Application, error) {
 	return apps.Items, nil
 }
 
+// UpdateSpec updates the spec for given application
 func (client *argoCD) UpdateSpec(ctx context.Context, in *application.ApplicationUpdateSpecRequest) (*v1alpha1.ApplicationSpec, error) {
 	conn, appClient, err := client.Client.NewApplicationClient()
 	if err != nil {
diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go
new file mode 100644
index 0000000..9505b26
--- /dev/null
+++ b/pkg/argocd/update.go
@@ -0,0 +1,145 @@
+package argocd
+
+import (
+	"fmt"
+
+	"github.com/argoproj-labs/argocd-image-updater/pkg/client"
+	"github.com/argoproj-labs/argocd-image-updater/pkg/image"
+	"github.com/argoproj-labs/argocd-image-updater/pkg/log"
+	"github.com/argoproj-labs/argocd-image-updater/pkg/registry"
+)
+
+// Stores some statistics about the results of a run
+type ImageUpdaterResult struct {
+	NumApplicationsProcessed int
+	NumImagesUpdated         int
+	NumImagesConsidered      int
+	NumSkipped               int
+	NumErrors                int
+}
+
+// UpdateApplication update all images of a single application. Will run in a goroutine.
+func UpdateApplication(newRegFn registry.NewRegistryClient, argoClient ArgoCD, kubeClient *client.KubernetesClient, curApplication *ApplicationImages, dryRun bool) ImageUpdaterResult {
+	result := ImageUpdaterResult{}
+	app := curApplication.Application.GetName()
+
+	// Get all images that are deployed with the current application
+	applicationImages := GetImagesFromApplication(&curApplication.Application)
+
+	result.NumApplicationsProcessed += 1
+
+	// Loop through all images of current application, and check whether one of
+	// its images is eligible for updating.
+	//
+	// Whether an image qualifies for update is dependent on semantic version
+	// constraints which are part of the application's annotation values.
+	//
+	for _, applicationImage := range applicationImages {
+		updateableImage := curApplication.Images.ContainsImage(applicationImage, false)
+		if updateableImage == nil {
+			log.WithContext().AddField("application", app).Debugf("Image %s not in list of allowed images, skipping", applicationImage.ImageName)
+			result.NumSkipped += 1
+			continue
+		}
+
+		result.NumImagesConsidered += 1
+
+		imgCtx := log.WithContext().
+			AddField("application", app).
+			AddField("registry", applicationImage.RegistryURL).
+			AddField("image_name", applicationImage.ImageName).
+			AddField("image_tag", applicationImage.ImageTag)
+
+		imgCtx.Debugf("Considering this image for update")
+
+		rep, err := registry.GetRegistryEndpoint(applicationImage.RegistryURL)
+		if err != nil {
+			imgCtx.Errorf("Could not get registry endpoint from configuration: %v", err)
+			result.NumErrors += 1
+			continue
+		}
+
+		var vc image.VersionConstraint
+		if updateableImage.ImageTag != nil {
+			vc.Constraint = updateableImage.ImageTag.TagName
+			imgCtx.Debugf("Using version constraint '%s' when looking for a new tag", vc.Constraint)
+		} else {
+			imgCtx.Debugf("Using no version constraint when looking for a new tag")
+		}
+
+		vc.SortMode = updateableImage.GetParameterUpdateStrategy(curApplication.Application.Annotations)
+
+		err = rep.SetEndpointCredentials(kubeClient)
+		if err != nil {
+			imgCtx.Errorf("Could not set registry endpoint credentials: %v", err)
+			continue
+		}
+
+		regClient, err := newRegFn(rep)
+		if err != nil {
+			imgCtx.Errorf("Could not create registry client: %v", err)
+			continue
+		}
+
+		// Get list of available image tags from the repository
+		tags, err := rep.GetTags(applicationImage, regClient, &vc)
+		if err != nil {
+			imgCtx.Errorf("Could not get tags from registry: %v", err)
+			result.NumErrors += 1
+			continue
+		}
+
+		imgCtx.Tracef("List of available tags found: %v", tags.Tags())
+
+		// Get the latest available tag matching any constraint that might be set
+		// for allowed updates.
+		latest, err := applicationImage.GetNewestVersionFromTags(&vc, tags)
+		if err != nil {
+			imgCtx.Errorf("Unable to find newest version from available tags: %v", err)
+			result.NumErrors += 1
+			continue
+		}
+
+		// If we have no latest tag information, it means there was no tag which
+		// has met our version constraint (or there was no semantic versioned tag
+		// at all in the repository)
+		if latest == nil {
+			imgCtx.Debugf("No suitable image tag for upgrade found in list of available tags.")
+			result.NumSkipped += 1
+			continue
+		}
+
+		// If the latest tag does not match image's current tag, it means we have
+		// an update candidate.
+		if applicationImage.ImageTag.TagName != latest.TagName {
+			if dryRun {
+				imgCtx.Infof("Would upgrade image to %s, but this is a dry run. Skipping.", applicationImage.WithTag(latest).String())
+				continue
+			}
+
+			imgCtx.Infof("Upgrading image to %s", applicationImage.WithTag(latest).String())
+
+			if appType := GetApplicationType(&curApplication.Application); appType == ApplicationTypeKustomize {
+				err = SetKustomizeImage(argoClient, &curApplication.Application, updateableImage.WithTag(latest))
+			} else if appType == ApplicationTypeHelm {
+				err = SetHelmImage(argoClient, &curApplication.Application, updateableImage.WithTag(latest))
+			} else {
+				result.NumErrors += 1
+				err = fmt.Errorf("Could not update application %s - neither Helm nor Kustomize application", app)
+			}
+
+			if err != nil {
+				imgCtx.Errorf("Error while trying to update image: %v", err)
+				result.NumErrors += 1
+				continue
+			} else {
+				imgCtx.Infof("Successfully updated image '%s' to '%s'", applicationImage.GetFullNameWithTag(), applicationImage.WithTag(latest).GetFullNameWithTag())
+				result.NumImagesUpdated += 1
+			}
+		} else {
+			imgCtx.Debugf("Image '%s' already on latest allowed version", applicationImage.GetFullNameWithTag())
+		}
+	}
+
+	return result
+}
diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go
new file mode 100644
index 0000000..0e72568
--- /dev/null
+++ b/pkg/argocd/update_test.go
@@ -0,0 +1,167 @@
+package argocd
+
+import (
+	"testing"
+
+	argomock "github.com/argoproj-labs/argocd-image-updater/pkg/argocd/mocks"
+	"github.com/argoproj-labs/argocd-image-updater/pkg/client"
+	"github.com/argoproj-labs/argocd-image-updater/pkg/image"
+	"github.com/argoproj-labs/argocd-image-updater/pkg/registry"
+	regmock "github.com/argoproj-labs/argocd-image-updater/pkg/registry/mocks"
+	"github.com/argoproj-labs/argocd-image-updater/test/fake"
+
+	"github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func Test_UpdateApplication(t *testing.T) {
+	t.Run("Test successful update", func(t *testing.T) {
+		mockClientFn := func(endpoint *registry.RegistryEndpoint) (registry.RegistryClient, error) {
+			regMock := regmock.RegistryClient{}
+			regMock.On("Tags", mock.Anything).Return([]string{"1.0.1"}, nil)
+			return &regMock, nil
+		}
+
+		argoClient := argomock.ArgoCD{}
+		argoClient.On("UpdateSpec", mock.Anything, mock.Anything).Return(nil, nil)
+
+		kubeClient := client.KubernetesClient{
+			Clientset: fake.NewFakeKubeClient(),
+		}
+		appImages := &ApplicationImages{
+			Application: v1alpha1.Application{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "guestbook",
+					Namespace: "guestbook",
+				},
+				Spec: v1alpha1.ApplicationSpec{
+					Source: v1alpha1.ApplicationSource{
+						Kustomize: &v1alpha1.ApplicationSourceKustomize{
+							Images: v1alpha1.KustomizeImages{
+								"jannfis/foobar:1.0.0",
+							},
+						},
+					},
+				},
+				Status: v1alpha1.ApplicationStatus{
+					SourceType: v1alpha1.ApplicationSourceTypeKustomize,
+					Summary: v1alpha1.ApplicationSummary{
+						Images: []string{
+							"jannfis/foobar:1.0.0",
+						},
+					},
+				},
+			},
+			Images: image.ContainerImageList{
+				image.NewFromIdentifier("jannfis/foobar:1.0.1"),
+			},
+		}
+		res := UpdateApplication(mockClientFn, &argoClient, &kubeClient, appImages, false)
+		assert.Equal(t, 0, res.NumErrors)
+		assert.Equal(t, 0, res.NumSkipped)
+		assert.Equal(t, 1, res.NumApplicationsProcessed)
+		assert.Equal(t, 1, res.NumImagesConsidered)
+		assert.Equal(t, 1, res.NumImagesUpdated)
+	})
+
+	t.Run("Test skip because of image not in list", func(t *testing.T) {
+		mockClientFn := func(endpoint *registry.RegistryEndpoint) (registry.RegistryClient, error) {
+			regMock := regmock.RegistryClient{}
+			regMock.On("Tags", mock.Anything).Return([]string{"1.0.1"}, nil)
+			return &regMock, nil
+		}
+
+		argoClient := argomock.ArgoCD{}
+		argoClient.On("UpdateSpec", mock.Anything, mock.Anything).Return(nil, nil)
+
+		kubeClient := client.KubernetesClient{
+			Clientset: fake.NewFakeKubeClient(),
+		}
+		appImages := &ApplicationImages{
+			Application: v1alpha1.Application{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "guestbook",
+					Namespace: "guestbook",
+				},
+				Spec: v1alpha1.ApplicationSpec{
+					Source: v1alpha1.ApplicationSource{
+						Kustomize: &v1alpha1.ApplicationSourceKustomize{
+							Images: v1alpha1.KustomizeImages{
+								"jannfis/foobar:1.0.0",
+							},
+						},
+					},
+				},
+				Status: v1alpha1.ApplicationStatus{
+					SourceType: v1alpha1.ApplicationSourceTypeKustomize,
+					Summary: v1alpha1.ApplicationSummary{
+						Images: []string{
+							"jannfis/foobar:1.0.0",
+						},
+					},
+				},
+			},
+			Images: image.ContainerImageList{
+				image.NewFromIdentifier("jannfis/barbar:1.0.1"),
+			},
+		}
+		res := UpdateApplication(mockClientFn, &argoClient, &kubeClient, appImages, false)
+		assert.Equal(t, 0, res.NumErrors)
+		assert.Equal(t, 1, res.NumSkipped)
+		assert.Equal(t, 1, res.NumApplicationsProcessed)
+		assert.Equal(t, 0, res.NumImagesConsidered)
+		assert.Equal(t, 0, res.NumImagesUpdated)
+	})
+
+	t.Run("Test skip because of image up-to-date", func(t *testing.T) {
+		mockClientFn := func(endpoint *registry.RegistryEndpoint) (registry.RegistryClient, error) {
+			regMock := regmock.RegistryClient{}
+			regMock.On("Tags", mock.Anything).Return([]string{"1.0.1"}, nil)
+			return &regMock, nil
+		}
+
+		argoClient := argomock.ArgoCD{}
+		argoClient.On("UpdateSpec", mock.Anything, mock.Anything).Return(nil, nil)
+
+		kubeClient := client.KubernetesClient{
+			Clientset: fake.NewFakeKubeClient(),
+		}
+		appImages := &ApplicationImages{
+			Application: v1alpha1.Application{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "guestbook",
+					Namespace: "guestbook",
+				},
+				Spec: v1alpha1.ApplicationSpec{
+					Source: v1alpha1.ApplicationSource{
+						Kustomize: &v1alpha1.ApplicationSourceKustomize{
+							Images: v1alpha1.KustomizeImages{
+								"jannfis/foobar:1.0.1",
+							},
+						},
+					},
+				},
+				Status: v1alpha1.ApplicationStatus{
+					SourceType: v1alpha1.ApplicationSourceTypeKustomize,
+					Summary: v1alpha1.ApplicationSummary{
+						Images: []string{
+							"jannfis/foobar:1.0.1",
+						},
+					},
+				},
+			},
+			Images: image.ContainerImageList{
+				image.NewFromIdentifier("jannfis/foobar:1.0.1"),
+			},
+		}
+		res := UpdateApplication(mockClientFn, &argoClient, &kubeClient, appImages, false)
+		assert.Equal(t, 0, res.NumErrors)
+		assert.Equal(t, 0, res.NumSkipped)
+		assert.Equal(t, 1, res.NumApplicationsProcessed)
+		assert.Equal(t, 1, res.NumImagesConsidered)
+		assert.Equal(t, 0, res.NumImagesUpdated)
+	})
+
+}
diff --git a/pkg/registry/client.go b/pkg/registry/client.go
index 0fc0153..e349bda 100644
--- a/pkg/registry/client.go
+++ b/pkg/registry/client.go
@@ -1,6 +1,8 @@
 package registry
 
 import (
+	"github.com/argoproj-labs/argocd-image-updater/pkg/registry/mocks"
+
 	"github.com/docker/distribution/manifest/schema1"
 	"github.com/nokia/docker-registry-client/registry"
 )
@@ -11,11 +13,18 @@ type RegistryClient interface {
 	ManifestV1(repository string, reference string) (*schema1.SignedManifest, error)
 }
 
+type NewRegistryClient func(*RegistryEndpoint) (RegistryClient, error)
+
 // Helper type for registry clients
 type registryClient struct {
 	regClient *registry.Registry
 }
 
+// NewMockClient returns a new mocked RegistryClient
+func NewMockClient(endpoint *RegistryEndpoint) (RegistryClient, error) {
+	return &mocks.RegistryClient{}, nil
+}
+
 // NewClient returns a new RegistryClient for the given endpoint information
 func NewClient(endpoint *RegistryEndpoint) (RegistryClient, error) {
 	client, err := registry.NewCustom(endpoint.RegistryAPI, registry.Options{