From 61c16536003d66dd802979d699b749c6e1f8c151 Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Sun, 19 Jan 2025 02:05:40 +0100 Subject: [PATCH] Skip copy for given container registry domain --- cmd/root.go | 1 + pkg/config/config.go | 9 ++-- pkg/webhook/image_swapper.go | 83 ++++++++++++++++++++----------- pkg/webhook/image_swapper_test.go | 71 ++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 33 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 7d2121b4..3bb924b6 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -108,6 +108,7 @@ A mutating webhook for Kubernetes, pointing the images to a new location.`, webhook.ImageSwapPolicy(imageSwapPolicy), webhook.ImageCopyPolicy(imageCopyPolicy), webhook.ImageCopyDeadline(imageCopyDeadline), + webhook.ImageCopySkipRegistries(cfg.ImageCopySkipRegistries), ) if err != nil { log.Err(err).Msg("error creating webhook") diff --git a/pkg/config/config.go b/pkg/config/config.go index a4bb860d..249a8dcb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -38,10 +38,11 @@ type Config struct { ListenAddress string - DryRun bool `yaml:"dryRun"` - ImageSwapPolicy string `yaml:"imageSwapPolicy" validate:"oneof=always exists"` - ImageCopyPolicy string `yaml:"imageCopyPolicy" validate:"oneof=delayed immediate force none"` - ImageCopyDeadline time.Duration `yaml:"imageCopyDeadline"` + DryRun bool `yaml:"dryRun"` + ImageSwapPolicy string `yaml:"imageSwapPolicy" validate:"oneof=always exists"` + ImageCopyPolicy string `yaml:"imageCopyPolicy" validate:"oneof=delayed immediate force none"` + ImageCopyDeadline time.Duration `yaml:"imageCopyDeadline"` + ImageCopySkipRegistries []string `yaml:"skipRegistries"` Source Source `yaml:"source"` Target Registry `yaml:"target"` diff --git a/pkg/webhook/image_swapper.go b/pkg/webhook/image_swapper.go index ee2aea96..f482b58b 100644 --- a/pkg/webhook/image_swapper.go +++ b/pkg/webhook/image_swapper.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strings" "time" "github.com/alitto/pond" @@ -61,6 +62,13 @@ func ImageCopyDeadline(deadline time.Duration) Option { } } +// ImageCopySkipRegistries allows to pass the skipRegistries option +func ImageCopySkipRegistries(registries []string) Option { + return func(swapper *ImageSwapper) { + swapper.imageCopySkipRegistries = registries + } +} + // Copier allows to pass the copier option func Copier(pool *pond.WorkerPool) Option { return func(swapper *ImageSwapper) { @@ -83,10 +91,12 @@ type ImageSwapper struct { imageSwapPolicy types.ImageSwapPolicy imageCopyPolicy types.ImageCopyPolicy + + imageCopySkipRegistries []string } // NewImageSwapper returns a new ImageSwapper initialized. -func NewImageSwapper(registryClient registry.Client, imagePullSecretProvider secrets.ImagePullSecretsProvider, filters []config.JMESPathFilter, imageSwapPolicy types.ImageSwapPolicy, imageCopyPolicy types.ImageCopyPolicy, imageCopyDeadline time.Duration) kwhmutating.Mutator { +func NewImageSwapper(registryClient registry.Client, imagePullSecretProvider secrets.ImagePullSecretsProvider, filters []config.JMESPathFilter, imageSwapPolicy types.ImageSwapPolicy, imageCopyPolicy types.ImageCopyPolicy, imageCopyDeadline time.Duration, skipRegistries []string) kwhmutating.Mutator { return &ImageSwapper{ registryClient: registryClient, imagePullSecretProvider: imagePullSecretProvider, @@ -95,6 +105,7 @@ func NewImageSwapper(registryClient registry.Client, imagePullSecretProvider sec imageSwapPolicy: imageSwapPolicy, imageCopyPolicy: imageCopyPolicy, imageCopyDeadline: imageCopyDeadline, + imageCopySkipRegistries: skipRegistries, } } @@ -106,6 +117,7 @@ func NewImageSwapperWithOpts(registryClient registry.Client, opts ...Option) kwh filters: []config.JMESPathFilter{}, imageSwapPolicy: types.ImageSwapPolicyExists, imageCopyPolicy: types.ImageCopyPolicyDelayed, + imageCopySkipRegistries: []string{}, } for _, opt := range opts { @@ -132,8 +144,8 @@ func NewImageSwapperWebhookWithOpts(registryClient registry.Client, opts ...Opti return kwhmutating.NewWebhook(mcfg) } -func NewImageSwapperWebhook(registryClient registry.Client, imagePullSecretProvider secrets.ImagePullSecretsProvider, filters []config.JMESPathFilter, imageSwapPolicy types.ImageSwapPolicy, imageCopyPolicy types.ImageCopyPolicy, imageCopyDeadline time.Duration) (webhook.Webhook, error) { - imageSwapper := NewImageSwapper(registryClient, imagePullSecretProvider, filters, imageSwapPolicy, imageCopyPolicy, imageCopyDeadline) +func NewImageSwapperWebhook(registryClient registry.Client, imagePullSecretProvider secrets.ImagePullSecretsProvider, filters []config.JMESPathFilter, imageSwapPolicy types.ImageSwapPolicy, imageCopyPolicy types.ImageCopyPolicy, imageCopyDeadline time.Duration, skipRegistries []string) (webhook.Webhook, error) { + imageSwapper := NewImageSwapper(registryClient, imagePullSecretProvider, filters, imageSwapPolicy, imageCopyPolicy, imageCopyDeadline, skipRegistries) mt := kwhmutating.MutatorFunc(imageSwapper.Mutate) mcfg := kwhmutating.WebhookConfig{ ID: "k8s-image-swapper", @@ -211,34 +223,47 @@ func (p *ImageSwapper) Mutate(ctx context.Context, ar *kwhmodel.AdmissionReview, targetRef := p.targetRef(srcRef) targetImage := targetRef.DockerReference().String() - imageCopierLogger := logger.With(). - Str("source-image", srcRef.DockerReference().String()). - Str("target-image", targetImage). - Logger() - - imageCopierContext := imageCopierLogger.WithContext(lctx) - // create an object responsible for the image copy - imageCopier := ImageCopier{ - sourcePod: pod, - sourceImageRef: srcRef, - targetImageRef: targetRef, - imagePullPolicy: container.ImagePullPolicy, - imageSwapper: p, - context: imageCopierContext, + // check if the registry domain is in the skip list and skip the image if it is - used when the private registry already has configured pull through cache for the source registry + domain := reference.Domain(srcRef.DockerReference()) + makeCopy := true + for _, skipRegistry := range p.imageCopySkipRegistries { + if strings.EqualFold(domain, skipRegistry) { + log.Ctx(lctx).Debug().Str("registry", domain).Msg("skip due to source registry being in the skip list") + makeCopy = false + break + } } - // imageCopyPolicy - switch p.imageCopyPolicy { - case types.ImageCopyPolicyDelayed: - p.copier.Submit(imageCopier.start) - case types.ImageCopyPolicyImmediate: - p.copier.SubmitAndWait(imageCopier.withDeadline().start) - case types.ImageCopyPolicyForce: - imageCopier.withDeadline().start() - case types.ImageCopyPolicyNone: - // do not copy image - default: - panic("unknown imageCopyPolicy") + if makeCopy { + imageCopierLogger := logger.With(). + Str("source-image", srcRef.DockerReference().String()). + Str("target-image", targetImage). + Logger() + + imageCopierContext := imageCopierLogger.WithContext(lctx) + // create an object responsible for the image copy + imageCopier := ImageCopier{ + sourcePod: pod, + sourceImageRef: srcRef, + targetImageRef: targetRef, + imagePullPolicy: container.ImagePullPolicy, + imageSwapper: p, + context: imageCopierContext, + } + + // imageCopyPolicy + switch p.imageCopyPolicy { + case types.ImageCopyPolicyDelayed: + p.copier.Submit(imageCopier.start) + case types.ImageCopyPolicyImmediate: + p.copier.SubmitAndWait(imageCopier.withDeadline().start) + case types.ImageCopyPolicyForce: + imageCopier.withDeadline().start() + case types.ImageCopyPolicyNone: + // do not copy image + default: + panic("unknown imageCopyPolicy") + } } // imageSwapPolicy diff --git a/pkg/webhook/image_swapper_test.go b/pkg/webhook/image_swapper_test.go index 66612ac0..2f99f314 100644 --- a/pkg/webhook/image_swapper_test.go +++ b/pkg/webhook/image_swapper_test.go @@ -298,6 +298,7 @@ func TestImageSwapper_Mutate(t *testing.T) { copier.StopAndWait() ecrClient.AssertExpectations(t) + assert.Equal(t, uint64(4), copier.SubmittedTasks()) } // TestImageSwapper_MutateWithImagePullSecrets tests mutating with imagePullSecret support @@ -391,6 +392,7 @@ func TestImageSwapper_MutateWithImagePullSecrets(t *testing.T) { copier.StopAndWait() ecrClient.AssertExpectations(t) + assert.Equal(t, uint64(1), copier.SubmittedTasks()) } func TestImageSwapper_GAR_Mutate(t *testing.T) { @@ -422,4 +424,73 @@ func TestImageSwapper_GAR_Mutate(t *testing.T) { assert.JSONEq(t, expected, string(resp.(*model.MutatingAdmissionResponse).JSONPatchPatch)) assert.Nil(t, resp.(*model.MutatingAdmissionResponse).Warnings) assert.NoError(t, err, "Webhook executed without errors") + assert.Equal(t, uint64(4), copier.SubmittedTasks()) +} + +func TestImageSwapper_skipRegistryGcrIo_Mutate(t *testing.T) { + registryClient, _ := registry.NewMockGARClient(nil, "us-central1-docker.pkg.dev/gcp-project-123/main") + + admissionReview, _ := readAdmissionReviewFromFile("admissionreview-simple.json") + admissionReviewModel := model.NewAdmissionReviewV1(admissionReview) + + copier := pond.New(1, 1) + wh, err := NewImageSwapperWebhookWithOpts( + registryClient, + Copier(copier), + ImageSwapPolicy(types.ImageSwapPolicyAlways), + ImageCopySkipRegistries([]string{"k8s.gcr.io"}), + ) + + assert.NoError(t, err, "NewImageSwapperWebhookWithOpts executed without errors") + + resp, err := wh.Review(context.TODO(), admissionReviewModel) + + // container with name "skip-test-gar" should be skipped, hence there is no "replace" operation for it + expected := `[ + {"op":"replace","path":"/spec/initContainers/0/image","value":"us-central1-docker.pkg.dev/gcp-project-123/main/docker.io/library/init-container:latest"}, + {"op":"replace","path":"/spec/containers/0/image","value":"us-central1-docker.pkg.dev/gcp-project-123/main/docker.io/library/nginx:latest"}, + {"op":"replace","path":"/spec/containers/1/image","value":"us-central1-docker.pkg.dev/gcp-project-123/main/k8s.gcr.io/ingress-nginx/controller@sha256:9bba603b99bf25f6d117cf1235b6598c16033ad027b143c90fa5b3cc583c5713"}, + {"op":"replace","path":"/spec/containers/2/image","value":"us-central1-docker.pkg.dev/gcp-project-123/main/123456789.dkr.ecr.ap-southeast-2.amazonaws.com/k8s.gcr.io/ingress-nginx/controller@sha256:9bba603b99bf25f6d117cf1235b6598c16033ad027b143c90fa5b3cc583c5713"} + ]` + + assert.JSONEq(t, expected, string(resp.(*model.MutatingAdmissionResponse).JSONPatchPatch)) + assert.Nil(t, resp.(*model.MutatingAdmissionResponse).Warnings) + assert.NoError(t, err, "Webhook executed without errors") + + // check the amount of submitted tasks for the copier + assert.Equal(t, uint64(3), copier.SubmittedTasks()) +} + +func TestImageSwapper_skipRegistryDockerIo_Mutate(t *testing.T) { + registryClient, _ := registry.NewMockGARClient(nil, "us-central1-docker.pkg.dev/gcp-project-123/main") + + admissionReview, _ := readAdmissionReviewFromFile("admissionreview-simple.json") + admissionReviewModel := model.NewAdmissionReviewV1(admissionReview) + + copier := pond.New(1, 1) + wh, err := NewImageSwapperWebhookWithOpts( + registryClient, + Copier(copier), + ImageSwapPolicy(types.ImageSwapPolicyAlways), + ImageCopySkipRegistries([]string{"docker.io"}), + ) + + assert.NoError(t, err, "NewImageSwapperWebhookWithOpts executed without errors") + + resp, err := wh.Review(context.TODO(), admissionReviewModel) + + // container with name "skip-test-gar" should be skipped, hence there is no "replace" operation for it + expected := `[ + {"op":"replace","path":"/spec/initContainers/0/image","value":"us-central1-docker.pkg.dev/gcp-project-123/main/docker.io/library/init-container:latest"}, + {"op":"replace","path":"/spec/containers/0/image","value":"us-central1-docker.pkg.dev/gcp-project-123/main/docker.io/library/nginx:latest"}, + {"op":"replace","path":"/spec/containers/1/image","value":"us-central1-docker.pkg.dev/gcp-project-123/main/k8s.gcr.io/ingress-nginx/controller@sha256:9bba603b99bf25f6d117cf1235b6598c16033ad027b143c90fa5b3cc583c5713"}, + {"op":"replace","path":"/spec/containers/2/image","value":"us-central1-docker.pkg.dev/gcp-project-123/main/123456789.dkr.ecr.ap-southeast-2.amazonaws.com/k8s.gcr.io/ingress-nginx/controller@sha256:9bba603b99bf25f6d117cf1235b6598c16033ad027b143c90fa5b3cc583c5713"} + ]` + + assert.JSONEq(t, expected, string(resp.(*model.MutatingAdmissionResponse).JSONPatchPatch)) + assert.Nil(t, resp.(*model.MutatingAdmissionResponse).Warnings) + assert.NoError(t, err, "Webhook executed without errors") + + // check the amount of submitted tasks for the copier + assert.Equal(t, uint64(2), copier.SubmittedTasks()) }