Improve conditionFuncFor expression parsing for wait --for jsonpath

Make it possible to parse jsonpath filter expressions: Split
jsonpath expressions on single '=' only and leave '==' as part of the
string.

Reported-at: https://github.com/kubernetes/kubernetes/issues/119206
Signed-off-by: Andreas Karis <ak.karis@gmail.com>

Kubernetes-commit: 41889984304c44b879d8bcae92c9ed7aa3fd8c67

pkg/cmd/wait/wait.go

@@ -74,6 +74,9 @@ var (
 		# Wait for the pod "busybox1" to contain the status phase to be "Running"
 		kubectl wait --for=jsonpath='{.status.phase}'=Running pod/busybox1
 
+		# Wait for pod "busybox1" to be Ready
+		kubectl wait --for='jsonpath={.status.conditions[?(@.type=="Ready")].status}=True' pod/busybox1
+
 		# Wait for the service "loadbalancer" to have ingress.
 		kubectl wait --for=jsonpath='{.status.loadBalancer.ingress}' service/loadbalancer
 
@@ -209,8 +212,8 @@ func conditionFuncFor(condition string, errOut io.Writer) (ConditionFunc, error)
 		}.IsConditionMet, nil
 	}
 	if strings.HasPrefix(condition, "jsonpath=") {
-		splitStr := strings.Split(condition, "=")
-		jsonPathExp, jsonPathValue, err := processJSONPathInput(splitStr[1:])
+		jsonPathInput := strings.TrimPrefix(condition, "jsonpath=")
+		jsonPathExp, jsonPathValue, err := processJSONPathInput(jsonPathInput)
 		if err != nil {
 			return nil, err
 		}
@@ -241,8 +244,10 @@ func newJSONPathParser(jsonPathExpression string) (*jsonpath.JSONPath, error) {
 	return j, nil
 }
 
-// processJSONPathInput will parses the user's JSONPath input containing JSON expression and, optionally, JSON value for matching condition and process it
-func processJSONPathInput(jsonPathInput []string) (string, string, error) {
+// processJSONPathInput will parse and process the provided JSONPath input containing a JSON expression and optionally
+// a value for the matching condition.
+func processJSONPathInput(input string) (string, string, error) {
+	jsonPathInput := splitJSONPathInput(input)
 	if numOfArgs := len(jsonPathInput); numOfArgs < 1 || numOfArgs > 2 {
 		return "", "", fmt.Errorf("jsonpath wait format must be --for=jsonpath='{.status.readyReplicas}'=3 or --for=jsonpath='{.status.readyReplicas}'")
 	}
@@ -260,6 +265,27 @@ func processJSONPathInput(jsonPathInput []string) (string, string, error) {
 	return relaxedJSONPathExp, jsonPathValue, nil
 }
 
+// splitJSONPathInput splits the provided input string on single '='. Double '==' will not cause the string to be
+// split. E.g., "a.b.c====d.e.f===g.h.i===" will split to ["a.b.c====d.e.f==","g.h.i==",""].
+func splitJSONPathInput(input string) []string {
+	var output []string
+	var element strings.Builder
+	for i := 0; i < len(input); i++ {
+		if input[i] == '=' {
+			if i < len(input)-1 && input[i+1] == '=' {
+				element.WriteString("==")
+				i++
+				continue
+			}
+			output = append(output, element.String())
+			element.Reset()
+			continue
+		}
+		element.WriteByte(input[i])
+	}
+	return append(output, element.String())
+}
+
 // ResourceLocation holds the location of a resource
 type ResourceLocation struct {
 	GroupResource schema.GroupResource

pkg/cmd/wait/wait_test.go

@@ -1534,39 +1534,112 @@ func TestWaitForJSONPathCondition(t *testing.T) {
 	}
 }
 
-// TestWaitForJSONPathBadConditionParsing will test errors in parsing JSONPath bad condition expressions
-// except for parsing JSONPath expression itself (i.e. call to cmdget.RelaxedJSONPathExpression())
-func TestWaitForJSONPathBadConditionParsing(t *testing.T) {
+// TestConditionFuncFor tests that the condition string can be properly parsed into a ConditionFunc.
+func TestConditionFuncFor(t *testing.T) {
 	tests := []struct {
-		name           string
-		condition      string
-		expectedResult JSONPathWait
-		expectedErr    string
+		name        string
+		condition   string
+		expectedErr string
 	}{
 		{
-			name:        "missing JSONPath expression",
+			name:        "jsonpath missing JSONPath expression",
 			condition:   "jsonpath=",
 			expectedErr: "jsonpath expression cannot be empty",
 		},
 		{
-			name:        "value in JSONPath expression has equal sign",
+			name:        "jsonpath check for condition without value",
+			condition:   "jsonpath={.metadata.name}",
+			expectedErr: None,
+		},
+		{
+			name:        "jsonpath check for condition without value relaxed parsing",
+			condition:   "jsonpath=abc",
+			expectedErr: None,
+		},
+		{
+			name:        "jsonpath check for expression and value",
+			condition:   "jsonpath={.metadata.name}=foo-b6699dcfb-rnv7t",
+			expectedErr: None,
+		},
+		{
+			name:        "jsonpath check for expression and value relaxed parsing",
+			condition:   "jsonpath=.metadata.name=foo-b6699dcfb-rnv7t",
+			expectedErr: None,
+		},
+		{
+			name:        "jsonpath selecting based on condition",
+			condition:   `jsonpath={.status.containerStatuses[?(@.name=="foo")].ready}=True`,
+			expectedErr: None,
+		},
+		{
+			name:        "jsonpath selecting based on condition relaxed parsing",
+			condition:   "jsonpath=status.conditions[?(@.type==\"Available\")].status=True",
+			expectedErr: None,
+		},
+		{
+			name:        "jsonpath selecting based on condition without value",
+			condition:   `jsonpath={.status.containerStatuses[?(@.name=="foo")].ready}`,
+			expectedErr: None,
+		},
+		{
+			name:        "jsonpath selecting based on condition without value relaxed parsing",
+			condition:   `jsonpath=.status.containerStatuses[?(@.name=="foo")].ready`,
+			expectedErr: None,
+		},
+		{
+			name:        "jsonpath invalid expression with repeated '='",
 			condition:   "jsonpath={.metadata.name}='test=wrong'",
 			expectedErr: "jsonpath wait format must be --for=jsonpath='{.status.readyReplicas}'=3 or --for=jsonpath='{.status.readyReplicas}'",
 		},
 		{
-			name:        "undefined value",
+			name:        "jsonpath undefined value after '='",
 			condition:   "jsonpath={.metadata.name}=",
-			expectedErr: "jsonpath wait has to have a value after equal sign, like --for=jsonpath='{.status.readyReplicas}'=3",
+			expectedErr: "jsonpath wait has to have a value after equal sign",
+		},
+		{
+			name:        "jsonpath complex expressions not supported",
+			condition:   "jsonpath={.status.conditions[?(@.type==\"Failed\"||@.type==\"Complete\")].status}=True",
+			expectedErr: "unrecognized character in action: U+007C '|'",
+		},
+		{
+			name:      "jsonpath invalid expression",
+			condition: "jsonpath={=True",
+			expectedErr: "unexpected path string, expected a 'name1.name2' or '.name1.name2' or '{name1.name2}' or " +
+				"'{.name1.name2}'",
+		},
+		{
+			name:        "condition delete",
+			condition:   "delete",
+			expectedErr: None,
+		},
+		{
+			name:        "condition true",
+			condition:   "condition=hello",
+			expectedErr: None,
+		},
+		{
+			name:        "condition with value",
+			condition:   "condition=hello=world",
+			expectedErr: None,
+		},
+		{
+			name:        "unrecognized condition",
+			condition:   "cond=invalid",
+			expectedErr: "unrecognized condition: \"cond=invalid\"",
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			_, err := conditionFuncFor(test.condition, io.Discard)
-			if err == nil && test.expectedErr != "" {
-				t.Fatalf("expected %q, got empty", test.expectedErr)
-			}
-			if !strings.Contains(err.Error(), test.expectedErr) {
-				t.Fatalf("expected %q, got %q", test.expectedErr, err.Error())
+			switch {
+			case err == nil && test.expectedErr != None:
+				t.Fatalf("expected error %q, got nil", test.expectedErr)
+			case err != nil && test.expectedErr == None:
+				t.Fatalf("expected no error, got %q", err)
+			case err != nil && test.expectedErr != None:
+				if !strings.Contains(err.Error(), test.expectedErr) {
+					t.Fatalf("expected error %q, got %q", test.expectedErr, err.Error())
+				}
 			}
 		})
 	}