Skip to content

Adding context timeout for Ansible proxy #2264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Apr 22, 2020
13 changes: 13 additions & 0 deletions changelog/fragments/fix-issue-bug-1701041.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# entries is a list of entries to include in
# release notes and/or the migration guide
entries:
- description: >
Added timeout to the Ansible based-operator proxy, which enables error reporting for requests that fail due to RBAC permissions issues to List and Watch the resources.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabianvf done


# kind iss one of:
# - addition
# - change
# - deprecation
# - removal
# - bugfix
kind: "bugfix"
8 changes: 6 additions & 2 deletions pkg/ansible/proxy/cache_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,9 @@ func (c *cacheResponseHandler) getListFromCache(r *requestfactory.RequestInfo, r
k.Kind = k.Kind + "List"
un := unstructured.UnstructuredList{}
un.SetGroupVersionKind(k)
err := c.informerCache.List(context.Background(), &un, clientListOpts...)
ctx, cancel := context.WithTimeout(context.Background(), cacheEstablishmentTimeout)
defer cancel()
err := c.informerCache.List(ctx, &un, clientListOpts...)
if err != nil {
// break here in case resource doesn't exist in cache but exists on APIserver
// This is very unlikely but provides user with expected 404
Expand All @@ -287,7 +289,9 @@ func (c *cacheResponseHandler) getObjectFromCache(r *requestfactory.RequestInfo,
un := &unstructured.Unstructured{}
un.SetGroupVersionKind(k)
obj := client.ObjectKey{Namespace: r.Namespace, Name: r.Name}
err := c.informerCache.Get(context.Background(), obj, un)
ctx, cancel := context.WithTimeout(context.Background(), cacheEstablishmentTimeout)
defer cancel()
err := c.informerCache.Get(ctx, obj, un)
if err != nil {
// break here in case resource doesn't exist in cache but exists on APIserver
// This is very unlikely but provides user with expected 404
Expand Down
8 changes: 8 additions & 0 deletions pkg/ansible/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"net/http"
"strings"
"sync"
"time"

"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/controllermap"
"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/kubeconfig"
Expand All @@ -44,6 +45,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"
)

// This is the default timeout to wait for the cache to respond
// todo(shawn-hurley): Eventually this should be configurable
const cacheEstablishmentTimeout = 6 * time.Second
const AutoSkipCacheREList = "^/api/.*/pods/.*/exec,^/api/.*/pods/.*/attach"

// RequestLogHandler - log the requests that come through the proxy.
Expand Down Expand Up @@ -244,6 +248,8 @@ func addWatchToController(owner kubeconfig.NamespacedOwnerReference, cMap *contr
&handler.EnqueueRequestForOwner{OwnerType: u}, dependentPredicate)
// Store watch in map
if err != nil {
log.Error(err, "Failed to watch child resource",
"kind", resource.GroupVersionKind(), "enqueue_kind", u.GroupVersionKind())
return err
}
case (!useOwnerRef && dataNamespaceScoped) || contents.WatchClusterScopedResources:
Expand All @@ -259,6 +265,8 @@ func addWatchToController(owner kubeconfig.NamespacedOwnerReference, cMap *contr
err = contents.Controller.Watch(&source.Kind{Type: resource},
&osdkHandler.EnqueueRequestForAnnotation{Type: typeString}, dependentPredicate)
if err != nil {
log.Error(err, "Failed to watch child resource",
"kind", resource.GroupVersionKind(), "enqueue_kind", u.GroupVersionKind())
return err
}
}
Expand Down