From 1ede4b07ca0a97f3566111aed87ca99c44cb0625 Mon Sep 17 00:00:00 2001 From: Enrico Stahn Date: Sat, 25 Sep 2021 00:30:58 +1000 Subject: [PATCH] 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 | 8 +++- pkg/registry/ecr.go | 2 +- pkg/webhook/image_swapper.go | 72 +++++++++++++++++++++++++++++++ pkg/webhook/image_swapper_test.go | 50 +++++++++++++++++---- 4 files changed, 121 insertions(+), 11 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index fb0842ce..6c775114 100644 --- a/cmd/root.go +++ b/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) diff --git a/pkg/registry/ecr.go b/pkg/registry/ecr.go index b6897139..c266522e 100644 --- a/pkg/registry/ecr.go +++ b/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") diff --git a/pkg/webhook/image_swapper.go b/pkg/webhook/image_swapper.go index 84a1a581..f5127762 100644 --- a/pkg/webhook/image_swapper.go +++ b/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) diff --git a/pkg/webhook/image_swapper_test.go b/pkg/webhook/image_swapper_test.go index e29a74c8..056dedce 100644 --- a/pkg/webhook/image_swapper_test.go +++ b/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) }