From 3f280ca1337b752a1e6be3d9ea2fa63202d5f2d2 Mon Sep 17 00:00:00 2001 From: Dan Ramich Date: Sat, 3 Dec 2022 07:12:27 -0700 Subject: [PATCH] Update ScanSA calls to account for blank service accounts (#1871) The original check in ScanSA assumed the service account on the pod spec was not blank. It can be blank and when it is kubernetes will default to using the 'default' service account. This change extends the check logic to also include that. --- internal/dao/cronjob.go | 2 +- internal/dao/dp.go | 2 +- internal/dao/ds.go | 2 +- internal/dao/helpers.go | 12 ++++++++++++ internal/dao/helpers_test.go | 38 ++++++++++++++++++++++++++++++++++++ internal/dao/job.go | 2 +- internal/dao/pod.go | 2 +- internal/dao/sts.go | 2 +- 8 files changed, 56 insertions(+), 6 deletions(-) diff --git a/internal/dao/cronjob.go b/internal/dao/cronjob.go index 38ed4807..b3faf365 100644 --- a/internal/dao/cronjob.go +++ b/internal/dao/cronjob.go @@ -98,7 +98,7 @@ func (c *CronJob) ScanSA(ctx context.Context, fqn string, wait bool) (Refs, erro if err != nil { return nil, errors.New("expecting CronJob resource") } - if cj.Spec.JobTemplate.Spec.Template.Spec.ServiceAccountName == n { + if serviceAccountMatches(cj.Spec.JobTemplate.Spec.Template.Spec.ServiceAccountName, n) { refs = append(refs, Ref{ GVR: c.GVR(), FQN: client.FQN(cj.Namespace, cj.Name), diff --git a/internal/dao/dp.go b/internal/dao/dp.go index f59be35a..3c426e7f 100644 --- a/internal/dao/dp.go +++ b/internal/dao/dp.go @@ -167,7 +167,7 @@ func (d *Deployment) ScanSA(ctx context.Context, fqn string, wait bool) (Refs, e if err != nil { return nil, errors.New("expecting Deployment resource") } - if dp.Spec.Template.Spec.ServiceAccountName == n { + if serviceAccountMatches(dp.Spec.Template.Spec.ServiceAccountName, n) { refs = append(refs, Ref{ GVR: d.GVR(), FQN: client.FQN(dp.Namespace, dp.Name), diff --git a/internal/dao/ds.go b/internal/dao/ds.go index e1b04633..1ef7bf39 100644 --- a/internal/dao/ds.go +++ b/internal/dao/ds.go @@ -187,7 +187,7 @@ func (d *DaemonSet) ScanSA(ctx context.Context, fqn string, wait bool) (Refs, er if err != nil { return nil, errors.New("expecting DaemonSet resource") } - if ds.Spec.Template.Spec.ServiceAccountName == n { + if serviceAccountMatches(ds.Spec.Template.Spec.ServiceAccountName, n) { refs = append(refs, Ref{ GVR: d.GVR(), FQN: client.FQN(ds.Namespace, ds.Name), diff --git a/internal/dao/helpers.go b/internal/dao/helpers.go index 19d05aa9..c1b0a047 100644 --- a/internal/dao/helpers.go +++ b/internal/dao/helpers.go @@ -14,6 +14,8 @@ import ( "k8s.io/cli-runtime/pkg/printers" ) +const defaultServiceAccount = "default" + var ( inverseRx = regexp.MustCompile(`\A\!`) fuzzyRx = regexp.MustCompile(`\A\-f`) @@ -81,3 +83,13 @@ func ToYAML(o runtime.Object, showManaged bool) (string, error) { return buff.String(), nil } + +// serviceAccountMatches validates that the ServiceAccount referenced in the PodSpec matches the incoming +// ServiceAccount. If the PodSpec ServiceAccount is blank kubernetes will use the "default" ServiceAccount +// when deploying the pod, so if the incoming SA is "default" and podSA is an empty string that is also a match. +func serviceAccountMatches(podSA, saName string) bool { + if podSA == "" { + podSA = defaultServiceAccount + } + return podSA == saName +} diff --git a/internal/dao/helpers_test.go b/internal/dao/helpers_test.go index 1f6280e8..8687e119 100644 --- a/internal/dao/helpers_test.go +++ b/internal/dao/helpers_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" ) func TestToPerc(t *testing.T) { @@ -19,3 +20,40 @@ func TestToPerc(t *testing.T) { assert.Equal(t, u.e, toPerc(u.v1, u.v2)) } } + +func TestServiceAccountMatches(t *testing.T) { + uu := []struct { + podTemplate *v1.PodSpec + saName string + expect bool + }{ + {podTemplate: &v1.PodSpec{ + ServiceAccountName: "", + }, + saName: defaultServiceAccount, + expect: true, + }, + {podTemplate: &v1.PodSpec{ + ServiceAccountName: "", + }, + saName: "foo", + expect: false, + }, + {podTemplate: &v1.PodSpec{ + ServiceAccountName: "foo", + }, + saName: "foo", + expect: true, + }, + {podTemplate: &v1.PodSpec{ + ServiceAccountName: "foo", + }, + saName: "bar", + expect: false, + }, + } + + for _, u := range uu { + assert.Equal(t, u.expect, serviceAccountMatches(u.podTemplate.ServiceAccountName, u.saName)) + } +} diff --git a/internal/dao/job.go b/internal/dao/job.go index dfc35c8e..6264dff3 100644 --- a/internal/dao/job.go +++ b/internal/dao/job.go @@ -91,7 +91,7 @@ func (j *Job) ScanSA(ctx context.Context, fqn string, wait bool) (Refs, error) { if err != nil { return nil, errors.New("expecting Job resource") } - if job.Spec.Template.Spec.ServiceAccountName == n { + if serviceAccountMatches(job.Spec.Template.Spec.ServiceAccountName, n) { refs = append(refs, Ref{ GVR: j.GVR(), FQN: client.FQN(job.Namespace, job.Name), diff --git a/internal/dao/pod.go b/internal/dao/pod.go index 4e5658b6..6b616347 100644 --- a/internal/dao/pod.go +++ b/internal/dao/pod.go @@ -242,7 +242,7 @@ func (p *Pod) ScanSA(ctx context.Context, fqn string, wait bool) (Refs, error) { if len(pod.ObjectMeta.OwnerReferences) > 0 { continue } - if pod.Spec.ServiceAccountName == n { + if serviceAccountMatches(pod.Spec.ServiceAccountName, n) { refs = append(refs, Ref{ GVR: p.GVR(), FQN: client.FQN(pod.Namespace, pod.Name), diff --git a/internal/dao/sts.go b/internal/dao/sts.go index d51e7de3..d1582203 100644 --- a/internal/dao/sts.go +++ b/internal/dao/sts.go @@ -184,7 +184,7 @@ func (s *StatefulSet) ScanSA(ctx context.Context, fqn string, wait bool) (Refs, if err != nil { return nil, errors.New("expecting StatefulSet resource") } - if sts.Spec.Template.Spec.ServiceAccountName == n { + if serviceAccountMatches(sts.Spec.Template.Spec.ServiceAccountName, n) { refs = append(refs, Ref{ GVR: s.GVR(), FQN: client.FQN(sts.Namespace, sts.Name),