From 3911d091530fc743585c72c7366db3a9c7932bfd Mon Sep 17 00:00:00 2001 From: Dominik Deren Date: Tue, 7 Dec 2021 06:25:20 +0100 Subject: [PATCH] fix: improve error message for ListArchivedWorkflows (#7345) * fix: improve error message for ListArchivedWorkflows Error message that is returned when user tries to list archived workflows and doesn't specify a namespace is very generic and hard to understand. This PR tries to make it a bit more clear as to what might be the cause of the issue, to help guide the user. I thought that it might be a good idea to check if `namespace` is an empty string, and if it is, return a custom error for this case, telling user that `namespace` can't be empty. But I assume that this would not be correct for cases when Argo-Workflows runs with ClusterRole permissions? Signed-off-by: Dominik Deren * Fixing test & adding string formatting. Signed-off-by: Dominik Deren --- server/workflowarchive/archived_workflow_server.go | 2 +- server/workflowarchive/archived_workflow_server_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/workflowarchive/archived_workflow_server.go b/server/workflowarchive/archived_workflow_server.go index f9dfe9ab1fcf..806d844cd09b 100644 --- a/server/workflowarchive/archived_workflow_server.go +++ b/server/workflowarchive/archived_workflow_server.go @@ -83,7 +83,7 @@ func (w *archivedWorkflowServer) ListArchivedWorkflows(ctx context.Context, req return nil, err } if !allowed { - return nil, status.Error(codes.PermissionDenied, "permission denied") + return nil, status.Error(codes.PermissionDenied, fmt.Sprintf("Permission denied, you are not allowed to list workflows in namespace \"%s\". Maybe you want to specify a namespace with `listOptions.fieldSelector=metadata.namespace=your-ns`?", namespace)) } // When the zero value is passed, we should treat this as returning all results diff --git a/server/workflowarchive/archived_workflow_server_test.go b/server/workflowarchive/archived_workflow_server_test.go index 6b00f73d2dfc..c613c765e338 100644 --- a/server/workflowarchive/archived_workflow_server_test.go +++ b/server/workflowarchive/archived_workflow_server_test.go @@ -73,7 +73,7 @@ func Test_archivedWorkflowServer(t *testing.T) { t.Run("ListArchivedWorkflows", func(t *testing.T) { allowed = false _, err := w.ListArchivedWorkflows(ctx, &workflowarchivepkg.ListArchivedWorkflowsRequest{ListOptions: &metav1.ListOptions{Limit: 1}}) - assert.Equal(t, err, status.Error(codes.PermissionDenied, "permission denied")) + assert.Equal(t, err, status.Error(codes.PermissionDenied, "Permission denied, you are not allowed to list workflows in namespace \"\". Maybe you want to specify a namespace with `listOptions.fieldSelector=metadata.namespace=your-ns`?")) allowed = true resp, err := w.ListArchivedWorkflows(ctx, &workflowarchivepkg.ListArchivedWorkflowsRequest{ListOptions: &metav1.ListOptions{Limit: 1}}) if assert.NoError(t, err) {