refactor: restructure to improve read-/testability

Added options (...WithOpts) to improve readibility but primarily to
allow injecting various options such as the worker pool. This is being
used in unit tests to have control over the flow.

cmd/root.go

@@ -82,7 +82,13 @@ A mutating webhook for Kubernetes, pointing the images to a new location.`,
 
 		imagePullSecretProvider := setupImagePullSecretsProvider()
 
-		wh, err := webhook.NewImageSwapperWebhook(rClient, imagePullSecretProvider, cfg.Source.Filters, imageSwapPolicy, imageCopyPolicy)
+		wh, err := webhook.NewImageSwapperWebhookWithOpts(
+			rClient,
+			webhook.Filters(cfg.Source.Filters),
+			webhook.ImagePullSecretProvider(imagePullSecretProvider),
+			webhook.ImageSwapPolicy(imageSwapPolicy),
+			webhook.ImageCopyPolicy(imageCopyPolicy),
+		)
 		if err != nil {
 			log.Err(err).Msg("error creating webhook")
 			os.Exit(1)

pkg/registry/ecr.go

@@ -91,7 +91,7 @@ func (e *ECRClient) ImageExists(ref string) bool {
 		"inspect",
 		"--retry-times", "3",
 		"docker://" + ref,
-		"--creds", string(e.Credentials()),
+		"--creds", e.Credentials(),
 	}
 
 	log.Trace().Str("app", app).Strs("args", args).Msg("executing command to inspect image")

pkg/webhook/image_swapper.go

@@ -51,6 +51,78 @@ func NewImageSwapper(registryClient registry.Client, imagePullSecretProvider sec
 	}
 }
 
+// NewImageSwapperWithOpts returns a configured ImageSwapper instance
+func NewImageSwapperWithOpts(registryClient registry.Client, opts ...Option) kwhmutating.Mutator {
+	swapper := &ImageSwapper{
+		registryClient: registryClient,
+		imagePullSecretProvider: secrets.NewDummyImagePullSecretsProvider(),
+		filters: []config.JMESPathFilter{},
+		imageSwapPolicy: types.ImageSwapPolicyExists,
+		imageCopyPolicy: types.ImageCopyPolicyDelayed,
+	}
+
+	for _, opt := range opts {
+		opt(swapper)
+	}
+
+	// Initialise worker pool if not configured
+	if swapper.copier == nil {
+		swapper.copier = pond.New(100, 1000)
+	}
+
+	return swapper
+}
+
+// Option represents an option that can be passed when instantiating a worker pool to customize it
+type Option func(*ImageSwapper)
+
+// ImagePullSecretProvider allows to pass a provider reading out Kubernetes secrets
+func ImagePullSecretProvider(provider secrets.ImagePullSecretsProvider) Option {
+	return func(swapper *ImageSwapper) {
+		swapper.imagePullSecretProvider = provider
+	}
+}
+
+// Filters allows to pass JMESPathFilter to select the images to be swapped
+func Filters(filters []config.JMESPathFilter) Option {
+	return func(swapper *ImageSwapper) {
+		swapper.filters = filters
+	}
+}
+
+// ImageSwapPolicy allows to pass the ImageSwapPolicy option
+func ImageSwapPolicy(policy types.ImageSwapPolicy) Option {
+	return func(swapper *ImageSwapper) {
+		swapper.imageSwapPolicy = policy
+	}
+}
+
+// ImageCopyPolicy allows to pass the ImageCopyPolicy option
+func ImageCopyPolicy(policy types.ImageCopyPolicy) Option {
+	return func(swapper *ImageSwapper) {
+		swapper.imageCopyPolicy = policy
+	}
+}
+
+// Copier allows to pass the copier option
+func Copier(pool *pond.WorkerPool) Option {
+	return func(swapper *ImageSwapper) {
+		swapper.copier = pool
+	}
+}
+
+func NewImageSwapperWebhookWithOpts(registryClient registry.Client, opts ...Option) (webhook.Webhook, error) {
+	imageSwapper := NewImageSwapperWithOpts(registryClient, opts...)
+	mt := kwhmutating.MutatorFunc(imageSwapper.Mutate)
+	mcfg := kwhmutating.WebhookConfig{
+		ID:      "k8s-image-swapper",
+		Obj:     &corev1.Pod{},
+		Mutator: mt,
+	}
+
+	return kwhmutating.NewWebhook(mcfg)
+}
+
 func NewImageSwapperWebhook(registryClient registry.Client, imagePullSecretProvider secrets.ImagePullSecretsProvider, filters []config.JMESPathFilter, imageSwapPolicy types.ImageSwapPolicy, imageCopyPolicy types.ImageCopyPolicy) (webhook.Webhook, error) {
 	imageSwapper := NewImageSwapper(registryClient, imagePullSecretProvider, filters, imageSwapPolicy, imageCopyPolicy)
 	mt := kwhmutating.MutatorFunc(imageSwapper.Mutate)

pkg/webhook/image_swapper_test.go

@@ -6,15 +6,17 @@ import (
 	"io/ioutil"
 	"testing"
 
+	"github.com/alitto/pond"
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ecr"
 	"github.com/aws/aws-sdk-go/service/ecr/ecriface"
 	"github.com/davecgh/go-spew/spew"
 	"github.com/estahn/k8s-image-swapper/pkg/config"
 	"github.com/estahn/k8s-image-swapper/pkg/registry"
-	"github.com/estahn/k8s-image-swapper/pkg/secrets"
 	"github.com/estahn/k8s-image-swapper/pkg/types"
 	"github.com/slok/kubewebhook/v2/pkg/model"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	admissionv1 "k8s.io/api/admission/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -189,10 +191,12 @@ func TestFilterMatch(t *testing.T) {
 }
 
 type mockECRClient struct {
+	mock.Mock
 	ecriface.ECRAPI
 }
 
-func (m *mockECRClient) CreateRepository(*ecr.CreateRepositoryInput) (*ecr.CreateRepositoryOutput, error) {
+func (m *mockECRClient) CreateRepository(createRepositoryInput *ecr.CreateRepositoryInput) (*ecr.CreateRepositoryOutput, error) {
+	m.Called(createRepositoryInput)
 	return &ecr.CreateRepositoryOutput{}, nil
 }
 
@@ -211,15 +215,43 @@ func readAdmissionReviewFromFile(filename string) (*admissionv1.AdmissionReview,
 }
 
 func TestImageSwapper_Mutate(t *testing.T) {
-	ecrClient := &mockECRClient{}
-	registryClient, _ := registry.NewMockECRClient(ecrClient, "", "")
-	imagePullSecretProvider := secrets.NewDummyImagePullSecretsProvider()
+	ecrClient := new(mockECRClient)
+	ecrClient.On(
+		"CreateRepository",
+		&ecr.CreateRepositoryInput{
+			ImageScanningConfiguration: &ecr.ImageScanningConfiguration{
+				ScanOnPush: aws.Bool(true),
+			},
+			ImageTagMutability: aws.String("MUTABLE"),
+			RepositoryName:     aws.String("docker.io/library/nginx"),
+			Tags: []*ecr.Tag{{
+				Key:   aws.String("CreatedBy"),
+				Value: aws.String("k8s-image-swapper"),
+			}},
+		}).Return(mock.Anything)
+
+	registryClient, _ := registry.NewMockECRClient(ecrClient, "ap-southeast-2", "123456789.dkr.ecr.ap-southeast-2.amazonaws.com")
 
 	admissionReview, _ := readAdmissionReviewFromFile("../../test/requests/admissionreview.json")
-	arm := model.NewAdmissionReviewV1(admissionReview)
+	admissionReviewModel := model.NewAdmissionReviewV1(admissionReview)
+
+	copier := pond.New(1, 1)
+	// TODO: test types.ImageSwapPolicyExists
+	wh, err := NewImageSwapperWebhookWithOpts(
+		registryClient,
+		Copier(copier),
+		ImageSwapPolicy(types.ImageSwapPolicyAlways),
+	)
+
+	assert.NoError(t, err, "NewImageSwapperWebhookWithOpts executed without errors")
 
-	wh, _ := NewImageSwapperWebhook(registryClient, imagePullSecretProvider, []config.JMESPathFilter{}, types.ImageSwapPolicyAlways, types.ImageSwapPolicyAlways)
-	resp, err := wh.Review(context.TODO(), arm)
+	resp, err := wh.Review(context.TODO(), admissionReviewModel)
 	spew.Dump(resp)
-	spew.Dump(err)
+
+	assert.NoError(t, err, "Webhook executed without errors")
+
+	// Ensure the worker pool is empty before asserting ecrClient
+	copier.StopAndWait()
+
+	ecrClient.AssertExpectations(t)
 }