Skip to content

Commit dbe77b6

Browse files
jsilverio22jsilverio22
committed
add check for SA if not found in cache but expected to be there
Signed-off-by: Joshua Silverio <[email protected]>
1 parent c33767b commit dbe77b6

File tree

4 files changed

+77
-29
lines changed

4 files changed

+77
-29
lines changed

‎pkg/cache/cache.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type CacheResponse struct {
4141

4242
type ServiceAccountCache interface {
4343
Start(stop chan struct{})
44-
Get(name, namespace string) (role, aud string, useRegionalSTS bool, tokenExpiration int64)
44+
Get(name, namespace string) (role, aud string, useRegionalSTS bool, tokenExpiration int64, err error)
4545
// ToJSON returns cache contents as JSON string
4646
ToJSON() string
4747
}
@@ -76,22 +76,26 @@ func init() {
7676
// Get will return the cached configuration of the given ServiceAccount.
7777
// It will first look at the set of ServiceAccounts configured using annotations. If none are found, it will look for any
7878
// ServiceAccount configured through the pod-identity-webhook ConfigMap.
79-
func (c *serviceAccountCache) Get(name, namespace string) (role, aud string, useRegionalSTS bool, tokenExpiration int64) {
79+
func (c *serviceAccountCache) Get(name, namespace string) (role, aud string, useRegionalSTS bool, tokenExpiration int64, err error) {
8080
klog.V(5).Infof("Fetching sa %s/%s from cache", namespace, name)
81+
var respSA *CacheResponse
8182
{
82-
resp := c.getSA(name, namespace)
83-
if resp != nil && resp.RoleARN != "" {
84-
return resp.RoleARN, resp.Audience, resp.UseRegionalSTS, resp.TokenExpiration
83+
respSA = c.getSA(name, namespace)
84+
if respSA != nil && respSA.RoleARN != "" {
85+
return respSA.RoleARN, respSA.Audience, respSA.UseRegionalSTS, respSA.TokenExpiration, nil
8586
}
8687
}
8788
{
88-
resp := c.getCM(name, namespace)
89-
if resp != nil {
90-
return resp.RoleARN, resp.Audience, resp.UseRegionalSTS, resp.TokenExpiration
89+
respCM := c.getCM(name, namespace)
90+
if respCM != nil {
91+
return respCM.RoleARN, respCM.Audience, respCM.UseRegionalSTS, respCM.TokenExpiration, nil
9192
}
9293
}
93-
klog.V(5).Infof("Service account %s/%s not found in cache", namespace, name)
94-
return "", "", false, pkg.DefaultTokenExpiration
94+
if respSA == nil {
95+
return "", "", false, pkg.DefaultTokenExpiration, fmt.Errorf("service account %s/%s not found in cache and one is expected", namespace, name)
96+
}
97+
98+
return "", "", false, pkg.DefaultTokenExpiration, nil
9599
}
96100

97101
func (c *serviceAccountCache) getSA(name, namespace string) *CacheResponse {

‎pkg/cache/cache_test.go‎

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,17 @@ func TestSaCache(t *testing.T) {
3535
webhookUsage: prometheus.NewGauge(prometheus.GaugeOpts{}),
3636
}
3737

38-
role, aud, useRegionalSTS, tokenExpiration := cache.Get("default", "default")
39-
38+
role, aud, useRegionalSTS, tokenExpiration, err := cache.Get("default", "default")
39+
if err == nil {
40+
t.Fatal("Expected err to not be empty")
41+
}
4042
if role != "" || aud != "" {
4143
t.Errorf("Expected role and aud to be empty, got %s, %s, %t, %d", role, aud, useRegionalSTS, tokenExpiration)
4244
}
4345

4446
cache.addSA(testSA)
4547

46-
role, aud, useRegionalSTS, tokenExpiration = cache.Get("default", "default")
48+
role, aud, useRegionalSTS, tokenExpiration, err = cache.Get("default", "default")
4749

4850
assert.Equal(t, roleArn, role, "Expected role to be %s, got %s", roleArn, role)
4951
assert.Equal(t, "sts.amazonaws.com", aud, "Expected aud to be sts.amzonaws.com, got %s", aud)
@@ -154,7 +156,10 @@ func TestNonRegionalSTS(t *testing.T) {
154156
t.Fatalf("cache never called addSA: %v", err)
155157
}
156158

157-
gotRoleArn, gotAudience, useRegionalSTS, gotTokenExpiration := cache.Get("default", "default")
159+
gotRoleArn, gotAudience, useRegionalSTS, gotTokenExpiration, err := cache.Get("default", "default")
160+
if err != nil {
161+
t.Fatal(err)
162+
}
158163
if gotRoleArn != roleArn {
159164
t.Errorf("got roleArn %v, expected %v", gotRoleArn, roleArn)
160165
}
@@ -199,7 +204,10 @@ func TestPopulateCacheFromCM(t *testing.T) {
199204
t.Errorf("failed to build cache: %v", err)
200205
}
201206

202-
role, _, _, _ := c.Get("mysa2", "myns2")
207+
role, _, _, _, err := c.Get("mysa2", "myns2")
208+
if err != nil {
209+
t.Fatal(err)
210+
}
203211
if role == "" {
204212
t.Errorf("cloud not find entry that should have been added")
205213
}
@@ -211,7 +219,10 @@ func TestPopulateCacheFromCM(t *testing.T) {
211219
t.Errorf("failed to build cache: %v", err)
212220
}
213221

214-
role, _, _, _ := c.Get("mysa2", "myns2")
222+
role, _, _, _, err := c.Get("mysa2", "myns2")
223+
if err != nil {
224+
t.Fatal(err)
225+
}
215226
if role == "" {
216227
t.Errorf("cloud not find entry that should have been added")
217228
}
@@ -223,7 +234,8 @@ func TestPopulateCacheFromCM(t *testing.T) {
223234
t.Errorf("failed to build cache: %v", err)
224235
}
225236

226-
role, _, _, _ := c.Get("mysa2", "myns2")
237+
role, _, _, _, _ := c.Get("mysa2", "myns2")
238+
227239
if role != "" {
228240
t.Errorf("found entry that should have been removed")
229241
}
@@ -253,7 +265,10 @@ func TestSAAnnotationRemoval(t *testing.T) {
253265
c.addSA(oldSA)
254266

255267
{
256-
gotRoleArn, _, _, _ := c.Get("default", "default")
268+
gotRoleArn, _, _, _, err := c.Get("default", "default")
269+
if err != nil {
270+
t.Fatal(err)
271+
}
257272
if gotRoleArn != roleArn {
258273
t.Errorf("got roleArn %q, expected %q", gotRoleArn, roleArn)
259274
}
@@ -265,7 +280,10 @@ func TestSAAnnotationRemoval(t *testing.T) {
265280
c.addSA(newSA)
266281

267282
{
268-
gotRoleArn, _, _, _ := c.Get("default", "default")
283+
gotRoleArn, _, _, _, err := c.Get("default", "default")
284+
if err != nil {
285+
t.Fatal(err)
286+
}
269287
if gotRoleArn != "" {
270288
t.Errorf("got roleArn %v, expected %q", gotRoleArn, "")
271289
}
@@ -320,7 +338,10 @@ func TestCachePrecedence(t *testing.T) {
320338
t.Errorf("failed to build cache: %v", err)
321339
}
322340

323-
role, _, _, exp := c.Get("mysa2", "myns2")
341+
role, _, _, exp, err := c.Get("mysa2", "myns2")
342+
if err != nil {
343+
t.Fatal(err)
344+
}
324345
if role == "" {
325346
t.Errorf("could not find entry that should have been added")
326347
}
@@ -337,7 +358,10 @@ func TestCachePrecedence(t *testing.T) {
337358
}
338359

339360
// Removing sa2 from CM, but SA still exists
340-
role, _, _, exp := c.Get("mysa2", "myns2")
361+
role, _, _, exp, err := c.Get("mysa2", "myns2")
362+
if err != nil {
363+
t.Fatal(err)
364+
}
341365
if role == "" {
342366
t.Errorf("could not find entry that should still exist")
343367
}
@@ -353,7 +377,10 @@ func TestCachePrecedence(t *testing.T) {
353377
c.addSA(sa2)
354378

355379
// Neither cache should return any hits now
356-
role, _, _, _ := c.Get("myns2", "mysa2")
380+
role, _, _, _, err := c.Get("myns2", "mysa2")
381+
if err == nil {
382+
t.Errorf("found entry that should not exist")
383+
}
357384
if role != "" {
358385
t.Errorf("found entry that should not exist")
359386
}
@@ -367,7 +394,10 @@ func TestCachePrecedence(t *testing.T) {
367394
t.Errorf("failed to build cache: %v", err)
368395
}
369396

370-
role, _, _, exp := c.Get("mysa2", "myns2")
397+
role, _, _, exp, err := c.Get("mysa2", "myns2")
398+
if err != nil {
399+
t.Fatal(err)
400+
}
371401
if role == "" {
372402
t.Errorf("cloud not find entry that should have been added")
373403
}

‎pkg/cache/fake.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ package cache
22

33
import (
44
"encoding/json"
5-
"k8s.io/api/core/v1"
65
"strconv"
76
"sync"
87

8+
"k8s.io/api/core/v1"
9+
910
"github.com/aws/amazon-eks-pod-identity-webhook/pkg"
1011
)
1112

@@ -44,14 +45,14 @@ var _ ServiceAccountCache = &FakeServiceAccountCache{}
4445
func (f *FakeServiceAccountCache) Start(chan struct{}) {}
4546

4647
// Get gets a service account from the cache
47-
func (f *FakeServiceAccountCache) Get(name, namespace string) (role, aud string, useRegionalSTS bool, tokenExpiration int64) {
48+
func (f *FakeServiceAccountCache) Get(name, namespace string) (role, aud string, useRegionalSTS bool, tokenExpiration int64, err error) {
4849
f.mu.RLock()
4950
defer f.mu.RUnlock()
5051
resp, ok := f.cache[namespace+"/"+name]
5152
if !ok {
52-
return "", "", false, pkg.DefaultTokenExpiration
53+
return "", "", false, pkg.DefaultTokenExpiration, nil
5354
}
54-
return resp.RoleARN, resp.Audience, resp.UseRegionalSTS, resp.TokenExpiration
55+
return resp.RoleARN, resp.Audience, resp.UseRegionalSTS, resp.TokenExpiration, nil
5556
}
5657

5758
// Add adds a cache entry

‎pkg/handler/handler.go‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,9 +388,22 @@ func (m *Modifier) MutatePod(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResp
388388
// audience: serviceaccount annotation > flag
389389
// regionalSTS: serviceaccount annotation > flag
390390
// tokenExpiration: pod annotation > serviceaccount annotation > flag
391-
podRole, audience, regionalSTS, tokenExpiration := m.Cache.Get(pod.Spec.ServiceAccountName, pod.Namespace)
392-
391+
podRole, audience, regionalSTS, tokenExpiration, err := m.Cache.Get(pod.Spec.ServiceAccountName, pod.Namespace)
393392
// determine whether to perform mutation
393+
if err != nil {
394+
klog.Errorf("Pod was not mutated. Reason: "+
395+
"Service account was not found in cache and was expected. %s",
396+
logContext(pod.Name,
397+
pod.GenerateName,
398+
pod.Spec.ServiceAccountName,
399+
pod.Namespace))
400+
return &v1beta1.AdmissionResponse{
401+
Allowed: false,
402+
Result: &metav1.Status{
403+
Message: err.Error(),
404+
},
405+
}
406+
}
394407
if podRole == "" {
395408
klog.V(4).Infof("Pod was not mutated. Reason: "+
396409
"Service account did not have the right annotations or was not found in the cache. %s",

0 commit comments

Comments
 (0)