Implement --skip-prefixes parameter

* Allows specific image prefixes to be skipped from digest resolution
* Implemented for webhook and kpt
* kpt support also includes environment variable SKIP_PREFIXES bound to the same parameter
* refactors kubeconfig parsing of multi value strings into util.StringArray for reuse
* common-issues.md updated describing the use case

build/examples/pod-diff-registry.yaml

@@ -0,0 +1,25 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: v1
+kind: Pod
+metadata:
+  name: hello-pod
+spec:
+  containers:
+  - name: busybox
+    image: busybox:latest
+  initContainers:
+  - name: init
+    image: gcr.io/google-samples/hello-app:1.0

cmd/function/function.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/go-logr/logr"
 	"github.com/spf13/cobra"
@@ -33,6 +32,7 @@ import (
 
 	"github.com/google/k8s-digester/pkg/logging"
 	"github.com/google/k8s-digester/pkg/resolve"
+	"github.com/google/k8s-digester/pkg/util"
 )
 
 // Cmd creates the KRM function command. This is the root command.
@@ -50,24 +50,23 @@ func createResourceFn(ctx context.Context, log logr.Logger) framework.ResourceLi
 	return func(resourceList *framework.ResourceList) error {
 		log.V(2).Info("kubeconfig", "kubeconfig", viper.GetString("kubeconfig"))
 		log.V(2).Info("offline", "offline", viper.GetBool("offline"))
+		log.V(2).Info("skip-prefixes", "skip-prefixes", util.StringArray(viper.GetString("skip-prefixes")))
 		var config *rest.Config
 		if !viper.GetBool("offline") {
 			var kubeconfig string
 			var err error
-			kubeconfigs := strings.FieldsFunc(viper.GetString("kubeconfig"), func(r rune) bool {
-				return r == ':' || r == ';'
-			})
+			kubeconfigs := util.StringArray(viper.GetString("kubeconfig"))
 			if len(kubeconfigs) > 0 {
 				kubeconfig = kubeconfigs[0]
 			}
-			
+
 			config, err = createConfig(log, kubeconfig)
 			if err != nil {
 				return fmt.Errorf("could not create k8s client config: %w", err)
 			}
 		}
 		for _, r := range resourceList.Items {
-			if err := resolve.ImageTags(ctx, log, config, r); err != nil {
+			if err := resolve.ImageTags(ctx, log, config, r, util.StringArray(viper.GetString("skip-prefixes"))); err != nil {
 				return err
 			}
 		}
@@ -92,6 +91,9 @@ func customizeCmd(cmd *cobra.Command) {
 		"do not connect to Kubernetes API server to retrieve imagePullSecrets")
 	viper.BindPFlag("offline", cmd.Flags().Lookup("offline"))
 	viper.BindEnv("offline")
+	cmd.Flags().String("skip-prefixes", "", "(optional) image prefixes that should not be resolved to digests, colon separated")
+	viper.BindPFlag("skip-prefixes", cmd.Flags().Lookup("skip-prefixes"))
+	viper.BindEnv("skip-prefixes", "SKIP_PREFIXES")
 }
 
 // getKubeconfigDefault determines the default value of the --kubeconfig flag.

cmd/webhook/webhook.go

@@ -78,6 +78,7 @@ var (
 	offline             bool
 	port                int
 	ignoreErrors        bool
+	skipPrefixes        string
 )
 
 func init() {
@@ -90,6 +91,7 @@ func init() {
 	Cmd.Flags().BoolVar(&offline, "offline", false, "do not connect to API server to retrieve imagePullSecrets")
 	Cmd.Flags().IntVar(&port, "port", defaultPort, "webhook server port")
 	Cmd.Flags().BoolVar(&ignoreErrors, "ignore-errors", false, "do not fail on webhook admission errors, just log them")
+	Cmd.Flags().StringVar(&skipPrefixes, "skip-prefixes", "", "(optional) image prefixes that should not be resolved to digests, colon separated")
 }
 
 func run(ctx context.Context) error {
@@ -146,7 +148,7 @@ func run(ctx context.Context) error {
 		close(certSetupFinished)
 	}
 
-	go setupControllers(mgr, log, dryRun, ignoreErrors, certSetupFinished)
+	go setupControllers(mgr, log, dryRun, ignoreErrors, certSetupFinished, util.StringArray(skipPrefixes))
 
 	log.Info("starting manager")
 	if err := mgr.Start(ctx); err != nil {
@@ -155,7 +157,7 @@ func run(ctx context.Context) error {
 	return nil
 }
 
-func setupControllers(mgr manager.Manager, log logr.Logger, dryRun bool, ignoreErrors bool, certSetupFinished chan struct{}) {
+func setupControllers(mgr manager.Manager, log logr.Logger, dryRun bool, ignoreErrors bool, certSetupFinished chan struct{}, skipPrefixes []string) {
 	log.Info("waiting for cert rotation setup")
 	<-certSetupFinished
 	log.Info("done waiting for cert rotation setup")
@@ -168,6 +170,7 @@ func setupControllers(mgr manager.Manager, log logr.Logger, dryRun bool, ignoreE
 		DryRun:       dryRun,
 		IgnoreErrors: ignoreErrors,
 		Config:       config,
+		SkipPrefixes: skipPrefixes,
 	}
 	mwh := &admission.Webhook{Handler: whh}
 	log.Info("starting webhook server", "path", webhookPath)

docs/common-issues.md

@@ -108,3 +108,40 @@ The steps below use the `HTTPS_PROXY` environment variable.
 Note that this will not work for proxies that require NTLM authentication.
 
 Ref: https://knative.dev/docs/serving/tag-resolution/#corporate-proxy
+
+## Interaction with systems expecting tags, particularly cloud managed services
+
+If digester updates an image tag that is being actively managed by a cloud controller then
+it may cause the cloud controller to behave unexpectedly.
+
+One example of this is the Anthos Service Mesh Managed Dataplane Controller which looks
+for specific tagged versions of the istio-proxy sidecar injected by the mutating webhook.
+
+Replacement of the tagged names with digest values can, under these circumstances, create
+an edge case for the cloud managed services handling unepected values in unforseen ways such
+as updating pods and terminating them once they have already been updated (since the image
+does not match the value set by the controller with only the tag).
+
+In these circumstances and if you are using digester to provide a tag feature when using
+Binary Authorisation it is worth noting that there is a capability to whitelist certain
+image registries and repo locations within Binary Authorisation. ASM images are by default
+whitelisted by the policy.
+
+To avoid digester replacing the tagged version expected by mdp-controller in these instances
+one can utilise the --skip-prefixes option to the webhook which takes a set of prefixes
+separated by a colon (if multiple prefixes are needed).
+
+The parameter can be added to the webhook args in the deployment, the following is an
+example
+```
+        args:
+        - webhook
+        - --cert-dir=/certs # kpt-set: --cert-dir=${cert-dir}
+        - --disable-cert-rotation=false # kpt-set: --disable-cert-rotation=${disable-cert-rotation}
+        - --dry-run=false # kpt-set: --dry-run=${dry-run}
+        - --health-addr=:9090 # kpt-set: --health-addr=:${health-port}
+        - --metrics-addr=:8888 # kpt-set: --metrics-addr=:${metrics-port}
+        - --offline=false # kpt-set: --offline=${offline}
+        - --port=8443 # kpt-set: --port=${port}
+        - --skip-prefixes=gcr.io/gke-release/asm/mdp:gcr.io/gke-release/asm/proxyv2
+```

pkg/handler/handler.go

@@ -44,6 +44,7 @@ type Handler struct {
 	DryRun       bool
 	IgnoreErrors bool
 	Config       *rest.Config
+	SkipPrefixes []string
 }
 
 var resolveImageTags = resolve.ImageTags // override for testing
@@ -74,7 +75,7 @@ func (h *Handler) Handle(ctx context.Context, req admission.Request) admission.R
 		return h.admissionError(err)
 	}
 
-	if err = resolveImageTags(ctx, h.Log, h.Config, r); err != nil {
+	if err = resolveImageTags(ctx, h.Log, h.Config, r, h.SkipPrefixes); err != nil {
 		return h.admissionError(err)
 	}
 

pkg/handler/handler_test.go

@@ -123,7 +123,7 @@ func Test_Handle_NotPatchedWhenNoChange(t *testing.T) {
 			},
 		},
 	}
-	resolveImageTags = func(_ context.Context, _ logr.Logger, _ *rest.Config, _ *yaml.RNode) error {
+	resolveImageTags = func(_ context.Context, _ logr.Logger, _ *rest.Config, _ *yaml.RNode, _ []string) error {
 		return nil
 	}
 	h := &Handler{Log: log}
@@ -146,7 +146,7 @@ func Test_Handle_Patch(t *testing.T) {
 		},
 	}
 	imageWithDigest := "registry.example.com/repository/image:tag@sha256:digest"
-	resolveImageTags = func(_ context.Context, _ logr.Logger, _ *rest.Config, n *yaml.RNode) error {
+	resolveImageTags = func(_ context.Context, _ logr.Logger, _ *rest.Config, n *yaml.RNode, _ []string) error {
 		return n.PipeE(yaml.Lookup("spec", "containers", "0", "image"), yaml.FieldSetter{StringValue: imageWithDigest})
 	}
 	h := &Handler{Log: log}

pkg/resolve/resolve.go

@@ -45,14 +45,15 @@ var resolveTagFn = resolveTag // override for unit testing
 // - `spec.jobTemplate.spec.template.spec.initContainers`
 // The `config` input parameter can be null. In this case, the function
 // will not attempt to retrieve imagePullSecrets from the cluster.
-func ImageTags(ctx context.Context, log logr.Logger, config *rest.Config, n *yaml.RNode) error {
+func ImageTags(ctx context.Context, log logr.Logger, config *rest.Config, n *yaml.RNode, skipPrefixes []string) error {
 	kc, err := keychain.Create(ctx, log, config, n)
 	if err != nil {
 		return fmt.Errorf("could not create keychain: %w", err)
 	}
 	imageTagFilter := &ImageTagFilter{
-		Log:      log,
-		Keychain: kc,
+		Log:          log,
+		Keychain:     kc,
+		SkipPrefixes: &skipPrefixes,
 	}
 	// if input is a CronJob, we need to look up the image tags in the
 	// `spec.jobTemplate.spec.template.spec` path as well
@@ -76,8 +77,9 @@ func ImageTags(ctx context.Context, log logr.Logger, config *rest.Config, n *yam
 
 // ImageTagFilter resolves image tags to digests
 type ImageTagFilter struct {
-	Log      logr.Logger
-	Keychain authn.Keychain
+	Log          logr.Logger
+	Keychain     authn.Keychain
+	SkipPrefixes *[]string
 }
 
 var _ yaml.Filter = &ImageTagFilter{}
@@ -97,6 +99,12 @@ func (f *ImageTagFilter) filterImage(n *yaml.RNode) error {
 		return fmt.Errorf("could not lookup image in node %v: %w", s, err)
 	}
 	image := yaml.GetValue(imageNode)
+	for _, prefix := range *f.SkipPrefixes {
+		if strings.HasPrefix(image, prefix) {
+			// Image should be excluded from digest resolution
+			return nil
+		}
+	}
 	if strings.Contains(image, "@") {
 		return nil // already has digest, skip
 	}

pkg/resolve/resolve_test.go

@@ -42,8 +42,9 @@ var (
 	ctx    = context.Background()
 	log    = logging.CreateDiscardLogger()
 	filter = &ImageTagFilter{
-		Log:      log,
-		Keychain: &anonymousKeychain{},
+		Log:          log,
+		Keychain:     &anonymousKeychain{},
+		SkipPrefixes: &[]string{},
 	}
 )
 
@@ -100,7 +101,7 @@ func Test_ImageTags_Pod(t *testing.T) {
 		t.Fatalf("could not create pod node: %v", err)
 	}
 
-	if err := ImageTags(ctx, log, nil, node); err != nil {
+	if err := ImageTags(ctx, log, nil, node, []string{}); err != nil {
 		t.Fatalf("problem resolving image tags: %v", err)
 	}
 	t.Log(node.MustString())
@@ -111,13 +112,30 @@ func Test_ImageTags_Pod(t *testing.T) {
 	assertContainer(t, node, "image3@sha256:b0542da3f90bad69318e16ec7fcb6b13b089971886999e08bec91cea34891f0f", "spec", "initContainers", "[name=initcontainer1]")
 }
 
+func Test_ImageTags_Pod_Skip_Prefixes(t *testing.T) {
+	node, err := createPodNode([]string{"image0", "skip1.local/image1"}, []string{"image2", "skip2.local/image3"})
+	if err != nil {
+		t.Fatalf("could not create pod node: %v", err)
+	}
+
+	if err := ImageTags(ctx, log, nil, node, []string{"skip1.local", "skip2.local"}); err != nil {
+		t.Fatalf("problem resolving image tags: %v", err)
+	}
+	t.Log(node.MustString())
+
+	assertContainer(t, node, "image0@sha256:07d7d43fe9dd151e40f0a8d54c5211a8601b04e4a8fa7ad57ea5e73e4ffa7e4a", "spec", "containers", "[name=container0]")
+	assertContainer(t, node, "skip1.local/image1", "spec", "containers", "[name=container1]")
+	assertContainer(t, node, "image2@sha256:5bb21ac469b5e7df4e17899d4aae0adfb430f0f0b336a2242ef1a22d25bd2e53", "spec", "initContainers", "[name=initcontainer0]")
+	assertContainer(t, node, "skip2.local/image3", "spec", "initContainers", "[name=initcontainer1]")
+}
+
 func Test_ImageTags_CronJob(t *testing.T) {
 	node, err := createCronJobNode([]string{"image0", "image1"}, []string{"image2", "image3"})
 	if err != nil {
 		t.Fatalf("could not create pod node: %v", err)
 	}
 
-	if err := ImageTags(ctx, log, nil, node); err != nil {
+	if err := ImageTags(ctx, log, nil, node, []string{}); err != nil {
 		t.Fatalf("problem resolving image tags: %v", err)
 	}
 	t.Log(node.MustString())
@@ -134,7 +152,7 @@ func Test_ImageTags_Deployment(t *testing.T) {
 		t.Fatalf("could not create deployment node: %v", err)
 	}
 
-	if err := ImageTags(ctx, log, nil, node); err != nil {
+	if err := ImageTags(ctx, log, nil, node, []string{}); err != nil {
 		t.Fatalf("problem resolving image tags: %v", err)
 	}
 	t.Log(node.MustString())

pkg/util/stringarray.go

@@ -0,0 +1,11 @@
+package util
+
+import (
+	"strings"
+)
+
+func StringArray(str string) []string {
+	return strings.FieldsFunc(str, func(r rune) bool {
+		return r == ':' || r == ';'
+	})
+}