Do not skip cache on requests without owner refs

operator-framework · Jan 9, 2020 · 37d28fc · 37d28fc
1 parent 2734d23
commit 37d28fc
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 17 deletions.
diff --git a/pkg/ansible/proxy/cache_response.go b/pkg/ansible/proxy/cache_response.go
@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+
 	"fmt"
 	"net/http"
 	"strings"
@@ -149,12 +150,12 @@ func (c *cacheResponseHandler) skipCacheLookup(r *requestfactory.RequestInfo, gv
 	owner, err := getRequestOwnerRef(req)
 	if err != nil {
 		log.Error(err, "Could not get owner reference from proxy.")
-		return true
+		return false
 	}
 	ownerGV, err := schema.ParseGroupVersion(owner.APIVersion)
 	if err != nil {
-		// TODO(asmacdo) what circumstances cause this path?
-		log.Error(err, "Could not get owner reference from proxy.")
+		m := fmt.Sprintf("Could not get group version for: %v.", owner)
+		log.Error(err, m)
 		return true
 	}
 	ownerGVK := schema.GroupVersionKind{
@@ -166,16 +167,10 @@ func (c *cacheResponseHandler) skipCacheLookup(r *requestfactory.RequestInfo, gv
 	// Make sure that the gvk is not blacklisted
 	relatedController, ok := c.cMap.Get(ownerGVK)
 	// TODO(asmacdo) If !ok, what should the behavior be?
-	// **Option 1**
-	// if !ok {
-	// 	return false
-	// } else {
-	// 	if gvkInList(gvk, relatedController.Blacklist) {
-	// 		return true
-	// 	}
-	// }
-	// **Option 2** this looks right to asmacdo
-	if !ok || gvkInList(gvk, relatedController.Blacklist) {
+	if !ok {
+		return false
+	}
+	if gvkInList(gvk, relatedController.Blacklist) {
 		return true
 	}
 
@@ -216,7 +211,8 @@ func (c *cacheResponseHandler) recoverDependentWatches(req *http.Request, un *un
 	if typeString, ok := un.GetAnnotations()[osdkHandler.TypeAnnotation]; ok {
 		ownerGV, err := schema.ParseGroupVersion(ownerRef.APIVersion)
 		if err != nil {
-			log.Error(err, "Could not get ownerRef from proxy")
+			m := fmt.Sprintf("could not get group version for: %v", ownerGV)
+			log.Error(err, m)
 			return
 		}
 		if typeString == fmt.Sprintf("%v.%v", ownerRef.Kind, ownerGV.Group) {

diff --git a/pkg/ansible/proxy/inject_owner.go b/pkg/ansible/proxy/inject_owner.go
@@ -127,7 +127,7 @@ func (i *injectOwnerReferenceHandler) ServeHTTP(w http.ResponseWriter, req *http
 		} else {
 			ownerGV, err := schema.ParseGroupVersion(owner.APIVersion)
 			if err != nil {
-				m := fmt.Sprintf("could not get broup version for: %v", owner)
+				m := fmt.Sprintf("could not get group version for: %v", owner)
 				log.Error(err, m)
 				http.Error(w, m, http.StatusBadRequest)
 				return

diff --git a/pkg/ansible/watches/watches.go b/pkg/ansible/watches/watches.go
@@ -44,10 +44,10 @@ type Watch struct {
 	Vars                        map[string]interface{}    `yaml:"vars"`
 	MaxRunnerArtifacts          int                       `yaml:"maxRunnerArtifacts"`
 	ReconcilePeriod             time.Duration             `yaml:"reconcilePeriod"`
+	Finalizer                   *Finalizer                `yaml:"finalizer"`
 	ManageStatus                bool                      `yaml:"manageStatus"`
 	WatchDependentResources     bool                      `yaml:"watchDependentResources"`
 	WatchClusterScopedResources bool                      `yaml:"watchClusterScopedResources"`
-	Finalizer                   *Finalizer                `yaml:"finalizer"`
 
 	// Not configurable via watches.yaml
 	MaxWorkers       int `yaml:"maxWorkers"`
@@ -93,8 +93,8 @@ func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		ReconcilePeriod             string                    `yaml:"reconcilePeriod"`
 		ManageStatus                bool                      `yaml:"manageStatus"`
 		WatchDependentResources     bool                      `yaml:"watchDependentResources"`
-		Blacklist                   []schema.GroupVersionKind `yaml:"blacklist"`
 		WatchClusterScopedResources bool                      `yaml:"watchClusterScopedResources"`
+		Blacklist                   []schema.GroupVersionKind `yaml:"blacklist"`
 		Finalizer                   *Finalizer                `yaml:"finalizer"`
 	}
 	var tmp alias