Add imagePullSecrets change

paretl · paretl · commit 7260b737f4f5 · 2025-03-20T17:02:34.000+01:00
diff --git 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/Makefile b/Makefile
@@ -17,7 +17,7 @@ DOCKER_BUILDX_BUILDER ?= "harbor-container-webhook"
 all: $(addprefix build-,$(ARCH))
 
 # Image registry for build/push image targets
-IMAGE_REGISTRY ?= ghcr.io/indeedeng/harbor-container-webhook
+IMAGE_REGISTRY ?= harbor.monitoring-lz800.mindee.net/mindee/harbor-container-webhook
 
 HELM_DIR    ?= deploy/charts/harbor-container-webhook
 
diff --git 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
@@ -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
@@ -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
@@ -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"
+    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
@@ -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
@@ -13,6 +13,11 @@
       "metadata": {
       },
       "spec": {
+        "imagePullSecrets": [
+          {
+            "name": "foo"
+          }
+        ],
         "containers": [
           {
             "name": "foo",
diff --git 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
@@ -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)
@@ -52,6 +53,16 @@ func (p *PodContainerProxier) Handle(ctx context.Context, req admission.Request)
 	pod.Spec.InitContainers = initContainers
 	pod.Spec.Containers = containers
 
+	// imagePullSecrets
+	imagePullSecrets, updatedImagePullSecrets, err := p.updateImagePullSecrets(pod.Spec.ImagePullSecrets)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+	if !updatedImagePullSecrets {
+		return admission.Allowed("no updates")
+	}
+	pod.Spec.ImagePullSecrets = imagePullSecrets
+
 	marshaledPod, err := json.Marshal(pod)
 	if err != nil {
 		return admission.Errored(http.StatusInternalServerError, err)
@@ -68,7 +79,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 +129,15 @@ func (p *PodContainerProxier) InjectDecoder(d admission.Decoder) error {
 	p.Decoder = d
 	return nil
 }
+
+func (p *PodContainerProxier) updateImagePullSecrets(imagePullSecrets []corev1.LocalObjectReference) (newImagePullSecrets []corev1.LocalObjectReference, updated bool, err error) {
+	pod := &corev1.Pod{}
+	for _, transformer := range p.Transformers {
+		updated, newImagePullSecrets, err = transformer.RewriteImagePullSecrets(imagePullSecrets)
+		if err != nil {
+			return imagePullSecrets, false, err
+		}
+		logger.Info(fmt.Sprintf("rewriting the imagePullSecrets of the pod %q from %q to %q", pod.ObjectMeta.Name, imagePullSecrets, newImagePullSecrets))
+	}
+	return newImagePullSecrets, updated, nil
+}
diff --git 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,95 @@ 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
+		platform         string
+		os               string
+		expected         []corev1.LocalObjectReference
+	}
+	tests := []testcase{
+		{
+			name:             "imagePullSecrets is empty, replacement is expected and secret name should be added",
+			imagePullSecrets: []corev1.LocalObjectReference{},
+			os:               "linux",
+			platform:         "amd64",
+			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"}},
+			os:               "linux",
+			platform:         "amd64",
+			expected:         []corev1.LocalObjectReference{{Name: "mysecret"}, {Name: "secret-test"}},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			newImagePullSecrets, _, err := proxier.updateImagePullSecrets(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
+		platform         string
+		os               string
+		expected         []corev1.LocalObjectReference
+	}
+	tests := []testcase{
+		{
+			name:             "imagePullSecrets is empty, replacement is not expected",
+			imagePullSecrets: []corev1.LocalObjectReference{},
+			os:               "linux",
+			platform:         "amd64",
+			expected:         []corev1.LocalObjectReference{},
+		},
+		{
+			name:             "imagePullSecrets has a secret, replacement is not expected",
+			imagePullSecrets: []corev1.LocalObjectReference{{Name: "mysecret"}},
+			os:               "linux",
+			platform:         "amd64",
+			expected:         []corev1.LocalObjectReference{{Name: "mysecret"}},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			newImagePullSecrets, _, err := proxier.updateImagePullSecrets(tc.imagePullSecrets)
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, newImagePullSecrets)
+		})
+	}
+}
diff --git 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
+}
