Stop erroring in selector if operator is DoesNotExist (#803)

pkg/eval/jsonpath.go

@@ -49,11 +49,11 @@ func (e Evaluator) EvaluateJsonPath(path string, obj interface{}) (interface{},
 	}
 
 	if len(interfaceList) > 1 {
-		return "", fmt.Errorf("too many results for the query: %s", path)
+		return nil, fmt.Errorf("too many results for the query: %s", path)
 	}
 
 	if len(interfaceList) == 0 {
-		return "", JsonPathDoesNotExistError{Path: path}
+		return nil, JsonPathDoesNotExistError{Path: path}
 	}
 
 	return interfaceList[0], nil

pkg/realizer/deliverable/component_test.go

@@ -321,7 +321,7 @@ var _ = Describe("Resource", func() {
 				Expect(template.GetKind()).To(Equal("ClusterSourceTemplate"))
 
 				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("find results: does-not-exist is not found"))
+				Expect(err.Error()).To(ContainSubstring("jsonpath returned empty list: data.does-not-exist"))
 				Expect(reflect.TypeOf(err).String()).To(Equal("deliverable.RetrieveOutputError"))
 			})
 		})

pkg/realizer/runnable/gc/gc_test.go

@@ -101,7 +101,7 @@ var _ = Describe("CleanupRunnableStampedObjects", func() {
 		_, deletedObject1 := repo.DeleteArgsForCall(0)
 		Expect(deletedObject1).To(Equal(successfulRunnableStampedObjectToBeDeleted))
 
-		Expect(out).To(Say("failed evaluating jsonpath to determine runnable stamped object success.*ThingWithoutSucceededCondition.*status is not found"))
+		Expect(out).To(Say(`"error":"jsonpath returned empty list: status.conditions\[\?\(\@.type==\\"Succeeded\\"\)\].status"`))
 	})
 
 	Context("when runnable stamped objects outside the retention policy are processed", func() {

pkg/realizer/runnable/realizer_test.go

@@ -489,7 +489,7 @@ var _ = Describe("Realizer", func() {
 		It("returns RetrieveOutputError", func() {
 			_, _, err := rlzr.Realize(ctx, runnable, systemRepo, runnableRepo, discoveryClient)
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring(`unable to retrieve outputs from stamped object [my-important-ns/my-stamped-resource-] of type [configmap] for run template [my-template]: failed to evaluate path [data.hasnot]: evaluate: failed to find results: hasnot is not found`))
+			Expect(err.Error()).To(ContainSubstring(`unable to retrieve outputs from stamped object [my-important-ns/my-stamped-resource-] of type [configmap] for run template [my-template]: failed to evaluate path [data.hasnot]: jsonpath returned empty list: data.hasnot`))
 			Expect(reflect.TypeOf(err).String()).To(Equal("runnable.RetrieveOutputError"))
 		})
 	})

pkg/realizer/workload/component_test.go

@@ -323,7 +323,7 @@ var _ = Describe("Resource", func() {
 				Expect(template.GetKind()).To(Equal("ClusterImageTemplate"))
 
 				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("find results: does-not-exist is not found"))
+				Expect(err.Error()).To(ContainSubstring("jsonpath returned empty list: data.does-not-exist"))
 				Expect(reflect.TypeOf(err).String()).To(Equal("workload.RetrieveOutputError"))
 			})
 		})
