diff --git a/pkg/eval/jsonpath.go b/pkg/eval/jsonpath.go index 6800d75c8..3c5849aa7 100644 --- a/pkg/eval/jsonpath.go +++ b/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 diff --git a/pkg/realizer/deliverable/component_test.go b/pkg/realizer/deliverable/component_test.go index 5af2f5895..14f37e92a 100644 --- a/pkg/realizer/deliverable/component_test.go +++ b/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")) }) }) diff --git a/pkg/realizer/runnable/gc/gc_test.go b/pkg/realizer/runnable/gc/gc_test.go index e9b4c365e..32cd45cd7 100644 --- a/pkg/realizer/runnable/gc/gc_test.go +++ b/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() { diff --git a/pkg/realizer/runnable/realizer_test.go b/pkg/realizer/runnable/realizer_test.go index 69f2dcc57..b0681bbbd 100644 --- a/pkg/realizer/runnable/realizer_test.go +++ b/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")) }) }) diff --git a/pkg/realizer/workload/component_test.go b/pkg/realizer/workload/component_test.go index 07b75aaa5..02c3d7c15 100644 --- a/pkg/realizer/workload/component_test.go +++ b/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`)) }) }) diff --git a/pkg/selector/selector.go b/pkg/selector/selector.go index 2287772ee..fcfa64d03 100644 --- a/pkg/selector/selector.go +++ b/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) } -} \ No newline at end of file +} diff --git a/pkg/selector/selector_test.go b/pkg/selector/selector_test.go index 7392bf380..ca9a052b9 100644 --- a/pkg/selector/selector_test.go +++ b/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) diff --git a/pkg/templates/cluster_run_template_test.go b/pkg/templates/cluster_run_template_test.go index 37399ec8c..472209c79 100644 --- a/pkg/templates/cluster_run_template_test.go +++ b/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")) }) }) }) diff --git a/pkg/utils/json_path.go b/pkg/utils/json_path.go index 95589a017..cf4a59586 100644 --- a/pkg/utils/json_path.go +++ b/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 { diff --git a/pkg/utils/json_path_test.go b/pkg/utils/json_path_test.go index 291d672b0..b0e3eb08d 100644 --- a/pkg/utils/json_path_test.go +++ b/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: ") diff --git a/tests/integration/delivery/deliverable_reconciler_test.go b/tests/integration/delivery/deliverable_reconciler_test.go index d006c2917..f360eded0 100644 --- a/tests/integration/delivery/deliverable_reconciler_test.go +++ b/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`), }), })) }) diff --git a/tests/kuttl/delivery/deploy-w-missing-match/01-assert.yaml b/tests/kuttl/delivery/deploy-w-missing-match/01-assert.yaml index 9aa328ac3..286b64ff6 100644 --- a/tests/kuttl/delivery/deploy-w-missing-match/01-assert.yaml +++ b/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