diff --git a/CHANGELOG.md b/CHANGELOG.md index f2004d5..6c32621 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.9.0] - 2025-03-20 +### Added +- Add imagePullSecrets update when an image is modified + ## [0.8.1] - 2025-03-17 ### Fixed - Fixed chart pdb rendering diff --git a/README.md b/README.md index d01f356..9f1df91 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,8 @@ reference which matches at least one match rule and none of the exclusion rules, by the `replace` contents of the rule. If `checkUpstream` is enabled, the webhook will first fetch the manifest the rewritten container image reference and verify it exists before rewriting the image. +You can also update the `imagePullSecrets` of a modified pod to have the right docker secret to connect to the modified registry. For that, put `replaceImagePullSecrets` to `true` and be sure that `authSecretName` is set with the Kubernetes secret that you want to add to `imagePullSecrets`. If `imagePullSecrets` already contains a secret, the `authSecretName` will be added to the list anyway. + Example configuration: ```yaml port: 9443 @@ -67,6 +69,12 @@ rules: replace: 'harbor.example.com/ubuntu-proxy' checkUpstream: true # tests if the manifest for the rewritten image exists authSecretName: harbor-example-image-pull-secret # optional, defaults to "" - secret in the webhook namespace for authenticating to harbor.example.com + - name: 'docker.io rewrite rule with imagePullSecrets update' + matches: + - '^docker.io' + replace: 'harbor.example.com/dockerhub-proxy' + replaceImagePullSecrets: true # enable imagePullSecrets change for modified images + authSecretName: harbor-example-image-pull-secret # secret to add to imagePullSecrets on the modified pod ``` Local Development === diff --git a/deploy/charts/harbor-container-webhook/Chart.yaml b/deploy/charts/harbor-container-webhook/Chart.yaml index 3f0c1e3..d9651cb 100644 --- a/deploy/charts/harbor-container-webhook/Chart.yaml +++ b/deploy/charts/harbor-container-webhook/Chart.yaml @@ -3,7 +3,7 @@ name: harbor-container-webhook description: Webhook to configure pods with harbor proxy cache projects type: application version: 0.8.1 -appVersion: "0.8.0" +appVersion: "0.9.0" kubeVersion: ">= 1.16.0-0" home: https://github.com/IndeedEng/harbor-container-webhook maintainers: diff --git a/deploy/charts/harbor-container-webhook/values.yaml b/deploy/charts/harbor-container-webhook/values.yaml index b2d4db2..1dc6ffc 100644 --- a/deploy/charts/harbor-container-webhook/values.yaml +++ b/deploy/charts/harbor-container-webhook/values.yaml @@ -113,6 +113,12 @@ rules: [] # platforms: # defaults to linux/amd64, only used if checkUpstream is set # - linux/amd64 # - linux/arm64 +# - name: 'docker.io rewrite rule with imagePullSecrets update' +# matches: +# - '^docker.io' +# replace: 'harbor.example.com/dockerhub-proxy' +# replaceImagePullSecrets: true # enable imagePullSecrets change for modified images +# authSecretName: harbor-example-image-pull-secret # secret to add to imagePullSecrets on the modified pod extraRules: [] diff --git a/hack/config.yaml b/hack/config.yaml index 495c118..87a0414 100644 --- a/hack/config.yaml +++ b/hack/config.yaml @@ -18,4 +18,11 @@ rules: - '^docker.io/(library/)?ubuntu:.*$' replace: 'harbor-v2.awscmhqa2.k8s.indeed.tech/dockerhub-proxy-auth' checkUpstream: true # tests if the manifest for the rewritten image exists - authSecretName: "harborv2-qa" \ No newline at end of file + authSecretName: "harborv2-qa" + - name: 'docker.io rewrite rule with imagePullSecret change' + # image refs must match at least one of the rules, and not match any excludes + matches: + - '^docker.io' + replace: 'harbor.example.com/dockerhub-proxy' + authSecretName: "harborv2-qa" + replaceImagePullSecrets: true diff --git a/hack/test/admission.json b/hack/test/admission.json index d2de16a..64e096d 100644 --- a/hack/test/admission.json +++ b/hack/test/admission.json @@ -13,6 +13,11 @@ "metadata": { }, "spec": { + "imagePullSecrets": [ + { + "name": "foo" + } + ], "containers": [ { "name": "foo", diff --git a/hack/test/no-op.json b/hack/test/no-op.json index f77fa53..0065f24 100644 --- a/hack/test/no-op.json +++ b/hack/test/no-op.json @@ -13,6 +13,11 @@ "metadata": { }, "spec": { + "imagePullSecrets": [ + { + "name": "foo" + } + ], "containers": [ { "name": "foo", diff --git a/internal/config/config.go b/internal/config/config.go index 99fb944..8793f41 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -79,10 +79,13 @@ type ProxyRule struct { // If the webhook lacks permissions to fetch the image manifest or the registry is down, the image // will not be rewritten. Experimental. CheckUpstream bool `yaml:"checkUpstream"` + // ReplaceImagePullSecrets enables the replacement of the imagePullSecrets of the pod in addition to the image + ReplaceImagePullSecrets bool `yaml:"replaceImagePullSecrets"` // List of the required platforms to check for if CheckUpstream is set. Defaults to "linux/amd64" if unset. Platforms []string `yaml:"platforms"` // AuthSecretName is a reference to an image pull secret (must be .dockerconfigjson type) which - // will be used to authenticate if `checkUpstream` is set. Unused if not specified or `checkUpstream` is false. + // will be used to authenticate if `checkUpstream` is set or to modify the imagePullSecrets if + // `replaceImagePullSecrets` is set. AuthSecretName string `yaml:"authSecretName"` // Namespace that the webhook is running in, used for accessing secrets for authenticated proxy rules Namespace string diff --git a/internal/webhook/mutate.go b/internal/webhook/mutate.go index cd81d04..51ae7ca 100644 --- a/internal/webhook/mutate.go +++ b/internal/webhook/mutate.go @@ -38,6 +38,7 @@ func (p *PodContainerProxier) Handle(ctx context.Context, req admission.Request) return admission.Errored(http.StatusBadRequest, err) } + // container images initContainers, updatedInit, err := p.updateContainers(ctx, pod.Spec.InitContainers, "init") if err != nil { return admission.Errored(http.StatusInternalServerError, err) @@ -46,11 +47,19 @@ func (p *PodContainerProxier) Handle(ctx context.Context, req admission.Request) if err != nil { return admission.Errored(http.StatusInternalServerError, err) } + pod.Spec.InitContainers = initContainers + pod.Spec.Containers = containers + if !updated && !updatedInit { return admission.Allowed("no updates") } - pod.Spec.InitContainers = initContainers - pod.Spec.Containers = containers + + // imagePullSecrets + imagePullSecrets, err := p.updateImagePullSecrets(p.getPodName(pod), pod.Spec.ImagePullSecrets) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + pod.Spec.ImagePullSecrets = imagePullSecrets marshaledPod, err := json.Marshal(pod) if err != nil { @@ -68,7 +77,7 @@ func (p *PodContainerProxier) lookupNodeArchAndOS(ctx context.Context, restClien return node.Status.NodeInfo.Architecture, node.Status.NodeInfo.OperatingSystem, nil } -func (p *PodContainerProxier) updateContainers(ctx context.Context, containers []corev1.Container, kind string) ([]corev1.Container, bool, error) { +func (p *PodContainerProxier) updateContainers(ctx context.Context, containers []corev1.Container, _ string) ([]corev1.Container, bool, error) { containersReplacement := make([]corev1.Container, 0, len(containers)) updated := false for i := range containers { @@ -118,3 +127,31 @@ func (p *PodContainerProxier) InjectDecoder(d admission.Decoder) error { p.Decoder = d return nil } + +func (p *PodContainerProxier) updateImagePullSecrets(podName string, imagePullSecrets []corev1.LocalObjectReference) (newImagePullSecrets []corev1.LocalObjectReference, err error) { + updated := false + for _, transformer := range p.Transformers { + updated, newImagePullSecrets, err = transformer.RewriteImagePullSecrets(imagePullSecrets) + if err != nil { + return imagePullSecrets, err + } + if !updated { + return imagePullSecrets, nil + } + logger.Info(fmt.Sprintf("rewriting the imagePullSecrets of the pod %s from %q to %q", podName, imagePullSecrets, newImagePullSecrets)) + } + return newImagePullSecrets, nil +} + +func (p *PodContainerProxier) getPodName(pod *corev1.Pod) (podName string) { + if pod.Name != "" { + return pod.Name + } + if pod.ObjectMeta.Labels["app.kubernetes.io/name"] != "" { + return pod.ObjectMeta.Labels["app.kubernetes.io/name"] + } + if pod.ObjectMeta.Labels["app"] != "" { + return pod.ObjectMeta.Labels["app"] + } + return pod.GenerateName +} diff --git a/internal/webhook/mutate_test.go b/internal/webhook/mutate_test.go index fda5917..d4eefd7 100644 --- a/internal/webhook/mutate_test.go +++ b/internal/webhook/mutate_test.go @@ -4,6 +4,8 @@ import ( "context" "testing" + corev1 "k8s.io/api/core/v1" + "github.com/indeedeng-alpha/harbor-container-webhook/internal/config" "github.com/stretchr/testify/require" @@ -27,6 +29,11 @@ func TestPodContainerProxier_rewriteImage(t *testing.T) { Matches: []string{"^docker.io/(library/)?ubuntu"}, Replace: "harbor.example.com/ubuntu-proxy", }, + { + Name: "docker.io proxy cache with imagePullSecret change", + Matches: []string{"^docker.io/(library/)?ubuntu"}, + Replace: "harbor.example.com/ubuntu-proxy", + }, }, nil) require.NoError(t, err) proxier := PodContainerProxier{ @@ -99,3 +106,83 @@ func TestPodContainerProxier_rewriteImage(t *testing.T) { }) } } + +func TestPodContainerProxier_updateImagePullSecretsWithReplaceEnabled(t *testing.T) { + transformers, err := MakeTransformers([]config.ProxyRule{ + { + Name: "docker.io proxy cache with imagePullSecrets change", + Matches: []string{"^docker.io"}, + Replace: "harbor.example.com/dockerhub-proxy", + ReplaceImagePullSecrets: true, + AuthSecretName: "secret-test", + }, + }, nil) + require.NoError(t, err) + proxier := PodContainerProxier{ + Transformers: transformers, + } + + type testcase struct { + name string + imagePullSecrets []corev1.LocalObjectReference + expected []corev1.LocalObjectReference + } + tests := []testcase{ + { + name: "imagePullSecrets is empty, replacement is expected and secret name should be added", + imagePullSecrets: []corev1.LocalObjectReference{}, + expected: []corev1.LocalObjectReference{{Name: "secret-test"}}, + }, + { + name: "imagePullSecrets has a secret, replacement is expected and secret name should be added", + imagePullSecrets: []corev1.LocalObjectReference{{Name: "mysecret"}}, + expected: []corev1.LocalObjectReference{{Name: "mysecret"}, {Name: "secret-test"}}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + newImagePullSecrets, err := proxier.updateImagePullSecrets("pod-test", tc.imagePullSecrets) + require.NoError(t, err) + require.Equal(t, tc.expected, newImagePullSecrets) + }) + } +} + +func TestPodContainerProxier_updateImagePullSecretsWithReplaceDinabled(t *testing.T) { + transformers, err := MakeTransformers([]config.ProxyRule{ + { + Name: "docker.io proxy cache without imagePullSecrets change", + Matches: []string{"^docker.io"}, + Replace: "harbor.example.com/dockerhub-proxy", + }, + }, nil) + require.NoError(t, err) + proxier := PodContainerProxier{ + Transformers: transformers, + } + + type testcase struct { + name string + imagePullSecrets []corev1.LocalObjectReference + expected []corev1.LocalObjectReference + } + tests := []testcase{ + { + name: "imagePullSecrets is empty, replacement is not expected", + imagePullSecrets: []corev1.LocalObjectReference{}, + expected: []corev1.LocalObjectReference{}, + }, + { + name: "imagePullSecrets has a secret, replacement is not expected", + imagePullSecrets: []corev1.LocalObjectReference{{Name: "mysecret"}}, + expected: []corev1.LocalObjectReference{{Name: "mysecret"}}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + newImagePullSecrets, err := proxier.updateImagePullSecrets("pod-test", tc.imagePullSecrets) + require.NoError(t, err) + require.Equal(t, tc.expected, newImagePullSecrets) + }) + } +} diff --git a/internal/webhook/transformer.go b/internal/webhook/transformer.go index a41e7f1..a56709e 100644 --- a/internal/webhook/transformer.go +++ b/internal/webhook/transformer.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "regexp" + "slices" "strings" "time" @@ -76,6 +77,9 @@ type ContainerTransformer interface { // CheckUpstream ensures that the docker image reference exists in the upstream registry // and returns if the image exists, or an error if the registry can't be contacted. CheckUpstream(ctx context.Context, imageRef string) (bool, error) + + // RewriteImagePullSecrets takes a list of kubernetes secret name and add the AuthSecretName parameter + RewriteImagePullSecrets(imagePullSecrets []corev1.LocalObjectReference) (bool, []corev1.LocalObjectReference, error) } func MakeTransformers(rules []config.ProxyRule, client client.Client) ([]ContainerTransformer, error) { @@ -272,3 +276,43 @@ func (t *ruleTransformer) anyExclusion(imageRef string) bool { } return false } + +func (t *ruleTransformer) RewriteImagePullSecrets(imagePullSecrets []corev1.LocalObjectReference) (updated bool, newImagePullSecrets []corev1.LocalObjectReference, err error) { + if t.rule.AuthSecretName == "" && t.rule.ReplaceImagePullSecrets { + return false, imagePullSecrets, fmt.Errorf("replaceImagePullSecrets is enabled but no authSecretName parameter") + } + if !t.rule.ReplaceImagePullSecrets { + return false, imagePullSecrets, nil + } + + start := time.Now() + updated, imagePullSecrets = t.doRewriteImagePullSecrets(imagePullSecrets) + duration := time.Since(start) + if updated { + rewrite.WithLabelValues(t.metricName).Inc() + rewriteTime.WithLabelValues(t.metricName).Observe(duration.Seconds()) + } else if !updated { + return false, imagePullSecrets, nil + } + return true, imagePullSecrets, nil +} + +func (t *ruleTransformer) doRewriteImagePullSecrets(imagePullSecrets []corev1.LocalObjectReference) (bool, []corev1.LocalObjectReference) { + existingSecrets := t.getExistingSecrets(imagePullSecrets) + + if slices.Contains(existingSecrets, t.rule.AuthSecretName) { + return false, imagePullSecrets + } + newImagePullSecret := corev1.LocalObjectReference{ + Name: t.rule.AuthSecretName, + } + imagePullSecrets = append(imagePullSecrets, newImagePullSecret) + return true, imagePullSecrets +} + +func (t *ruleTransformer) getExistingSecrets(imagePullSecrets []corev1.LocalObjectReference) (existingSecrets []string) { + for _, secret := range imagePullSecrets { + existingSecrets = append(existingSecrets, secret.Name) + } + return existingSecrets +}