fix: Fix panic and provide better error message on watch endpoint (ar…

…goproj#3605)

server/workflow/workflow_server.go

@@ -146,7 +146,7 @@ func (s *workflowServer) WatchWorkflows(req *workflowpkg.WatchWorkflowsRequest,
 		opts = req.ListOptions
 		wfName, err := argoutil.RecoverWorkflowNameFromSelectorString(opts.FieldSelector)
 		if err != nil {
-			return err
+			return fmt.Errorf("malformed request: workflows must be specified with a 'metadata.name=...' field selector; unable to parse: %w", err)
 		}
 		if len(wfName) > 0 {
 			wf, err := s.getWorkflow(wfClient, req.Namespace, wfName, metav1.GetOptions{})

util/util.go

@@ -97,21 +97,21 @@ func RecoverIndexFromNodeName(name string) int {
 func GenerateFieldSelectorFromWorkflowName(wfName string) string {
 	result := fields.ParseSelectorOrDie(fmt.Sprintf("metadata.name=%s", wfName)).String()
 	compare, err := RecoverWorkflowNameFromSelectorString(result)
-	if err != nil {
+	if err != nil || wfName != compare {
 		log.WithFields(log.Fields{"wfName": wfName}).Error(err)
-	}
-	if wfName != compare {
 		panic(fmt.Sprintf("Could not recover field selector from workflow name. Expected '%s' but got '%s'\n", wfName, compare))
 	}
 	return result
 }
 
 func RecoverWorkflowNameFromSelectorString(selector string) (string, error) {
 	nameIndex := strings.Index(selector, "=")
+	if nameIndex < 0 {
+		return "", fmt.Errorf("incorrect prefix: selector is not formatted correctly")
+	}
 	prefix := selector[:nameIndex]
 	if prefix != "metadata.name" {
-		return "", fmt.Errorf("Incorrect prefix. Expected 'metadata.name' but got '%s'", prefix)
+		return "", fmt.Errorf("incorrect prefix: expected 'metadata.name' but got '%s'", prefix)
 	}
-	name := selector[nameIndex+1:]
-	return name, nil
+	return selector[nameIndex+1:], nil
 }

util/util_test.go

@@ -53,4 +53,7 @@ func TestRecoverWorkflowNameFromSelectorStringError(t *testing.T) {
 	name, err := RecoverWorkflowNameFromSelectorString("whatever=whalesay")
 	assert.NotNil(t, err)
 	assert.Equal(t, name, "")
+	assert.NotPanics(t, func() {
+		_, _ = RecoverWorkflowNameFromSelectorString("whatever")
+	})
 }