Skip to content

Commit

Permalink
fix(taskrun): emit warning for missing secret in ServiceAccount inste…
Browse files Browse the repository at this point in the history
…ad of failing

fix #7760

Log a warning if a Secrets in service account does not exist
  • Loading branch information
l-qing authored and tekton-robot committed Apr 25, 2024
1 parent fba68b7 commit a494d6a
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 3 deletions.
24 changes: 23 additions & 1 deletion pkg/pod/creds_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"regexp"
"strings"

"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
Expand All @@ -29,8 +30,12 @@ import (
"github.com/tektoncd/pipeline/pkg/credentials/gitcreds"
"github.com/tektoncd/pipeline/pkg/names"
corev1 "k8s.io/api/core/v1"
k8serrors "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 (
Expand All @@ -51,7 +56,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
Expand All @@ -73,6 +79,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
Expand All @@ -87,6 +104,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 k8serrors.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
}
Expand Down
55 changes: 54 additions & 1 deletion pkg/pod/creds_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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",
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
}
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit a494d6a

Please sign in to comment.