From 16f4ccf9a8c7632a1e9f9ae63b437ab2bdcb6e76 Mon Sep 17 00:00:00 2001 From: qingliu Date: Sun, 17 Mar 2024 14:15:53 +0800 Subject: [PATCH] fix(taskrun): emit warning for missing secret in ServiceAccount instead of failing fix #7760 Log a warning if a Secrets in service account does not exist --- pkg/pod/creds_init.go | 24 ++++++++++++++++- pkg/pod/creds_init_test.go | 55 +++++++++++++++++++++++++++++++++++++- pkg/pod/pod.go | 2 +- 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/pkg/pod/creds_init.go b/pkg/pod/creds_init.go index 1556f95395f..3fe46e1b43b 100644 --- a/pkg/pod/creds_init.go +++ b/pkg/pod/creds_init.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "regexp" + "strings" "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" @@ -28,8 +29,12 @@ import ( "github.com/tektoncd/pipeline/pkg/credentials/gitcreds" "github.com/tektoncd/pipeline/pkg/names" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" + "knative.dev/pkg/controller" + "knative.dev/pkg/logging" ) const ( @@ -50,7 +55,8 @@ var dnsLabel1123Forbidden = regexp.MustCompile("[^a-zA-Z0-9-]+") // Any errors encountered during this process are returned to the // caller. If no matching annotated secrets are found, nil lists with a // nil error are returned. -func credsInit(ctx context.Context, serviceAccountName, namespace string, kubeclient kubernetes.Interface) ([]string, []corev1.Volume, []corev1.VolumeMount, error) { +func credsInit(ctx context.Context, obj runtime.Object, serviceAccountName, namespace string, kubeclient kubernetes.Interface) ([]string, []corev1.Volume, []corev1.VolumeMount, error) { + logger := logging.FromContext(ctx) cfg := config.FromContextOrDefaults(ctx) if cfg != nil && cfg.FeatureFlags != nil && cfg.FeatureFlags.DisableCredsInit { return nil, nil, nil, nil @@ -72,6 +78,17 @@ func credsInit(ctx context.Context, serviceAccountName, namespace string, kubecl var volumeMounts []corev1.VolumeMount var volumes []corev1.Volume var args []string + var missingSecrets []string + + defer func() { + recorder := controller.GetEventRecorder(ctx) + if len(missingSecrets) > 0 && recorder != nil && obj != nil { + recorder.Eventf(obj, corev1.EventTypeWarning, "FailedToRetrieveSecret", + "Unable to retrieve some secrets (%s); attempting to use them may not succeed.", + strings.Join(missingSecrets, ", ")) + } + }() + // Track duplicated secrets, prevent errors like this: // Pod "xxx" is invalid: spec.containers[0].volumeMounts[12].mountPath: Invalid value: // "/tekton/creds-secrets/demo-docker-credentials": must be unique @@ -86,6 +103,11 @@ func credsInit(ctx context.Context, serviceAccountName, namespace string, kubecl visitedSecrets[secretEntry.Name] = struct{}{} secret, err := kubeclient.CoreV1().Secrets(namespace).Get(ctx, secretEntry.Name, metav1.GetOptions{}) + if errors.IsNotFound(err) { + missingSecrets = append(missingSecrets, secretEntry.Name) + logger.Warnf("Secret %q in ServiceAccount %s/%s not found, skipping", secretEntry.Name, namespace, serviceAccountName) + continue + } if err != nil { return nil, nil, nil, err } diff --git a/pkg/pod/creds_init_test.go b/pkg/pod/creds_init_test.go index 25b2c883abb..c7dcacf81c3 100644 --- a/pkg/pod/creds_init_test.go +++ b/pkg/pod/creds_init_test.go @@ -28,6 +28,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" fakek8s "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/record" + "knative.dev/pkg/controller" logtesting "knative.dev/pkg/logging/testing" "knative.dev/pkg/system" ) @@ -49,6 +51,7 @@ func TestCredsInit(t *testing.T) { wantVolumeMounts []corev1.VolumeMount objs []runtime.Object envVars []corev1.EnvVar + events []string ctx context.Context }{{ desc: "service account exists with no secrets; nothing to initialize", @@ -288,11 +291,52 @@ func TestCredsInit(t *testing.T) { MountPath: "/tekton/creds-secrets/my-creds", }}, ctx: context.Background(), + }, { + desc: "service account has not found secrets", + objs: []runtime.Object{ + &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: serviceAccountName, Namespace: namespace}, + Secrets: []corev1.ObjectReference{ + {Name: "my-creds"}, + {Name: "not-exist-1"}, + {Name: "not-exist-2"}, + }, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-creds", + Namespace: namespace, + Annotations: map[string]string{ + "tekton.dev/git-0": "github.com", + }, + }, + Type: "kubernetes.io/basic-auth", + Data: map[string][]byte{ + "username": []byte("foo"), + "password": []byte("BestEver"), + }, + }, + }, + envVars: []corev1.EnvVar{}, + wantArgs: []string{ + "-basic-git=my-creds=github.com", + }, + wantVolumeMounts: []corev1.VolumeMount{{ + Name: "tekton-internal-secret-volume-my-creds-9l9zj", + MountPath: "/tekton/creds-secrets/my-creds", + }}, + events: []string{ + `Warning FailedToRetrieveSecret Unable to retrieve some secrets (not-exist-1, not-exist-2); attempting to use them may not succeed.`, + }, + ctx: context.Background(), }} { t.Run(c.desc, func(t *testing.T) { names.TestingSeed() + eventObj := &corev1.Event{} kubeclient := fakek8s.NewSimpleClientset(c.objs...) - args, volumes, volumeMounts, err := credsInit(c.ctx, serviceAccountName, namespace, kubeclient) + recorder := record.NewFakeRecorder(1000) + c.ctx = controller.WithEventRecorder(c.ctx, recorder) + args, volumes, volumeMounts, err := credsInit(c.ctx, eventObj, serviceAccountName, namespace, kubeclient) if err != nil { t.Fatalf("credsInit: %v", err) } @@ -305,6 +349,15 @@ func TestCredsInit(t *testing.T) { if d := cmp.Diff(c.wantVolumeMounts, volumeMounts); d != "" { t.Fatalf("Diff %s", diff.PrintWantGot(d)) } + if len(recorder.Events) != len(c.events) { + t.Fatalf("Expected %d events, got %d", len(c.events), len(recorder.Events)) + } + for i, e := range c.events { + message := <-recorder.Events + if message != e { + t.Fatalf("Expected event %d to be %q, got %q", i, e, message) + } + } }) } } diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index d63111db427..585224d6ed3 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -179,7 +179,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta if config.IsSpireEnabled(ctx) { commonExtraEntrypointArgs = append(commonExtraEntrypointArgs, "-enable_spire") } - credEntrypointArgs, credVolumes, credVolumeMounts, err := credsInit(ctx, taskRun.Spec.ServiceAccountName, taskRun.Namespace, b.KubeClient) + credEntrypointArgs, credVolumes, credVolumeMounts, err := credsInit(ctx, taskRun, taskRun.Spec.ServiceAccountName, taskRun.Namespace, b.KubeClient) if err != nil { return nil, err }