@@ -449,7 +449,7 @@ var _ = Describe("Resource", func() {
 				branch := "main"
 				workload = v1alpha1.Workload{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:                       "my-workload",
+						Name: "my-workload",
 					},
 					TypeMeta: metav1.TypeMeta{
 						Kind:       "Workload",
@@ -581,17 +581,34 @@ var _ = Describe("Resource", func() {
 				})
 			})
 
-			When("key does not exist in the spec", func() {
-				It("returns a ResolveTemplateOptionError", func() {
+			When("one option has key that does not exist in the spec", func() {
+				It("does not error", func() {
 					resource.TemplateRef.Options[0].Selector.MatchFields[0].Key = `spec.env[?(@.name=="some-name")].bad`
 
 					template, _, _, err := r.Do(ctx, &resource, supplyChainName, outputs)
 
+					Expect(template.GetName()).To(Equal("template-chosen"))
+					Expect(template.GetKind()).To(Equal("ClusterImageTemplate"))
+
+					Expect(err).NotTo(HaveOccurred())
+					_, name, kind := fakeSystemRepo.GetTemplateArgsForCall(0)
+					Expect(name).To(Equal("template-chosen"))
+					Expect(kind).To(Equal("ClusterImageTemplate"))
+				})
+			})
+
+			When("key is malformed", func() {
+				It("returns a ResolveTemplateOptionError", func() {
+					resource.TemplateRef.Options[0].Selector.MatchFields[0].Key = `spec.env[`
+
+					template, _, _, err := r.Do(ctx, &resource, supplyChainName, outputs)
+
 					Expect(template).To(BeNil())
 
 					Expect(err).To(HaveOccurred())
+					Expect(reflect.TypeOf(err).String()).To(Equal("workload.ResolveTemplateOptionError"))
 					Expect(err.Error()).To(ContainSubstring(`error matching against template option [template-not-chosen] for resource [resource-1] in supply chain [supply-chain-name]`))
-					Expect(err.Error()).To(ContainSubstring(`failed to evaluate selector matchFields: unable to match field requirement with key [spec.env[?(@.name=="some-name")].bad] operator [Exists] values [[]]: evaluate: failed to find results: bad is not found`))
+					Expect(err.Error()).To(ContainSubstring(`failed to evaluate selector matchFields: unable to match field requirement with key [spec.env[] operator [Exists] values [[]]: evaluate: failed to parse jsonpath '{.spec.env[}': unterminated array`))
 				})
 			})
 

pkg/selector/selector.go

@@ -21,12 +21,13 @@ import (
 	"github.com/vmware-tanzu/cartographer/pkg/eval"
 )
 
-
 func Matches(req v1alpha1.FieldSelectorRequirement, context interface{}) (bool, error) {
 	evaluator := eval.EvaluatorBuilder()
 	actualValue, err := evaluator.EvaluateJsonPath(req.Key, context)
 	if err != nil {
-		return false, err
+		if _, ok := err.(eval.JsonPathDoesNotExistError); !ok || req.Operator != v1alpha1.FieldSelectorOpDoesNotExist {
+			return false, err
+		}
 	}
 
 	switch req.Operator {
@@ -51,4 +52,4 @@ func Matches(req v1alpha1.FieldSelectorRequirement, context interface{}) (bool,
 	default:
 		return false, fmt.Errorf("invalid operator %s for field selector", req.Operator)
 	}
-}
+}

pkg/selector/selector_test.go

@@ -30,7 +30,7 @@ var _ = Describe("Selector", func() {
 		BeforeEach(func() {
 			context = map[string]interface{}{
 				"hello": "world",
-				"bad":   nil,
+				"empty": nil,
 			}
 		})
 
@@ -50,7 +50,7 @@ var _ = Describe("Selector", func() {
 			Context("when the key is valid but returns a nil value", func() {
 				It("returns false and does not error", func() {
 					req := v1alpha1.FieldSelectorRequirement{
-						Key:      "bad",
+						Key:      "empty",
 						Operator: v1alpha1.FieldSelectorOpExists,
 					}
 					ret, err := selector.Matches(req, context)

pkg/templates/cluster_run_template_test.go

@@ -154,7 +154,7 @@ var _ = Describe("ClusterRunTemplate", func() {
 					template := templates.NewRunTemplateModel(apiTemplate)
 					_, _, err := template.GetOutput(stampedObjects)
 					Expect(err).To(HaveOccurred())
-					Expect(err.Error()).To(Equal("failed to evaluate path [spec.nonexistant]: evaluate: failed to find results: nonexistant is not found"))
+					Expect(err.Error()).To(Equal("failed to evaluate path [spec.nonexistant]: jsonpath returned empty list: spec.nonexistant"))
 				})
 			})
 		})
@@ -230,7 +230,7 @@ var _ = Describe("ClusterRunTemplate", func() {
 					template := templates.NewRunTemplateModel(apiTemplate)
 					_, _, err := template.GetOutput(stampedObjects)
 					Expect(err).To(HaveOccurred())
-					Expect(err.Error()).To(Equal("failed to evaluate path [spec.nonexistant]: evaluate: failed to find results: nonexistant is not found"))
+					Expect(err.Error()).To(Equal("failed to evaluate path [spec.nonexistant]: jsonpath returned empty list: spec.nonexistant"))
 				})
 
 				Context("and one does not have succeeded condition", func() {
@@ -241,7 +241,7 @@ var _ = Describe("ClusterRunTemplate", func() {
 						template := templates.NewRunTemplateModel(apiTemplate)
 						_, _, err := template.GetOutput(stampedObjects)
 						Expect(err).To(HaveOccurred())
-						Expect(err.Error()).To(ContainSubstring("failed to evaluate path [spec.nonexistant]: evaluate: failed to find results: nonexistant is not found"))
+						Expect(err.Error()).To(ContainSubstring("failed to evaluate path [spec.nonexistant]: jsonpath returned empty list: spec.nonexistant"))
 					})
 				})
 			})

pkg/utils/json_path.go

@@ -29,6 +29,7 @@ func SinglePathEvaluate(jsonpathExpression string, obj interface{}) ([]interface
 	)
 
 	parser := jsonpath.New("")
+	parser.AllowMissingKeys(true)
 
 	err := parser.Parse(jsonpathExpression)
 	if err != nil {

pkg/utils/json_path_test.go

@@ -69,7 +69,7 @@ var _ = Describe("JsonPath", func() {
 
 	Context("when find fails", func() {
 		BeforeEach(func() {
-			path = `{.bye}`
+			path = `{.hello[1]}`
 		})
 
 		ItReturnsAHelpfulError("find results: ")

tests/integration/delivery/deliverable_reconciler_test.go

@@ -508,7 +508,7 @@ var _ = Describe("DeliverableReconciler", func() {
 							"ResourcesSubmitted": MatchFields(IgnoreExtras, Fields{
 								"Status":  Equal(metav1.ConditionUnknown),
 								"Reason":  Equal("ConditionNotMet"),
-								"Message": ContainSubstring(`resource [deployer] condition not met: failed to evaluate succeededCondition.Key [status.conditions[?(@.type=="Succeeded")].status]: evaluate: failed to find results: conditions is not found`),
+								"Message": ContainSubstring(`resource [deployer] condition not met: failed to evaluate succeededCondition.Key [status.conditions[?(@.type=="Succeeded")].status]: jsonpath returned empty list: status.conditions[?(@.type=="Succeeded")].status`),
 							}),
 						}))
 					})

tests/kuttl/delivery/deploy-w-missing-match/01-assert.yaml

@@ -25,7 +25,7 @@ status:
     - type: ResourcesSubmitted
       status: Unknown
       reason: ConditionNotMet
-      message: "resource [deployer] condition not met: could not find value on output [spec.value.some-missing-key]: evaluate: failed to find results: some-missing-key is not found"
+      message: "resource [deployer] condition not met: could not find value on output [spec.value.some-missing-key]: jsonpath returned empty list: spec.value.some-missing-key"
     - type: Ready
       status: Unknown
       reason: ConditionNotMet