AlinsRan commented on code in PR #2817:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2817#discussion_r3656099539
##########
pkg/utils/k8s.go:
##########
@@ -52,29 +121,124 @@ func HasAPIResourceWithLogger(mgr ctrl.Manager, obj
client.Object, logger logr.L
"groupVersion", groupVersion,
)
- // Create discovery client
- discoveryClient, err :=
discovery.NewDiscoveryClientForConfig(mgr.GetConfig())
- if err != nil {
- logger.Info("failed to create discovery client", "error", err)
- return false
- }
-
// Query server resources for the specific group/version
- apiResources, err :=
discoveryClient.ServerResourcesForGroupVersion(groupVersion)
- if err != nil {
+ var apiResources *metav1.APIResourceList
+ err := retryUntilDefinitive(backoff, logger, func() error {
+ var err error
+ apiResources, err =
discoveryClient.ServerResourcesForGroupVersion(groupVersion)
+ return err
+ })
+ switch {
+ case err == nil:
+ case apierrors.IsNotFound(err):
logger.Info("group/version not available in cluster", "error",
err)
- return false
+ return false, nil
+ case apierrors.IsForbidden(err):
Review Comment:
Agreed — you and the EE reviewer landed on the same conclusion
independently, and my earlier justification was wrong. The
`nonResourceURLs`-denies-one-group case is real but rare, and it is not worth
the failure mode: the result is cached for the manager's lifetime, so any RBAC
slip permanently omits a controller and its indexes behind a ready manager.
Fixed in 9490f6d6: `isResourceAbsent` is now `isDefinitive` (it only decides
whether retrying can change the answer), and `hasAPIResource` maps the two
definitive cases apart — 404 resolves to absent, Forbidden returns an error
naming the service account's discovery permissions.
`TestHasAPIResource/discovery_forbidden` now asserts the error.
Same change in api7/api7-ingress-controller#444.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
