Skip to content

Commit e54c8ef

Browse files
authored
Merge pull request kubernetes#126038 from mprahl/retry-watcher-forbidden
Stop the RetryWatcher when failing due to permissions issue
2 parents 8db6fc7 + db2218d commit e54c8ef

File tree

2 files changed

+65
-0
lines changed

2 files changed

+65
-0
lines changed

staging/src/k8s.io/client-go/tools/watch/retrywatcher.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,35 @@ func (rw *RetryWatcher) doReceive() (bool, time.Duration) {
128128
return false, 0
129129
}
130130

131+
// Check if the watch failed due to the client not having permission to watch the resource or the credentials
132+
// being invalid (e.g. expired token).
133+
if apierrors.IsForbidden(err) || apierrors.IsUnauthorized(err) {
134+
// Add more detail since the forbidden message returned by the Kubernetes API is just "unknown".
135+
klog.ErrorS(err, msg+": ensure the client has valid credentials and watch permissions on the resource")
136+
137+
if apiStatus, ok := err.(apierrors.APIStatus); ok {
138+
statusErr := apiStatus.Status()
139+
140+
sent := rw.send(watch.Event{
141+
Type: watch.Error,
142+
Object: &statusErr,
143+
})
144+
if !sent {
145+
// This likely means the RetryWatcher is stopping but return false so the caller to doReceive can
146+
// verify this and potentially retry.
147+
klog.Error("Failed to send the Unauthorized or Forbidden watch event")
148+
149+
return false, 0
150+
}
151+
} else {
152+
// This should never happen since apierrors only handles apierrors.APIStatus. Still, this is an
153+
// unrecoverable error, so still allow it to return true below.
154+
klog.ErrorS(err, msg+": encountered an unexpected Unauthorized or Forbidden error type")
155+
}
156+
157+
return true, 0
158+
}
159+
131160
klog.ErrorS(err, msg)
132161
// Retry
133162
return false, 0

staging/src/k8s.io/client-go/tools/watch/retrywatcher_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,42 @@ func TestRetryWatcher(t *testing.T) {
288288
},
289289
},
290290
},
291+
{
292+
name: "fails on Forbidden",
293+
initialRV: "5",
294+
watchClient: &cache.ListWatch{
295+
WatchFunc: func() func(options metav1.ListOptions) (watch.Interface, error) {
296+
return func(options metav1.ListOptions) (watch.Interface, error) {
297+
return nil, apierrors.NewForbidden(schema.GroupResource{}, "", errors.New("unknown"))
298+
}
299+
}(),
300+
},
301+
watchCount: 1,
302+
expected: []watch.Event{
303+
{
304+
Type: watch.Error,
305+
Object: &apierrors.NewForbidden(schema.GroupResource{}, "", errors.New("unknown")).ErrStatus,
306+
},
307+
},
308+
},
309+
{
310+
name: "fails on Unauthorized",
311+
initialRV: "5",
312+
watchClient: &cache.ListWatch{
313+
WatchFunc: func() func(options metav1.ListOptions) (watch.Interface, error) {
314+
return func(options metav1.ListOptions) (watch.Interface, error) {
315+
return nil, apierrors.NewUnauthorized("")
316+
}
317+
}(),
318+
},
319+
watchCount: 1,
320+
expected: []watch.Event{
321+
{
322+
Type: watch.Error,
323+
Object: &apierrors.NewUnauthorized("").ErrStatus,
324+
},
325+
},
326+
},
291327
{
292328
name: "recovers from timeout error",
293329
initialRV: "5",

0 commit comments

Comments
 (0)