From 34d75c2244c9c465a8336e628ad16c1480b3eec0 Mon Sep 17 00:00:00 2001 From: Andy Smith Date: Wed, 22 Nov 2023 11:31:05 +0000 Subject: [PATCH] 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 | 25 ++++++++++++++++++ cmd/function/function.go | 14 +++++----- cmd/webhook/webhook.go | 7 +++-- docs/common-issues.md | 37 +++++++++++++++++++++++++++ pkg/handler/handler.go | 3 ++- pkg/handler/handler_test.go | 4 +-- pkg/resolve/resolve.go | 18 +++++++++---- pkg/resolve/resolve_test.go | 28 ++++++++++++++++---- pkg/util/stringarray.go | 11 ++++++++ 9 files changed, 126 insertions(+), 21 deletions(-) create mode 100644 build/examples/pod-diff-registry.yaml create mode 100644 pkg/util/stringarray.go diff --git a/build/examples/pod-diff-registry.yaml b/build/examples/pod-diff-registry.yaml new file mode 100644 index 0000000..64e31d5 --- /dev/null +++ b/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 diff --git a/cmd/function/function.go b/cmd/function/function.go index dc6702e..a4fa68f 100644 --- a/cmd/function/function.go +++ b/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. diff --git a/cmd/webhook/webhook.go b/cmd/webhook/webhook.go index fcefe5f..b49a991 100644 --- a/cmd/webhook/webhook.go +++ b/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) diff --git a/docs/common-issues.md b/docs/common-issues.md index 1cf7b92..5aecfce 100644 --- a/docs/common-issues.md +++ b/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 Authorization it is worth noting that there is a capability to whitelist certain +image registries and repo locations within Binary Authorization. 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 +``` \ No newline at end of file diff --git a/pkg/handler/handler.go b/pkg/handler/handler.go index 290c79e..8d06c3e 100644 --- a/pkg/handler/handler.go +++ b/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) } diff --git a/pkg/handler/handler_test.go b/pkg/handler/handler_test.go index 03df2a5..9088ef8 100644 --- a/pkg/handler/handler_test.go +++ b/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} diff --git a/pkg/resolve/resolve.go b/pkg/resolve/resolve.go index ff41bad..372be78 100644 --- a/pkg/resolve/resolve.go +++ b/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 } diff --git a/pkg/resolve/resolve_test.go b/pkg/resolve/resolve_test.go index 562cbe3..d380bf5 100644 --- a/pkg/resolve/resolve_test.go +++ b/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()) diff --git a/pkg/util/stringarray.go b/pkg/util/stringarray.go new file mode 100644 index 0000000..0baf62c --- /dev/null +++ b/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 == ';' + }) +}